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

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

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...