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

[Roland McGrath <mcgrathr at google dot com>]

On Fri, Oct 11, 2013 at 2:26 PM, Cary Coutant <ccoutant@google.com> wrote:
> 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()?

Separate compilation.  In output.cc the compiler only sees the abstract
class definition, where the method is const.  In arm.cc the override is
non-const, but that (evidently) doesn't make it a different overload (in
the sense of having a separate vtable slot).  GCC (at least 4.6.3, I
haven't tried newer ones) does not warn about an override of a virtual
function having mismatching constness, which is all that can be seen when
compiling arm.cc.

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

Like this?


Thanks,
Roland


gold/
	* target.h (Target::adjust_elf_header, Target::do_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.
	* output.h (Output_file_header): Remove const from member target_
	and corresponding constructor argument.
	* output.cc (Output_file_header::Output_file_header): Update prototype.
	(Output_file_header::do_sized_write): Use this->target_ in place
	of parameters()->target().

--- a/gold/output.cc
+++ b/gold/output.cc
@@ -435,7 +435,7 @@ Output_segment_headers::do_size() const

 // Output_file_header methods.

-Output_file_header::Output_file_header(const Target* target,
+Output_file_header::Output_file_header(Target* target,
 				       const Symbol_table* symtab,
 				       const Output_segment_headers* osh)
   : target_(target),
@@ -577,7 +577,7 @@ Output_file_header::do_sized_write(Output_file* of)

   // Let the target adjust the ELF header, e.g., to set EI_OSABI in
   // the e_ident field.
-  parameters->target().adjust_elf_header(view, ehdr_size);
+  this->target_->adjust_elf_header(view, ehdr_size);

   of->write_output_view(0, ehdr_size, view);
 }
--- a/gold/output.h
+++ b/gold/output.h
@@ -573,7 +573,7 @@ class Output_segment_headers : public Output_data
 class Output_file_header : public Output_data
 {
  public:
-  Output_file_header(const Target*,
+  Output_file_header(Target*,
 		     const Symbol_table*,
 		     const Output_segment_headers*);

@@ -617,7 +617,7 @@ class Output_file_header : public Output_data
   off_t
   do_size() const;

-  const Target* target_;
+  Target* target_;
   const Symbol_table* symtab_;
   const Output_segment_headers* segment_header_;
   const Output_section_headers* section_header_;
--- 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:

@@ -4339,7 +4339,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