gold: --export-dynamic-symbol: don't imply -u

Message ID 20200609223414.149502-1-maskray@google.com
State New
Headers show
Series
  • gold: --export-dynamic-symbol: don't imply -u
Related show

Commit Message

Cui, Lili via Binutils June 9, 2020, 10:34 p.m.
to match GNU ld.

gold/

	* archive.cc (Library_base::should_include_member): Don't handle
	--export-dynamic-symbol.
	* symtab.cc (Symbol_table::do_add_undefined_symbols_from_command_line):
	Likewise.
---
 gold/archive.cc | 7 -------
 gold/symtab.cc  | 6 ------
 2 files changed, 13 deletions(-)

-- 
2.27.0.278.ge193c7cf3a9-goog

Comments

Cui, Lili via Binutils June 14, 2020, 12:22 a.m. | #1
> to match GNU ld.

>

> gold/

>

>         * archive.cc (Library_base::should_include_member): Don't handle

>         --export-dynamic-symbol.

>         * symtab.cc (Symbol_table::do_add_undefined_symbols_from_command_line):

>         Likewise.


You say we're doing this to match Gnu ld, but Gnu ld didn't implement
this option until several years after it was added to Gold. Shouldn't
the burden of compatibility be in the other direction? I'm not
comfortable changing the behavior of --export-dynamic-symbol like
this, and my memory isn't good enough to remember what the rationale
was for making it work like -u -- whether it was just "it seems like
it makes sense" or "there's a specific need for it to work that way".

-cary
Cui, Lili via Binutils June 14, 2020, 1:08 a.m. | #2
On Sat, Jun 13, 2020 at 5:23 PM Cary Coutant <ccoutant@gmail.com> wrote:
>

> > to match GNU ld.

> >

> > gold/

> >

> >         * archive.cc (Library_base::should_include_member): Don't handle

> >         --export-dynamic-symbol.

> >         * symtab.cc (Symbol_table::do_add_undefined_symbols_from_command_line):

> >         Likewise.

>

> You say we're doing this to match Gnu ld, but Gnu ld didn't implement

> this option until several years after it was added to Gold. Shouldn't

> the burden of compatibility be in the other direction? I'm not

> comfortable changing the behavior of --export-dynamic-symbol like

> this, and my memory isn't good enough to remember what the rationale

> was for making it work like -u -- whether it was just "it seems like

> it makes sense" or "there's a specific need for it to work that way".

>

> -cary


I did try to be compatible with Gold for my initial patch but I
quickly realized not implying -u makes more sense.
https://sourceware.org/pipermail/binutils/2020-May/110953.html

This option does not appear to have open source uses (according to
Debian Code Search) since GNU ld did not implement it until a few days
ago.
The updated semantics did cause some trouble to me - I had to fix a
few --export-dynamic-symbol= uses for my company. Everything is good
now.

My conjecture about the design of implied -u: we do ugly things like
--export-dynamic-symbol=_Unwind_Resume (and a few other symbols), so
that the executable exposes unwind symbols
which can prevent glibc's ugly dlopen("libgcc.so.1") . If
--export-dynamic-symbol= implies -u, things are easier:

we can write: -Wl,--export-dynamic-symbol=_Unwind_Backtrace
-Wl,--export-dynamic-symbol=_Unwind_Resume
instead of: -Wl,--export-dynamic-symbol=_Unwind_Backtrace
-Wl,--export-dynamic-symbol=_Unwind_Resume -Wl,-u,_Unwind_Backtrace
-Wl,-u,_Unwind_Resume

-----

There are still a few differences between gold and GNU ld:

* --export-dynamic-symbol=non_exist adds an undefined symbol in .symtab
* --export-dynamic-symbol does not take a glob in gold
* --export-dynamic-symbol does not make symbol preemptable in -shared
link if -Bsymbolic or -Bsymbolic-functions or --dynamic-list is
specified.

---

A short summary of --export-dynamic-symbol's intended behavior:

-no-pie or -pie: matched non-local defined symbols will be added to
the dynamic symbol table.
-shared: matched non-local STV_DEFAULT symbols will not be bound to
definitions within the shared object even if they would otherwise be
due to -Bsymbolic, -Bsymbolic-functions, or --dynamic-list.

Patch

diff --git a/gold/archive.cc b/gold/archive.cc
index 75ad5517b8..d682d69ed6 100644
--- a/gold/archive.cc
+++ b/gold/archive.cc
@@ -120,13 +120,6 @@  Library_base::should_include_member(Symbol_table* symtab, Layout* layout,
       return Library_base::SHOULD_INCLUDE_YES;
     }
 
-  if (parameters->options().is_export_dynamic_symbol(sym_name))
-    {
-      *why = "--export-dynamic-symbol ";
-      *why += sym_name;
-      return Library_base::SHOULD_INCLUDE_YES;
-    }
-
   if (layout->script_options()->is_referenced(sym_name))
     {
       size_t alc = 100 + strlen(sym_name);
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 205444b5ea..f5a14e9736 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -2475,12 +2475,6 @@  Symbol_table::do_add_undefined_symbols_from_command_line(Layout* layout)
        ++p)
     this->add_undefined_symbol_from_command_line<size>(p->c_str());
 
-  for (options::String_set::const_iterator p =
-	 parameters->options().export_dynamic_symbol_begin();
-       p != parameters->options().export_dynamic_symbol_end();
-       ++p)
-    this->add_undefined_symbol_from_command_line<size>(p->c_str());
-
   for (Script_options::referenced_const_iterator p =
 	 layout->script_options()->referenced_begin();
        p != layout->script_options()->referenced_end();