[SDK] Fixes loading appended snapshots when executed with PATH.

Loading of appended snapshots used to try read the executable itself
via arg[0] which holds the "path" to the executable.
However, when the executable is being invoked via PATH the "path"
can be just the name of the executable with no actual path.
This would cause the file reading to fail to find the file and
therefore fail to read.
This in turn caused standalone executables to fail to run when invoked
via PATH.

Bug: #38912
Change-Id: I08501661441db90ce6cff96a9337a5770ec3524d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/121853
Commit-Queue: Clement Skau <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>

Author: Clement Skau

runtime/bin/file_android.cc

@@ -597,11 +597,16 @@ intptr_t File::ReadLinkInto(const char* pathname,
     errno = ENOENT;
     return -1;
   }
-  const size_t target_size =
+  size_t target_size =
       TEMP_FAILURE_RETRY(readlink(pathname, result, result_size));
   if (target_size <= 0) {
     return -1;
   }
+  // readlink returns non-zero terminated strings. Append.
+  if (target_size < result_size) {
+    result[target_size] = '\0';
+    target_size++;
+  }
   return target_size;
 }
 
@@ -615,10 +620,9 @@ const char* File::ReadLink(const char* pathname) {
   if (target_size <= 0) {
     return NULL;
   }
-  char* target_name = DartUtils::ScopedCString(target_size + 1);
+  char* target_name = DartUtils::ScopedCString(target_size);
   ASSERT(target_name != NULL);
   memmove(target_name, target, target_size);
-  target_name[target_size] = '\0';
   return target_name;
 }
 

runtime/bin/file_linux.cc

@@ -590,11 +590,16 @@ intptr_t File::ReadLinkInto(const char* pathname,
     errno = ENOENT;
     return -1;
   }
-  const size_t target_size =
+  size_t target_size =
       TEMP_FAILURE_RETRY(readlink(pathname, result, result_size));
   if (target_size <= 0) {
     return -1;
   }
+  // readlink returns non-zero terminated strings. Append.
+  if (target_size < result_size) {
+    result[target_size] = '\0';
+    target_size++;
+  }
   return target_size;
 }
 
@@ -608,10 +613,9 @@ const char* File::ReadLink(const char* pathname) {
   if (target_size <= 0) {
     return NULL;
   }
-  char* target_name = DartUtils::ScopedCString(target_size + 1);
+  char* target_name = DartUtils::ScopedCString(target_size);
   ASSERT(target_name != NULL);
   memmove(target_name, target, target_size);
-  target_name[target_size] = '\0';
   return target_name;
 }
 

runtime/bin/main.cc

