Re: [PATCH] gold: Ignore definition from a dynamic object for __start/__stop

[Cary Coutant <ccoutant at gmail dot com>]

> Since __start and __stop symbols must be defined in a regular object,
> definition from a dynamic object should be ignored.  Also __start and
> __stop symbols in a dynamic object shouldn't be preempted.
>
>         PR gold/22291
>         * layout.cc (Layout::define_section_symbols): Use STV_PROTECTED
>         and set must_be_in_reg to true for __start and __stop symbols.

Shouldn't we use HIDDEN here instead of PROTECTED? If the symbols must
be defined in a regular object, and should not be pre-empted, it seems
to me that references to start and stop symbols should always be from
within the same load module.

>         * symtab.cc (Symbol_table::define_special_symbol): Add an
>         argument, must_be_in_reg.  If the symbol must be defined in a
>         regular object, ignore definition from a dynamic object.

I'd think we'd also want to ignore references from a dynamic object as well.

I think the must_be_in_reg flag is unnecessary -- only_if_ref should
be sufficient. I looked through all the symbols that are created with
only_if_ref true, and they all look like they should ignore
definitions (and references) in dynamic objects:

  __rel_iplt_start (global hidden)
  __rel_iplt_end (global hidden)
 __exidx_start (arm, global hidden)
  __exidx_end (arm, global hidden)
  _TLS_MODULE_BASE_ (local hidden)
  __preinit_array_start (global hidden)
  __preinit_array_end (global hidden)
  __init_array_start (global hidden)
  __init_array_end (global hidden)
  __fini_array_start (global hidden)
  __fini_array_end (global hidden)
  __stack (global default)
  __executable_start (global default)
  __ehdr_start (global hidden)
  etext, _etext, __etext (global default)
  edata (global default)
  end (global default)

Certainly the ones that are hidden should ignore both defs and refs in
dynamic objects. The others (__stack, __executable_start, [_][_]etext,
edata, and end) should at least ignore defs in dynamic objects.

@@ -1507,7 +1507,8 @@ class Symbol_table
                        Output_data*, uint64_t value, uint64_t symsize,
                        elfcpp::STT type, elfcpp::STB binding,
                        elfcpp::STV visibility, unsigned char nonvis,
-                       bool offset_is_from_end, bool only_if_ref);
+                       bool offset_is_from_end, bool only_if_ref,
+                       bool must_be_in_reg = false);

I'd prefer not to use a default parameter value here
(define_in_output_data), but there are a lot of other calls that would
need to be adjusted. Better if we can get away without the extra
parameter at all.

   // Define a special symbol based on an Output_segment.  It is a
   // multiple definition error if this symbol is already defined.
@@ -1831,7 +1832,8 @@ class Symbol_table
   Sized_symbol<size>*
   define_special_symbol(const char** pname, const char** pversion,
                        bool only_if_ref, Sized_symbol<size>** poldsym,
-                       bool* resolve_oldsym, bool is_forced_local);
+                       bool* resolve_oldsym, bool is_forced_local,
+                       bool must_be_in_reg = false);

This parameter shouldn't need a default value -- I think you've
already added the extra parameter to all calls.

   // Define a symbol in an Output_data, sized version.
   template<int size>
@@ -1842,7 +1844,8 @@ class Symbol_table
                           typename elfcpp::Elf_types<size>::Elf_WXword ssize,
                           elfcpp::STT type, elfcpp::STB binding,
                           elfcpp::STV visibility, unsigned char nonvis,
-                          bool offset_is_from_end, bool only_if_ref);
+                          bool offset_is_from_end, bool only_if_ref,
+                          bool must_be_in_reg = false);

This (do_define_in_output_data) shouldn't need a default value either,
since the only calls are from define_in_output_data.

-cary