ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions

Message ID 20200521013139.114917-1-maskray@google.com
State New
Headers show
Series
  • ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
Related show

Commit Message

They aren't used together in reality, so it is safe to change the
semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions
should be considered as a subset of -Bsymbolic, so --dynamic-list
overridding -Bsymbolic implies that --dynamic-list overridding
-Bsymbolic-functions.

This also helps PR ld/25910, which would make the situation more
difficult to understand.

	PR ld/26018
	* lexsup.c: Simplify.
	* ld.texi: Update documentation.
	* testsuite/ld-elf/shared.exp: Update tests.
	* testsuite/ld-elf/dl4e.out: New.
---
 ld/ld.texi                     |  3 ++-
 ld/lexsup.c                    | 33 +++++++++++++--------------------
 ld/testsuite/ld-elf/dl4e.out   |  6 ++++++
 ld/testsuite/ld-elf/shared.exp | 10 +++++++---
 4 files changed, 28 insertions(+), 24 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/dl4e.out

-- 
2.26.2.761.g0e0b3e54be-goog

Comments

On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils
<binutils@sourceware.org> wrote:
>

> They aren't used together in reality, so it is safe to change the

> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

> should be considered as a subset of -Bsymbolic, so --dynamic-list

> overridding -Bsymbolic implies that --dynamic-list overridding

> -Bsymbolic-functions.


--dynamic-list=DYNAMIC-LIST-FILE'
     Specify the name of a dynamic list file to the linker.  This is
     typically used when creating shared libraries to specify a list of
     global symbols whose references shouldn't be bound to the
     definition within the shared library, or creating dynamically
     linked executables to specify a list of symbols which should be
     added to the symbol table in the executable.  This option is only
     meaningful on ELF platforms which support shared libraries.

The --dynamic-list* options are intended for shared libraries.  The goal
IS NOT put them in dynamic symbol table since all global symbols are in
dynamic symbol table already in a shared library.  The goal is to make
them PREEMPTIBLE.  So making the --dynamic-list* options override -Bsymbolic
and -Bsymbolic-functions is incorrect since there is NOTHING to override.

To do it properly:

1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.
Only add symbols to dynamic symbol table.
2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.
and ignore --export-dynamic-symbol for -shared.  It has to be done at the
end of command-line parsing.

> This also helps PR ld/25910, which would make the situation more

> difficult to understand.

>

>         PR ld/26018

>         * lexsup.c: Simplify.

>         * ld.texi: Update documentation.

>         * testsuite/ld-elf/shared.exp: Update tests.

>         * testsuite/ld-elf/dl4e.out: New.




-- 
H.J.
On 2020-05-22, H.J. Lu wrote:
>On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

><binutils@sourceware.org> wrote:

>>

>> They aren't used together in reality, so it is safe to change the

>> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

>> should be considered as a subset of -Bsymbolic, so --dynamic-list

>> overridding -Bsymbolic implies that --dynamic-list overridding

>> -Bsymbolic-functions.

>

>--dynamic-list=DYNAMIC-LIST-FILE'

>     Specify the name of a dynamic list file to the linker.  This is

>     typically used when creating shared libraries to specify a list of

>     global symbols whose references shouldn't be bound to the

>     definition within the shared library, or creating dynamically

>     linked executables to specify a list of symbols which should be

>     added to the symbol table in the executable.  This option is only

>     meaningful on ELF platforms which support shared libraries.

>

>The --dynamic-list* options are intended for shared libraries. 


--dynamic-list* work for both executables and shared libraries.
For an executable, export some symbols.
For a shared library, specify preemptible symbols.

>The goal

>IS NOT put them in dynamic symbol table since all global symbols are in

>dynamic symbol table already in a shared library.  The goal is to make

>them PREEMPTIBLE.  


I agree with this sentence and the patch respects it.

>So making the --dynamic-list* options override -Bsymbolic

>and -Bsymbolic-functions is incorrect since there is NOTHING to override.


I don't agree with this statement. --dynamic-list* have function overlay
with -Bsymbolic. When two options overlap in functionality, many users
expect the more fine-grained option to win, thus my thought that
--dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

(FWIW LLD's preemptibility logic is
   ...
   if (config->hasDynamicList)
     return sym.inDynamicList;
   return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());
   This works fine and I won't change it.
)

>To do it properly:

>

>1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

>Only add symbols to dynamic symbol table.


This was already implemented when you implemented --dynamic-list in
2006. This patch does not change the fact.

>2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

>and ignore --export-dynamic-symbol for -shared.  It has to be done at the

>end of command-line parsing.


I am happy with the description about --export-dynamic-symbol.

>> This also helps PR ld/25910, which would make the situation more

>> difficult to understand.

>>

>>         PR ld/26018

>>         * lexsup.c: Simplify.

>>         * ld.texi: Update documentation.

>>         * testsuite/ld-elf/shared.exp: Update tests.

>>         * testsuite/ld-elf/dl4e.out: New.

>

>

>

>-- 

>H.J.
On Fri, May 22, 2020 at 8:56 AM Fangrui Song <maskray@google.com> wrote:
>

>

> On 2020-05-22, H.J. Lu wrote:

> >On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

> ><binutils@sourceware.org> wrote:

> >>

> >> They aren't used together in reality, so it is safe to change the

> >> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

> >> should be considered as a subset of -Bsymbolic, so --dynamic-list

> >> overridding -Bsymbolic implies that --dynamic-list overridding

> >> -Bsymbolic-functions.

> >

> >--dynamic-list=DYNAMIC-LIST-FILE'

> >     Specify the name of a dynamic list file to the linker.  This is

> >     typically used when creating shared libraries to specify a list of

> >     global symbols whose references shouldn't be bound to the

> >     definition within the shared library, or creating dynamically

> >     linked executables to specify a list of symbols which should be

> >     added to the symbol table in the executable.  This option is only

> >     meaningful on ELF platforms which support shared libraries.

> >

> >The --dynamic-list* options are intended for shared libraries.

>

> --dynamic-list* work for both executables and shared libraries.

> For an executable, export some symbols.

> For a shared library, specify preemptible symbols.

>

> >The goal

> >IS NOT put them in dynamic symbol table since all global symbols are in

> >dynamic symbol table already in a shared library.  The goal is to make

> >them PREEMPTIBLE.

>

> I agree with this sentence and the patch respects it.

>

> >So making the --dynamic-list* options override -Bsymbolic

> >and -Bsymbolic-functions is incorrect since there is NOTHING to override.

>

> I don't agree with this statement. --dynamic-list* have function overlay

> with -Bsymbolic. When two options overlap in functionality, many users

> expect the more fine-grained option to win, thus my thought that

> --dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

>

> (FWIW LLD's preemptibility logic is

>    ...

>    if (config->hasDynamicList)

>      return sym.inDynamicList;

>    return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());

>    This works fine and I won't change it.

> )

>

> >To do it properly:

> >

> >1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

> >Only add symbols to dynamic symbol table.

>

> This was already implemented when you implemented --dynamic-list in

> 2006. This patch does not change the fact.

>

> >2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

> >and ignore --export-dynamic-symbol for -shared.  It has to be done at the

> >end of command-line parsing.


There is no need to change linker manual.  --dynamic-list* should work both
before and after -Bsymbolic and -Bsymbolic-functions.  Your patch used
libdl2d.so without creating it first.

Here is the updated version of your patch.  I took the liberty to make
the changes above.

-- 
H.J.
From de4eb142701b740f4680bb9084e59f2d62990dc0 Mon Sep 17 00:00:00 2001
From: Fangrui Song via Binutils <binutils@sourceware.org>
Date: Wed, 20 May 2020 18:31:39 -0700
Subject: [PATCH] ld: Make --dynamic-list* work before -Bsymbolic
 -Bsymbolic-functions

--dynamic-list* should work both before and after -Bsymbolic and
-Bsymbolic-functions.

	PR ld/26018
	* lexsup.c (parse_args): Simplify.
	* testsuite/ld-elf/dl4e.out: New.
	* testsuite/ld-elf/shared.exp: Updated for PR ld/26018 tests.
---
 ld/lexsup.c                    | 33 +++++++++++++--------------------
 ld/testsuite/ld-elf/dl4e.out   |  6 ++++++
 ld/testsuite/ld-elf/shared.exp | 12 +++++++++---
 3 files changed, 28 insertions(+), 23 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/dl4e.out

diff --git a/ld/lexsup.c b/ld/lexsup.c
index fe9526b527..49e013b5bd 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -1390,22 +1390,16 @@ parse_args (unsigned argc, char **argv)
 	  break;
 	case OPTION_DYNAMIC_LIST_DATA:
 	  opt_dynamic_list = dynamic_list_data;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_DYNAMIC_LIST_CPP_TYPEINFO:
 	  lang_append_dynamic_list_cpp_typeinfo ();
 	  if (opt_dynamic_list != dynamic_list_data)
 	    opt_dynamic_list = dynamic_list;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_DYNAMIC_LIST_CPP_NEW:
 	  lang_append_dynamic_list_cpp_new ();
 	  if (opt_dynamic_list != dynamic_list_data)
 	    opt_dynamic_list = dynamic_list;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_DYNAMIC_LIST:
 	  /* This option indicates a small script that only specifies
@@ -1422,8 +1416,6 @@ parse_args (unsigned argc, char **argv)
 	  }
 	  if (opt_dynamic_list != dynamic_list_data)
 	    opt_dynamic_list = dynamic_list;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_WARN_COMMON:
 	  config.warn_common = TRUE;
@@ -1632,6 +1624,19 @@ parse_args (unsigned argc, char **argv)
       && command_line.check_section_addresses < 0)
     command_line.check_section_addresses = 0;
 
+  switch (opt_dynamic_list)
+    {
+    case dynamic_list_unset:
+      break;
+    case dynamic_list_data:
+      link_info.dynamic_data = TRUE;
+      /* Fall through.  */
+    case dynamic_list:
+      link_info.dynamic = TRUE;
+      opt_symbolic = symbolic_unset;
+      break;
+    }
+
   /* -Bsymbolic and -Bsymbols-functions are for shared library output.  */
   if (bfd_link_dll (&link_info))
     switch (opt_symbolic)
