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

Message ID CAB=4xhqb63g_b8K_BJiKne8N+X9zRO_0sYZw+0UNTTFTbdV3OA@mail.gmail.com
State New
Headers show
Series
  • [users/roland/ld-start-stop-visibility] gold, ld: Implement -z start-stop-visibility=... option.
Related show

Commit Message

David Faust via Binutils June 13, 2020, 7 a.m.
[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.

Comments

Fangrui Song June 13, 2020, 8:13 a.m. | #1
On Sat, Jun 13, 2020 at 12:00 AM Roland McGrath via Binutils
<binutils@sourceware.org> wrote:
>

> [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);


Link: https://sourceware.org/pipermail/binutils/2020-June/111466.html

Looks reasonable to me. You may pursue the LLD patch if this is
accepted in GNU ld. (If it is accepted, I'd be happy to argue with
other contributors to fight for -z start-stop-visibility= )
David Faust via Binutils June 13, 2020, 11:43 p.m. | #2
> +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"));

> +}


> +  DEFINE_special(start_stop_visibility, options::DASH_Z, '\0',

> +                 N_("ELF symbol visibility for synthesized "

> +                    "__start_* and __stop_* symbols"),

> +                 N_("[default,internal,hidden,protected]"));


Please use DEFINE_enum instead, and then convert to an STV value in
General_options::finalize(). See the code for the --icf or
--orphan-handling options for an example.

-cary
David Faust via Binutils June 15, 2020, 6:35 a.m. | #3
On Sat, Jun 13, 2020 at 12:00:55AM -0700, Roland McGrath via Binutils wrote:
> [My MUA is mangling the patch, but it's in git on branch

> users/roland/ld-start-stop-visibility.]


The ld.bfd part is OK.

-- 
Alan Modra
Australia Development Lab, IBM
David Faust via Binutils June 15, 2020, 6:47 p.m. | #4
On Sat, Jun 13, 2020 at 4:43 PM Cary Coutant <ccoutant@gmail.com> wrote:
> Please use DEFINE_enum instead, and then convert to an STV value in

> General_options::finalize(). See the code for the --icf or

> --orphan-handling options for an example.


Done and committed.

Thanks,
Roland
Fangrui Song June 15, 2020, 8:48 p.m. | #5
On 2020-06-15, Roland McGrath via Binutils wrote:
>On Sat, Jun 13, 2020 at 4:43 PM Cary Coutant <ccoutant@gmail.com> wrote:

>> Please use DEFINE_enum instead, and then convert to an STV value in

>> General_options::finalize(). See the code for the --icf or

>> --orphan-handling options for an example.

>

>Done and committed.

>

>Thanks,

>Roland


Hope we can re-consider the resolution to
https://sourceware.org/bugzilla/show_bug.cgi?id=21964

-z start-stop-visibility=hidden  is a better default.
David Faust via Binutils June 15, 2020, 8:52 p.m. | #6
On Mon, Jun 15, 2020 at 1:48 PM Fangrui Song <i@maskray.me> wrote:
> Hope we can re-consider the resolution to

> https://sourceware.org/bugzilla/show_bug.cgi?id=21964

>

> -z start-stop-visibility=hidden  is a better default.


I agree.  But realistically, changing arcane things affecting
compatibility of existing code bases is nearly impossible.  It's far
more tractable for those who really understand the issues and are
using the features in new code to use the new switch explicitly, and
for people defining new ABIs without legacy compatibility concerns to
make the compiler driver pass the better default to the linker
implicitly (as we will do for Fuchsia).
Fangrui Song June 16, 2020, 3:48 p.m. | #7
On 2020-06-15, Roland McGrath via Binutils wrote:
>On Sat, Jun 13, 2020 at 4:43 PM Cary Coutant <ccoutant@gmail.com> wrote:

>> Please use DEFINE_enum instead, and then convert to an STV value in

>> General_options::finalize(). See the code for the --icf or

>> --orphan-handling options for an example.

>

>Done and committed.

>

>Thanks,

>Roland


I see that your original message included quite a bit of description
while commit cae64165f47b64898c4f1982d294862cfae89a47 dropped most of
them.

To understand this practice, I have found the justification in 
https://www.gnu.org/prep/standards/html_node/Change-Log-Concepts.html#Change-Log-Concepts

"The best place to explain how parts of the new code work with other code
is in comments in the code, not in the change log."

However, many modern binutils-gdb commits add rationale to the git descriptions.
The practice is adopted by most influential projects:)
Is there a need to amend
https://www.gnu.org/prep/standards/html_node/Change-Log-Concepts.html#Change-Log-Concepts
?

Patch

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