Re: [PATCH] gold: remove false const from Target::do_adjust_elf_header

[Cary Coutant <ccoutant@google.com>]

There are a lot of places where we call parameters->target() and
really do want a const Target&. If we're going to make it non-const,
it really should be Target* instead of Target& (as a general rule, we
don't like to return const references).

The call to parameters->target().adjust_elf_header() in output.cc is
so clearly wrong that I don't understand how this compiles with GCC
(for ARM, that is). Just by leaving the "const" off of the override in
Target_arm, we can completely subvert the const-ness of the reference
returned by parameters->target()?

I think a better approach would be to remove the "const" from the
Target* passed to the Output_file_header ctor and stored in a private
data member, then modify the call to adjust_elf_header() to be
"this->target_->adjust_elf_header()". There's no reason to call
parameters->target() here when we already have a pointer to the target
readily available.

-cary


On Fri, Oct 11, 2013 at 11:46 AM, Roland McGrath <mcgrathr@google.com> wrote:
> The claim of constness is false for Target_arm (because it calls
> set_processor_specific_flags), so it should not be claimed for the
> general case.
>
> OK for trunk and 2.24?
>
>
> Thanks,
> Roland
>
>
> gold/
>         * target.h (Target::adjust_elf_header, Target::adjust_elf_header):
>         Remove const from declaration.
>         * target.cc (Sized_target::do_adjust_elf_header): Update definition.
>         * sparc.cc (Target_sparc::do_adjust_elf_header): Likewise.
>         * parameters.h (Parameters::target): Make return value non-const.
>
> --- a/gold/parameters.h
> +++ b/gold/parameters.h
> @@ -99,7 +99,7 @@ class Parameters
>    { return this->target_ != NULL; }
>
>    // The target of the output file we are generating.
> -  const Target&
> +  Target&
>    target() const
>    {
>      gold_assert(this->target_valid());
> --- a/gold/sparc.cc
> +++ b/gold/sparc.cc
> @@ -217,7 +217,7 @@ class Target_sparc : public Sized_target<size, big_endian>
>                      const elfcpp::Ehdr<size, big_endian>& ehdr);
>
>    void
> -  do_adjust_elf_header(unsigned char* view, int len) const;
> +  do_adjust_elf_header(unsigned char* view, int len);
>
>   private:
>
> @@ -4341,7 +4341,7 @@ template<int size, bool big_endian>
>  void
>  Target_sparc<size, big_endian>::do_adjust_elf_header(
>      unsigned char* view,
> -    int len) const
> +    int len)
>  {
>    elfcpp::Ehdr_write<size, big_endian> oehdr(view);
>
> --- a/gold/target.cc
> +++ b/gold/target.cc
> @@ -1,6 +1,6 @@
>  // target.cc -- target support for gold.
>
> -// Copyright 2009, 2010, 2011 Free Software Foundation, Inc.
> +// Copyright 2009, 2010, 2011, 2013 Free Software Foundation, Inc.
>  // Written by Doug Kwan <dougkwan@google.com>.
>
>  // This file is part of gold.
> @@ -219,7 +219,7 @@ Target::do_plt_fde_location(const Output_data*
> plt, unsigned char*,
>  template<int size, bool big_endian>
>  void
>  Sized_target<size, big_endian>::do_adjust_elf_header(unsigned char* view,
> -                                                    int len) const
> +                                                    int len)
>  {
>    elfcpp::ELFOSABI osabi = this->osabi();
>    if (osabi != elfcpp::ELFOSABI_NONE)
> --- a/gold/target.h
> +++ b/gold/target.h
> @@ -1,6 +1,6 @@
>  // target.h -- target support for gold   -*- C++ -*-
>
> -// Copyright 2006, 2007, 2008, 2009, 2010, 2011, 2012
> +// Copyright 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013
>  // Free Software Foundation, Inc.
>  // Written by Ian Lance Taylor <iant@google.com>.
>
> @@ -238,7 +238,7 @@ class Target
>    // Adjust the output file header before it is written out.  VIEW
>    // points to the header in external form.  LEN is the length.
>    void
> -  adjust_elf_header(unsigned char* view, int len) const
> +  adjust_elf_header(unsigned char* view, int len)
>    { return this->do_adjust_elf_header(view, len); }
>
>    // Return address and size to plug into eh_frame FDEs associated with a PLT.
> @@ -548,7 +548,7 @@ class Target
>    // By default, we set the EI_OSABI field if requested (in
>    // Sized_target).
>    virtual void
> -  do_adjust_elf_header(unsigned char*, int) const = 0;
> +  do_adjust_elf_header(unsigned char*, int) = 0;
>
>    // Return address and size to plug into eh_frame FDEs associated with a PLT.
>    virtual void
> @@ -1018,7 +1018,7 @@ class Sized_target : public Target
>
>    // Set the EI_OSABI field if requested.
>    virtual void
> -  do_adjust_elf_header(unsigned char*, int) const;
> +  do_adjust_elf_header(unsigned char*, int);
>
>    // Handle target specific gc actions when adding a gc reference.
>    virtual void