-Bsymbolic is not for PIEs

Message ID 20191002015837.GA7064@bubble.grove.modra.org
State New
Headers show
Series
  • -Bsymbolic is not for PIEs
Related show

Commit Message

Alan Modra Oct. 2, 2019, 1:58 a.m.
Despite PR19615, it doesn't make sense to use -Bsymbolic with PIEs.
Dynamic symbols in an executable won't be overridden anyway, so if
-Bsymbolic makes any difference in a PIE it can only be papering over
other bugs.

	* ld.texi (-Bsymbolic, -Bsymbolic-functions): Don't mention PIEs.
	* ld.h (symbolic_enum, dynamic_list_enum),
	(args_type <symbolic, dynamic_list>): Move to..
	* lexsup.c (parse_args): ..here, using auto vars opt_symbolic
	and opt_dynamic_list rather than command_line fields.  Only
	act on -Bsymbolic and -Bsymbolic-functions for shared library
	output.  Free dynamic_list.


-- 
Alan Modra
Australia Development Lab, IBM

Comments

Fangrui Song Oct. 2, 2019, 10:05 a.m. | #1
On 2019-10-02, Alan Modra wrote:
>Despite PR19615, it doesn't make sense to use -Bsymbolic with PIEs.

>Dynamic symbols in an executable won't be overridden anyway, so if

>-Bsymbolic makes any difference in a PIE it can only be papering over

>other bugs.

>

>	* ld.texi (-Bsymbolic, -Bsymbolic-functions): Don't mention PIEs.

>	* ld.h (symbolic_enum, dynamic_list_enum),

>	(args_type <symbolic, dynamic_list>): Move to..

>	* lexsup.c (parse_args): ..here, using auto vars opt_symbolic

>	and opt_dynamic_list rather than command_line fields.  Only

>	act on -Bsymbolic and -Bsymbolic-functions for shared library

>	output.  Free dynamic_list.

>

>diff --git a/ld/ld.h b/ld/ld.h

>index 55078a9637..5243346f37 100644

>--- a/ld/ld.h

>+++ b/ld/ld.h

>@@ -119,20 +119,6 @@ struct wildcard_list

>

> enum endian_enum { ENDIAN_UNSET = 0, ENDIAN_BIG, ENDIAN_LITTLE };

>

>-enum symbolic_enum

>-{

>-  symbolic_unset = 0,

>-  symbolic,

>-  symbolic_functions,

>-};

>-

>-enum dynamic_list_enum

>-{

>-  dynamic_list_unset = 0,

>-  dynamic_list_data,

>-  dynamic_list

>-};

>-

> typedef struct

