diff --git a/CHANGELOG.md b/CHANGELOG.md index dc58f20e10d..d76e779f1b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ ## 1.20.0 +### Dart VM + +* We have improved the way that the VM locates the native code library for a + native extension (e.g. `dart-ext:` import). We have updated this + [article on native extensions](https://www.dartlang.org/articles/dart-vm/native-extensions) + to reflect the VM's improved behavior. + ### Core library changes * `dart:core`: Remove deprecated `Resource` class. Use the class in `package:resource` instead. diff --git a/runtime/bin/extensions.cc b/runtime/bin/extensions.cc index d319ce771c1..4584d0175d0 100644 --- a/runtime/bin/extensions.cc +++ b/runtime/bin/extensions.cc @@ -9,6 +9,7 @@ #include "bin/dartutils.h" #include "bin/file.h" #include "bin/platform.h" +#include "bin/utils.h" #include "include/dart_api.h" #include "platform/assert.h" #include "platform/globals.h" @@ -16,41 +17,100 @@ namespace dart { namespace bin { -Dart_Handle Extensions::LoadExtension(const char* extension_directory, - const char* extension_name, - Dart_Handle parent_library) { - // For example on Linux: directory/libfoo-arm.so - const char* library_strings[] = { - extension_directory, // directory/ - Platform::LibraryPrefix(), // lib - extension_name, // foo - "-", - Platform::HostArchitecture(), // arm - ".", - Platform::LibraryExtension(), // so - NULL, - }; - const char* library_file = Concatenate(library_strings); - void* library_handle = LoadExtensionLibrary(library_file); - if (library_handle == NULL) { - // Fallback on a library file name that does not specify the host - // architecture. For example on Linux: directory/libfoo.so - const char* fallback_library_strings[] = { - extension_directory, // directory/ - Platform::LibraryPrefix(), // lib - extension_name, // foo +static char PathSeparator() { + const char* sep = File::PathSeparator(); + ASSERT(strlen(sep) == 1); + return sep[0]; +} + + +void* Extensions::MakePathAndResolve(const char* dir, const char* name) { + // First try to find the library with a suffix specifying the architecture. + { + const char* path_components[] = { + dir, + Platform::LibraryPrefix(), + name, + "-", + Platform::HostArchitecture(), // arm ".", Platform::LibraryExtension(), // so NULL, }; - const char* fallback_library_file = Concatenate(fallback_library_strings); - library_handle = LoadExtensionLibrary(fallback_library_file); - if (library_handle == NULL) { - return GetError(); + const char* library_file = Concatenate(path_components); + void* library_handle = LoadExtensionLibrary(library_file); + if (library_handle != NULL) { + return library_handle; } } - const char* strings[] = { extension_name, "_Init", NULL }; + // Fall back on a library name without the suffix. + { + const char* path_components[] = { + dir, + Platform::LibraryPrefix(), + name, + ".", + Platform::LibraryExtension(), // so + NULL, + }; + const char* library_file = Concatenate(path_components); + return LoadExtensionLibrary(library_file); + } +} + + +// IMPORTANT: In the absolute path case, do not extract the filename and search +// for that by passing it to LoadExtensionLibrary. That can lead to confusion in +// which the absolute path is wrong, and a different version of the library is +// loaded from the standard location. +void* Extensions::ResolveAbsPathExtension(const char* extension_path) { + const char* last_slash = strrchr(extension_path, PathSeparator()) + 1; + char* name = strdup(last_slash); + char* dir = StringUtils::StrNDup(extension_path, last_slash - extension_path); + void* library_handle = MakePathAndResolve(dir, name); + free(dir); + free(name); + return library_handle; +} + + +void* Extensions::ResolveExtension(const char* extension_directory, + const char* extension_name) { + // If the path following dart-ext is an absolute path, then only look for the + // library there. + if (File::IsAbsolutePath(extension_name)) { + return ResolveAbsPathExtension(extension_name); + } + + // If the path following dart-ext is just a file name, first look next to + // the importing Dart library. + void* library_handle = MakePathAndResolve(extension_directory, + extension_name); + if (library_handle != NULL) { + return library_handle; + } + + // Then pass the library name down to the platform. E.g. dlopen will do its + // own search in standard search locations. + return MakePathAndResolve("", extension_name); +} + + +Dart_Handle Extensions::LoadExtension(const char* extension_directory, + const char* extension_name, + Dart_Handle parent_library) { + void* library_handle = ResolveExtension(extension_directory, extension_name); + if (library_handle == NULL) { + return GetError(); + } + + const char* extension = extension_name; + if (File::IsAbsolutePath(extension_name)) { + extension = strrchr(extension_name, PathSeparator()) + 1; + } + + const char* strings[] = { extension, "_Init", NULL }; const char* init_function_name = Concatenate(strings); void* init_function = ResolveSymbol(library_handle, init_function_name); Dart_Handle result = GetError(); diff --git a/runtime/bin/extensions.h b/runtime/bin/extensions.h index d2663036381..bcbcac3c7f8 100644 --- a/runtime/bin/extensions.h +++ b/runtime/bin/extensions.h @@ -26,6 +26,11 @@ class Extensions { private: static Dart_Handle GetError(); + static void* MakePathAndResolve(const char* dir, const char* name); + static void* ResolveAbsPathExtension(const char* extension_path); + static void* ResolveExtension(const char* extensioion_directory, + const char* extension_name); + // The returned string is scope allocated. static const char* Concatenate(const char** strings); diff --git a/runtime/bin/loader.cc b/runtime/bin/loader.cc index 122e851a302..4d35728d228 100644 --- a/runtime/bin/loader.cc +++ b/runtime/bin/loader.cc @@ -8,6 +8,7 @@ #include "bin/builtin.h" #include "bin/dartutils.h" #include "bin/extensions.h" +#include "bin/file.h" #include "bin/lockers.h" #include "bin/utils.h" @@ -261,12 +262,10 @@ static bool LibraryHandleError(Dart_Handle library, Dart_Handle error) { } -static bool IsWindowsHost() { -#if defined(TARGET_OS_WINDOWS) - return true; -#else // defined(TARGET_OS_WINDOWS) - return false; -#endif // defined(TARGET_OS_WINDOWS) +static bool PathContainsSeparator(const char* path) { + return (strchr(path, '/') != NULL) || + ((strncmp(File::PathSeparator(), "/", 1) != 0) && + (strstr(path, File::PathSeparator()) != NULL)); } @@ -300,7 +299,6 @@ bool Loader::ProcessResultLocked(Loader* loader, Loader::IOResult* result) { return false; } - if (result->tag == _Dart_kImportExtension) { ASSERT(library_uri != Dart_Null()); Dart_Handle library = Dart_LookupLibrary(library_uri); @@ -320,10 +318,10 @@ bool Loader::ProcessResultLocked(Loader* loader, Loader::IOResult* result) { lib_path = lib_uri; } const char* extension_path = DartUtils::RemoveScheme(extension_uri); - if (strchr(extension_path, '/') != NULL || - (IsWindowsHost() && strchr(extension_path, '\\') != NULL)) { + if (!File::IsAbsolutePath(extension_path) && + PathContainsSeparator(extension_path)) { loader->error_ = DartUtils::NewError( - "Relative paths for dart extensions are not supported: '%s'", + "Native extension path must be absolute, or simply the file name: %s", extension_path); return false; } diff --git a/runtime/bin/utils.h b/runtime/bin/utils.h index ab02ebe647d..5311d361e65 100644 --- a/runtime/bin/utils.h +++ b/runtime/bin/utils.h @@ -84,6 +84,9 @@ class StringUtils { intptr_t len = -1, intptr_t* result_len = NULL); + // Not all platforms support strndup. + static char* StrNDup(const char* s, intptr_t n); + private: DISALLOW_ALLOCATION(); DISALLOW_IMPLICIT_CONSTRUCTORS(StringUtils); diff --git a/runtime/bin/utils_android.cc b/runtime/bin/utils_android.cc index c7a4d63f98c..8c6ad24b3d7 100644 --- a/runtime/bin/utils_android.cc +++ b/runtime/bin/utils_android.cc @@ -71,6 +71,11 @@ char* StringUtils::Utf8ToConsoleString( } +char* StringUtils::StrNDup(const char* s, intptr_t n) { + return strndup(s, n); +} + + bool ShellUtils::GetUtf8Argv(int argc, char** argv) { return false; } diff --git a/runtime/bin/utils_fuchsia.cc b/runtime/bin/utils_fuchsia.cc index 1df2aba4e45..0809723b8a0 100644 --- a/runtime/bin/utils_fuchsia.cc +++ b/runtime/bin/utils_fuchsia.cc @@ -68,6 +68,11 @@ char* StringUtils::Utf8ToConsoleString( } +char* StringUtils::StrNDup(const char* s, intptr_t n) { + return strndup(s, n); +} + + bool ShellUtils::GetUtf8Argv(int argc, char** argv) { return false; } diff --git a/runtime/bin/utils_linux.cc b/runtime/bin/utils_linux.cc index 2029071abeb..8266102a893 100644 --- a/runtime/bin/utils_linux.cc +++ b/runtime/bin/utils_linux.cc @@ -69,6 +69,11 @@ char* StringUtils::Utf8ToConsoleString( } +char* StringUtils::StrNDup(const char* s, intptr_t n) { + return strndup(s, n); +} + + bool ShellUtils::GetUtf8Argv(int argc, char** argv) { return false; } diff --git a/runtime/bin/utils_macos.cc b/runtime/bin/utils_macos.cc index 136881d37f5..2421d2a6a8e 100644 --- a/runtime/bin/utils_macos.cc +++ b/runtime/bin/utils_macos.cc @@ -77,6 +77,11 @@ char* StringUtils::Utf8ToConsoleString( } +char* StringUtils::StrNDup(const char* s, intptr_t n) { + return strndup(s, n); +} + + bool ShellUtils::GetUtf8Argv(int argc, char** argv) { return false; } diff --git a/runtime/bin/utils_win.cc b/runtime/bin/utils_win.cc index 7e3cfb689a7..4b0b1fa01e7 100644 --- a/runtime/bin/utils_win.cc +++ b/runtime/bin/utils_win.cc @@ -159,6 +159,23 @@ const wchar_t* StringUtilsWin::Utf8ToWide( } +char* StringUtils::StrNDup(const char* s, intptr_t n) { + intptr_t len = strlen(s); + if ((n < 0) || (len < 0)) { + return NULL; + } + if (n < len) { + len = n; + } + char* result = reinterpret_cast(malloc(len + 1)); + if (result == NULL) { + return NULL; + } + result[len] = '\0'; + return reinterpret_cast(memmove(result, s, len)); +} + + bool ShellUtils::GetUtf8Argv(int argc, char** argv) { wchar_t* command_line = GetCommandLineW(); int unicode_argc; diff --git a/tests/standalone/io/test_extension_fail_test.dart b/tests/standalone/io/test_extension_fail_test.dart index 3a9d0cfb401..c9893678c11 100644 --- a/tests/standalone/io/test_extension_fail_test.dart +++ b/tests/standalone/io/test_extension_fail_test.dart @@ -35,37 +35,55 @@ String getExtensionPath(String buildDirectory) { } } -void main() { +bool checkExitCode(int code) { + return ((code == 255) || (code == 253)); +} + +bool checkStdError(String err) { + return err.contains("Unhandled exception:") || + err.contains( + "Native extension path must be absolute, or simply the file name"); +} + +// name is either "extension" or "relative_extension" +Future test(String name, bool checkForBall) { String scriptDirectory = dirname(Platform.script.toFilePath()); String buildDirectory = dirname(Platform.executable); Directory tempDirectory = - Directory.systemTemp.createTempSync('dart_test_extension_fail'); + Directory.systemTemp.createTempSync('dart_test_${name}_fail'); String testDirectory = tempDirectory.path; // Copy test_extension shared library, test_extension.dart and // test_extension_fail_tester.dart to the temporary test directory. copyFileToDirectory(getExtensionPath(buildDirectory), testDirectory).then((_) { - var extensionDartFile = join(scriptDirectory, 'test_extension.dart'); + var extensionDartFile = join(scriptDirectory, 'test_${name}.dart'); return copyFileToDirectory(extensionDartFile, testDirectory); }).then((_) { var testExtensionTesterFile = - join(scriptDirectory, 'test_extension_fail_tester.dart'); + join(scriptDirectory, 'test_${name}_fail_tester.dart'); return copyFileToDirectory(testExtensionTesterFile, testDirectory); }).then((_) { - var script = join(testDirectory, 'test_extension_fail_tester.dart'); - return Process.run(Platform.executable, [script]); + var script = join(testDirectory, 'test_${name}_fail_tester.dart'); + return Process.run(Platform.executable, ['--trace-loading', script]); }).then((ProcessResult result) { print("ERR: ${result.stderr}\n\n"); print("OUT: ${result.stdout}\n\n"); - if (result.exitCode != 255) { - throw new StateError("bad exit code"); + if (!checkExitCode(result.exitCode)) { + throw new StateError("bad exit code: ${result.exitCode}"); } - if (!result.stderr.contains("Unhandled exception:")) { + if (!checkStdError(result.stderr)) { throw new StateError("stderr doesn't contain unhandled exception."); } - if (!result.stderr.contains("ball")) { - throw new StateError("stderr doesn't contain 'ball'."); + if (checkForBall) { + if (!result.stderr.contains("ball")) { + throw new StateError("stderr doesn't contain 'ball'."); + } } }).whenComplete(() => tempDirectory.deleteSync(recursive: true)); } + +main() async { + await test("extension", true); + await test("relative_extension", false); +} diff --git a/tests/standalone/io/test_relative_extension.dart b/tests/standalone/io/test_relative_extension.dart new file mode 100644 index 00000000000..79f9489c1c3 --- /dev/null +++ b/tests/standalone/io/test_relative_extension.dart @@ -0,0 +1,20 @@ +// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +library test_extension; + +import "dart-ext:extension/test_extension"; + +class Cat { + Cat(this.x); + + num x; + + String toString() => 'cat $x'; + + // Implements (a != null) ? a : b using a native C++ function and the API. + static int ifNull(a, b) native 'TestExtension_IfNull'; + + static int throwMeTheBall(ball) native 'TestExtension_ThrowMeTheBall'; +} diff --git a/tests/standalone/io/test_relative_extension_fail_tester.dart b/tests/standalone/io/test_relative_extension_fail_tester.dart new file mode 100644 index 00000000000..ad37cf09dd6 --- /dev/null +++ b/tests/standalone/io/test_relative_extension_fail_tester.dart @@ -0,0 +1,21 @@ +// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +library test_extension_test; + +import "dart:async"; +import "dart:isolate"; +import "test_relative_extension.dart"; + +main() { + try { + Cat.throwMeTheBall("ball"); + } on String catch (e) { + if (e != "ball") throw new StateError("exception not equal to 'ball'"); + } + // Make sure the exception is thrown out to the event handler from C++ code. + // The harness expects the string "ball" to be thrown and the process to + // end with an unhandled exception. + Timer.run(() => Cat.throwMeTheBall("ball")); +}