@@ -1083,16 +1083,21 @@ void main(int argc, char** argv) {
   // snapshot, load and run that.
   // Any arguments passed to such an executable are meant for the actual
   // application so skip all Dart VM flag parsing.
-  app_snapshot = Snapshot::TryReadAppendedAppSnapshotElf(argv[0]);
-  if (app_snapshot != nullptr) {
-    script_name = argv[0];
 
-    // Store the executable name.
-    Platform::SetExecutableName(argv[0]);
+  const size_t kPathBufSize = PATH_MAX + 1;
+  char executable_path[kPathBufSize];
+  if (Platform::ResolveExecutablePathInto(executable_path, kPathBufSize) > 0) {
+    app_snapshot = Snapshot::TryReadAppendedAppSnapshotElf(executable_path);
+    if (app_snapshot != nullptr) {
+      script_name = argv[0];
+
+      // Store the executable name.
+      Platform::SetExecutableName(argv[0]);
 
-    // Parse out options to be passed to dart main.
-    for (int i = 1; i < argc; i++) {
-      dart_options.AddArgument(argv[i]);
+      // Parse out options to be passed to dart main.
+      for (int i = 1; i < argc; i++) {
+        dart_options.AddArgument(argv[i]);
+      }
     }
   }
 #endif

runtime/bin/platform.h

@@ -64,13 +64,12 @@ class Platform {
 
   static const char* ResolveExecutablePath();
 
-  // On Linux and Android this has the same effect as calling
-  // ResolveExecutablePath except that Dart_ScopeAllocate is not called and that
-  // the result goes into the given parameters.
-  // On all other platforms it returns -1, i.e. doesn't work.
+  // This has the same effect as calling ResolveExecutablePath except that
+  // Dart_ScopeAllocate is not called and that the result goes into the given
+  // parameters.
+  // WARNING: On Fuchsia it returns -1, i.e. doesn't work.
   // Note that `result` should be pre-allocated with size `result_size`.
   // The return-value is the length read into `result` or -1 on failure.
-  // The content of `result` is not guranteed to be null-terminated.
   static intptr_t ResolveExecutablePathInto(char* result, size_t result_size);
 
   // Stores the executable name.

runtime/bin/platform_macos.cc

@@ -252,7 +252,18 @@ const char* Platform::ResolveExecutablePath() {
 }
 
 intptr_t Platform::ResolveExecutablePathInto(char* result, size_t result_size) {
-  return -1;
+  // Get the required length of the buffer.
+  uint32_t path_size = 0;
+  if (_NSGetExecutablePath(nullptr, &path_size) == 0) {
+    return -1;
+  }
+  if (path_size > result_size) {
+    return -1;
+  }
+  if (_NSGetExecutablePath(result, &path_size) != 0) {
+    return -1;
+  }
+  return path_size;
 }
 
 void Platform::Exit(int exit_code) {

runtime/bin/platform_win.cc

@@ -275,7 +275,7 @@ const char* Platform::ResolveExecutablePath() {
   // Ensure no last error before calling GetModuleFileNameW.
   SetLastError(ERROR_SUCCESS);
   // Get the required length of the buffer.
-  int path_length = GetModuleFileNameW(NULL, tmp_buffer, kTmpBufferSize);
+  GetModuleFileNameW(nullptr, tmp_buffer, kTmpBufferSize);
   if (GetLastError() != ERROR_SUCCESS) {
     return NULL;
   }
@@ -286,6 +286,20 @@ const char* Platform::ResolveExecutablePath() {
 }
 
 intptr_t Platform::ResolveExecutablePathInto(char* result, size_t result_size) {
+  // Ensure no last error before calling GetModuleFileNameW.
+  SetLastError(ERROR_SUCCESS);
+  const int kTmpBufferSize = 32768;
+  wchar_t tmp_buffer[kTmpBufferSize];
+  // Get the required length of the buffer.
+  GetModuleFileNameW(nullptr, tmp_buffer, kTmpBufferSize);
+  if (GetLastError() != ERROR_SUCCESS) {
+    return -1;
+  }
+  WideToUtf8Scope wide_to_utf8_scope(tmp_buffer);
+  if (wide_to_utf8_scope.length() <= result_size) {
+    strncpy(result, wide_to_utf8_scope.utf8(), result_size);
+    return wide_to_utf8_scope.length();
+  }
   return -1;
 }
 

runtime/platform/globals.h

@@ -722,6 +722,11 @@ static inline void StoreUnaligned(T* ptr, T value) {
 #define STDERR_FILENO 2
 #endif
 
+#ifndef PATH_MAX
+// Most platforms use PATH_MAX, but in Windows it's called MAX_PATH.
+#define PATH_MAX MAX_PATH
+#endif
+
 }  // namespace dart
 
 #endif  // RUNTIME_PLATFORM_GLOBALS_H_

runtime/tests/vm/dart/run_appended_aot_snapshot_test.dart

@@ -22,9 +22,10 @@ Future<void> main(List<String> args) async {
       'runtime', 'tests', 'vm', 'dart', 'run_appended_aot_snapshot_test.dart');
 
   await withTempDir((String tmp) async {
+    final String exeName = 'test.exe';
     final String dillPath = path.join(tmp, 'test.dill');
     final String aotPath = path.join(tmp, 'test.aot');
-    final String exePath = path.join(tmp, 'test.exe');
+    final String exePath = path.join(tmp, exeName);
 
     {
       final result = await generateAotKernel(checkedInDartVM, genKernel,
@@ -51,8 +52,32 @@ Future<void> main(List<String> args) async {
       Expect.equals(result.stdout, '');
     }
 
-    final runResult =
-        await runBinary('run appended aot snapshot', exePath, ['--child']);
-    expectOutput('Hello, Appended AOT', runResult);
+    {
+      final runResult =
+          await runBinary('run appended aot snapshot', exePath, ['--child']);
+      expectOutput('Hello, Appended AOT', runResult);
+    }
+
+    {
+      // Test that it runs when invoked via PATH as well.
+      Map<String, String> environment = {'PATH': tmp};
+      final runResult = await runBinary(
+          'run appended aot snapshot from PATH', exeName, ['--child'],
+          environment: environment);
+      expectOutput('Hello, Appended AOT', runResult);
+    }
+
+    // Windows allows leaving out .exe. Make sure we can load that as well.
+    if (Platform.isWindows) {
+      final String exeNameWithoutExt =
+          exeName.replaceFirst(new RegExp(r'.exe$'), '');
+      Map<String, String> environment = {'PATH': tmp};
+      final runResult = await runBinary(
+          'run appended aot snapshot without extension',
+          exeNameWithoutExt,
+          ['--child'],
+          environment: environment);
+      expectOutput('Hello, Appended AOT', runResult);
+    }
   });
 }

runtime/tests/vm/dart/snapshot_test_helper.dart

@@ -72,10 +72,11 @@ Future<Result> runGenSnapshot(String prefix, List<String> arguments) {
   return runBinary(prefix, genSnapshot, arguments);
 }
 
-Future<Result> runBinary(
-    String prefix, String binary, List<String> arguments) async {
+Future<Result> runBinary(String prefix, String binary, List<String> arguments,
+    {Map<String, String> environment}) async {
   print("+ $binary " + arguments.join(" "));
-  final processResult = await Process.run(binary, arguments);
+  final processResult =
+      await Process.run(binary, arguments, environment: environment);
   final result =
       new Result('[$prefix] ${binary} ${arguments.join(' ')}', processResult);