@@ -1658,18 +1663,6 @@ parse_args (unsigned argc, char **argv)
 	break;
       }
 
-  switch (opt_dynamic_list)
-    {
-    case dynamic_list_unset:
-      break;
-    case dynamic_list_data:
-      link_info.dynamic_data = TRUE;
-      /* Fall through.  */
-    case dynamic_list:
-      link_info.dynamic = TRUE;
-      break;
-    }
-
   if (!bfd_link_dll (&link_info))
     {
       if (command_line.filter_shlib)
diff --git a/ld/testsuite/ld-elf/dl4e.out b/ld/testsuite/ld-elf/dl4e.out
new file mode 100644
index 0000000000..e5da6e2185
--- /dev/null
+++ b/ld/testsuite/ld-elf/dl4e.out
@@ -0,0 +1,6 @@
+bar OK2
+bar OK4
+DSO1
+DSO2
+OK2
+OK4
diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index 3366430515..7d35f3f379 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -598,6 +598,9 @@ set build_tests {
   {"Build libdl2c.so with --dynamic-list-data and dl2xxx.list"
    "-shared -Wl,--dynamic-list-data,--dynamic-list=dl2xxx.list" "-fPIC"
    {dl2.c dl2xxx.c} {} "libdl2c.so"}
+  {"Build libdl2d.so with --dynamic-list-data -Bsymbolic"
+   "-shared -Wl,-Bsymbolic,--dynamic-list-data" "-fPIC"
+   {dl2.c dl2xxx.c} {} "libdl2d.so"}
   {"Build libdl4a.so with --dynamic-list=dl4.list"
    "-shared -Wl,--dynamic-list=dl4.list" "-fPIC"
    {dl4.c dl4xxx.c} {} "libdl4a.so"}
@@ -874,6 +877,9 @@ set run_tests [list \
     [list "Run with libdl2c.so" \
      "-Wl,--no-as-needed tmpdir/libdl2c.so" "" \
      {dl2main.c} "dl2c" "dl2b.out" ] \
+    [list "Run with libdl2d.so" \
+     "-Wl,--no-as-needed tmpdir/libdl2d.so" "" \
+     {dl2main.c} "dl2d" "dl2a.out" ] \
     [list "Run with libdl4a.so" \
      "-Wl,--no-as-needed tmpdir/libdl4a.so" "" \
      {dl4main.c} "dl4a" "dl4a.out" ] \
@@ -888,10 +894,10 @@ set run_tests [list \
      {dl4main.c} "dl4d" "dl4b.out" ] \
     [list "Run with libdl4e.so" \
      "-Wl,--no-as-needed tmpdir/libdl4e.so" "" \
-     {dl4main.c} "dl4e" "dl4a.out" ] \
+     {dl4main.c} "dl4e" "dl4e.out" ] \
     [list "Run with libdl4f.so" \
      "-Wl,--no-as-needed tmpdir/libdl4f.so" "" \
-     {dl4main.c} "dl4f" "dl4a.out" ] \
+     {dl4main.c} "dl4f" "dl4e.out" ] \
     [list "Run with libdata1.so" \
      "-Wl,--no-as-needed tmpdir/libdata1.so" "" \
      {dynbss1.c} "dynbss1" "pass.out" ] \
@@ -988,7 +994,7 @@ set dlopen_run_tests [list \
      {dl6cmain.c} "dl6c1" "dl6b.out" ] \
     [list "Run dl6d1 with --dynamic-list-data and dlopen on libdl6d.so" \
      "-Wl,--no-as-needed,--dynamic-list-data $extralibs" "" \
-     {dl6dmain.c} "dl6d1" "dl6b.out" ] \
+     {dl6dmain.c} "dl6d1" "dl6a.out" ] \
     [list "Run pr21964-2" \
      "-Wl,--no-as-needed,-rpath,tmpdir tmpdir/pr21964-2a.so $extralibs" "" \
      {pr21964-2c.c} "pr21964-2" "pass.out" ] \
On 2020-05-23, H.J. Lu wrote:
>On Fri, May 22, 2020 at 8:56 AM Fangrui Song <maskray@google.com> wrote:

>>

>>

>> On 2020-05-22, H.J. Lu wrote:

>> >On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

>> ><binutils@sourceware.org> wrote:

>> >>

>> >> They aren't used together in reality, so it is safe to change the

>> >> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

>> >> should be considered as a subset of -Bsymbolic, so --dynamic-list

>> >> overridding -Bsymbolic implies that --dynamic-list overridding

>> >> -Bsymbolic-functions.

>> >

>> >--dynamic-list=DYNAMIC-LIST-FILE'

>> >     Specify the name of a dynamic list file to the linker.  This is

>> >     typically used when creating shared libraries to specify a list of

>> >     global symbols whose references shouldn't be bound to the

>> >     definition within the shared library, or creating dynamically

>> >     linked executables to specify a list of symbols which should be

>> >     added to the symbol table in the executable.  This option is only

>> >     meaningful on ELF platforms which support shared libraries.

>> >

>> >The --dynamic-list* options are intended for shared libraries.

>>

>> --dynamic-list* work for both executables and shared libraries.

>> For an executable, export some symbols.

>> For a shared library, specify preemptible symbols.

>>

>> >The goal

>> >IS NOT put them in dynamic symbol table since all global symbols are in

>> >dynamic symbol table already in a shared library.  The goal is to make

>> >them PREEMPTIBLE.

>>

>> I agree with this sentence and the patch respects it.

>>

>> >So making the --dynamic-list* options override -Bsymbolic

>> >and -Bsymbolic-functions is incorrect since there is NOTHING to override.

>>

>> I don't agree with this statement. --dynamic-list* have function overlay

>> with -Bsymbolic. When two options overlap in functionality, many users

>> expect the more fine-grained option to win, thus my thought that

>> --dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

>>

>> (FWIW LLD's preemptibility logic is

>>    ...

>>    if (config->hasDynamicList)

>>      return sym.inDynamicList;

>>    return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());

>>    This works fine and I won't change it.

>> )

>>

>> >To do it properly:

>> >

>> >1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

>> >Only add symbols to dynamic symbol table.

>>

>> This was already implemented when you implemented --dynamic-list in

>> 2006. This patch does not change the fact.

>>

>> >2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

>> >and ignore --export-dynamic-symbol for -shared.  It has to be done at the

>> >end of command-line parsing.

>

>There is no need to change linker manual.  --dynamic-list* should work both

>before and after -Bsymbolic and -Bsymbolic-functions.  Your patch used

>libdl2d.so without creating it first.

>

>Here is the updated version of your patch.  I took the liberty to make

>the changes above.

>

>-- 

>H.J.


Thanks for the fix (libdl4e.so->libdl2d.so)

The updated patch looks good to me.
On 2020-05-23, Fangrui Song wrote:
>On 2020-05-23, H.J. Lu wrote:

>>On Fri, May 22, 2020 at 8:56 AM Fangrui Song <maskray@google.com> wrote:

>>>

>>>

>>>On 2020-05-22, H.J. Lu wrote:

>>>>On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

>>>><binutils@sourceware.org> wrote:

>>>>>

>>>>> They aren't used together in reality, so it is safe to change the

>>>>> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

>>>>> should be considered as a subset of -Bsymbolic, so --dynamic-list

>>>>> overridding -Bsymbolic implies that --dynamic-list overridding

>>>>> -Bsymbolic-functions.

>>>>

>>>>--dynamic-list=DYNAMIC-LIST-FILE'

>>>>     Specify the name of a dynamic list file to the linker.  This is

>>>>     typically used when creating shared libraries to specify a list of

>>>>     global symbols whose references shouldn't be bound to the

>>>>     definition within the shared library, or creating dynamically

>>>>     linked executables to specify a list of symbols which should be

>>>>     added to the symbol table in the executable.  This option is only

>>>>     meaningful on ELF platforms which support shared libraries.

>>>>

>>>>The --dynamic-list* options are intended for shared libraries.

>>>

>>>--dynamic-list* work for both executables and shared libraries.

>>>For an executable, export some symbols.

>>>For a shared library, specify preemptible symbols.

>>>

>>>>The goal

>>>>IS NOT put them in dynamic symbol table since all global symbols are in

>>>>dynamic symbol table already in a shared library.  The goal is to make

>>>>them PREEMPTIBLE.

>>>

>>>I agree with this sentence and the patch respects it.

>>>

>>>>So making the --dynamic-list* options override -Bsymbolic

>>>>and -Bsymbolic-functions is incorrect since there is NOTHING to override.

>>>

>>>I don't agree with this statement. --dynamic-list* have function overlay

>>>with -Bsymbolic. When two options overlap in functionality, many users

