This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] gold: Ignore definition from a dynamic object for __start/__stop
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Cary Coutant <ccoutant at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Fri, 20 Oct 2017 09:18:17 -0700
- Subject: Re: [PATCH] gold: Ignore definition from a dynamic object for __start/__stop
- Authentication-results: sourceware.org; auth=none
- References: <20171018132021.GA4627@gmail.com> <CAJimCsEYhYtcPosB6vjsUsp02G69KKgn3MZ9=a=5XUowcx6diA@mail.gmail.com>
On Thu, Oct 19, 2017 at 5:39 PM, Cary Coutant <ccoutant@gmail.com> wrote:
>> 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.
Done.
> 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
As Alan mentioned, __start and __stop symbols must be exported,
but not preempted. STV_PROTECTED should be used.
> 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.
It is needed since I didn't change:
symtab.cc: sym = this->define_special_symbol<size, true>(&name, &version,
symtab.cc: sym = this->define_special_symbol<size, false>(&name, &version,
symtab.cc: sym = this->define_special_symbol<size, true>(&name, &version,
symtab.cc: sym = this->define_special_symbol<size, false>(&name, &version,
symtab.cc: sym = this->define_special_symbol<size, true>(&name, &version,
symtab.cc: sym = this->define_special_symbol<size, false>(&name, &version,
symtab.cc: sym = this->define_special_symbol<size, true>(&name, &version,
symtab.cc: sym = this->define_special_symbol<size, false>(&name, &version,
> // 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.
Done.
Here is the updated patch. OK for master?
Thanks.
--
H.J.
From 90d9ebfbeb3fc183e5ca562c5b9dbf12b4271c2a Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 13 Oct 2017 05:02:19 -0700
Subject: [PATCH] gold: Ignore def/ref from a dynamic object for __start/__stop
Since __start and __stop symbols must be defined in a regular object,
definition and reference 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.
* 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.
(Symbol_table::define_in_output_data): Add an argument,
must_be_in_reg and pass it to do_define_in_output_data.
(Symbol_table::do_define_in_output_data): Add an argument,
must_be_in_reg and pass it to define_special_symbol.
* symtab.h (Symbol_table::define_in_output_data): Add an
argument, must_be_in_reg, and default to false.
(Symbol_table::define_special_symbol): Likewise.
(Symbol_table::do_define_in_output_data): Add an argument,
must_be_in_reg.
---
gold/layout.cc | 10 ++++++----
gold/symtab.cc | 30 ++++++++++++++++++++++--------
gold/symtab.h | 9 ++++++---
3 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/gold/layout.cc b/gold/layout.cc
index 5f25faea55..bdfceee3b2 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -2228,10 +2228,11 @@ Layout::define_section_symbols(Symbol_table* symtab)
0, // symsize
elfcpp::STT_NOTYPE,
elfcpp::STB_GLOBAL,
- elfcpp::STV_DEFAULT,
+ elfcpp::STV_PROTECTED,
0, // nonvis
false, // offset_is_from_end
- true); // only_if_ref
+ true, // only_if_ref
+ true); // must_be_in_reg
symtab->define_in_output_data(stop_name.c_str(),
NULL, // version
@@ -2241,10 +2242,11 @@ Layout::define_section_symbols(Symbol_table* symtab)
0, // symsize
elfcpp::STT_NOTYPE,
elfcpp::STB_GLOBAL,
- elfcpp::STV_DEFAULT,
+ elfcpp::STV_PROTECTED,
0, // nonvis
true, // offset_is_from_end
- true); // only_if_ref
+ true, // only_if_ref
+ true); // must_be_in_reg
}
}
}
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 1555de6e5b..f830f8dc05 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -1760,7 +1760,9 @@ Sized_symbol<size>*
Symbol_table::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)
{
*resolve_oldsym = false;
*poldsym = NULL;
@@ -1797,7 +1799,13 @@ Symbol_table::define_special_symbol(const char** pname, const char** pversion,
oldsym = this->lookup(*pname, *pversion);
if (oldsym == NULL && is_default_version)
oldsym = this->lookup(*pname, NULL);
- if (oldsym == NULL || !oldsym->is_undefined())
+ // If the symbol must be defined in a regular object, ignore
+ // definition and reference from a dynamic object.
+ if (oldsym == NULL
+ || (!oldsym->is_undefined()
+ && (!must_be_in_reg
+ || (oldsym->in_reg()
+ && !oldsym->is_from_dynobj()))))
return NULL;
*pname = oldsym->name();
@@ -1916,7 +1924,8 @@ Symbol_table::define_in_output_data(const char* name,
elfcpp::STV visibility,
unsigned char nonvis,
bool offset_is_from_end,
- bool only_if_ref)
+ bool only_if_ref,
+ bool must_be_in_reg)
{
if (parameters->target().get_size() == 32)
{
@@ -1925,7 +1934,8 @@ Symbol_table::define_in_output_data(const char* name,
value, symsize, type, binding,
visibility, nonvis,
offset_is_from_end,
- only_if_ref);
+ only_if_ref,
+ must_be_in_reg);
#else
gold_unreachable();
#endif
@@ -1937,7 +1947,8 @@ Symbol_table::define_in_output_data(const char* name,
value, symsize, type, binding,
visibility, nonvis,
offset_is_from_end,
- only_if_ref);
+ only_if_ref,
+ must_be_in_reg);
#else
gold_unreachable();
#endif
@@ -1962,7 +1973,8 @@ Symbol_table::do_define_in_output_data(
elfcpp::STV visibility,
unsigned char nonvis,
bool offset_is_from_end,
- bool only_if_ref)
+ bool only_if_ref,
+ bool must_be_in_reg)
{
Sized_symbol<size>* sym;
Sized_symbol<size>* oldsym;
@@ -1975,7 +1987,8 @@ Symbol_table::do_define_in_output_data(
sym = this->define_special_symbol<size, true>(&name, &version,
only_if_ref, &oldsym,
&resolve_oldsym,
- is_forced_local);
+ is_forced_local,
+ must_be_in_reg);
#else
gold_unreachable();
#endif
@@ -1986,7 +1999,8 @@ Symbol_table::do_define_in_output_data(
sym = this->define_special_symbol<size, false>(&name, &version,
only_if_ref, &oldsym,
&resolve_oldsym,
- is_forced_local);
+ is_forced_local,
+ must_be_in_reg);
#else
gold_unreachable();
#endif
diff --git a/gold/symtab.h b/gold/symtab.h
index a67d5eb90d..53ce84a0d6 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -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);
// 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);
// 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);
// Define a symbol in an Output_segment, sized version.
template<int size>
--
2.13.6