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 13:39:13 -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> <CAMe9rOoC=tOzrQJwdY=esUJpyE4n5d19NKLRutk066F9SJHMtg@mail.gmail.com> <CAJimCsFQiqTWALj4F8Ko7b_PhNuyFUqc8PmUW-5KGpD-kDD5eA@mail.gmail.com> <CAMe9rOqiJ4s9+oWM6XpL=r3cawTDR+o1Myn9nargOiN+_pNq9g@mail.gmail.com>
On Fri, Oct 20, 2017 at 11:50 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Oct 20, 2017 at 11:11 AM, Cary Coutant <ccoutant@gmail.com> wrote:
>>>> 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.
>>
>> You didn't answer why you think it's not reasonable to use only_if_ref
>> by itself, rather than adding the new must_be_in_reg flag.
>>
>
> only_if_ref isn't sufficient. Passing visibility to define_special_symbol
> may work. If visibility != STV_DEFAULT, both refs and defs in dynamic
> objects can be ignored. If visibility == STV_DEFAULT, defs in dynamic
> objects can be ignored.
>
Here is the updated patch.
--
H.J.
From 78e03a33628ee801c93f006fa43ebb8cdb8dd243 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 special
symbols
Since special symbol must be defined in a regular object, definition
from a dynamic object should be ignored. If special symbol has the
hidden or internal visibility, reference from a dynamic object should
also 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
for __start and __stop symbols.
* symtab.cc (Symbol_table::define_special_symbol): Add an
argument, visibility. Ignore definition and reference from
a dynamic object, depending on visibility.
(Symbol_table::do_define_in_output_data): Pass visibility to
define_special_symbol.
(Symbol_table::do_define_in_output_segment): Likewise.
(Symbol_table::do_define_as_constant): Likewise.
(Symbol_table::add_undefined_symbol_from_command_line): Pass
STV_DEFAULT to define_special_symbol.
* symtab.h (Symbol_table::define_special_symbol): Add an
argument, visibility.
ixx
---
gold/layout.cc | 4 ++--
gold/symtab.cc | 51 +++++++++++++++++++++++++++++++++++++++++----------
gold/symtab.h | 3 ++-
3 files changed, 45 insertions(+), 13 deletions(-)
diff --git a/gold/layout.cc b/gold/layout.cc
index 5f25faea55..c9dd823536 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -2228,7 +2228,7 @@ 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
@@ -2241,7 +2241,7 @@ 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
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 1555de6e5b..13ec5de277 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -1759,8 +1759,10 @@ template<int size, bool big_endian>
Sized_symbol<size>*
Symbol_table::define_special_symbol(const char** pname, const char** pversion,
bool only_if_ref,
+ elfcpp::STV visibility,
Sized_symbol<size>** poldsym,
- bool* resolve_oldsym, bool is_forced_local)
+ bool* resolve_oldsym,
+ bool is_forced_local)
{
*resolve_oldsym = false;
*poldsym = NULL;
@@ -1797,8 +1799,21 @@ 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 (oldsym == NULL)
return NULL;
+ if (!oldsym->is_undefined())
+ {
+ // Skip if the old definition is from a regular object.
+ if (!oldsym->is_from_dynobj())
+ return NULL;
+
+ // If the symbol has the hidden or internal visibility, ignore
+ // definition and reference from a dynamic object.
+ if ((visibility == elfcpp::STV_HIDDEN
+ || visibility == elfcpp::STV_INTERNAL)
+ && !oldsym->in_reg())
+ return NULL;
+ }
*pname = oldsym->name();
if (is_default_version)
@@ -1973,7 +1988,9 @@ Symbol_table::do_define_in_output_data(
{
#if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
sym = this->define_special_symbol<size, true>(&name, &version,
- only_if_ref, &oldsym,
+ only_if_ref,
+ visibility,
+ &oldsym,
&resolve_oldsym,
is_forced_local);
#else
@@ -1984,7 +2001,9 @@ Symbol_table::do_define_in_output_data(
{
#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_64_LITTLE)
sym = this->define_special_symbol<size, false>(&name, &version,
- only_if_ref, &oldsym,
+ only_if_ref,
+ visibility,
+ &oldsym,
&resolve_oldsym,
is_forced_local);
#else
@@ -2092,7 +2111,9 @@ Symbol_table::do_define_in_output_segment(
{
#if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
sym = this->define_special_symbol<size, true>(&name, &version,
- only_if_ref, &oldsym,
+ only_if_ref,
+ visibility,
+ &oldsym,
&resolve_oldsym,
is_forced_local);
#else
@@ -2103,7 +2124,9 @@ Symbol_table::do_define_in_output_segment(
{
#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_64_LITTLE)
sym = this->define_special_symbol<size, false>(&name, &version,
- only_if_ref, &oldsym,
+ only_if_ref,
+ visibility,
+ &oldsym,
&resolve_oldsym,
is_forced_local);
#else
@@ -2209,7 +2232,9 @@ Symbol_table::do_define_as_constant(
{
#if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
sym = this->define_special_symbol<size, true>(&name, &version,
- only_if_ref, &oldsym,
+ only_if_ref,
+ visibility,
+ &oldsym,
&resolve_oldsym,
is_forced_local);
#else
@@ -2220,7 +2245,9 @@ Symbol_table::do_define_as_constant(
{
#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_64_LITTLE)
sym = this->define_special_symbol<size, false>(&name, &version,
- only_if_ref, &oldsym,
+ only_if_ref,
+ visibility,
+ &oldsym,
&resolve_oldsym,
is_forced_local);
#else
@@ -2447,7 +2474,9 @@ Symbol_table::add_undefined_symbol_from_command_line(const char* name)
{
#if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
sym = this->define_special_symbol<size, true>(&name, &version,
- false, &oldsym,
+ false,
+ elfcpp::STV_DEFAULT,
+ &oldsym,
&resolve_oldsym,
false);
#else
@@ -2458,7 +2487,9 @@ Symbol_table::add_undefined_symbol_from_command_line(const char* name)
{
#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_64_LITTLE)
sym = this->define_special_symbol<size, false>(&name, &version,
- false, &oldsym,
+ false,
+ elfcpp::STV_DEFAULT,
+ &oldsym,
&resolve_oldsym,
false);
#else
diff --git a/gold/symtab.h b/gold/symtab.h
index a67d5eb90d..edf1c1729c 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -1830,7 +1830,8 @@ class Symbol_table
template<int size, bool big_endian>
Sized_symbol<size>*
define_special_symbol(const char** pname, const char** pversion,
- bool only_if_ref, Sized_symbol<size>** poldsym,
+ bool only_if_ref, elfcpp::STV visibility,
+ Sized_symbol<size>** poldsym,
bool* resolve_oldsym, bool is_forced_local);
// Define a symbol in an Output_data, sized version.
--
2.13.6