>>>expect the more fine-grained option to win, thus my thought that

>>>--dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

>>>

>>>(FWIW LLD's preemptibility logic is

>>>   ...

>>>   if (config->hasDynamicList)

>>>     return sym.inDynamicList;

>>>   return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());

>>>   This works fine and I won't change it.

>>>)

>>>

>>>>To do it properly:

>>>>

>>>>1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

>>>>Only add symbols to dynamic symbol table.

>>>

>>>This was already implemented when you implemented --dynamic-list in

>>>2006. This patch does not change the fact.

>>>

>>>>2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

>>>>and ignore --export-dynamic-symbol for -shared.  It has to be done at the

>>>>end of command-line parsing.

>>

>>There is no need to change linker manual.  --dynamic-list* should work both

>>before and after -Bsymbolic and -Bsymbolic-functions.  Your patch used

>>libdl2d.so without creating it first.

>>

>>Here is the updated version of your patch.  I took the liberty to make

>>the changes above.

>>

>>-- 

>>H.J.

>

>Thanks for the fix (libdl4e.so->libdl2d.so)

>

>The updated patch looks good to me.


Around lexsup.c:1162

`case symbolic_functions:` should set link_info.dynamic_data and link_info.dynamic , otherwise
-Bsymbolic-functions is broken.


.globl _start, foo
_start:
   call foo
foo

ld-new -shared -Bsymbolic-functions a.o -o a.so

foo@plt is not created
On Sat, May 23, 2020 at 5:11 PM Fangrui Song <maskray@google.com> wrote:
>

> On 2020-05-23, Fangrui Song wrote:

> >On 2020-05-23, H.J. Lu wrote:

> >>On Fri, May 22, 2020 at 8:56 AM Fangrui Song <maskray@google.com> wrote:

> >>>

> >>>

> >>>On 2020-05-22, H.J. Lu wrote:

> >>>>On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

> >>>><binutils@sourceware.org> wrote:

> >>>>>

> >>>>> They aren't used together in reality, so it is safe to change the

> >>>>> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

> >>>>> should be considered as a subset of -Bsymbolic, so --dynamic-list

> >>>>> overridding -Bsymbolic implies that --dynamic-list overridding

> >>>>> -Bsymbolic-functions.

> >>>>

> >>>>--dynamic-list=DYNAMIC-LIST-FILE'

> >>>>     Specify the name of a dynamic list file to the linker.  This is

> >>>>     typically used when creating shared libraries to specify a list of

> >>>>     global symbols whose references shouldn't be bound to the

> >>>>     definition within the shared library, or creating dynamically

> >>>>     linked executables to specify a list of symbols which should be

> >>>>     added to the symbol table in the executable.  This option is only

> >>>>     meaningful on ELF platforms which support shared libraries.

> >>>>

> >>>>The --dynamic-list* options are intended for shared libraries.

> >>>

> >>>--dynamic-list* work for both executables and shared libraries.

> >>>For an executable, export some symbols.

> >>>For a shared library, specify preemptible symbols.

> >>>

> >>>>The goal

> >>>>IS NOT put them in dynamic symbol table since all global symbols are in

> >>>>dynamic symbol table already in a shared library.  The goal is to make

> >>>>them PREEMPTIBLE.

> >>>

> >>>I agree with this sentence and the patch respects it.

> >>>

> >>>>So making the --dynamic-list* options override -Bsymbolic

> >>>>and -Bsymbolic-functions is incorrect since there is NOTHING to override.

> >>>

> >>>I don't agree with this statement. --dynamic-list* have function overlay

> >>>with -Bsymbolic. When two options overlap in functionality, many users

> >>>expect the more fine-grained option to win, thus my thought that

> >>>--dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

> >>>

> >>>(FWIW LLD's preemptibility logic is

> >>>   ...

> >>>   if (config->hasDynamicList)

> >>>     return sym.inDynamicList;

> >>>   return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());

> >>>   This works fine and I won't change it.

> >>>)

> >>>

> >>>>To do it properly:

> >>>>

> >>>>1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

> >>>>Only add symbols to dynamic symbol table.

> >>>

> >>>This was already implemented when you implemented --dynamic-list in

> >>>2006. This patch does not change the fact.

> >>>

> >>>>2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

> >>>>and ignore --export-dynamic-symbol for -shared.  It has to be done at the

> >>>>end of command-line parsing.

> >>

> >>There is no need to change linker manual.  --dynamic-list* should work both

> >>before and after -Bsymbolic and -Bsymbolic-functions.  Your patch used

> >>libdl2d.so without creating it first.

> >>

> >>Here is the updated version of your patch.  I took the liberty to make

> >>the changes above.

> >>

> >>--

> >>H.J.

> >

> >Thanks for the fix (libdl4e.so->libdl2d.so)

> >

> >The updated patch looks good to me.

>

> Around lexsup.c:1162

>

> `case symbolic_functions:` should set link_info.dynamic_data and link_info.dynamic , otherwise

> -Bsymbolic-functions is broken.

>

>

> .globl _start, foo

> _start:

>    call foo

> foo

>

> ld-new -shared -Bsymbolic-functions a.o -o a.so

>

> foo@plt is not created


'-Bsymbolic-functions'
     When creating a shared library, bind references to global function
     symbols to the definition within the shared library, if any.  This
     option is only meaningful on ELF platforms which support shared
     libraries.

There should be no foo@plt.

-- 
H.J.
On 2020-05-23, H.J. Lu wrote:
>On Sat, May 23, 2020 at 5:11 PM Fangrui Song <maskray@google.com> wrote:

>>

>> On 2020-05-23, Fangrui Song wrote:

>> >On 2020-05-23, H.J. Lu wrote:

>> >>On Fri, May 22, 2020 at 8:56 AM Fangrui Song <maskray@google.com> wrote:

>> >>>

>> >>>

>> >>>On 2020-05-22, H.J. Lu wrote:

>> >>>>On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

>> >>>><binutils@sourceware.org> wrote:

>> >>>>>

>> >>>>> They aren't used together in reality, so it is safe to change the

>> >>>>> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

>> >>>>> should be considered as a subset of -Bsymbolic, so --dynamic-list

>> >>>>> overridding -Bsymbolic implies that --dynamic-list overridding

>> >>>>> -Bsymbolic-functions.

>> >>>>

>> >>>>--dynamic-list=DYNAMIC-LIST-FILE'

>> >>>>     Specify the name of a dynamic list file to the linker.  This is

>> >>>>     typically used when creating shared libraries to specify a list of

>> >>>>     global symbols whose references shouldn't be bound to the

>> >>>>     definition within the shared library, or creating dynamically

>> >>>>     linked executables to specify a list of symbols which should be

>> >>>>     added to the symbol table in the executable.  This option is only

>> >>>>     meaningful on ELF platforms which support shared libraries.

>> >>>>

>> >>>>The --dynamic-list* options are intended for shared libraries.

>> >>>

>> >>>--dynamic-list* work for both executables and shared libraries.

>> >>>For an executable, export some symbols.

>> >>>For a shared library, specify preemptible symbols.

>> >>>

>> >>>>The goal

>> >>>>IS NOT put them in dynamic symbol table since all global symbols are in

>> >>>>dynamic symbol table already in a shared library.  The goal is to make

>> >>>>them PREEMPTIBLE.

>> >>>

>> >>>I agree with this sentence and the patch respects it.

>> >>>

>> >>>>So making the --dynamic-list* options override -Bsymbolic

>> >>>>and -Bsymbolic-functions is incorrect since there is NOTHING to override.

>> >>>

>> >>>I don't agree with this statement. --dynamic-list* have function overlay

>> >>>with -Bsymbolic. When two options overlap in functionality, many users

>> >>>expect the more fine-grained option to win, thus my thought that

>> >>>--dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

>> >>>

>> >>>(FWIW LLD's preemptibility logic is

>> >>>   ...

>> >>>   if (config->hasDynamicList)

>> >>>     return sym.inDynamicList;

>> >>>   return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());

>> >>>   This works fine and I won't change it.

>> >>>)

>> >>>

>> >>>>To do it properly:

>> >>>>

>> >>>>1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

>> >>>>Only add symbols to dynamic symbol table.

>> >>>

>> >>>This was already implemented when you implemented --dynamic-list in

>> >>>2006. This patch does not change the fact.

>> >>>

>> >>>>2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

>> >>>>and ignore --export-dynamic-symbol for -shared.  It has to be done at the

>> >>>>end of command-line parsing.

>> >>

>> >>There is no need to change linker manual.  --dynamic-list* should work both

>> >>before and after -Bsymbolic and -Bsymbolic-functions.  Your patch used

>> >>libdl2d.so without creating it first.

>> >>

>> >>Here is the updated version of your patch.  I took the liberty to make

>> >>the changes above.

>> >>

>> >>--

>> >>H.J.

>> >

>> >Thanks for the fix (libdl4e.so->libdl2d.so)

>> >

>> >The updated patch looks good to me.

>>

>> Around lexsup.c:1162

>>

>> `case symbolic_functions:` should set link_info.dynamic_data and link_info.dynamic , otherwise

>> -Bsymbolic-functions is broken.

>>

>>

>> .globl _start, foo

>> _start:

>>    call foo

>> foo

>>

>> ld-new -shared -Bsymbolic-functions a.o -o a.so

>>

>> foo@plt is not created

>

>'-Bsymbolic-functions'

>     When creating a shared library, bind references to global function

