[PATCH users/roland/ld-start-stop-visibility] gold, ld: Implement -z start-stop-visibility=... option.

[Roland McGrath]

[My MUA is mangling the patch, but it's in git on branch
users/roland/ld-start-stop-visibility.]

The history of visibility for __start_* / __stop_* symbols is a messy
and sad one.  As one of those involved in the original creation of the
feature (to replace the GNU a.out "symbol set" feature), I can say for
sure that we should have used STV_HIDDEN from the start for the
semantics we originally had in mind.  But we didn't, and history
ensued.  It's probably hopeless nowadays to really change the default
behavior (which is not STV_DEFAULT behavior any more, at least) to
STV_HIDDEN.  But we can at least offer the user flexibility so users
more interested in sensible semantics than historical compatibility
can choose -z start-stop-visibility=hidden to clean up their symbol
tables in the future.

Ok for trunk?


Thanks,
Roland

bfd/
2020-06-12  Roland McGrath  <mcgrathr@google.com>

* elflink.c (bfd_elf_define_start_stop): Use start_stop_visibility
field of bfd_link_info.

gold/
2020-06-12  Roland McGrath  <mcgrathr@google.com>

Implement -z start-stop-visibility=... option.
* options.h (class General_options): Handle -z start-stop-visibility=.
(General_options::start_stop_visibility): New public method.
(General_options::start_stop_visibility_): New private member.
* options.cc (General_options::General_options): Add initializer.
(General_options::parse_start_stop_visibility): New function.
* layout.cc (Layout::define_section_symbols): Use option setting.

include/
2020-06-12  Roland McGrath  <mcgrathr@google.com>

* bfdlink.h (struct bfd_link_info): New field start_stop_visibility.

ld/
2020-06-12  Roland McGrath  <mcgrathr@google.com>

* NEWS: Mention -z start-stop-visibility=... option for ELF.
* ld.texi (Options): Document -z start-stop-visibility=... option.
* ldmain.c (main): Initialize link_info.start_stop_visibility.
* emultempl/elf.em (gld${EMULATION_NAME}_handle_option):
Parse -z start-stop-visibility=... option.

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 3e56a297f6..d7ed468045 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -14829,7 +14829,8 @@ bfd_elf_define_start_stop (struct bfd_link_info *info,
       else
  {
   if (ELF_ST_VISIBILITY (h->other) == STV_DEFAULT)
-    h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_PROTECTED;
+    h->other = ((h->other & ~ELF_ST_VISIBILITY (-1))
+ | info->start_stop_visibility);
   if (was_dynamic)
     bfd_elf_link_record_dynamic_symbol (info, h);
  }
diff --git a/gold/layout.cc b/gold/layout.cc
index be437f3900..bb7fd497b4 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -2474,6 +2474,7 @@
Layout::create_initial_dynamic_sections(Symbol_table* symtab)
 void
 Layout::define_section_symbols(Symbol_table* symtab)
 {
+  const elfcpp::STV visibility = parameters->options().start_stop_visibility();
   for (Section_list::const_iterator p = this->section_list_.begin();
        p != this->section_list_.end();
        ++p)
@@ -2495,7 +2496,7 @@ Layout::define_section_symbols(Symbol_table* symtab)
  0, // symsize
  elfcpp::STT_NOTYPE,
  elfcpp::STB_GLOBAL,
- elfcpp::STV_PROTECTED,
+ visibility,
  0, // nonvis
  false, // offset_is_from_end
  true); // only_if_ref
@@ -2508,7 +2509,7 @@ Layout::define_section_symbols(Symbol_table* symtab)
  0, // symsize
  elfcpp::STT_NOTYPE,
  elfcpp::STB_GLOBAL,
- elfcpp::STV_PROTECTED,
+ visibility,
  0, // nonvis
  true, // offset_is_from_end
  true); // only_if_ref
diff --git a/gold/options.cc b/gold/options.cc
index 94867b361a..9936619685 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -760,6 +760,23 @@ General_options::parse_pop_state(const char*,
const char*, Command_line*)
   delete posdep;
 }

+void
+General_options::parse_start_stop_visibility(char const*, char const* arg,
+                                             Command_line*)
+{
+  if (strcmp(arg, "default") == 0)
+    this->start_stop_visibility_ = elfcpp::STV_DEFAULT;
+  else if (strcmp(arg, "internal") == 0)
+    this->start_stop_visibility_ = elfcpp::STV_INTERNAL;
+  else if (strcmp(arg, "hidden") == 0)
+    this->start_stop_visibility_ = elfcpp::STV_HIDDEN;
+  else if (strcmp(arg, "protected") == 0)
+    this->start_stop_visibility_ = elfcpp::STV_PROTECTED;
+  else
+    gold_fatal(_("-z start-stop-visibility: must take one of the following "
+                 "arguments: default, internal, hidden, protected"));
+}
+
 } // End namespace gold.

 namespace
@@ -997,7 +1014,8 @@ General_options::General_options()
     fix_v4bx_(FIX_V4BX_NONE),
     endianness_(ENDIANNESS_NOT_SET),
     discard_locals_(DISCARD_SEC_MERGE),
-    orphan_handling_enum_(ORPHAN_PLACE)
+    orphan_handling_enum_(ORPHAN_PLACE),
+    start_stop_visibility_(elfcpp::STV_PROTECTED)
 {
   // Turn off option registration once construction is complete.
   gold::options::ready_to_register = false;
diff --git a/gold/options.h b/gold/options.h
index b2059d984c..ed050c3b02 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1500,6 +1500,10 @@ class General_options
       N_("Don't mark variables read-only after relocation"));
   DEFINE_uint64(stack_size, options::DASH_Z, '\0', 0,
  N_("Set PT_GNU_STACK segment p_memsz to SIZE"), N_("SIZE"));