> {

>   /* 1 => assign space to common symbols even if `relocatable_output'.  */

>@@ -183,13 +169,6 @@ typedef struct

>   /* Big or little endian as set on command line.  */

>   enum endian_enum endian;

>

>-  /* -Bsymbolic and -Bsymbolic-functions, as set on command line.  */

>-  enum symbolic_enum symbolic;

>-

>-  /* --dynamic-list, --dynamic-list-cpp-new, --dynamic-list-cpp-typeinfo

>-     and --dynamic-list FILE, as set on command line.  */

>-  enum dynamic_list_enum dynamic_list;

>-

>   /* Name of runtime interpreter to invoke.  */

>   char *interpreter;

>

>diff --git a/ld/ld.texi b/ld/ld.texi

>index fcbc335c95..3b2d05411f 100644

>--- a/ld/ld.texi

>+++ b/ld/ld.texi

>@@ -1436,21 +1436,15 @@ libraries.

> When creating a shared library, bind references to global symbols to the

> definition within the shared library, if any.  Normally, it is possible

> for a program linked against a shared library to override the definition

>-within the shared library.  This option can also be used with the

>-@option{--export-dynamic} option, when creating a position independent

>-executable, to bind references to global symbols to the definition within

>-the executable.  This option is only meaningful on ELF platforms which

>-support shared libraries and position independent executables.

>+within the shared library.  This option is only meaningful on ELF

>+platforms which support shared libraries.

>

> @kindex -Bsymbolic-functions

> @item -Bsymbolic-functions

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

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

>-This option can also be used with the @option{--export-dynamic} option,

>-when creating a position independent executable, to bind references

>-to global function symbols to the definition within the executable.

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

>-libraries and position independent executables.

>+libraries.

>

> @kindex --dynamic-list=@var{dynamic-list-file}

> @item --dynamic-list=@var{dynamic-list-file}

>diff --git a/ld/lexsup.c b/ld/lexsup.c

>index 1c15ac29c0..f91549697e 100644

>--- a/ld/lexsup.c

>+++ b/ld/lexsup.c

>@@ -565,6 +565,18 @@ parse_args (unsigned argc, char **argv)

>   struct option *really_longopts;

>   int last_optind;

>   enum report_method how_to_report_unresolved_symbols = RM_GENERATE_ERROR;

>+  enum symbolic_enum

>+  {

>+    symbolic_unset = 0,

>+    symbolic,

>+    symbolic_functions,

>+  } opt_symbolic = symbolic_unset;

>+  enum dynamic_list_enum

>+  {

>+    dynamic_list_unset = 0,

>+    dynamic_list_data,

>+    dynamic_list

>+  } opt_dynamic_list = dynamic_list_unset;

>

>   shortopts = (char *) xmalloc (OPTION_COUNT * 3 + 2);

>   longopts = (struct option *)

>@@ -1233,10 +1245,10 @@ parse_args (unsigned argc, char **argv)

> 	  config.stats = TRUE;

> 	  break;

> 	case OPTION_SYMBOLIC:

>-	  command_line.symbolic = symbolic;

>+	  opt_symbolic = symbolic;

> 	  break;

> 	case OPTION_SYMBOLIC_FUNCTIONS:

>-	  command_line.symbolic = symbolic_functions;

>+	  opt_symbolic = symbolic_functions;

> 	  break;

> 	case 't':

> 	  ++trace_files;

>@@ -1381,23 +1393,23 @@ parse_args (unsigned argc, char **argv)

> 	  command_line.version_exports_section = optarg;

> 	  break;

> 	case OPTION_DYNAMIC_LIST_DATA:

>-	  command_line.dynamic_list = dynamic_list_data;

>-	  if (command_line.symbolic == symbolic)

>-	    command_line.symbolic = symbolic_unset;

>+	  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 (command_line.dynamic_list != dynamic_list_data)

>-	    command_line.dynamic_list = dynamic_list;

>-	  if (command_line.symbolic == symbolic)

>-	    command_line.symbolic = symbolic_unset;

>+	  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 (command_line.dynamic_list != dynamic_list_data)

>-	    command_line.dynamic_list = dynamic_list;

>-	  if (command_line.symbolic == symbolic)

>-	    command_line.symbolic = symbolic_unset;

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

>@@ -1412,10 +1424,10 @@ parse_args (unsigned argc, char **argv)

> 	    parser_input = input_dynamic_list;

> 	    yyparse ();

> 	  }

>-	  if (command_line.dynamic_list != dynamic_list_data)

>-	    command_line.dynamic_list = dynamic_list;

>-	  if (command_line.symbolic == symbolic)

>-	    command_line.symbolic = symbolic_unset;

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

>@@ -1625,32 +1637,33 @@ parse_args (unsigned argc, char **argv)

>       && command_line.check_section_addresses < 0)

>     command_line.check_section_addresses = 0;

>

>-  /* We may have -Bsymbolic, -Bsymbolic-functions, --dynamic-list-data,

>-     --dynamic-list-cpp-new, --dynamic-list-cpp-typeinfo and

>-     --dynamic-list FILE.  -Bsymbolic and -Bsymbolic-functions are

>-     for PIC outputs.  -Bsymbolic overrides all others and vice versa.  */

>-  switch (command_line.symbolic)

>-    {

>-    case symbolic_unset:

>-      break;

>-    case symbolic:

>-      /* -Bsymbolic is for PIC output only.  */

>-      if (bfd_link_pic (&link_info))

>-	{

>-	  link_info.symbolic = TRUE;

>-	  /* Should we free the unused memory?  */

>-	  link_info.dynamic_list = NULL;

>-	  command_line.dynamic_list = dynamic_list_unset;

>-	}

>-      break;

>-    case symbolic_functions:

>-      /* -Bsymbolic-functions is for PIC output only.  */

>-      if (bfd_link_pic (&link_info))

>-	command_line.dynamic_list = dynamic_list_data;

>-      break;

>-    }