>     symbols to the definition within the shared library, if any.  This

>     option is only meaningful on ELF platforms which support shared

>     libraries.

>

>There should be no foo@plt.


Sorry, my example missed .type:

.global _start, foo
.type foo, @function
_start:
   call foo
foo:


ld-new -shared -Bsymbolic-functions a.o -o a.so
foo@plt was created without the patch.
On Sat, May 23, 2020 at 6:05 PM Fangrui Song <maskray@google.com> wrote:
>

> On 2020-05-23, H.J. Lu wrote:

> >On Sat, May 23, 2020 at 5:11 PM Fangrui Song <maskray@google.com> wrote:

> >>

> >> On 2020-05-23, Fangrui Song wrote:

> >> >On 2020-05-23, H.J. Lu wrote:

> >> >>On Fri, May 22, 2020 at 8:56 AM Fangrui Song <maskray@google.com> wrote:

> >> >>>

> >> >>>

> >> >>>On 2020-05-22, H.J. Lu wrote:

> >> >>>>On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

> >> >>>><binutils@sourceware.org> wrote:

> >> >>>>>

> >> >>>>> They aren't used together in reality, so it is safe to change the

> >> >>>>> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

> >> >>>>> should be considered as a subset of -Bsymbolic, so --dynamic-list

> >> >>>>> overridding -Bsymbolic implies that --dynamic-list overridding

> >> >>>>> -Bsymbolic-functions.

> >> >>>>

> >> >>>>--dynamic-list=DYNAMIC-LIST-FILE'

> >> >>>>     Specify the name of a dynamic list file to the linker.  This is

> >> >>>>     typically used when creating shared libraries to specify a list of

> >> >>>>     global symbols whose references shouldn't be bound to the

> >> >>>>     definition within the shared library, or creating dynamically

> >> >>>>     linked executables to specify a list of symbols which should be

> >> >>>>     added to the symbol table in the executable.  This option is only

> >> >>>>     meaningful on ELF platforms which support shared libraries.

> >> >>>>

> >> >>>>The --dynamic-list* options are intended for shared libraries.

> >> >>>

> >> >>>--dynamic-list* work for both executables and shared libraries.

> >> >>>For an executable, export some symbols.

> >> >>>For a shared library, specify preemptible symbols.

> >> >>>

> >> >>>>The goal

> >> >>>>IS NOT put them in dynamic symbol table since all global symbols are in

> >> >>>>dynamic symbol table already in a shared library.  The goal is to make

> >> >>>>them PREEMPTIBLE.

> >> >>>

> >> >>>I agree with this sentence and the patch respects it.

> >> >>>

> >> >>>>So making the --dynamic-list* options override -Bsymbolic

> >> >>>>and -Bsymbolic-functions is incorrect since there is NOTHING to override.

> >> >>>

> >> >>>I don't agree with this statement. --dynamic-list* have function overlay

> >> >>>with -Bsymbolic. When two options overlap in functionality, many users

> >> >>>expect the more fine-grained option to win, thus my thought that

> >> >>>--dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

> >> >>>

> >> >>>(FWIW LLD's preemptibility logic is

> >> >>>   ...

> >> >>>   if (config->hasDynamicList)

> >> >>>     return sym.inDynamicList;

> >> >>>   return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());

> >> >>>   This works fine and I won't change it.

> >> >>>)

> >> >>>

> >> >>>>To do it properly:

> >> >>>>

> >> >>>>1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

> >> >>>>Only add symbols to dynamic symbol table.

> >> >>>

> >> >>>This was already implemented when you implemented --dynamic-list in

> >> >>>2006. This patch does not change the fact.

> >> >>>

> >> >>>>2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

> >> >>>>and ignore --export-dynamic-symbol for -shared.  It has to be done at the

> >> >>>>end of command-line parsing.

> >> >>

> >> >>There is no need to change linker manual.  --dynamic-list* should work both

> >> >>before and after -Bsymbolic and -Bsymbolic-functions.  Your patch used

> >> >>libdl2d.so without creating it first.

> >> >>

> >> >>Here is the updated version of your patch.  I took the liberty to make

> >> >>the changes above.

> >> >>

> >> >>--

> >> >>H.J.

> >> >

> >> >Thanks for the fix (libdl4e.so->libdl2d.so)

> >> >

> >> >The updated patch looks good to me.

> >>

> >> Around lexsup.c:1162

> >>

> >> `case symbolic_functions:` should set link_info.dynamic_data and link_info.dynamic , otherwise

> >> -Bsymbolic-functions is broken.

> >>

> >>

> >> .globl _start, foo

> >> _start:

> >>    call foo

> >> foo

> >>

> >> ld-new -shared -Bsymbolic-functions a.o -o a.so

> >>

> >> foo@plt is not created

> >

> >'-Bsymbolic-functions'

> >     When creating a shared library, bind references to global function

> >     symbols to the definition within the shared library, if any.  This

> >     option is only meaningful on ELF platforms which support shared

> >     libraries.

> >

> >There should be no foo@plt.

>

> Sorry, my example missed .type:

>

> .global _start, foo

> .type foo, @function

> _start:

>    call foo

> foo:

>

>

> ld-new -shared -Bsymbolic-functions a.o -o a.so

> foo@plt was created without the patch.


On master branch, I got

[hjl@gnu-cfl-2 pr26018]$ cat func.s
.global _start, foo
.type foo, %function
.text
_start:
call foo
foo:
ret
[hjl@gnu-cfl-2 pr26018]$ make func.so
as   -o func.o func.s
./ld -shared  -Bsymbolic-function -o func.so func.o
[hjl@gnu-cfl-2 pr26018]$ objdump -dw func.so

func.so:     file format elf64-x86-64


Disassembly of section .text:

0000000000001000 <_start>:
    1000: e8 00 00 00 00        callq  1005 <foo>

0000000000001005 <foo>:
    1005: c3                    retq
[hjl@gnu-cfl-2 pr26018]$

It looks OK to me.

-- 
H.J.
On 2020-05-23, H.J. Lu wrote:
>On Sat, May 23, 2020 at 6:05 PM Fangrui Song <maskray@google.com> wrote:

>>

>> On 2020-05-23, H.J. Lu wrote:

>> >On Sat, May 23, 2020 at 5:11 PM Fangrui Song <maskray@google.com> wrote:

>> >>

>> >> On 2020-05-23, Fangrui Song wrote:

>> >> >On 2020-05-23, H.J. Lu wrote:

>> >> >>On Fri, May 22, 2020 at 8:56 AM Fangrui Song <maskray@google.com> wrote:

>> >> >>>

>> >> >>>

>> >> >>>On 2020-05-22, H.J. Lu wrote:

>> >> >>>>On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

>> >> >>>><binutils@sourceware.org> wrote:

>> >> >>>>>

>> >> >>>>> They aren't used together in reality, so it is safe to change the

>> >> >>>>> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

>> >> >>>>> should be considered as a subset of -Bsymbolic, so --dynamic-list

>> >> >>>>> overridding -Bsymbolic implies that --dynamic-list overridding

>> >> >>>>> -Bsymbolic-functions.

>> >> >>>>

>> >> >>>>--dynamic-list=DYNAMIC-LIST-FILE'

>> >> >>>>     Specify the name of a dynamic list file to the linker.  This is

>> >> >>>>     typically used when creating shared libraries to specify a list of

>> >> >>>>     global symbols whose references shouldn't be bound to the

>> >> >>>>     definition within the shared library, or creating dynamically

>> >> >>>>     linked executables to specify a list of symbols which should be

>> >> >>>>     added to the symbol table in the executable.  This option is only

>> >> >>>>     meaningful on ELF platforms which support shared libraries.

>> >> >>>>

>> >> >>>>The --dynamic-list* options are intended for shared libraries.

>> >> >>>

>> >> >>>--dynamic-list* work for both executables and shared libraries.

>> >> >>>For an executable, export some symbols.

>> >> >>>For a shared library, specify preemptible symbols.

>> >> >>>

>> >> >>>>The goal

>> >> >>>>IS NOT put them in dynamic symbol table since all global symbols are in

>> >> >>>>dynamic symbol table already in a shared library.  The goal is to make

>> >> >>>>them PREEMPTIBLE.

>> >> >>>

>> >> >>>I agree with this sentence and the patch respects it.

>> >> >>>

>> >> >>>>So making the --dynamic-list* options override -Bsymbolic

>> >> >>>>and -Bsymbolic-functions is incorrect since there is NOTHING to override.

>> >> >>>

>> >> >>>I don't agree with this statement. --dynamic-list* have function overlay

>> >> >>>with -Bsymbolic. When two options overlap in functionality, many users

>> >> >>>expect the more fine-grained option to win, thus my thought that

>> >> >>>--dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

>> >> >>>

>> >> >>>(FWIW LLD's preemptibility logic is

>> >> >>>   ...

>> >> >>>   if (config->hasDynamicList)

>> >> >>>     return sym.inDynamicList;

>> >> >>>   return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());

>> >> >>>   This works fine and I won't change it.

>> >> >>>)

>> >> >>>

>> >> >>>>To do it properly:

>> >> >>>>

>> >> >>>>1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

>> >> >>>>Only add symbols to dynamic symbol table.

>> >> >>>

>> >> >>>This was already implemented when you implemented --dynamic-list in

>> >> >>>2006. This patch does not change the fact.

>> >> >>>

>> >> >>>>2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

