-Bsymbolic is not for PIEs

[Fangrui Song]

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
)