From 0c89a490d016c0f4c566966edc10fd03467f42ac Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Fri, 24 Jan 2020 01:09:53 +0000 Subject: [PATCH] Revert "[vm] resolve symbolic link to find kernel_service snapshot" This reverts commit 8a8c19dd4b1d6d80a0fa7ec4bec2127e3050c1e0. Reason for revert: ASAN detected a leak: https://ci.chromium.org/p/dart/builders/ci.sandbox/vm-kernel-asan-linux-release-x64/4588 Original change's description: > [vm] resolve symbolic link to find kernel_service snapshot > > When VM starts, it will try to find kernel service snapshot which is located together with exe file. However, if symbolic link is provided, the directory to search will become the parent directory of link but not the actual target. > This cl will resolve the symbolic link to find the kernel service snapshot. > > Bug: https://github.com/dart-lang/sdk/issues/35188 > Change-Id: I842973a9b73439003a748122d9416158d155892e > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/129380 > Commit-Queue: Zichang Guo > Reviewed-by: Siva Annamalai TBR=asiva@google.com,zichangguo@google.com Change-Id: Iedc4a4ea466a183d1db3bb57dd30b29a61e1b933 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: https://github.com/dart-lang/sdk/issues/35188 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/133171 Reviewed-by: Ben Konyi Commit-Queue: Ben Konyi --- runtime/bin/dfe.cc | 9 --------- runtime/bin/file.h | 7 +------ runtime/bin/file_android.cc | 21 ++++++--------------- runtime/bin/file_fuchsia.cc | 21 ++++++--------------- runtime/bin/file_linux.cc | 21 ++++++--------------- runtime/bin/file_macos.cc | 21 ++++++--------------- runtime/bin/file_win.cc | 18 +++++------------- 7 files changed, 30 insertions(+), 88 deletions(-) diff --git a/runtime/bin/dfe.cc b/runtime/bin/dfe.cc index d941c0d6de7..be610de0578 100644 --- a/runtime/bin/dfe.cc +++ b/runtime/bin/dfe.cc @@ -55,15 +55,6 @@ static char* GetDirectoryPrefixFromExeName() { name = Platform::GetExecutableName(); target_size = strlen(name); } - Namespace* namespc = Namespace::Create(Namespace::Default()); - if (File::GetType(namespc, name, false) == File::kIsLink) { - // Resolve the link without creating Dart scope String. - name = File::LinkTarget(namespc, name, target, kTargetSize); - if (name == NULL) { - return strdup(""); - } - target_size = strlen(name); - } const char* sep = File::PathSeparator(); const intptr_t sep_length = strlen(sep); diff --git a/runtime/bin/file.h b/runtime/bin/file.h index c0ab0fb0777..9268cc91694 100644 --- a/runtime/bin/file.h +++ b/runtime/bin/file.h @@ -253,12 +253,7 @@ class File : public ReferenceCounted { static StdioHandleType GetStdioHandleType(int fd); // LinkTarget, GetCanonicalPath, and ReadLink may call Dart_ScopeAllocate. - // If dest and its size are provided, Dart String will not be created. - // The result will be populated into dest. - static const char* LinkTarget(Namespace* namespc, - const char* pathname, - char* dest = NULL, - int dest_size = 0); + static const char* LinkTarget(Namespace* namespc, const char* pathname); static const char* GetCanonicalPath(Namespace* namespc, const char* path); // Link LinkTarget, but pathname must be absolute. static const char* ReadLink(const char* pathname); diff --git a/runtime/bin/file_android.cc b/runtime/bin/file_android.cc index 59abc84eeab..a3bb55845a4 100644 --- a/runtime/bin/file_android.cc +++ b/runtime/bin/file_android.cc @@ -552,10 +552,7 @@ static int ReadLinkAt(int dirfd, return syscall(__NR_readlinkat, dirfd, pathname, buf, bufsize); } -const char* File::LinkTarget(Namespace* namespc, - const char* name, - char* dest, - int dest_size) { +const char* File::LinkTarget(Namespace* namespc, const char* name) { NamespaceScope ns(namespc, name); struct stat link_stats; const int status = TEMP_FAILURE_RETRY( @@ -577,17 +574,11 @@ const char* File::LinkTarget(Namespace* namespc, if (target_size <= 0) { return NULL; } - if (dest == NULL) { - dest = DartUtils::ScopedCString(target_size + 1); - } else { - ASSERT(dest_size > 0); - if (dest_size <= target_size) { - return NULL; - } - } - memmove(dest, target, target_size); - dest[target_size] = '\0'; - return dest; + char* target_name = DartUtils::ScopedCString(target_size + 1); + ASSERT(target_name != NULL); + memmove(target_name, target, target_size); + target_name[target_size] = '\0'; + return target_name; } bool File::IsAbsolutePath(const char* pathname) { diff --git a/runtime/bin/file_fuchsia.cc b/runtime/bin/file_fuchsia.cc index 98b6537b032..182b4a99772 100644 --- a/runtime/bin/file_fuchsia.cc +++ b/runtime/bin/file_fuchsia.cc @@ -540,10 +540,7 @@ bool File::SetLastModified(Namespace* namespc, return utimensat(ns.fd(), ns.path(), times, 0) == 0; } -const char* File::LinkTarget(Namespace* namespc, - const char* name, - char* dest, - int dest_size) { +const char* File::LinkTarget(Namespace* namespc, const char* name) { NamespaceScope ns(namespc, name); struct stat link_stats; const int status = TEMP_FAILURE_RETRY( @@ -565,17 +562,11 @@ const char* File::LinkTarget(Namespace* namespc, if (target_size <= 0) { return NULL; } - if (dest == NULL) { - dest = DartUtils::ScopedCString(target_size + 1); - } else { - ASSERT(dest_size > 0); - if (dest_size <= target_size) { - return NULL; - } - } - memmove(dest, target, target_size); - dest[target_size] = '\0'; - return dest; + char* target_name = DartUtils::ScopedCString(target_size + 1); + ASSERT(target_name != NULL); + memmove(target_name, target, target_size); + target_name[target_size] = '\0'; + return target_name; } bool File::IsAbsolutePath(const char* pathname) { diff --git a/runtime/bin/file_linux.cc b/runtime/bin/file_linux.cc index c012f8af489..bf51aba5cc7 100644 --- a/runtime/bin/file_linux.cc +++ b/runtime/bin/file_linux.cc @@ -549,10 +549,7 @@ bool File::SetLastModified(Namespace* namespc, return utimensat(ns.fd(), ns.path(), times, 0) == 0; } -const char* File::LinkTarget(Namespace* namespc, - const char* name, - char* dest, - int dest_size) { +const char* File::LinkTarget(Namespace* namespc, const char* name) { NamespaceScope ns(namespc, name); struct stat64 link_stats; const int status = TEMP_FAILURE_RETRY( @@ -574,17 +571,11 @@ const char* File::LinkTarget(Namespace* namespc, if (target_size <= 0) { return NULL; } - if (dest == NULL) { - dest = DartUtils::ScopedCString(target_size + 1); - } else { - ASSERT(dest_size > 0); - if (dest_size <= target_size) { - return NULL; - } - } - memmove(dest, target, target_size); - dest[target_size] = '\0'; - return dest; + char* target_name = DartUtils::ScopedCString(target_size + 1); + ASSERT(target_name != NULL); + memmove(target_name, target, target_size); + target_name[target_size] = '\0'; + return target_name; } bool File::IsAbsolutePath(const char* pathname) { diff --git a/runtime/bin/file_macos.cc b/runtime/bin/file_macos.cc index 456023c909e..2e132827c2d 100644 --- a/runtime/bin/file_macos.cc +++ b/runtime/bin/file_macos.cc @@ -518,10 +518,7 @@ bool File::SetLastModified(Namespace* namespc, return utime(name, ×) == 0; } -const char* File::LinkTarget(Namespace* namespc, - const char* pathname, - char* dest, - int dest_size) { +const char* File::LinkTarget(Namespace* namespc, const char* pathname) { struct stat link_stats; if (lstat(pathname, &link_stats) != 0) { return NULL; @@ -539,17 +536,11 @@ const char* File::LinkTarget(Namespace* namespc, if (target_size <= 0) { return NULL; } - if (dest == NULL) { - dest = DartUtils::ScopedCString(target_size + 1); - } else { - ASSERT(dest_size > 0); - if ((size_t)dest_size <= target_size) { - return NULL; - } - } - memmove(dest, target, target_size); - dest[target_size] = '\0'; - return dest; + char* target_name = DartUtils::ScopedCString(target_size + 1); + ASSERT(target_name != NULL); + memmove(target_name, target, target_size); + target_name[target_size] = '\0'; + return target_name; } bool File::IsAbsolutePath(const char* pathname) { diff --git a/runtime/bin/file_win.cc b/runtime/bin/file_win.cc index 975bd12184a..f5c6010f9cd 100644 --- a/runtime/bin/file_win.cc +++ b/runtime/bin/file_win.cc @@ -517,10 +517,7 @@ int64_t File::LengthFromPath(Namespace* namespc, const char* name) { return st.st_size; } -const char* File::LinkTarget(Namespace* namespc, - const char* pathname, - char* dest, - int dest_size) { +const char* File::LinkTarget(Namespace* namespc, const char* pathname) { const wchar_t* name = StringUtilsWin::Utf8ToWide(pathname); HANDLE dir_handle = CreateFileW( name, GENERIC_READ, @@ -574,18 +571,13 @@ const char* File::LinkTarget(Namespace* namespc, } int utf8_length = WideCharToMultiByte(CP_UTF8, 0, target, target_length, NULL, 0, NULL, NULL); - if (dest_size > 0 && dest_size <= utf8_length) { - return NULL; - } - if (dest == NULL) { - dest = DartUtils::ScopedCString(utf8_length + 1); - } - if (0 == WideCharToMultiByte(CP_UTF8, 0, target, target_length, dest, + char* utf8_target = DartUtils::ScopedCString(utf8_length + 1); + if (0 == WideCharToMultiByte(CP_UTF8, 0, target, target_length, utf8_target, utf8_length, NULL, NULL)) { return NULL; } - dest[utf8_length] = '\0'; - return dest; + utf8_target[utf8_length] = '\0'; + return utf8_target; } void File::Stat(Namespace* namespc, const char* name, int64_t* data) {