>> >> >>>>and ignore --export-dynamic-symbol for -shared.  It has to be done at the

>> >> >>>>end of command-line parsing.

>> >> >>

>> >> >>There is no need to change linker manual.  --dynamic-list* should work both

>> >> >>before and after -Bsymbolic and -Bsymbolic-functions.  Your patch used

>> >> >>libdl2d.so without creating it first.

>> >> >>

>> >> >>Here is the updated version of your patch.  I took the liberty to make

>> >> >>the changes above.

>> >> >>

>> >> >>--

>> >> >>H.J.

>> >> >

>> >> >Thanks for the fix (libdl4e.so->libdl2d.so)

>> >> >

>> >> >The updated patch looks good to me.

>> >>

>> >> Around lexsup.c:1162

>> >>

>> >> `case symbolic_functions:` should set link_info.dynamic_data and link_info.dynamic , otherwise

>> >> -Bsymbolic-functions is broken.

>> >>

>> >>

>> >> .globl _start, foo

>> >> _start:

>> >>    call foo

>> >> foo

>> >>

>> >> ld-new -shared -Bsymbolic-functions a.o -o a.so

>> >>

>> >> foo@plt is not created

>> >

>> >'-Bsymbolic-functions'

>> >     When creating a shared library, bind references to global function

>> >     symbols to the definition within the shared library, if any.  This

>> >     option is only meaningful on ELF platforms which support shared

>> >     libraries.

>> >

>> >There should be no foo@plt.

>>

>> Sorry, my example missed .type:

>>

>> .global _start, foo

>> .type foo, @function

>> _start:

>>    call foo

>> foo:

>>

>>

>> ld-new -shared -Bsymbolic-functions a.o -o a.so

>> foo@plt was created without the patch.

>

>On master branch, I got

>

>[hjl@gnu-cfl-2 pr26018]$ cat func.s

>.global _start, foo

>.type foo, %function

>.text

>_start:

>call foo

>foo:

>ret

>[hjl@gnu-cfl-2 pr26018]$ make func.so

>as   -o func.o func.s

>./ld -shared  -Bsymbolic-function -o func.so func.o

>[hjl@gnu-cfl-2 pr26018]$ objdump -dw func.so

>

>func.so:     file format elf64-x86-64

>

>

>Disassembly of section .text:

>

>0000000000001000 <_start>:

>    1000: e8 00 00 00 00        callq  1005 <foo>

>

>0000000000001005 <foo>:

>    1005: c3                    retq

>[hjl@gnu-cfl-2 pr26018]$

>

>It looks OK to me.


At master, foo is bound locally (intended).
With this patch, foo@plt is created

Fix:

>> >> `case symbolic_functions:` should set link_info.dynamic_data and link_info.dynamic , otherwise

>> >> -Bsymbolic-functions is broken.
On Sat, May 23, 2020 at 7:56 PM Fangrui Song <maskray@google.com> wrote:
>

> On 2020-05-23, H.J. Lu wrote:

> >On Sat, May 23, 2020 at 6:05 PM Fangrui Song <maskray@google.com> wrote:

> >>

> >> On 2020-05-23, H.J. Lu wrote:

> >> >On Sat, May 23, 2020 at 5:11 PM Fangrui Song <maskray@google.com> wrote:

> >> >>

> >> >> On 2020-05-23, Fangrui Song wrote:

> >> >> >On 2020-05-23, H.J. Lu wrote:

> >> >> >>On Fri, May 22, 2020 at 8:56 AM Fangrui Song <maskray@google.com> wrote:

> >> >> >>>

> >> >> >>>

> >> >> >>>On 2020-05-22, H.J. Lu wrote:

> >> >> >>>>On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

> >> >> >>>><binutils@sourceware.org> wrote:

> >> >> >>>>>

> >> >> >>>>> They aren't used together in reality, so it is safe to change the

> >> >> >>>>> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

> >> >> >>>>> should be considered as a subset of -Bsymbolic, so --dynamic-list

> >> >> >>>>> overridding -Bsymbolic implies that --dynamic-list overridding

> >> >> >>>>> -Bsymbolic-functions.

> >> >> >>>>

> >> >> >>>>--dynamic-list=DYNAMIC-LIST-FILE'

> >> >> >>>>     Specify the name of a dynamic list file to the linker.  This is

> >> >> >>>>     typically used when creating shared libraries to specify a list of

> >> >> >>>>     global symbols whose references shouldn't be bound to the

> >> >> >>>>     definition within the shared library, or creating dynamically

> >> >> >>>>     linked executables to specify a list of symbols which should be

> >> >> >>>>     added to the symbol table in the executable.  This option is only

> >> >> >>>>     meaningful on ELF platforms which support shared libraries.

> >> >> >>>>

> >> >> >>>>The --dynamic-list* options are intended for shared libraries.

> >> >> >>>

> >> >> >>>--dynamic-list* work for both executables and shared libraries.

> >> >> >>>For an executable, export some symbols.

> >> >> >>>For a shared library, specify preemptible symbols.

> >> >> >>>

> >> >> >>>>The goal

> >> >> >>>>IS NOT put them in dynamic symbol table since all global symbols are in

> >> >> >>>>dynamic symbol table already in a shared library.  The goal is to make

> >> >> >>>>them PREEMPTIBLE.

> >> >> >>>

> >> >> >>>I agree with this sentence and the patch respects it.

> >> >> >>>

> >> >> >>>>So making the --dynamic-list* options override -Bsymbolic

> >> >> >>>>and -Bsymbolic-functions is incorrect since there is NOTHING to override.

> >> >> >>>

> >> >> >>>I don't agree with this statement. --dynamic-list* have function overlay

> >> >> >>>with -Bsymbolic. When two options overlap in functionality, many users

> >> >> >>>expect the more fine-grained option to win, thus my thought that

> >> >> >>>--dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

> >> >> >>>

> >> >> >>>(FWIW LLD's preemptibility logic is

> >> >> >>>   ...

> >> >> >>>   if (config->hasDynamicList)

> >> >> >>>     return sym.inDynamicList;

> >> >> >>>   return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());

> >> >> >>>   This works fine and I won't change it.

> >> >> >>>)

> >> >> >>>

> >> >> >>>>To do it properly:

> >> >> >>>>

> >> >> >>>>1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

> >> >> >>>>Only add symbols to dynamic symbol table.

> >> >> >>>

> >> >> >>>This was already implemented when you implemented --dynamic-list in

> >> >> >>>2006. This patch does not change the fact.

> >> >> >>>

> >> >> >>>>2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

> >> >> >>>>and ignore --export-dynamic-symbol for -shared.  It has to be done at the

> >> >> >>>>end of command-line parsing.

> >> >> >>

> >> >> >>There is no need to change linker manual.  --dynamic-list* should work both

> >> >> >>before and after -Bsymbolic and -Bsymbolic-functions.  Your patch used

> >> >> >>libdl2d.so without creating it first.

> >> >> >>

> >> >> >>Here is the updated version of your patch.  I took the liberty to make

> >> >> >>the changes above.

> >> >> >>

> >> >> >>--

> >> >> >>H.J.

> >> >> >

> >> >> >Thanks for the fix (libdl4e.so->libdl2d.so)

> >> >> >

> >> >> >The updated patch looks good to me.

> >> >>

> >> >> Around lexsup.c:1162

> >> >>

> >> >> `case symbolic_functions:` should set link_info.dynamic_data and link_info.dynamic , otherwise

> >> >> -Bsymbolic-functions is broken.

> >> >>

> >> >>

> >> >> .globl _start, foo

> >> >> _start:

> >> >>    call foo

> >> >> foo

> >> >>

> >> >> ld-new -shared -Bsymbolic-functions a.o -o a.so

> >> >>

> >> >> foo@plt is not created

> >> >

> >> >'-Bsymbolic-functions'

> >> >     When creating a shared library, bind references to global function

> >> >     symbols to the definition within the shared library, if any.  This

> >> >     option is only meaningful on ELF platforms which support shared

> >> >     libraries.

> >> >

> >> >There should be no foo@plt.

> >>

> >> Sorry, my example missed .type:

> >>

> >> .global _start, foo

> >> .type foo, @function

> >> _start:

> >>    call foo

> >> foo:

> >>

> >>

> >> ld-new -shared -Bsymbolic-functions a.o -o a.so

> >> foo@plt was created without the patch.

> >

> >On master branch, I got

> >

> >[hjl@gnu-cfl-2 pr26018]$ cat func.s

> >.global _start, foo

> >.type foo, %function

> >.text

> >_start:

> >call foo

> >foo:

> >ret

> >[hjl@gnu-cfl-2 pr26018]$ make func.so

> >as   -o func.o func.s

> >./ld -shared  -Bsymbolic-function -o func.so func.o

> >[hjl@gnu-cfl-2 pr26018]$ objdump -dw func.so

> >

> >func.so:     file format elf64-x86-64

> >

> >

> >Disassembly of section .text:

> >

> >0000000000001000 <_start>:

> >    1000: e8 00 00 00 00        callq  1005 <foo>

> >

> >0000000000001005 <foo>:

> >    1005: c3                    retq

> >[hjl@gnu-cfl-2 pr26018]$

> >

> >It looks OK to me.

>

> At master, foo is bound locally (intended).

> With this patch, foo@plt is created

>


Fixed.


-- 
H.J.
On 2020-05-23, H.J. Lu wrote:
>On Sat, May 23, 2020 at 7:56 PM Fangrui Song <maskray@google.com> wrote:

>>

>> On 2020-05-23, H.J. Lu wrote:

>> >On Sat, May 23, 2020 at 6:05 PM Fangrui Song <maskray@google.com> wrote:

>> >>

>> >> On 2020-05-23, H.J. Lu wrote:

>> >> >On Sat, May 23, 2020 at 5:11 PM Fangrui Song <maskray@google.com> wrote:

>> >> >>

>> >> >> On 2020-05-23, Fangrui Song wrote:

>> >> >> >On 2020-05-23, H.J. Lu wrote:

>> >> >> >>On Fri, May 22, 2020 at 8:56 AM Fangrui Song <maskray@google.com> wrote:

>> >> >> >>>

>> >> >> >>>

>> >> >> >>>On 2020-05-22, H.J. Lu wrote:

>> >> >> >>>>On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

>> >> >> >>>><binutils@sourceware.org> wrote:

>> >> >> >>>>>

>> >> >> >>>>> They aren't used together in reality, so it is safe to change the

>> >> >> >>>>> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

>> >> >> >>>>> should be considered as a subset of -Bsymbolic, so --dynamic-list

>> >> >> >>>>> overridding -Bsymbolic implies that --dynamic-list overridding

>> >> >> >>>>> -Bsymbolic-functions.

>> >> >> >>>>

>> >> >> >>>>--dynamic-list=DYNAMIC-LIST-FILE'

>> >> >> >>>>     Specify the name of a dynamic list file to the linker.  This is

>> >> >> >>>>     typically used when creating shared libraries to specify a list of

>> >> >> >>>>     global symbols whose references shouldn't be bound to the

>> >> >> >>>>     definition within the shared library, or creating dynamically

>> >> >> >>>>     linked executables to specify a list of symbols which should be

>> >> >> >>>>     added to the symbol table in the executable.  This option is only

>> >> >> >>>>     meaningful on ELF platforms which support shared libraries.

>> >> >> >>>>

>> >> >> >>>>The --dynamic-list* options are intended for shared libraries.

>> >> >> >>>

>> >> >> >>>--dynamic-list* work for both executables and shared libraries.

>> >> >> >>>For an executable, export some symbols.

>> >> >> >>>For a shared library, specify preemptible symbols.

>> >> >> >>>

>> >> >> >>>>The goal

>> >> >> >>>>IS NOT put them in dynamic symbol table since all global symbols are in

>> >> >> >>>>dynamic symbol table already in a shared library.  The goal is to make

>> >> >> >>>>them PREEMPTIBLE.

>> >> >> >>>

>> >> >> >>>I agree with this sentence and the patch respects it.

>> >> >> >>>

>> >> >> >>>>So making the --dynamic-list* options override -Bsymbolic

>> >> >> >>>>and -Bsymbolic-functions is incorrect since there is NOTHING to override.

>> >> >> >>>

>> >> >> >>>I don't agree with this statement. --dynamic-list* have function overlay

>> >> >> >>>with -Bsymbolic. When two options overlap in functionality, many users

>> >> >> >>>expect the more fine-grained option to win, thus my thought that

>> >> >> >>>--dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

>> >> >> >>>

>> >> >> >>>(FWIW LLD's preemptibility logic is

>> >> >> >>>   ...

>> >> >> >>>   if (config->hasDynamicList)

>> >> >> >>>     return sym.inDynamicList;

>> >> >> >>>   return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());

>> >> >> >>>   This works fine and I won't change it.

>> >> >> >>>)

>> >> >> >>>

>> >> >> >>>>To do it properly:

>> >> >> >>>>

>> >> >> >>>>1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

>> >> >> >>>>Only add symbols to dynamic symbol table.

>> >> >> >>>

>> >> >> >>>This was already implemented when you implemented --dynamic-list in

>> >> >> >>>2006. This patch does not change the fact.

>> >> >> >>>

>> >> >> >>>>2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

>> >> >> >>>>and ignore --export-dynamic-symbol for -shared.  It has to be done at the

>> >> >> >>>>end of command-line parsing.

>> >> >> >>

>> >> >> >>There is no need to change linker manual.  --dynamic-list* should work both

>> >> >> >>before and after -Bsymbolic and -Bsymbolic-functions.  Your patch used

>> >> >> >>libdl2d.so without creating it first.

>> >> >> >>

>> >> >> >>Here is the updated version of your patch.  I took the liberty to make

>> >> >> >>the changes above.

>> >> >> >>

>> >> >> >>--

>> >> >> >>H.J.

>> >> >> >

>> >> >> >Thanks for the fix (libdl4e.so->libdl2d.so)

>> >> >> >

>> >> >> >The updated patch looks good to me.

>> >> >>

>> >> >> Around lexsup.c:1162

>> >> >>

>> >> >> `case symbolic_functions:` should set link_info.dynamic_data and link_info.dynamic , otherwise

>> >> >> -Bsymbolic-functions is broken.

>> >> >>

>> >> >>

>> >> >> .globl _start, foo

>> >> >> _start:

>> >> >>    call foo

>> >> >> foo

>> >> >>

>> >> >> ld-new -shared -Bsymbolic-functions a.o -o a.so

>> >> >>

>> >> >> foo@plt is not created

>> >> >

>> >> >'-Bsymbolic-functions'

>> >> >     When creating a shared library, bind references to global function

>> >> >     symbols to the definition within the shared library, if any.  This

>> >> >     option is only meaningful on ELF platforms which support shared

>> >> >     libraries.

>> >> >

>> >> >There should be no foo@plt.

>> >>

>> >> Sorry, my example missed .type:

>> >>

>> >> .global _start, foo

>> >> .type foo, @function

>> >> _start:

>> >>    call foo

>> >> foo:

>> >>

>> >>

>> >> ld-new -shared -Bsymbolic-functions a.o -o a.so

>> >> foo@plt was created without the patch.

>> >

>> >On master branch, I got

>> >

>> >[hjl@gnu-cfl-2 pr26018]$ cat func.s

>> >.global _start, foo

>> >.type foo, %function

>> >.text

>> >_start:

>> >call foo

>> >foo:

>> >ret

>> >[hjl@gnu-cfl-2 pr26018]$ make func.so

>> >as   -o func.o func.s

>> >./ld -shared  -Bsymbolic-function -o func.so func.o

>> >[hjl@gnu-cfl-2 pr26018]$ objdump -dw func.so

>> >

>> >func.so:     file format elf64-x86-64

>> >

>> >

>> >Disassembly of section .text:

>> >

>> >0000000000001000 <_start>:

>> >    1000: e8 00 00 00 00        callq  1005 <foo>

>> >

>> >0000000000001005 <foo>:

>> >    1005: c3                    retq

>> >[hjl@gnu-cfl-2 pr26018]$

>> >

>> >It looks OK to me.

>>

>> At master, foo is bound locally (intended).

>> With this patch, foo@plt is created

>>

>

>Fixed.


Thanks. LG
On Sat, May 23, 2020 at 9:34 PM Fangrui Song <maskray@google.com> wrote:
>

>

> On 2020-05-23, H.J. Lu wrote:

> >On Sat, May 23, 2020 at 7:56 PM Fangrui Song <maskray@google.com> wrote:

> >>

> >> On 2020-05-23, H.J. Lu wrote:

> >> >On Sat, May 23, 2020 at 6:05 PM Fangrui Song <maskray@google.com> wrote:

> >> >>

> >> >> On 2020-05-23, H.J. Lu wrote:

> >> >> >On Sat, May 23, 2020 at 5:11 PM Fangrui Song <maskray@google.com> wrote:

> >> >> >>

> >> >> >> On 2020-05-23, Fangrui Song wrote:

> >> >> >> >On 2020-05-23, H.J. Lu wrote:

> >> >> >> >>On Fri, May 22, 2020 at 8:56 AM Fangrui Song <maskray@google.com> wrote:

> >> >> >> >>>

> >> >> >> >>>

> >> >> >> >>>On 2020-05-22, H.J. Lu wrote:

> >> >> >> >>>>On Wed, May 20, 2020 at 6:48 PM Fangrui Song via Binutils

> >> >> >> >>>><binutils@sourceware.org> wrote:

> >> >> >> >>>>>

> >> >> >> >>>>> They aren't used together in reality, so it is safe to change the

> >> >> >> >>>>> semantics. --dynamic-list is refined -Bsymbolic. -Bsymbolic-functions

> >> >> >> >>>>> should be considered as a subset of -Bsymbolic, so --dynamic-list

> >> >> >> >>>>> overridding -Bsymbolic implies that --dynamic-list overridding

> >> >> >> >>>>> -Bsymbolic-functions.

> >> >> >> >>>>

> >> >> >> >>>>--dynamic-list=DYNAMIC-LIST-FILE'

> >> >> >> >>>>     Specify the name of a dynamic list file to the linker.  This is

> >> >> >> >>>>     typically used when creating shared libraries to specify a list of

> >> >> >> >>>>     global symbols whose references shouldn't be bound to the

> >> >> >> >>>>     definition within the shared library, or creating dynamically

> >> >> >> >>>>     linked executables to specify a list of symbols which should be

> >> >> >> >>>>     added to the symbol table in the executable.  This option is only

> >> >> >> >>>>     meaningful on ELF platforms which support shared libraries.

> >> >> >> >>>>

> >> >> >> >>>>The --dynamic-list* options are intended for shared libraries.

> >> >> >> >>>

