[PATCH] gold: --export-dynamic-symbol: don't imply -u

[PATCH] gold: --export-dynamic-symbol: don't imply -u

[Fangrui Song]

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(-)

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();
-- 
2.27.0.278.ge193c7cf3a9-goog

Re: [PATCH] gold: --export-dynamic-symbol: don't imply -u

[Cary Coutant]

> 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

Re: [PATCH] gold: --export-dynamic-symbol: don't imply -u

[Fāng-ruì Sòng]

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.

Re: [PATCH] gold: --export-dynamic-symbol: don't imply -u

[Fangrui Song]

On Sat, Jun 13, 2020 at 6:08 PM Fāng-ruì Sòng via Binutils
<binutils@sourceware.org> wrote:
>
> 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.

Ping :)

Re: [PATCH] gold: --export-dynamic-symbol: don't imply -u

[Cary Coutant]

> > 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

OK, I'll concede here. Sorry for the long delay.

> > > > 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.

This is OK. Thanks!

-cary