>+  /* -Bsymbolic and -Bsymbols-functions are for shared library output.  */

>+  if (bfd_link_dll (&link_info))

>+    switch (opt_symbolic)

>+      {

>+      case symbolic_unset:

>+	break;

>+      case symbolic:

>+	link_info.symbolic = TRUE;

>+	if (link_info.dynamic_list)

>+	  {

>+	    struct bfd_elf_version_expr *ent, *next;

>+	    for (ent = link_info.dynamic_list->head.list; ent; ent = next)

>+	      {

>+		next = ent->next;

>+		free (ent);

>+	      }

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

>+	break;

>+      }

>

>-  switch (command_line.dynamic_list)

>+  switch (opt_dynamic_list)

>     {

>     case dynamic_list_unset:

>       break;


Nice! FWIW, lld computes the preemptability of a symbol with a similar
rule that ignores -Bsymbolic for executables (-nopie / -pie):

  ...
  if (!b.isDefined())
    return true;

  if (!config->shared)
    return false;

  // If the dynamic list is present, it specifies preemptable symbols in a DSO.
  if (config->hasDynamicList)
    return b.inDynamicList;

  // -Bsymbolic means that definitions are not preempted.
  if (config->bsymbolic || (config->bsymbolicFunctions && b.isFunc()))
    return false;
  return true;


(This email also serves as a test for my neomutt-replying-to-a-raw-message workflow.
1) Downloaded the message via "Other format:  [Raw text]"
2) Delete the first line
3) Place under /tmp/INBOX/new/a
)

Patch

diff --git a/ld/ld.h b/ld/ld.h
index 55078a9637..5243346f37 100644
--- a/ld/ld.h
+++ b/ld/ld.h
@@ -119,20 +119,6 @@  struct wildcard_list
 
 enum endian_enum { ENDIAN_UNSET = 0, ENDIAN_BIG, ENDIAN_LITTLE };
 
-enum symbolic_enum
-{
-  symbolic_unset = 0,
-  symbolic,
-  symbolic_functions,
-};
-
-enum dynamic_list_enum
-{
-  dynamic_list_unset = 0,
-  dynamic_list_data,
-  dynamic_list
-};
-
 typedef struct
 {
   /* 1 => assign space to common symbols even if `relocatable_output'.  */
@@ -183,13 +169,6 @@  typedef struct
   /* Big or little endian as set on command line.  */
   enum endian_enum endian;
 
-  /* -Bsymbolic and -Bsymbolic-functions, as set on command line.  */
-  enum symbolic_enum symbolic;
-
-  /* --dynamic-list, --dynamic-list-cpp-new, --dynamic-list-cpp-typeinfo
-     and --dynamic-list FILE, as set on command line.  */
-  enum dynamic_list_enum dynamic_list;
-
   /* Name of runtime interpreter to invoke.  */
   char *interpreter;
 
diff --git a/ld/ld.texi b/ld/ld.texi
index fcbc335c95..3b2d05411f 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -1436,21 +1436,15 @@  libraries.
 When creating a shared library, bind references to global symbols to the
 definition within the shared library, if any.  Normally, it is possible
 for a program linked against a shared library to override the definition
-within the shared library.  This option can also be used with the
-@option{--export-dynamic} option, when creating a position independent
-executable, to bind references to global symbols to the definition within
-the executable.  This option is only meaningful on ELF platforms which
-support shared libraries and position independent executables.
+within the shared library.  This option is only meaningful on ELF
+platforms which support shared libraries.
 
 @kindex -Bsymbolic-functions
 @item -Bsymbolic-functions
 When creating a shared library, bind references to global function
 symbols to the definition within the shared library, if any.
-This option can also be used with the @option{--export-dynamic} option,
-when creating a position independent executable, to bind references
-to global function symbols to the definition within the executable.
 This option is only meaningful on ELF platforms which support shared
-libraries and position independent executables.
+libraries.
 
 @kindex --dynamic-list=@var{dynamic-list-file}
 @item --dynamic-list=@var{dynamic-list-file}
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 1c15ac29c0..f91549697e 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -565,6 +565,18 @@  parse_args (unsigned argc, char **argv)
   struct option *really_longopts;
   int last_optind;
   enum report_method how_to_report_unresolved_symbols = RM_GENERATE_ERROR;
+  enum symbolic_enum
+  {
+    symbolic_unset = 0,
+    symbolic,
+    symbolic_functions,
+  } opt_symbolic = symbolic_unset;
+  enum dynamic_list_enum
+  {
+    dynamic_list_unset = 0,
+    dynamic_list_data,
+    dynamic_list
+  } opt_dynamic_list = dynamic_list_unset;
 
   shortopts = (char *) xmalloc (OPTION_COUNT * 3 + 2);
   longopts = (struct option *)
@@ -1233,10 +1245,10 @@  parse_args (unsigned argc, char **argv)
 	  config.stats = TRUE;
 	  break;
 	case OPTION_SYMBOLIC:
-	  command_line.symbolic = symbolic;
+	  opt_symbolic = symbolic;
 	  break;
 	case OPTION_SYMBOLIC_FUNCTIONS:
-	  command_line.symbolic = symbolic_functions;
+	  opt_symbolic = symbolic_functions;
 	  break;
 	case 't':
 	  ++trace_files;
@@ -1381,23 +1393,23 @@  parse_args (unsigned argc, char **argv)
 	  command_line.version_exports_section = optarg;
 	  break;
 	case OPTION_DYNAMIC_LIST_DATA:
-	  command_line.dynamic_list = dynamic_list_data;
-	  if (command_line.symbolic == symbolic)
-	    command_line.symbolic = symbolic_unset;
+	  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 (command_line.dynamic_list != dynamic_list_data)
-	    command_line.dynamic_list = dynamic_list;
-	  if (command_line.symbolic == symbolic)
-	    command_line.symbolic = symbolic_unset;
+	  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 (command_line.dynamic_list != dynamic_list_data)
-	    command_line.dynamic_list = dynamic_list;
-	  if (command_line.symbolic == symbolic)
-	    command_line.symbolic = symbolic_unset;
+	  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
@@ -1412,10 +1424,10 @@  parse_args (unsigned argc, char **argv)
 	    parser_input = input_dynamic_list;
 	    yyparse ();
 	  }
