public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
@ 2020-05-21  1:31 Fangrui Song
  2020-05-22 13:12 ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Fangrui Song @ 2020-05-21  1:31 UTC (permalink / raw)
  To: binutils; +Cc: Fangrui Song

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

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" ] \
-- 
2.26.2.761.g0e0b3e54be-goog


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
  2020-05-21  1:31 [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions Fangrui Song
@ 2020-05-22 13:12 ` H.J. Lu
  2020-05-22 15:56   ` Fangrui Song
  0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2020-05-22 13:12 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
  2020-05-22 13:12 ` H.J. Lu
@ 2020-05-22 15:56   ` Fangrui Song
  2020-05-23 19:29     ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Fangrui Song @ 2020-05-22 15:56 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils


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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
  2020-05-22 15:56   ` Fangrui Song
@ 2020-05-23 19:29     ` H.J. Lu
  2020-05-23 21:45       ` Fangrui Song
  0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2020-05-23 19:29 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils

[-- Attachment #1: Type: text/plain, Size: 2946 bytes --]

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.

[-- Attachment #2: 0001-ld-Make-dynamic-list-work-before-Bsymbolic-Bsymbolic.patch --]
[-- Type: text/x-patch, Size: 5365 bytes --]

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" ] \
-- 
2.26.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
  2020-05-23 19:29     ` H.J. Lu
@ 2020-05-23 21:45       ` Fangrui Song
  2020-05-24  0:11         ` Fangrui Song
  0 siblings, 1 reply; 16+ messages in thread
From: Fangrui Song @ 2020-05-23 21:45 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
  2020-05-23 21:45       ` Fangrui Song
@ 2020-05-24  0:11         ` Fangrui Song
  2020-05-24  0:43           ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Fangrui Song @ 2020-05-24  0:11 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
  2020-05-24  0:11         ` Fangrui Song
@ 2020-05-24  0:43           ` H.J. Lu
  2020-05-24  1:05             ` Fangrui Song
  0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2020-05-24  0:43 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
  2020-05-24  0:43           ` H.J. Lu
@ 2020-05-24  1:05             ` Fangrui Song
  2020-05-24  1:15               ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Fangrui Song @ 2020-05-24  1:05 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
  2020-05-24  1:05             ` Fangrui Song
@ 2020-05-24  1:15               ` H.J. Lu
  2020-05-24  2:56                 ` Fangrui Song
  0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2020-05-24  1:15 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions
  2020-05-24  1:15               ` H.J. Lu
@ 2020-05-24  2:56                 ` Fangrui Song
  2020-05-24  3:47                   ` [PATCH] ld: Add -Bsymbolic-functions tests H.J. Lu
  2020-05-24  4:05                   ` V2 [PATCH] ld: Make --dynamic-list* work before -Bsymbolic -Bsymbolic-functions H.J. Lu
  0 siblings, 2 replies; 16+ messages in thread
From: Fangrui Song @ 2020-05-24  2:56 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] ld: Add -Bsymbolic-functions tests
  2020-05-24  2:56                 ` Fangrui Song
@ 2020-05-24  3:47                   ` H.J. Lu
  2020-05-24  3:56                     ` Fangrui Song
  2020-05-24  4:05                   ` V2 [PATCH] ld: Make --dynamic-list* work before -Bsymbolic -Bsymbolic-functions H.J. Lu
  1 sibling, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2020-05-24  3:47 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils

[-- Attachment #1: Type: text/plain, Size: 5839 bytes --]

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
>

I am checking in this.

-- 
H.J.

[-- Attachment #2: 0001-ld-Add-Bsymbolic-functions-tests.patch --]
[-- Type: text/x-patch, Size: 3039 bytes --]

From 756472b879cd559f24bb0255422d8c727bd0b120 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 23 May 2020 20:44:12 -0700
Subject: [PATCH] ld: Add -Bsymbolic-functions tests

	PR ld/26018
	* testsuite/ld-i386/i386.exp: Add a -Bsymbolic-functions test.
	* testsuite/ld-x86-64/x86-64.exp: Likewise.
	* testsuite/ld-i386/pr26018.d: New file.
	* testsuite/ld-x86-64/pr26018.d: Likewise.
	* testsuite/ld-x86-64/pr26018.s: Likewise.
---
 ld/testsuite/ld-i386/i386.exp     |  1 +
 ld/testsuite/ld-i386/pr26018.d    | 16 ++++++++++++++++
 ld/testsuite/ld-x86-64/pr26018.d  | 15 +++++++++++++++
 ld/testsuite/ld-x86-64/pr26018.s  |  7 +++++++
 ld/testsuite/ld-x86-64/x86-64.exp |  1 +
 5 files changed, 40 insertions(+)
 create mode 100644 ld/testsuite/ld-i386/pr26018.d
 create mode 100644 ld/testsuite/ld-x86-64/pr26018.d
 create mode 100644 ld/testsuite/ld-x86-64/pr26018.s

diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp
index 7bb3636d3b..e3a5f8b195 100644
--- a/ld/testsuite/ld-i386/i386.exp
+++ b/ld/testsuite/ld-i386/i386.exp
@@ -496,6 +496,7 @@ run_dump_test "pr23930"
 run_dump_test "pr24322a"
 run_dump_test "pr24322b"
 run_dump_test "align-branch-1"
+run_dump_test "pr26018"
 
 if { !([istarget "i?86-*-linux*"]
        || [istarget "i?86-*-gnu*"]
diff --git a/ld/testsuite/ld-i386/pr26018.d b/ld/testsuite/ld-i386/pr26018.d
new file mode 100644
index 0000000000..9ff16c1343
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr26018.d
@@ -0,0 +1,16 @@
+#source: ../ld-x86-64/pr26018.s
+#as: --32
+#ld: -shared -Bsymbolic-functions -melf_i386
+#objdump: -dw
+
+.*: +file format .*
+
+
+Disassembly of section .text:
+
+[0-9a-f]+ <_start>:
+ +[a-f0-9]+:	e8 00 00 00 00       	call   [0-9a-f]+ <foo>
+
+[0-9a-f]+ <foo>:
+ +[a-f0-9]+:	c3                   	ret    
+#pass
diff --git a/ld/testsuite/ld-x86-64/pr26018.d b/ld/testsuite/ld-x86-64/pr26018.d
new file mode 100644
index 0000000000..c2003aa790
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr26018.d
@@ -0,0 +1,15 @@
+#as: --64
+#ld: -shared -Bsymbolic-functions -melf_x86_64
+#objdump: -dw
+
+.*: +file format .*
+
+
+Disassembly of section .text:
+
+[0-9a-f]+ <_start>:
+ +[a-f0-9]+:	e8 00 00 00 00       	callq  [0-9a-f]+ <foo>
+
+[0-9a-f]+ <foo>:
+ +[a-f0-9]+:	c3                   	retq   
+#pass
diff --git a/ld/testsuite/ld-x86-64/pr26018.s b/ld/testsuite/ld-x86-64/pr26018.s
new file mode 100644
index 0000000000..0b36f2bb11
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr26018.s
@@ -0,0 +1,7 @@
+	.global _start, foo
+	.type foo, %function
+	.text
+_start:
+	call foo@PLT
+foo:
+	ret
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index d2e5ac7205..9db16c6b6e 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -467,6 +467,7 @@ run_dump_test "pr25416-2a"
 run_dump_test "pr25416-2b"
 run_dump_test "pr25416-3"
 run_dump_test "pr25416-4"
+run_dump_test "pr26018"
 
 if { ![istarget "x86_64-*-linux*"] && ![istarget "x86_64-*-nacl*"]} {
     return
-- 
2.26.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Add -Bsymbolic-functions tests
  2020-05-24  3:47                   ` [PATCH] ld: Add -Bsymbolic-functions tests H.J. Lu
@ 2020-05-24  3:56                     ` Fangrui Song
  2020-05-24  4:00                       ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Fangrui Song @ 2020-05-24  3:56 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

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
>>
>
>I am checking in this.

Thank for the test. I think the filename can be `Bsymbolic-functions.s`
to make it clear its relation with the option.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] ld: Add -Bsymbolic-functions tests
  2020-05-24  3:56                     ` Fangrui Song
@ 2020-05-24  4:00                       ` H.J. Lu
  0 siblings, 0 replies; 16+ messages in thread
From: H.J. Lu @ 2020-05-24  4:00 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils

On Sat, May 23, 2020 at 8:56 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
> >>
> >
> >I am checking in this.
>
> Thank for the test. I think the filename can be `Bsymbolic-functions.s`
> to make it clear its relation with the option.

Maybe next time.

-- 
H.J.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* V2 [PATCH] ld: Make --dynamic-list* work before -Bsymbolic -Bsymbolic-functions
  2020-05-24  2:56                 ` Fangrui Song
  2020-05-24  3:47                   ` [PATCH] ld: Add -Bsymbolic-functions tests H.J. Lu
@ 2020-05-24  4:05                   ` H.J. Lu
  2020-05-24  4:34                     ` Fangrui Song
  1 sibling, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2020-05-24  4:05 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils

[-- Attachment #1: Type: text/plain, Size: 5824 bytes --]

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.

[-- Attachment #2: 0001-ld-Make-dynamic-list-work-before-Bsymbolic-Bsymbolic.patch --]
[-- Type: application/x-patch, Size: 5629 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: V2 [PATCH] ld: Make --dynamic-list* work before -Bsymbolic -Bsymbolic-functions
  2020-05-24  4:05                   ` V2 [PATCH] ld: Make --dynamic-list* work before -Bsymbolic -Bsymbolic-functions H.J. Lu
@ 2020-05-24  4:34                     ` Fangrui Song
  2020-05-24 11:50                       ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Fangrui Song @ 2020-05-24  4:34 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: V2 [PATCH] ld: Make --dynamic-list* work before -Bsymbolic -Bsymbolic-functions
  2020-05-24  4:34                     ` Fangrui Song
@ 2020-05-24 11:50                       ` H.J. Lu
  0 siblings, 0 replies; 16+ messages in thread
From: H.J. Lu @ 2020-05-24 11:50 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Binutils

[-- Attachment #1: Type: text/plain, Size: 6461 bytes --]

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.

[-- Attachment #2: 0001-ld-Handle-dynamic-list-before-Bsymbolic-Bsymbolic-fu.patch --]
[-- Type: text/x-patch, Size: 5608 bytes --]

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" ] \
-- 
2.26.2


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-05-24 11:51 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21  1:31 [PATCH] ld: Make --dynamic-list* override -Bsymbolic -Bsymbolic-functions Fangrui Song
2020-05-22 13:12 ` H.J. Lu
2020-05-22 15:56   ` Fangrui Song
2020-05-23 19:29     ` H.J. Lu
2020-05-23 21:45       ` Fangrui Song
2020-05-24  0:11         ` Fangrui Song
2020-05-24  0:43           ` H.J. Lu
2020-05-24  1:05             ` Fangrui Song
2020-05-24  1:15               ` H.J. Lu
2020-05-24  2:56                 ` Fangrui Song
2020-05-24  3:47                   ` [PATCH] ld: Add -Bsymbolic-functions tests H.J. Lu
2020-05-24  3:56                     ` Fangrui Song
2020-05-24  4:00                       ` H.J. Lu
2020-05-24  4:05                   ` V2 [PATCH] ld: Make --dynamic-list* work before -Bsymbolic -Bsymbolic-functions H.J. Lu
2020-05-24  4:34                     ` Fangrui Song
2020-05-24 11:50                       ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).