Skip to content

Commit

Permalink
[build] Don't export all symbols to the dynamic table.
Browse files Browse the repository at this point in the history
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.

TEST=runtime/tests/vm/dart/exported_symbols_test.dart
TEST=runtime/tests/vm/dart/symbolized_crash_test.dart
Bug: #53267
Change-Id: I2ee494fba86f67127ba0f6f402622f01a4662207
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323702
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
  • Loading branch information
rmacnak-google authored and Commit Queue committed Sep 5, 2023
1 parent ebc96ec commit b8ee3a9
Show file tree
Hide file tree
Showing 22 changed files with 635 additions and 245 deletions.
9 changes: 4 additions & 5 deletions build/config/BUILDCONFIG.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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 += [
Expand Down
5 changes: 4 additions & 1 deletion build/toolchain/fuchsia/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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,
]
}

Expand Down
13 changes: 12 additions & 1 deletion build/toolchain/gcc_toolchain.gni
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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 ]
}
Expand Down
16 changes: 15 additions & 1 deletion build/toolchain/mac/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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 ]
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
49 changes: 35 additions & 14 deletions runtime/bin/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
19 changes: 19 additions & 0 deletions runtime/bin/exe_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions runtime/bin/exe_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down
2 changes: 2 additions & 0 deletions runtime/bin/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
6 changes: 6 additions & 0 deletions runtime/bin/gen_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions runtime/bin/main_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1152,6 +1153,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;
Expand Down
6 changes: 6 additions & 0 deletions runtime/bin/run_vm_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit b8ee3a9

Please sign in to comment.