[stable][build] Don't export all symbols to the dynamic table.

We were exporting all symbols to the dynamic table so that they could be looked up using `dladdr` for the profiler and backtracer. The symbols include our statically-linked libcxx, which can create trouble when another DSO has a different version of libcxx. Now we export only the VM embedding API functions (`Dart_*`) and use a specially produced table to do the symbolization. Bug: #53267 Change-Id: I50f8150d194a564c116d95383c28a1a2aba0714e Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/323702 Cherry-pick-request: #53503 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/325126 Reviewed-by: Michael Thomsen <mit@google.com> Reviewed-by: Daco Harkes <dacoharkes@google.com> Commit-Queue: Siva Annamalai <asiva@google.com>
dart-lang · Sep 25, 2023 · c05b29e · c05b29e
1 parent 8f28795
commit c05b29e
Show file tree

Hide file tree

Showing 22 changed files with 303 additions and 245 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,7 +6,13 @@ This is a patch release that:
   `@staticInterop` `@anonymous` factory constructors with type parameters (see
   issue [#53579] for more details).
 
+- The standalone Dart VM now exports symbols only for the Dart_* embedding API
+  functions, avoiding conflicts with other DSOs loaded into the same process,
+  such as shared libraries loaded through `dart:ffi`, that may have different
+  versions of the same symbols (issue [#53267]).
+
 [#53579]: https://github.com/dart-lang/sdk/issues/53579
+[#53267]: https://github.com/dart-lang/sdk/issues/53267
 
 ## 3.1.2 - 2023-09-13
 

diff --git a/build/config/BUILDCONFIG.gn b/build/config/BUILDCONFIG.gn
@@ -264,11 +264,10 @@ if (is_win) {
   ]
 }
 if (is_posix) {
-  _native_compiler_configs += [ "//build/config/gcc:relative_paths" ]
-  if (is_product) {
-    _native_compiler_configs +=
-        [ "//build/config/gcc:symbol_visibility_hidden" ]
-  }
+  _native_compiler_configs += [
+    "//build/config/gcc:relative_paths",
+    "//build/config/gcc:symbol_visibility_hidden",
+  ]
 }
 if (is_fuchsia) {
   _native_compiler_configs += [

diff --git a/build/toolchain/fuchsia/BUILD.gn b/build/toolchain/fuchsia/BUILD.gn
@@ -137,18 +137,21 @@ toolchain("fuchsia") {
     exename = "{{target_output_name}}{{output_extension}}"
     outfile = "{{root_out_dir}}/$exename"
     rspfile = "$outfile.rsp"
+    symfile = "$outfile.sym"
+    symbolizer_script = rebase_path("//runtime/tools/dart_profiler_symbols.py")
 
     # Note that the unstripped_outfile is in the exe.stripped folder.
     # We should probably clean this up, but changing this and dart.cmx
     # to point ./dart instead of ./exe.stripped/dart makes the build
     # fail because ./dart does not end up in the manifest file.
     unstripped_outfile = "{{root_out_dir}}/exe.stripped/$exename"
-    command = "$compiler_prefix $ld $target_triple_flags $sysroot_flags $lto_flags {{ldflags}} -o $unstripped_outfile -Wl,--build-id -Wl,--start-group @$rspfile {{solibs}} -Wl,--end-group {{libs}} && ${strip} -o $outfile $unstripped_outfile"
+    command = "$compiler_prefix $ld $target_triple_flags $sysroot_flags $lto_flags {{ldflags}} -o $unstripped_outfile -Wl,--build-id -Wl,--start-group @$rspfile {{solibs}} -Wl,--end-group {{libs}} && ${strip} -o $outfile $unstripped_outfile && $symbolizer_script --nm $nm --output $symfile --binary $unstripped_outfile"
     description = "LINK $outfile"
     rspfile_content = "{{inputs}}"
     outputs = [
       unstripped_outfile,
       outfile,
+      symfile,
     ]
   }
 

diff --git a/build/toolchain/gcc_toolchain.gni b/build/toolchain/gcc_toolchain.gni
@@ -191,12 +191,20 @@ template("gcc_toolchain") {
       exename = "{{target_output_name}}{{output_extension}}"
       outfile = "{{root_out_dir}}/$exename"
       rspfile = "$outfile.rsp"
+      symfile = "$outfile.sym"
 
       if (defined(invoker.strip) || defined(invoker.llvm_objcopy)) {
         stripped_outfile = "{{root_out_dir}}/exe.stripped/$exename"
       }
 
       command = "$ld {{ldflags}} -o $outfile -Wl,--start-group @$rspfile {{solibs}} -Wl,--end-group $libs_section_prefix {{libs}} $libs_section_postfix"
+
+      symbolizer_script =
+          rebase_path("//runtime/tools/dart_profiler_symbols.py")
+      symbolize_command =
+          "$symbolizer_script --nm $nm --output $symfile --binary $outfile"
+      command += " && $symbolize_command"
+
       if (defined(invoker.strip)) {
         strip = invoker.strip
         strip_command =
@@ -212,7 +220,10 @@ template("gcc_toolchain") {
       }
       description = "LINK $outfile"
       rspfile_content = "{{inputs}}"
-      outputs = [ outfile ]
+      outputs = [
+        outfile,
+        symfile,
+      ]
       if (defined(invoker.strip) || defined(invoker.llvm_objcopy)) {
         outputs += [ stripped_outfile ]
       }

diff --git a/build/toolchain/mac/BUILD.gn b/build/toolchain/mac/BUILD.gn
@@ -49,6 +49,7 @@ template("mac_toolchain") {
     cxx = invoker.cxx
     ar = invoker.ar
     ld = invoker.ld
+    nm = invoker.nm
 
     # Make these apply to all tools below.
     lib_switch = "-l"
@@ -175,12 +176,20 @@ template("mac_toolchain") {
       exename = "{{target_output_name}}{{output_extension}}"
       outfile = "{{root_out_dir}}/$exename"
       rspfile = "$outfile.rsp"
+      symfile = "$outfile.sym"
 
       if (defined(invoker.strip)) {
         stripped_outfile = "{{root_out_dir}}/exe.stripped/$exename"
       }
 
       command = "$ld $sysroot_flags $toolchain_flags {{ldflags}} -Xlinker -rpath -Xlinker @executable_path/Frameworks -o $outfile -Wl,-filelist,$rspfile {{solibs}} {{libs}} {{frameworks}}"
+
+      symbolizer_script =
+          rebase_path("//runtime/tools/dart_profiler_symbols.py")
+      symbolize_command =
+          "$symbolizer_script --nm $nm --output $symfile --binary $outfile"
+      command += " && $symbolize_command"
+
       if (defined(invoker.strip)) {
         strip = invoker.strip
         strip_command = "${strip} -x -o $stripped_outfile $outfile"
@@ -189,7 +198,10 @@ template("mac_toolchain") {
 
       description = "LINK $outfile"
       rspfile_content = "{{inputs_newline}}"
-      outputs = [ outfile ]
+      outputs = [
+        outfile,
+        symfile,
+      ]
       if (defined(invoker.strip)) {
         outputs += [ stripped_outfile ]
       }
@@ -229,6 +241,7 @@ mac_toolchain("clang_x64") {
   ar = "${prefix}/llvm-ar"
   ld = cxx
   strip = "${prefix}/llvm-strip"
+  nm = "${prefix}/llvm-nm"
   is_clang = true
   if (mac_enable_relative_sdk_path) {
     mac_sdk_path = rebase_path(mac_sdk_path, root_build_dir)
@@ -245,6 +258,7 @@ mac_toolchain("clang_arm64") {
   ar = "${prefix}/llvm-ar"
   ld = cxx
   strip = "${prefix}/llvm-strip"
+  nm = "${prefix}/llvm-nm"
   is_clang = true
   if (mac_enable_relative_sdk_path) {
     mac_sdk_path = rebase_path(mac_sdk_path, root_build_dir)

diff --git a/runtime/bin/BUILD.gn b/runtime/bin/BUILD.gn
@@ -204,11 +204,18 @@ template("build_gen_snapshot") {
       ]
     }
 
-    if (!is_win) {
-      # Adds all symbols to the dynamic symbol table, not just used ones.
-      # This is needed to make native extensions work. It is also needed to get
-      # symbols in VM-generated backtraces and profiles.
-      ldflags = [ "-rdynamic" ]
+    if (is_mac) {
+      ldflags = [
+        "-Wl,-exported_symbol",
+        "-Wl,_Dart_*",
+      ]
+    } else if (!is_win) {
+      if (!is_clang) {
+        # TODO(rmacnak): Remove once bots are updated to a newer gcc.
+        ldflags = [ "-rdynamic" ]
+      } else {
+        ldflags = [ "-Wl,--export-dynamic-symbol=Dart_*" ]
+      }
     }
 
     if (is_win) {
@@ -793,11 +800,18 @@ template("dart_executable") {
 
     if (is_win) {
       ldflags = [ "/EXPORT:Dart_True" ]
+    } else if (is_mac) {
+      ldflags = [
+        "-Wl,-exported_symbol",
+        "-Wl,_Dart_*",
+      ]
     } else {
-      # Adds all symbols to the dynamic symbol table, not just used ones.
-      # This is needed to make native extensions work. It is also needed to get
-      # symbols in VM-generated backtraces and profiles.
-      ldflags = [ "-rdynamic" ]
+      if (!is_clang) {
+        # TODO(rmacnak): Remove once bots are updated to a newer gcc.
+        ldflags = [ "-rdynamic" ]
+      } else {
+        ldflags = [ "-Wl,--export-dynamic-symbol=Dart_*" ]
+      }
     }
 
     ldflags += extra_ldflags
@@ -1018,11 +1032,18 @@ executable("run_vm_tests") {
             ] + builtin_impl_tests + vm_tests + compiler_tests + heap_tests +
             io_impl_tests
 
-  if (!is_win) {
-    # Adds all symbols to the dynamic symbol table, not just used ones.
-    # This is needed to make native extensions work. It is also needed to get
-    # symbols in VM-generated backtraces and profiles.
-    ldflags = [ "-rdynamic" ]
+  if (is_mac) {
+    ldflags = [
+      "-Wl,-exported_symbol",
+      "-Wl,_Dart_*",
+    ]
+  } else if (!is_win) {
+    if (!is_clang) {
+      # TODO(rmacnak): Remove once bots are updated to a newer gcc.
+      ldflags = [ "-rdynamic" ]
+    } else {
+      ldflags = [ "-Wl,--export-dynamic-symbol=Dart_*" ]
+    }
   }
 
   if (is_win) {

diff --git a/runtime/bin/exe_utils.cc b/runtime/bin/exe_utils.cc
@@ -121,5 +121,24 @@ Utils::CStringUniquePtr EXEUtils::GetDirectoryPrefixFromExeName() {
                                                          : result);
 }
 
+#if !defined(DART_HOST_OS_WINDOWS)
+void EXEUtils::LoadDartProfilerSymbols(const char* exepath) {
+  int len = strlen(exepath);
+  char* sympath = reinterpret_cast<char*>(malloc(len + 5));
+  memcpy(sympath, exepath, len);     // NOLINT
+  memcpy(sympath + len, ".sym", 5);  // NOLINT
+  File* file = File::Open(nullptr, sympath, File::kRead);
+  free(sympath);
+  if (file != nullptr) {
+    int64_t size = file->Length();
+    MappedMemory* mapping = file->Map(File::kReadOnly, 0, size);
+    Dart_AddSymbols(exepath, mapping->address(), size);
+    mapping->Leak();  // Let us delete the object but keep the mapping.
+    delete mapping;
+    file->Release();
+  }
+}
+#endif  // !defined(DART_HOST_OS_WINDOWS)
+
 }  // namespace bin
 }  // namespace dart
diff --git a/runtime/bin/exe_utils.h b/runtime/bin/exe_utils.h
@@ -20,6 +20,12 @@ class EXEUtils {
   // Returns the path to the directory the current executable resides in.
   static Utils::CStringUniquePtr GetDirectoryPrefixFromExeName();
 
+#if !defined(DART_HOST_OS_WINDOWS)
+  // Loads a compact symbolization table from "$exepath.sym" that is used by the
+  // VM's profiler and crash stack trace dumper to symbolize C frames.
+  static void LoadDartProfilerSymbols(const char* exepath);
+#endif
+
  private:
   DISALLOW_COPY_AND_ASSIGN(EXEUtils);
 };

diff --git a/runtime/bin/file.h b/runtime/bin/file.h
@@ -35,6 +35,8 @@ class MappedMemory {
   intptr_t size() const { return size_; }
   uword start() const { return reinterpret_cast<uword>(address()); }
 
+  void Leak() { should_unmap_ = false; }
+
  private:
   void Unmap();
 

diff --git a/runtime/bin/gen_snapshot.cc b/runtime/bin/gen_snapshot.cc
@@ -17,6 +17,7 @@
 #include "bin/dartutils.h"
 #include "bin/error_exit.h"
 #include "bin/eventhandler.h"
+#include "bin/exe_utils.h"
 #include "bin/file.h"
 #include "bin/loader.h"
 #include "bin/options.h"
@@ -844,6 +845,11 @@ static int CreateIsolateAndSnapshot(const CommandLineOptions& inputs) {
 }
 
 int main(int argc, char** argv) {
+#if !defined(DART_HOST_OS_WINDOWS)
+  // Very early so any crashes during startup can also be symbolized.
+  EXEUtils::LoadDartProfilerSymbols(argv[0]);
+#endif
+
   const int EXTRA_VM_ARGUMENTS = 7;
   CommandLineOptions vm_options(argc + EXTRA_VM_ARGUMENTS);
   CommandLineOptions inputs(argc);

diff --git a/runtime/bin/main_impl.cc b/runtime/bin/main_impl.cc
@@ -19,6 +19,7 @@
 #include "bin/dfe.h"
 #include "bin/error_exit.h"
 #include "bin/eventhandler.h"
+#include "bin/exe_utils.h"
 #include "bin/file.h"
 #include "bin/gzip.h"
 #include "bin/isolate_data.h"
@@ -1148,6 +1149,11 @@ static Dart_GetVMServiceAssetsArchive GetVMServiceAssetsArchiveCallback =
 #endif  // !defined(PRODUCT)
 
 void main(int argc, char** argv) {
+#if !defined(DART_HOST_OS_WINDOWS)
+  // Very early so any crashes during startup can also be symbolized.
+  EXEUtils::LoadDartProfilerSymbols(argv[0]);
+#endif
+
   char* script_name = nullptr;
   // Allows the dartdev process to point to the desired package_config.
   char* package_config_override = nullptr;

diff --git a/runtime/bin/run_vm_tests.cc b/runtime/bin/run_vm_tests.cc
@@ -7,6 +7,7 @@
 #include "bin/dartutils.h"
 #include "bin/dfe.h"
 #include "bin/eventhandler.h"
+#include "bin/exe_utils.h"
 #include "bin/file.h"
 #include "bin/loader.h"
 #include "bin/platform.h"
@@ -293,6 +294,11 @@ void ShiftArgs(int* argc, const char** argv) {
 }
 
 static int Main(int argc, const char** argv) {
+#if !defined(DART_HOST_OS_WINDOWS)
+  // Very early so any crashes during startup can also be symbolized.
+  bin::EXEUtils::LoadDartProfilerSymbols(argv[0]);
+#endif
+
   // Flags being passed to the Dart VM.
   int dart_argc = 0;
   const char** dart_argv = nullptr;

diff --git a/runtime/tests/vm/dart/symbolized_crash_test.dart b/runtime/tests/vm/dart/symbolized_crash_test.dart
@@ -0,0 +1,30 @@
+// Copyright (c) 2023, 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.
+
+import "dart:io";
+
+import 'package:expect/expect.dart';
+import 'package:path/path.dart' as path;
+
+main() {
+  if (Platform.isWindows) return; // posix exit codes
+  if (Platform.isAndroid) return; // run_vm_tests not available on test device
+
+  var run_vm_tests =
+      path.join(path.dirname(Platform.resolvedExecutable), "run_vm_tests");
+  var result = Process.runSync(run_vm_tests, ["Fatal"]);
+  print(result.exitCode);
+  print(result.stdout);
+  print(result.stderr);
+
+  Expect.contains(
+      "error: This test fails and produces a backtrace", result.stderr);
+
+  // Check for the frames that are marked never inline or have their address
+  // taken, and so should be stable to changes in the C compiler. There are of
+  // course more frames.
+  Expect.contains("dart::Assert::Fail", result.stderr);
+  Expect.contains("Dart_TestFatal", result.stderr);
+  Expect.contains("main", result.stderr);
+}