-	  if (command_line.dynamic_list != dynamic_list_data)
-	    command_line.dynamic_list = dynamic_list;
-	  if (command_line.symbolic == symbolic)
-	    command_line.symbolic = symbolic_unset;
+	  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;
@@ -1625,32 +1637,33 @@  parse_args (unsigned argc, char **argv)
       && command_line.check_section_addresses < 0)
     command_line.check_section_addresses = 0;
 
-  /* We may have -Bsymbolic, -Bsymbolic-functions, --dynamic-list-data,
-     --dynamic-list-cpp-new, --dynamic-list-cpp-typeinfo and
-     --dynamic-list FILE.  -Bsymbolic and -Bsymbolic-functions are
-     for PIC outputs.  -Bsymbolic overrides all others and vice versa.  */
-  switch (command_line.symbolic)
-    {
-    case symbolic_unset:
-      break;
-    case symbolic:
-      /* -Bsymbolic is for PIC output only.  */
-      if (bfd_link_pic (&link_info))
-	{
-	  link_info.symbolic = TRUE;
-	  /* Should we free the unused memory?  */
-	  link_info.dynamic_list = NULL;
-	  command_line.dynamic_list = dynamic_list_unset;
-	}
-      break;
-    case symbolic_functions:
-      /* -Bsymbolic-functions is for PIC output only.  */
-      if (bfd_link_pic (&link_info))
-	command_line.dynamic_list = dynamic_list_data;
-      break;
-    }
+  /* -Bsymbolic and -Bsymbols-functions are for shared library output.  */
+  if (bfd_link_dll (&link_info))
+    switch (opt_symbolic)
+      {
+      case symbolic_unset:
+	break;
+      case symbolic:
+	link_info.symbolic = TRUE;
+	if (link_info.dynamic_list)
+	  {
+	    struct bfd_elf_version_expr *ent, *next;
+	    for (ent = link_info.dynamic_list->head.list; ent; ent = next)
+	      {
+		next = ent->next;
+		free (ent);
+	      }
+	    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;
+	break;
+      }
 
-  switch (command_line.dynamic_list)
+  switch (opt_dynamic_list)
     {
     case dynamic_list_unset:
       break;