+  DEFINE_special(start_stop_visibility, options::DASH_Z, '\0',
+                 N_("ELF symbol visibility for synthesized "
+                    "__start_* and __stop_* symbols"),
+                 N_("[default,internal,hidden,protected]"));
   DEFINE_bool(text, options::DASH_Z, '\0', false,
       N_("Do not permit relocations in read-only segments"),
       N_("Permit relocations in read-only segments"));
@@ -1750,6 +1754,10 @@ class General_options
   orphan_handling_enum() const
   { return this->orphan_handling_enum_; }

+  elfcpp::STV
+  start_stop_visibility() const
+  { return this->start_stop_visibility_; }
+
  private:
   // Don't copy this structure.
   General_options(const General_options&);
@@ -1876,6 +1884,8 @@ class General_options
   std::vector<Position_dependent_options*> options_stack_;
   // Orphan handling option, decoded to an enum value.
   Orphan_handling orphan_handling_enum_;
+  // Symbol visibility for __start_* / __stop_* magic symbols.
+  elfcpp::STV start_stop_visibility_;
 };

 // The position-dependent options.  We use this to store the state of
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 34a0d2ec4e..7163433383 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -542,10 +542,10 @@ struct bfd_link_info
      Normally these optimizations are disabled by default but some targets
      prefer to enable them by default.  So this field is a tri-state variable.
      The values are:
-
+
      zero: Enable the optimizations (either from --relax being specified on
        the command line or the backend's before_allocation emulation function.
-
+
      positive: The user has requested that these optimizations be disabled.
        (Via the --no-relax command line option).

@@ -649,6 +649,9 @@ struct bfd_link_info
   /* May be used to set DT_FLAGS_1 for ELF. */
   bfd_vma flags_1;

+  /* May be used to set ELF visibility for __start_* / __stop_.  */
+  unsigned int start_stop_visibility;
+
   /* Start and end of RELRO region.  */
   bfd_vma relro_start, relro_end;

diff --git a/ld/NEWS b/ld/NEWS
index 485e1cf5b9..6955937429 100644
--- a/ld/NEWS
+++ b/ld/NEWS
@@ -25,6 +25,9 @@
   searched relative to the directory of the linker script before other search
   paths.

+* Add ELF linker command-line option `-z start-stop-visibility=...' to control
+  the visibility of synthetic `__start_SECNAME` and `__stop_SECNAME` symbols.
+
 Changes in 2.34:

 * The ld check for "PHDR segment not covered by LOAD segment" is more
diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em
index c4979eb953..c577e8b2e6 100644
--- a/ld/emultempl/elf.em
+++ b/ld/emultempl/elf.em
@@ -749,6 +749,21 @@ fragment <<EOF
  {
   link_info.flags_1 |= DF_1_GLOBAUDIT;
  }
+      else if (CONST_STRNEQ (optarg, "start-stop-visibility="))
+ {
+  if (strcmp (optarg, "start-stop-visibility=default") == 0)
+    link_info.start_stop_visibility = STV_DEFAULT;
+  else if (strcmp (optarg, "start-stop-visibility=internal") == 0)
+    link_info.start_stop_visibility = STV_INTERNAL;
+  else if (strcmp (optarg, "start-stop-visibility=hidden") == 0)
+    link_info.start_stop_visibility = STV_HIDDEN;
+  else if (strcmp (optarg, "start-stop-visibility=protected") == 0)
+    link_info.start_stop_visibility = STV_PROTECTED;
+  else
+    einfo (_("%F%P: invalid visibility in \`-z %s'; "
+     "must be default, internal, hidden, or protected"),
+   optarg);
+ }
 EOF

 if test x"$GENERATE_SHLIB_SCRIPT" = xyes; then
diff --git a/ld/ld.texi b/ld/ld.texi
index bf474d4c62..b89c1a57c0 100644
--- a/ld/ld.texi
+++ b/ld/ld.texi
@@ -1373,6 +1373,19 @@ Specify a stack size for an ELF
@code{PT_GNU_STACK} segment.
 Specifying zero will override any default non-zero sized
 @code{PT_GNU_STACK} segment creation.

+@item start-stop-visibility=@var{value}
+@cindex visibility
+@cindex ELF symbol visibility
+Specify the ELF symbol visibility for synthesized
+@code{__start_SECNAME} and @code{__stop_SECNAME} symbols (@pxref{Input
+Section Example}).  @var{value} must be exactly @samp{default},
+@samp{internal}, @samp{hidden}, or @samp{protected}.  If no @samp{-z
+start-stop-visibility} option is given, @samp{protected} is used for
+compatibility with historical practice.  However, it's highly
+recommended to use @samp{-z start-stop-visibility=hidden} in new
+programs and shared libraries so that these symbols are not exported
+between shared objects, which is not usually what's intended.
+
 @item text
 @itemx notext
 @itemx textoff
diff --git a/ld/ldmain.c b/ld/ldmain.c
index e2c559ea3e..b0ce69f118 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -27,6 +27,7 @@
 #include "bfdlink.h"
 #include "ctf-api.h"
 #include "filenames.h"
+#include "elf/common.h"

 #include "ld.h"
 #include "ldmain.h"
@@ -307,6 +308,7 @@ main (int argc, char **argv)
 #ifdef DEFAULT_NEW_DTAGS
   link_info.new_dtags = DEFAULT_NEW_DTAGS;
 #endif
+  link_info.start_stop_visibility = STV_PROTECTED;

   ldfile_add_arch ("");
   emulation = get_emulation (argc, argv);