Re: [PATCH] gold: Remove GNU ld incompatible --[no-]define-common [BZ #28871]

["Fāng-ruì Sòng" <maskray@google.com>]

On 2022-02-18, Fāng-ruì Sòng wrote:
>On Tue, Feb 8, 2022 at 1:08 AM Fangrui Song <maskray@google.com> wrote:
>>
>> In 2008, 0dfbdef4c43cfe12bb3e2505ebe5acc651a35c98 confused
>> --define-common with -d/FORCE_COMMON_ALLOCATION and implemented
>> --define-common with -d semantics.
>>
>> Just remove the GNU ld incompatible --[no-]define-common.
>> ---
>>  gold/gold.cc    |  2 +-
>>  gold/options.cc | 14 ++++----------
>>  gold/options.h  |  9 ++++-----
>>  3 files changed, 9 insertions(+), 16 deletions(-)
>>
>> diff --git a/gold/gold.cc b/gold/gold.cc
>> index 2891d16c7c9..a93489e225e 100644
>> --- a/gold/gold.cc
>> +++ b/gold/gold.cc
>> @@ -708,7 +708,7 @@ queue_middle_tasks(const General_options& options,
>>    // Allocate common symbols.  We use a blocker to run this before the
>>    // Scan_relocs tasks, because it writes to the symbol table just as
>>    // they do.
>> -  if (parameters->options().define_common())
>> +  if (parameters->options().dc())
>>      {
>>        this_blocker = new Task_token(true);
>>        this_blocker->add_blocker();
>> diff --git a/gold/options.cc b/gold/options.cc
>> index 04be98a3e39..a6c4bda1ae8 100644
>> --- a/gold/options.cc
>> +++ b/gold/options.cc
>> @@ -1136,16 +1136,10 @@ General_options::finalize()
>>        gold_assert(this->strip_debug());
>>      }
>>
>> -  // For us, -dc and -dp are synonyms for --define-common.
>> -  if (this->dc())
>> -    this->set_define_common(true);
>> -  if (this->dp())
>> -    this->set_define_common(true);
>> -
>> -  // We also set --define-common if we're not relocatable, as long as
>> -  // the user didn't explicitly ask for something different.
>> -  if (!this->user_set_define_common())
>> -    this->set_define_common(!this->relocatable());
>> +  // A non-relocatable link always allocates space to COMMON symbols.
>> +  // -dc, -d, and -dp can force allocation for a relocatable link.
>> +  if (!this->relocatable() || this->d() || this->dp())
>> +    this->set_dc(true);
>>
>>    // execstack_status_ is a three-state variable; update it based on
>>    // -z [no]execstack.
>> diff --git a/gold/options.h b/gold/options.h
>> index 9509a445e8e..a994bdcf42a 100644
>> --- a/gold/options.h
>> +++ b/gold/options.h
>> @@ -787,13 +787,12 @@ class General_options
>>
>>    // d
>>
>> -  DEFINE_bool(define_common, options::TWO_DASHES, 'd', false,
>> -             N_("Define common symbols"),
>> -             N_("Do not define common symbols in relocatable output"));
>>    DEFINE_bool(dc, options::ONE_DASH, '\0', false,
>> -             N_("Alias for -d"), NULL);
>> +    N_("Allocate space to COMMON symbols for the relocatable link"), NULL);
>> +  DEFINE_bool(d, options::ONE_DASH, '\0', false,
>> +             N_("Alias for -dc"), NULL);
>>    DEFINE_bool(dp, options::ONE_DASH, '\0', false,
>> -             N_("Alias for -d"), NULL);
>> +             N_("Alias for -dc"), NULL);
>>
>>    DEFINE_string(debug, options::TWO_DASHES, '\0', "",
>>                 N_("Turn on debugging"),
>> --
>> 2.35.0.263.gb82422642f-goog
>>
>
>Ping...

ping^2 :)