> >> >> >> >>>--dynamic-list* work for both executables and shared libraries.

> >> >> >> >>>For an executable, export some symbols.

> >> >> >> >>>For a shared library, specify preemptible symbols.

> >> >> >> >>>

> >> >> >> >>>>The goal

> >> >> >> >>>>IS NOT put them in dynamic symbol table since all global symbols are in

> >> >> >> >>>>dynamic symbol table already in a shared library.  The goal is to make

> >> >> >> >>>>them PREEMPTIBLE.

> >> >> >> >>>

> >> >> >> >>>I agree with this sentence and the patch respects it.

> >> >> >> >>>

> >> >> >> >>>>So making the --dynamic-list* options override -Bsymbolic

> >> >> >> >>>>and -Bsymbolic-functions is incorrect since there is NOTHING to override.

> >> >> >> >>>

> >> >> >> >>>I don't agree with this statement. --dynamic-list* have function overlay

> >> >> >> >>>with -Bsymbolic. When two options overlap in functionality, many users

> >> >> >> >>>expect the more fine-grained option to win, thus my thought that

> >> >> >> >>>--dynamic-list* override -Bsymbolic and -Bsymbolic-functions.

> >> >> >> >>>

> >> >> >> >>>(FWIW LLD's preemptibility logic is

> >> >> >> >>>   ...

> >> >> >> >>>   if (config->hasDynamicList)

> >> >> >> >>>     return sym.inDynamicList;

> >> >> >> >>>   return !(config->bsymbolic || config->bsymbolicFunctions && sym.isFunc());

> >> >> >> >>>   This works fine and I won't change it.

> >> >> >> >>>)

> >> >> >> >>>

> >> >> >> >>>>To do it properly:

> >> >> >> >>>>

> >> >> >> >>>>1.  Extend  --dynamic-list* to executables.  Symbol binding is unchanged.

> >> >> >> >>>>Only add symbols to dynamic symbol table.

> >> >> >> >>>

> >> >> >> >>>This was already implemented when you implemented --dynamic-list in

> >> >> >> >>>2006. This patch does not change the fact.

> >> >> >> >>>

> >> >> >> >>>>2. Make --export-dynamic-symbol an alias of  --dynamic-list* for executables.

> >> >> >> >>>>and ignore --export-dynamic-symbol for -shared.  It has to be done at the

> >> >> >> >>>>end of command-line parsing.

> >> >> >> >>

> >> >> >> >>There is no need to change linker manual.  --dynamic-list* should work both

> >> >> >> >>before and after -Bsymbolic and -Bsymbolic-functions.  Your patch used

> >> >> >> >>libdl2d.so without creating it first.

> >> >> >> >>

> >> >> >> >>Here is the updated version of your patch.  I took the liberty to make

> >> >> >> >>the changes above.

> >> >> >> >>

> >> >> >> >>--

> >> >> >> >>H.J.

> >> >> >> >

> >> >> >> >Thanks for the fix (libdl4e.so->libdl2d.so)

> >> >> >> >

> >> >> >> >The updated patch looks good to me.

> >> >> >>

> >> >> >> Around lexsup.c:1162

> >> >> >>

> >> >> >> `case symbolic_functions:` should set link_info.dynamic_data and link_info.dynamic , otherwise

> >> >> >> -Bsymbolic-functions is broken.

> >> >> >>

> >> >> >>

> >> >> >> .globl _start, foo

> >> >> >> _start:

> >> >> >>    call foo

> >> >> >> foo

> >> >> >>

> >> >> >> ld-new -shared -Bsymbolic-functions a.o -o a.so

> >> >> >>

> >> >> >> foo@plt is not created

> >> >> >

> >> >> >'-Bsymbolic-functions'

> >> >> >     When creating a shared library, bind references to global function

> >> >> >     symbols to the definition within the shared library, if any.  This

> >> >> >     option is only meaningful on ELF platforms which support shared

> >> >> >     libraries.

> >> >> >

> >> >> >There should be no foo@plt.

> >> >>

> >> >> Sorry, my example missed .type:

> >> >>

> >> >> .global _start, foo

> >> >> .type foo, @function

> >> >> _start:

> >> >>    call foo

> >> >> foo:

> >> >>

> >> >>

> >> >> ld-new -shared -Bsymbolic-functions a.o -o a.so

> >> >> foo@plt was created without the patch.

> >> >

> >> >On master branch, I got

> >> >

> >> >[hjl@gnu-cfl-2 pr26018]$ cat func.s

> >> >.global _start, foo

> >> >.type foo, %function

> >> >.text

> >> >_start:

> >> >call foo

> >> >foo:

> >> >ret

> >> >[hjl@gnu-cfl-2 pr26018]$ make func.so

> >> >as   -o func.o func.s

> >> >./ld -shared  -Bsymbolic-function -o func.so func.o

> >> >[hjl@gnu-cfl-2 pr26018]$ objdump -dw func.so

> >> >

> >> >func.so:     file format elf64-x86-64

> >> >

> >> >

> >> >Disassembly of section .text:

> >> >

> >> >0000000000001000 <_start>:

> >> >    1000: e8 00 00 00 00        callq  1005 <foo>

> >> >

> >> >0000000000001005 <foo>:

> >> >    1005: c3                    retq

> >> >[hjl@gnu-cfl-2 pr26018]$

> >> >

> >> >It looks OK to me.

> >>

> >> At master, foo is bound locally (intended).

> >> With this patch, foo@plt is created

> >>

> >

> >Fixed.

>

> Thanks. LG


This is the patch I am checking in.

Thanks.

-- 
H.J.
From 32c2196b05608e1a17cfb0bc56aaecdc9dfec5e8 Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@google.com>
Date: Wed, 20 May 2020 18:31:39 -0700
Subject: [PATCH] ld: Handle --dynamic-list* before -Bsymbolic
 -Bsymbolic-functions

--dynamic-list* should work both before and after -Bsymbolic and
-Bsymbolic-functions.

	PR ld/26018
	* lexsup.c (parse_args): Simplify.
	* testsuite/ld-elf/dl4e.out: New.
	* testsuite/ld-elf/shared.exp: Updated for PR ld/26018 tests.
---
 ld/lexsup.c                    | 37 ++++++++++++++--------------------
 ld/testsuite/ld-elf/dl4e.out   |  6 ++++++
 ld/testsuite/ld-elf/shared.exp | 12 ++++++++---
 3 files changed, 30 insertions(+), 25 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/dl4e.out

diff --git a/ld/lexsup.c b/ld/lexsup.c
index fe9526b527..04db2f129f 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -1390,22 +1390,16 @@ parse_args (unsigned argc, char **argv)
 	  break;
 	case OPTION_DYNAMIC_LIST_DATA:
 	  opt_dynamic_list = dynamic_list_data;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_DYNAMIC_LIST_CPP_TYPEINFO:
 	  lang_append_dynamic_list_cpp_typeinfo ();
 	  if (opt_dynamic_list != dynamic_list_data)
 	    opt_dynamic_list = dynamic_list;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_DYNAMIC_LIST_CPP_NEW:
 	  lang_append_dynamic_list_cpp_new ();
 	  if (opt_dynamic_list != dynamic_list_data)
 	    opt_dynamic_list = dynamic_list;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_DYNAMIC_LIST:
 	  /* This option indicates a small script that only specifies
@@ -1422,8 +1416,6 @@ parse_args (unsigned argc, char **argv)
 	  }
 	  if (opt_dynamic_list != dynamic_list_data)
 	    opt_dynamic_list = dynamic_list;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_WARN_COMMON:
 	  config.warn_common = TRUE;
@@ -1632,6 +1624,19 @@ parse_args (unsigned argc, char **argv)
       && command_line.check_section_addresses < 0)
     command_line.check_section_addresses = 0;
 
+  switch (opt_dynamic_list)
+    {
+    case dynamic_list_unset:
+      break;
+    case dynamic_list_data:
+      link_info.dynamic_data = TRUE;
+      /* Fall through.  */
+    case dynamic_list:
+      link_info.dynamic = TRUE;
+      opt_symbolic = symbolic_unset;
+      break;
+    }
+
   /* -Bsymbolic and -Bsymbols-functions are for shared library output.  */
   if (bfd_link_dll (&link_info))
     switch (opt_symbolic)
@@ -1651,25 +1656,13 @@ parse_args (unsigned argc, char **argv)
 	    free (link_info.dynamic_list);
 	    link_info.dynamic_list = NULL;
 	  }
-	opt_dynamic_list = dynamic_list_unset;
 	break;
       case symbolic_functions:
-	opt_dynamic_list = dynamic_list_data;
+	link_info.dynamic = TRUE;
+	link_info.dynamic_data = TRUE;
 	break;
       }
 
