From da3f5dd4ecf4faaba466ba41c7c30ba4f8f73bfd Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Sat, 11 Nov 2023 17:59:54 +0900 Subject: [PATCH] Fix --dynamic-list for DSOs --dynamic-list, --export-dynamic-symbol and --export-dynamic-symbol-list have different semantics for executables and DSOs. If the output is an executable, they specify a list of symbols that are to be exported. If the output is a shared object, they specify the list of symbols that are to be interposable. mold havne't implemented the latter semantics. This commit fixes that issue. Fixes https://github.com/rui314/mold/issues/1071 --- elf/cmdline.cc | 25 +++----- elf/linker-script.cc | 35 ++++++----- elf/main.cc | 16 ----- elf/mold.h | 17 +++-- elf/output-chunks.cc | 8 ++- elf/passes.cc | 122 +++++++++++++++++++++++++++++------- test/elf/dynamic-list4.sh | 44 +++++++++++++ test/elf/version-script6.sh | 4 +- 8 files changed, 194 insertions(+), 77 deletions(-) create mode 100755 test/elf/dynamic-list4.sh diff --git a/elf/cmdline.cc b/elf/cmdline.cc index 6bc13a3002..6c06b4b143 100644 --- a/elf/cmdline.cc +++ b/elf/cmdline.cc @@ -1104,21 +1104,21 @@ std::vector parse_nonpositional_args(Context &ctx) { } else if (read_flag("no-keep-memory")) { } else if (read_arg("max-cache-size")) { } else if (read_arg("version-script")) { - // --version-script, --dynamic-list and --export-dynamic-symbol[-list] - // are treated as positional arguments even though they are actually not - // positional. This is because linker scripts (a positional argument) - // can also specify a version script, and it's better to consolidate - // parsing in read_input_files. In particular, version scripts can - // modify ctx.default_version which we initialize *after* parsing - // non-positional args, so the parsing cannot be done right here. + // --version-script is treated as positional arguments even though + // they are actually not positional. This is because linker scripts + // (a positional argument) can also specify a version script, and + // it's better to consolidate parsing in read_input_files. In + // particular, version scripts can modify ctx.default_version which + // we initialize *after* parsing non-positional args, so the parsing + // cannot be done right here. remaining.push_back("--version-script=" + std::string(arg)); } else if (read_arg("dynamic-list")) { ctx.arg.Bsymbolic = true; - remaining.push_back("--dynamic-list=" + std::string(arg)); + append(ctx.dynamic_list_patterns, parse_dynamic_list(ctx, arg)); } else if (read_arg("export-dynamic-symbol")) { - remaining.push_back("--export-dynamic-symbol=" + std::string(arg)); + ctx.dynamic_list_patterns.push_back({arg, ""}); } else if (read_arg("export-dynamic-symbol-list")) { - remaining.push_back("--export-dynamic-symbol-list=" + std::string(arg)); + append(ctx.dynamic_list_patterns, parse_dynamic_list(ctx, arg)); } else if (read_flag("as-needed")) { remaining.push_back("--as-needed"); } else if (read_flag("no-as-needed")) { @@ -1228,11 +1228,6 @@ std::vector parse_nonpositional_args(Context &ctx) { if (char *env = getenv("MOLD_REPRO"); env && env[0]) ctx.arg.repro = true; - if (ctx.arg.shared || ctx.arg.export_dynamic) - ctx.default_version = VER_NDX_GLOBAL; - else - ctx.default_version = VER_NDX_LOCAL; - if (ctx.arg.default_symver) { std::string ver = ctx.arg.soname.empty() ? filepath(ctx.arg.output).filename().string() : std::string(ctx.arg.soname); diff --git a/elf/linker-script.cc b/elf/linker-script.cc index 4bdc19e7c0..7ad500bb80 100644 --- a/elf/linker-script.cc +++ b/elf/linker-script.cc @@ -312,7 +312,6 @@ read_version_script_commands(Context &ctx, std::span &tok, if (tok[0] == "*") { ctx.default_version = (is_global ? ver_idx : (u32)VER_NDX_LOCAL); - ctx.default_version_from_version_script = true; } else if (is_global) { ctx.version_patterns.push_back({unquote(tok[0]), current_file->name, ver_str, ver_idx, is_cpp}); @@ -367,7 +366,9 @@ void parse_version_script(Context &ctx, MappedFile> *mf) { } template -void read_dynamic_list_commands(Context &ctx, std::span &tok, +void read_dynamic_list_commands(Context &ctx, + std::vector &result, + std::span &tok, bool is_cpp) { while (!tok.empty() && tok[0] != "}") { if (tok[0] == "extern") { @@ -376,11 +377,11 @@ void read_dynamic_list_commands(Context &ctx, std::span &to if (!tok.empty() && tok[0] == "\"C\"") { tok = tok.subspan(1); tok = skip(ctx, tok, "{"); - read_dynamic_list_commands(ctx, tok, false); + read_dynamic_list_commands(ctx, result, tok, false); } else { tok = skip(ctx, tok, "\"C++\""); tok = skip(ctx, tok, "{"); - read_dynamic_list_commands(ctx, tok, true); + read_dynamic_list_commands(ctx, result, tok, true); } tok = skip(ctx, tok, "}"); @@ -388,29 +389,32 @@ void read_dynamic_list_commands(Context &ctx, std::span &to continue; } - if (tok[0] == "*") - ctx.default_version = VER_NDX_GLOBAL; - else - ctx.version_patterns.push_back({unquote(tok[0]), current_file->name, - "global", VER_NDX_GLOBAL, is_cpp}); - + result.push_back({unquote(tok[0]), "", is_cpp}); tok = skip(ctx, tok.subspan(1), ";"); } } template -void parse_dynamic_list(Context &ctx, MappedFile> *mf) { - current_file = mf; - std::vector vec = tokenize(ctx, mf->get_contents()); +std::vector +parse_dynamic_list(Context &ctx, std::string_view path) { + std::string_view contents = + MappedFile>::must_open(ctx, std::string(path))->get_contents(); + std::vector vec = tokenize(ctx, contents); std::span tok = vec; + std::vector result; tok = skip(ctx, tok, "{"); - read_dynamic_list_commands(ctx, tok, false); + read_dynamic_list_commands(ctx, result, tok, false); tok = skip(ctx, tok, "}"); tok = skip(ctx, tok, ";"); if (!tok.empty()) SyntaxError(ctx, tok[0]) << "trailing garbage token"; + + for (DynamicPattern &p : result) + p.source = path; + + return result; } using E = MOLD_TARGET; @@ -418,6 +422,7 @@ using E = MOLD_TARGET; template void parse_linker_script(Context &, MappedFile> *); template std::string_view get_script_output_type(Context &, MappedFile> *); template void parse_version_script(Context &, MappedFile> *); -template void parse_dynamic_list(Context &, MappedFile> *); +template std::vector parse_dynamic_list(Context &, std::string_view); + } // namespace mold::elf diff --git a/elf/main.cc b/elf/main.cc index c4f3cd6ffd..6df00cfe9b 100644 --- a/elf/main.cc +++ b/elf/main.cc @@ -299,22 +299,6 @@ static void read_input_files(Context &ctx, std::span args) { if (!mf) Fatal(ctx) << "--version-script: file not found: " << arg; parse_version_script(ctx, mf); - } else if (remove_prefix(arg, "--dynamic-list=")) { - MappedFile> *mf = find_from_search_paths(ctx, std::string(arg)); - if (!mf) - Fatal(ctx) << "--dynamic-list: file not found: " << arg; - parse_dynamic_list(ctx, mf); - } else if (remove_prefix(arg, "--export-dynamic-symbol=")) { - if (arg == "*") - ctx.default_version = VER_NDX_GLOBAL; - else - ctx.version_patterns.push_back({arg, "--export-dynamic-symbol", - "global", VER_NDX_GLOBAL, false}); - } else if (remove_prefix(arg, "--export-dynamic-symbol-list=")) { - MappedFile> *mf = find_from_search_paths(ctx, std::string(arg)); - if (!mf) - Fatal(ctx) << "--export-dynamic-symbol-list: file not found: " << arg; - parse_dynamic_list(ctx, mf); } else if (arg == "--push-state") { state.push_back({ctx.as_needed, ctx.whole_archive, ctx.is_static, ctx.in_lib}); diff --git a/elf/mold.h b/elf/mold.h index 7ff7c2e6f9..d593f68402 100644 --- a/elf/mold.h +++ b/elf/mold.h @@ -1281,8 +1281,15 @@ get_script_output_type(Context &ctx, MappedFile> *mf); template void parse_version_script(Context &ctx, MappedFile> *mf); +struct DynamicPattern { + std::string_view pattern; + std::string_view source; + bool is_cpp = false; +}; + template -void parse_dynamic_list(Context &ctx, MappedFile> *mf); +std::vector +parse_dynamic_list(Context &ctx, std::string_view path); // // lto.cc @@ -1733,13 +1740,11 @@ struct Context { } arg; std::vector version_patterns; - u16 default_version = VER_NDX_GLOBAL; + std::vector dynamic_list_patterns; + i64 default_version = -1; i64 page_size = E::page_size; std::optional global_lock_fd; - // true if default_version is set by a wildcard in version script. - bool default_version_from_version_script = false; - // Reader context bool as_needed = false; bool whole_archive = false; @@ -2034,7 +2039,7 @@ class Symbol { i32 sym_idx = -1; i32 aux_idx = -1; - u16 ver_idx = 0; + i32 ver_idx = -1; // `flags` has NEEDS_ flags. Atomic flags = 0; diff --git a/elf/output-chunks.cc b/elf/output-chunks.cc index f44d448ac2..00d5538dfd 100644 --- a/elf/output-chunks.cc +++ b/elf/output-chunks.cc @@ -2550,8 +2550,12 @@ void VerdefSection::construct(Context &ctx) { for (std::string_view verstr : ctx.arg.version_definitions) write(verstr, idx++, 0); - for (Symbol *sym : std::span *>(ctx.dynsym->symbols).subspan(1)) - ctx.versym->contents[sym->get_dynsym_idx(ctx)] = sym->ver_idx; + for (Symbol *sym : std::span *>(ctx.dynsym->symbols).subspan(1)) { + i64 ver = sym->ver_idx; + if (ver == -1) + ver = VER_NDX_GLOBAL; + ctx.versym->contents[sym->get_dynsym_idx(ctx)] = ver; + } } template diff --git a/elf/passes.cc b/elf/passes.cc index c6ee0f66b9..8c7d5d0f51 100644 --- a/elf/passes.cc +++ b/elf/passes.cc @@ -1612,9 +1612,6 @@ template void apply_version_script(Context &ctx) { Timer t(ctx, "apply_version_script"); - // If all patterns are simple (i.e. not containing any meta- - // characters and is not a C++ name), we can simply look up - // symbols. auto is_simple = [&] { for (VersionPattern &v : ctx.version_patterns) if (v.is_cpp || v.pattern.find_first_of("*?[") != v.pattern.npos) @@ -1622,6 +1619,9 @@ void apply_version_script(Context &ctx) { return true; }; + // If all patterns are simple (i.e. not containing any meta- + // characters and is not a C++ name), we can simply look up + // symbols. if (is_simple()) { for (VersionPattern &v : ctx.version_patterns) { Symbol *sym = get_symbol(ctx, v.pattern); @@ -1747,44 +1747,124 @@ void compute_import_export(Context &ctx) { if (!ctx.arg.shared) { tbb::parallel_for_each(ctx.dsos, [&](SharedFile *file) { for (Symbol *sym : file->symbols) { - if (sym->file && !sym->file->is_dso && sym->visibility != STV_HIDDEN) { - if (sym->ver_idx != VER_NDX_LOCAL || - !ctx.default_version_from_version_script) { - std::scoped_lock lock(sym->mu); - sym->is_exported = true; - } + if (sym->file && !sym->file->is_dso && sym->visibility != STV_HIDDEN && + sym->ver_idx != VER_NDX_LOCAL) { + std::scoped_lock lock(sym->mu); + sym->is_exported = true; } } }); } + auto should_export = [&](Symbol &sym) { + if (sym.visibility == STV_HIDDEN) + return false; + + switch (sym.ver_idx) { + case -1: + if (ctx.arg.shared) + return !((ObjectFile *)sym.file)->exclude_libs; + return ctx.arg.export_dynamic; + case VER_NDX_LOCAL: + return false; + default: + return true; + } + }; + // Export symbols that are not hidden or marked as local. // We also want to mark imported symbols as such. tbb::parallel_for_each(ctx.objs, [&](ObjectFile *file) { for (Symbol *sym : file->get_global_syms()) { - if (!sym->file || sym->visibility == STV_HIDDEN || - sym->ver_idx == VER_NDX_LOCAL) - continue; - - // If we are using a symbol in a DSO, we need to import it at runtime. - if (sym->file != file && sym->file->is_dso && !sym->is_absolute()) { - std::scoped_lock lock(sym->mu); - sym->is_imported = true; + // If we are using a symbol in a DSO, we need to import it. + if (sym->file && sym->file->is_dso) { + if (!sym->is_absolute()) { + std::scoped_lock lock(sym->mu); + sym->is_imported = true; + } continue; } - // If we are creating a DSO, all global symbols are exported by default. - if (sym->file == file) { - std::scoped_lock lock(sym->mu); + // If we have a definition of a symbol, we may want to export it. + if (sym->file == file && should_export(*sym)) { sym->is_exported = true; - if (ctx.arg.shared && sym->visibility != STV_PROTECTED && + // Exported symbols are marked as imported as well by default + // for DSOs. + if (ctx.arg.shared && + sym->visibility != STV_PROTECTED && !ctx.arg.Bsymbolic && !(ctx.arg.Bsymbolic_functions && sym->get_type() == STT_FUNC)) sym->is_imported = true; } } }); + + + // Apply --dynamic-list, --export-dynamic-symbol and + // --export-dynamic-symbol-list options. + // + // The semantics of these options vary depending on whether we are + // creating an executalbe or a shared object. + // + // For executable, matched symbols are exported. + // + // For shared objects, matched symbols are imported if it is already + // exported so that they are interposable. In other words, symbols + // that did not match will be bound locally within the output file, + // effectively turning them into protected symbols. + MultiGlob matcher; + MultiGlob cpp_matcher; + + auto handle_match = [&](Symbol *sym) { + if (ctx.arg.shared) { + if (sym->is_exported) + sym->is_imported = true; + } else { + if (sym->file && !sym->file->is_dso && sym->visibility != STV_HIDDEN) + sym->is_exported = true; + } + }; + + for (DynamicPattern &p : ctx.dynamic_list_patterns) { + if (p.is_cpp) { + if (!cpp_matcher.add(p.pattern, 1)) + Fatal(ctx) << p.source << ": invalid dynamic list entry: " + << p.pattern; + continue; + } + + if (p.pattern.find_first_of("*?[") != p.pattern.npos) { + if (!matcher.add(p.pattern, 1)) + Fatal(ctx) << p.source << ": invalid dynamic list entry: " + << p.pattern; + continue; + } + + handle_match(get_symbol(ctx, p.pattern)); + } + + if (!matcher.empty() || !cpp_matcher.empty()) { + tbb::parallel_for_each(ctx.objs, [&](ObjectFile *file) { + for (Symbol *sym : file->get_global_syms()) { + if (sym->file != file) + continue; + if (ctx.arg.shared && !sym->is_exported) + continue; + + std::string_view name = sym->name(); + + if (matcher.find(name)) { + handle_match(sym); + } else if (!cpp_matcher.empty()) { + if (std::optional s = cpp_demangle(name)) + name = *s; + if (cpp_matcher.find(name)) + handle_match(sym); + } + } + }); + } } // Compute the "address-taken" bit for each input section. diff --git a/test/elf/dynamic-list4.sh b/test/elf/dynamic-list4.sh new file mode 100755 index 0000000000..83d88887e5 --- /dev/null +++ b/test/elf/dynamic-list4.sh @@ -0,0 +1,44 @@ +#!/bin/bash +. $(dirname $0)/common.inc + +cat < + +void foo() { printf("foo1 "); } +void bar() { printf("bar1 "); } +void baz() { printf("baz1 "); } + +void print() { + foo(); + bar(); + baz(); + printf("\n"); +} +EOF + +cat < $t/dyn +{ foo; bar; }; +EOF + +$CC -B. -shared -o $t/b.so $t/a.o -Wl,--dynamic-list=$t/dyn + +cat < +void foo() { printf("foo2 "); } +void bar() { printf("bar2 "); } +void baz() { printf("baz2 "); } +EOF + +$CC -B. -shared -o $t/d.so $t/c.o + +cat < +void print(); +int main() { print(); } +EOF + +$CC -B. -o $t/exe1 $t/e.o -Wl,-push-state,-no-as-needed $t/b.so -Wl,-pop-state +$QEMU $t/exe1 | grep -q 'foo1 bar1 baz1' + +$CC -B. -o $t/exe2 $t/e.o -Wl,-push-state,-no-as-needed $t/d.so $t/b.so -Wl,-pop-state +$QEMU $t/exe2 | grep -q 'foo2 bar2 baz1' diff --git a/test/elf/version-script6.sh b/test/elf/version-script6.sh index 74e2f9a899..44f809ef31 100755 --- a/test/elf/version-script6.sh +++ b/test/elf/version-script6.sh @@ -9,10 +9,10 @@ EOF cat < $t/d.ver VER_Y1 { local; *; };