-  switch (opt_dynamic_list)
-    {
-    case dynamic_list_unset:
-      break;
-    case dynamic_list_data:
-      link_info.dynamic_data = TRUE;
-      /* Fall through.  */
-    case dynamic_list:
-      link_info.dynamic = TRUE;
-      break;
-    }
-
   if (!bfd_link_dll (&link_info))
     {
       if (command_line.filter_shlib)
diff --git a/ld/testsuite/ld-elf/dl4e.out b/ld/testsuite/ld-elf/dl4e.out
new file mode 100644
index 0000000000..e5da6e2185
--- /dev/null
+++ b/ld/testsuite/ld-elf/dl4e.out
@@ -0,0 +1,6 @@
+bar OK2
+bar OK4
+DSO1
+DSO2
+OK2
+OK4
diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index 3366430515..7d35f3f379 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -598,6 +598,9 @@ set build_tests {
   {"Build libdl2c.so with --dynamic-list-data and dl2xxx.list"
    "-shared -Wl,--dynamic-list-data,--dynamic-list=dl2xxx.list" "-fPIC"
    {dl2.c dl2xxx.c} {} "libdl2c.so"}
+  {"Build libdl2d.so with --dynamic-list-data -Bsymbolic"
+   "-shared -Wl,-Bsymbolic,--dynamic-list-data" "-fPIC"
+   {dl2.c dl2xxx.c} {} "libdl2d.so"}
   {"Build libdl4a.so with --dynamic-list=dl4.list"
    "-shared -Wl,--dynamic-list=dl4.list" "-fPIC"
    {dl4.c dl4xxx.c} {} "libdl4a.so"}
@@ -874,6 +877,9 @@ set run_tests [list \
     [list "Run with libdl2c.so" \
      "-Wl,--no-as-needed tmpdir/libdl2c.so" "" \
      {dl2main.c} "dl2c" "dl2b.out" ] \
+    [list "Run with libdl2d.so" \
+     "-Wl,--no-as-needed tmpdir/libdl2d.so" "" \
+     {dl2main.c} "dl2d" "dl2a.out" ] \
     [list "Run with libdl4a.so" \
      "-Wl,--no-as-needed tmpdir/libdl4a.so" "" \
      {dl4main.c} "dl4a" "dl4a.out" ] \
@@ -888,10 +894,10 @@ set run_tests [list \
      {dl4main.c} "dl4d" "dl4b.out" ] \
     [list "Run with libdl4e.so" \
      "-Wl,--no-as-needed tmpdir/libdl4e.so" "" \
-     {dl4main.c} "dl4e" "dl4a.out" ] \
+     {dl4main.c} "dl4e" "dl4e.out" ] \
     [list "Run with libdl4f.so" \
      "-Wl,--no-as-needed tmpdir/libdl4f.so" "" \
-     {dl4main.c} "dl4f" "dl4a.out" ] \
+     {dl4main.c} "dl4f" "dl4e.out" ] \
     [list "Run with libdata1.so" \
      "-Wl,--no-as-needed tmpdir/libdata1.so" "" \
      {dynbss1.c} "dynbss1" "pass.out" ] \
@@ -988,7 +994,7 @@ set dlopen_run_tests [list \
      {dl6cmain.c} "dl6c1" "dl6b.out" ] \
     [list "Run dl6d1 with --dynamic-list-data and dlopen on libdl6d.so" \
      "-Wl,--no-as-needed,--dynamic-list-data $extralibs" "" \
-     {dl6dmain.c} "dl6d1" "dl6b.out" ] \
+     {dl6dmain.c} "dl6d1" "dl6a.out" ] \
     [list "Run pr21964-2" \
      "-Wl,--no-as-needed,-rpath,tmpdir tmpdir/pr21964-2a.so $extralibs" "" \
      {pr21964-2c.c} "pr21964-2" "pass.out" ] \

Patch

diff --git a/ld/ld.texi b/ld/ld.texi
index 4dc78e65fa..6969e4f4e8 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -1496,7 +1496,8 @@  typically used when creating shared libraries to specify a list of
 global symbols whose references shouldn't be bound to the definition
 within the shared library, or creating dynamically linked executables
 to specify a list of symbols which should be added to the symbol table
-in the executable.  This option is only meaningful on ELF platforms
+in the executable.  This option overrides @option{-Bsymbolic} and
+@option{-Bsymbolic-functions}.  This option is only meaningful on ELF platforms
 which support shared libraries.
 
 The format of the dynamic list is the same as the version node without
diff --git a/ld/lexsup.c b/ld/lexsup.c
index c02041d5f1..26b14edfa3 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -1390,22 +1390,16 @@  parse_args (unsigned argc, char **argv)
 	  break;
 	case OPTION_DYNAMIC_LIST_DATA:
 	  opt_dynamic_list = dynamic_list_data;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_DYNAMIC_LIST_CPP_TYPEINFO:
 	  lang_append_dynamic_list_cpp_typeinfo ();
 	  if (opt_dynamic_list != dynamic_list_data)
 	    opt_dynamic_list = dynamic_list;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_DYNAMIC_LIST_CPP_NEW:
 	  lang_append_dynamic_list_cpp_new ();
 	  if (opt_dynamic_list != dynamic_list_data)
 	    opt_dynamic_list = dynamic_list;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_DYNAMIC_LIST:
 	  /* This option indicates a small script that only specifies
@@ -1422,8 +1416,6 @@  parse_args (unsigned argc, char **argv)
 	  }
 	  if (opt_dynamic_list != dynamic_list_data)
 	    opt_dynamic_list = dynamic_list;
-	  if (opt_symbolic == symbolic)
-	    opt_symbolic = symbolic_unset;
 	  break;
 	case OPTION_WARN_COMMON:
 	  config.warn_common = TRUE;
@@ -1633,6 +1625,19 @@  parse_args (unsigned argc, char **argv)
       && command_line.check_section_addresses < 0)
     command_line.check_section_addresses = 0;
 
+  switch (opt_dynamic_list)
+    {
+    case dynamic_list_unset:
+      break;
+    case dynamic_list_data:
+      link_info.dynamic_data = TRUE;
+      /* Fall through.  */
+    case dynamic_list:
+      link_info.dynamic = TRUE;
+      opt_symbolic = symbolic_unset;
+      break;
+    }
+
   /* -Bsymbolic and -Bsymbols-functions are for shared library output.  */
   if (bfd_link_dll (&link_info))
     switch (opt_symbolic)
@@ -1659,18 +1664,6 @@  parse_args (unsigned argc, char **argv)
 	break;
       }
 
-  switch (opt_dynamic_list)
-    {
-    case dynamic_list_unset:
-      break;
-    case dynamic_list_data:
-      link_info.dynamic_data = TRUE;
-      /* Fall through.  */
-    case dynamic_list:
-      link_info.dynamic = TRUE;
-      break;
-    }
-
   if (!bfd_link_dll (&link_info))
     {
       if (command_line.filter_shlib)
diff --git a/ld/testsuite/ld-elf/dl4e.out b/ld/testsuite/ld-elf/dl4e.out
new file mode 100644
index 0000000000..e5da6e2185
--- /dev/null
+++ b/ld/testsuite/ld-elf/dl4e.out
@@ -0,0 +1,6 @@ 
+bar OK2
+bar OK4
+DSO1
+DSO2
+OK2
+OK4
diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index 3366430515..df810d91d2 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -610,6 +610,7 @@  set build_tests {
   {"Build libdl4d.so with --dynamic-list-data and dl4xxx.list"
    "-shared -Wl,--dynamic-list-data,--dynamic-list=dl4xxx.list" "-fPIC"
    {dl4.c dl4xxx.c} {} "libdl4d.so"}
+  # --dynamic-list* overrides -Bsymbolic*.
   {"Build libdl4e.so with -Bsymbolic-functions --dynamic-list-cpp-new"
    "-shared -Wl,-Bsymbolic-functions,--dynamic-list-cpp-new" "-fPIC"
    {dl4.c dl4xxx.c} {} "libdl4e.so"}
@@ -874,6 +875,9 @@  set run_tests [list \
     [list "Run with libdl2c.so" \
      "-Wl,--no-as-needed tmpdir/libdl2c.so" "" \
      {dl2main.c} "dl2c" "dl2b.out" ] \
+    [list "Run with libdl2d.so" \
+     "-Wl,--no-as-needed tmpdir/libdl2d.so" "" \
+     {dl2main.c} "dl2d" "dl2a.out" ] \
     [list "Run with libdl4a.so" \
      "-Wl,--no-as-needed tmpdir/libdl4a.so" "" \
      {dl4main.c} "dl4a" "dl4a.out" ] \
@@ -888,10 +892,10 @@  set run_tests [list \
      {dl4main.c} "dl4d" "dl4b.out" ] \
     [list "Run with libdl4e.so" \
      "-Wl,--no-as-needed tmpdir/libdl4e.so" "" \
-     {dl4main.c} "dl4e" "dl4a.out" ] \
+     {dl4main.c} "dl4e" "dl4e.out" ] \
     [list "Run with libdl4f.so" \
      "-Wl,--no-as-needed tmpdir/libdl4f.so" "" \
-     {dl4main.c} "dl4f" "dl4a.out" ] \
+     {dl4main.c} "dl4f" "dl4e.out" ] \
     [list "Run with libdata1.so" \
      "-Wl,--no-as-needed tmpdir/libdata1.so" "" \
      {dynbss1.c} "dynbss1" "pass.out" ] \
@@ -988,7 +992,7 @@  set dlopen_run_tests [list \
      {dl6cmain.c} "dl6c1" "dl6b.out" ] \
     [list "Run dl6d1 with --dynamic-list-data and dlopen on libdl6d.so" \
      "-Wl,--no-as-needed,--dynamic-list-data $extralibs" "" \
-     {dl6dmain.c} "dl6d1" "dl6b.out" ] \
+     {dl6dmain.c} "dl6d1" "dl6a.out" ] \
     [list "Run pr21964-2" \
      "-Wl,--no-as-needed,-rpath,tmpdir tmpdir/pr21964-2a.so $extralibs" "" \
      {pr21964-2c.c} "pr21964-2" "pass.out" ] \