* [patch][gold] Remove empty Merge_map class
@ 2015-02-06 23:32 Rafael Espíndola
2015-02-09 16:30 ` Rafael Espíndola
2015-02-17 15:56 ` Rafael Espíndola
0 siblings, 2 replies; 9+ messages in thread
From: Rafael Espíndola @ 2015-02-06 23:32 UTC (permalink / raw)
To: Binutils; +Cc: Cary Coutant
[-- Attachment #1: Type: text/plain, Size: 834 bytes --]
When looking at the constant merging code to see if the
mapping could be reused for gc, the most confusing part for
me was the empty Merge_map class.
It is only used as a key to implement is_merge_section_for (which is
only used by find_starting_output_address).
Since it is just a key, we can use the Output_section_data as a key
instead. This patch does that. IMHO removing the indirection makes
things a bit easier to understand. It also removes the need for the
virtual do_is_merge_section_for.
Last but not least, it makes some inefficiencies easier to spot. For
example, Output_merge_data::do_add_input_section handles only one
input section and therefore only one merge map, but it does
object->add_merge_mapping(this, shndx, i, entsize, k);
in a loop. It could extract Input_merge_map* before the loop.
Cheers,
Rafael
[-- Attachment #2: t.patch --]
[-- Type: text/x-patch, Size: 24206 bytes --]
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index faea1a8..711103b 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -412,7 +412,7 @@ Cie::~Cie()
section_offset_type
Cie::set_output_offset(section_offset_type output_offset,
unsigned int addralign,
- Merge_map* merge_map)
+ Output_section_data *output_data)
{
size_t length = this->contents_.length();
@@ -422,8 +422,9 @@ Cie::set_output_offset(section_offset_type output_offset,
if (this->object_ != NULL)
{
// Add a mapping so that relocations are applied correctly.
- merge_map->add_mapping(this->object_, this->shndx_, this->input_offset_,
- length, output_offset);
+ this->object_->add_merge_mapping(output_data, this->shndx_,
+ this->input_offset_, length,
+ output_offset);
}
length = align_address(length, addralign);
@@ -432,7 +433,7 @@ Cie::set_output_offset(section_offset_type output_offset,
p != this->fdes_.end();
++p)
{
- (*p)->add_mapping(output_offset + length, merge_map);
+ (*p)->add_mapping(output_offset + length, output_data);
size_t fde_length = (*p)->length();
fde_length = align_address(fde_length, addralign);
@@ -531,7 +532,6 @@ Eh_frame::Eh_frame()
eh_frame_hdr_(NULL),
cie_offsets_(),
unmergeable_cie_offsets_(),
- merge_map_(),
mappings_are_done_(false),
final_data_size_(0)
{
@@ -958,8 +958,8 @@ Eh_frame::read_cie(Sized_relobj_file<size, big_endian>* object,
// know for sure that we are doing a special mapping for this
// input section, but that's OK--if we don't do a special
// mapping, nobody will ever ask for the mapping we add here.
- this->merge_map_.add_mapping(object, shndx, (pcie - 8) - pcontents,
- pcieend - (pcie - 8), -1);
+ object->add_merge_mapping(this, shndx, (pcie - 8) - pcontents,
+ pcieend - (pcie - 8), -1);
}
// Record this CIE plus the offset in the input section.
@@ -1026,8 +1026,8 @@ Eh_frame::read_fde(Sized_relobj_file<size, big_endian>* object,
{
// This FDE applies to a section which we are discarding. We
// can discard this FDE.
- this->merge_map_.add_mapping(object, shndx, (pfde - 8) - pcontents,
- pfdeend - (pfde - 8), -1);
+ object->add_merge_mapping(this, shndx, (pfde - 8) - pcontents,
+ pfdeend - (pfde - 8), -1);
return true;
}
@@ -1107,14 +1107,14 @@ Eh_frame::set_final_data_size()
++p)
output_offset = (*p)->set_output_offset(output_offset,
this->addralign(),
- &this->merge_map_);
+ this);
for (Cie_offsets::iterator p = this->cie_offsets_.begin();
p != this->cie_offsets_.end();
++p)
output_offset = (*p)->set_output_offset(output_offset,
this->addralign(),
- &this->merge_map_);
+ this);
this->mappings_are_done_ = true;
this->final_data_size_ = output_offset - output_start;
@@ -1130,16 +1130,7 @@ Eh_frame::do_output_offset(const Relobj* object, unsigned int shndx,
section_offset_type offset,
section_offset_type* poutput) const
{
- return this->merge_map_.get_output_offset(object, shndx, offset, poutput);
-}
-
-// Return whether this is the merge section for an input section.
-
-bool
-Eh_frame::do_is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- return this->merge_map_.is_merge_section_for(object, shndx);
+ return object->merge_output_offset(shndx, offset, poutput);
}
// Write the data to the output file.
diff --git a/gold/ehframe.h b/gold/ehframe.h
index aa2bd31..d7152ad 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -192,12 +192,13 @@ class Fde
// Add a mapping for this FDE to MERGE_MAP, so that relocations
// against the FDE are applied to right part of the output file.
void
- add_mapping(section_offset_type output_offset, Merge_map* merge_map) const
+ add_mapping(section_offset_type output_offset,
+ Output_section_data* output_data) const
{
if (this->object_ != NULL)
- merge_map->add_mapping(this->object_, this->u_.from_object.shndx,
- this->u_.from_object.input_offset, this->length(),
- output_offset);
+ this->object_->add_merge_mapping(output_data, this->u_.from_object.shndx,
+ this->u_.from_object.input_offset, this->length(),
+ output_offset);
}
// Return whether this FDE was added after merge mapping.
@@ -308,7 +309,7 @@ class Cie
// mapping. It returns the new output offset.
section_offset_type
set_output_offset(section_offset_type output_offset, unsigned int addralign,
- Merge_map*);
+ Output_section_data*);
// Write the CIE to OVIEW starting at OFFSET. Round up the bytes to
// ADDRALIGN. ADDRESS is the virtual address of OVIEW.
@@ -406,10 +407,6 @@ class Eh_frame : public Output_section_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for an input section.
- bool
- do_is_merge_section_for(const Relobj*, unsigned int shndx) const;
-
// Write the data to the file.
void
do_write(Output_file*);
@@ -504,8 +501,6 @@ class Eh_frame : public Output_section_data
// A mapping from unmergeable CIEs to their offset in the output
// file.
Unmergeable_cie_offsets unmergeable_cie_offsets_;
- // A mapping from input sections to the output section.
- Merge_map merge_map_;
// Whether we have created the mappings to the output section.
bool mappings_are_done_;
// The final data size. This is only set if mappings_are_done_ is
diff --git a/gold/merge.cc b/gold/merge.cc
index f547388..eba3da5 100644
--- a/gold/merge.cc
+++ b/gold/merge.cc
@@ -62,15 +62,14 @@ Object_merge_map::get_input_merge_map(unsigned int shndx)
// Get or create the Input_merge_map to use for an input section.
Object_merge_map::Input_merge_map*
-Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
- unsigned int shndx)
-{
+Object_merge_map::get_or_make_input_merge_map(
+ const Output_section_data* output_data, unsigned int shndx) {
Input_merge_map* map = this->get_input_merge_map(shndx);
if (map != NULL)
{
// For a given input section in a given object, every mapping
// must be done with the same Merge_map.
- gold_assert(map->merge_map == merge_map);
+ gold_assert(map->output_data == output_data);
return map;
}
@@ -78,18 +77,18 @@ Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
if (this->first_shnum_ == -1U)
{
this->first_shnum_ = shndx;
- this->first_map_.merge_map = merge_map;
+ this->first_map_.output_data = output_data;
return &this->first_map_;
}
if (this->second_shnum_ == -1U)
{
this->second_shnum_ = shndx;
- this->second_map_.merge_map = merge_map;
+ this->second_map_.output_data = output_data;
return &this->second_map_;
}
Input_merge_map* new_map = new Input_merge_map;
- new_map->merge_map = merge_map;
+ new_map->output_data = output_data;
this->section_merge_maps_[shndx] = new_map;
return new_map;
}
@@ -97,12 +96,13 @@ Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
// Add a mapping.
void
-Object_merge_map::add_mapping(const Merge_map* merge_map, unsigned int shndx,
+Object_merge_map::add_mapping(const Output_section_data* output_data,
+ unsigned int shndx,
section_offset_type input_offset,
section_size_type length,
section_offset_type output_offset)
{
- Input_merge_map* map = this->get_or_make_input_merge_map(merge_map, shndx);
+ Input_merge_map* map = this->get_or_make_input_merge_map(output_data, shndx);
// Try to merge the new entry in the last one we saw.
if (!map->entries.empty())
@@ -142,14 +142,12 @@ Object_merge_map::add_mapping(const Merge_map* merge_map, unsigned int shndx,
// Get the output offset for an input address.
bool
-Object_merge_map::get_output_offset(const Merge_map* merge_map,
- unsigned int shndx,
+Object_merge_map::get_output_offset(unsigned int shndx,
section_offset_type input_offset,
section_offset_type* output_offset)
{
Input_merge_map* map = this->get_input_merge_map(shndx);
- if (map == NULL
- || (merge_map != NULL && map->merge_map != merge_map))
+ if (map == NULL)
return false;
if (!map->sorted)
@@ -184,12 +182,12 @@ Object_merge_map::get_output_offset(const Merge_map* merge_map,
// Return whether this is the merge map for section SHNDX.
-inline bool
-Object_merge_map::is_merge_section_for(const Merge_map* merge_map,
+bool
+Object_merge_map::is_merge_section_for(const Output_section_data* output_data,
unsigned int shndx)
{
Input_merge_map* map = this->get_input_merge_map(shndx);
- return map != NULL && map->merge_map == merge_map;
+ return map != NULL && map->output_data == output_data;
}
// Initialize a mapping from input offsets to output addresses.
@@ -231,62 +229,11 @@ Object_merge_map::initialize_input_to_output_map(
}
}
-// Class Merge_map.
-
-// Add a mapping for the bytes from OFFSET to OFFSET + LENGTH in input
-// section SHNDX in object OBJECT to an OUTPUT_OFFSET in merged data
-// in an output section.
-
-void
-Merge_map::add_mapping(Relobj* object, unsigned int shndx,
- section_offset_type offset, section_size_type length,
- section_offset_type output_offset)
-{
- gold_assert(object != NULL);
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- {
- object_merge_map = new Object_merge_map();
- object->set_merge_map(object_merge_map);
- }
-
- object_merge_map->add_mapping(this, shndx, offset, length, output_offset);
-}
-
-// Return the output offset for an input address. The input address
-// is at offset OFFSET in section SHNDX in OBJECT. This sets
-// *OUTPUT_OFFSET to the offset in the merged data in the output
-// section. This returns true if the mapping is known, false
-// otherwise.
-
-bool
-Merge_map::get_output_offset(const Relobj* object, unsigned int shndx,
- section_offset_type offset,
- section_offset_type* output_offset) const
-{
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- return false;
- return object_merge_map->get_output_offset(this, shndx, offset,
- output_offset);
-}
-
-// Return whether this is the merge section for SHNDX in OBJECT.
-
-bool
-Merge_map::is_merge_section_for(const Relobj* object, unsigned int shndx) const
-{
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- return false;
- return object_merge_map->is_merge_section_for(this, shndx);
-}
-
// Class Output_merge_base.
// Return the output offset for an input offset. The input address is
// at offset OFFSET in section SHNDX in OBJECT. If we know the
-// offset, set *POUTPUT and return true. Otherwise return false.
+// offset, set *POUTPUT and return true. Otherwise return false
bool
Output_merge_base::do_output_offset(const Relobj* object,
@@ -294,16 +241,7 @@ Output_merge_base::do_output_offset(const Relobj* object,
section_offset_type offset,
section_offset_type* poutput) const
{
- return this->merge_map_.get_output_offset(object, shndx, offset, poutput);
-}
-
-// Return whether this is the merge section for SHNDX in OBJECT.
-
-bool
-Output_merge_base::do_is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- return this->merge_map_.is_merge_section_for(object, shndx);
+ return object->merge_output_offset(shndx, offset, poutput);
}
// Record a merged input section for script processing.
@@ -439,7 +377,7 @@ Output_merge_data::do_add_input_section(Relobj* object, unsigned int shndx)
}
// Record the offset of this constant in the output section.
- this->add_mapping(object, shndx, i, entsize, k);
+ object->add_merge_mapping(this, shndx, i, entsize, k);
}
// For script processing, we keep the input sections.
@@ -628,8 +566,8 @@ Output_merge_string<Char_type>::finalize_merged_data()
{
section_size_type length = p->offset - last_input_offset;
if (length > 0)
- this->add_mapping((*l)->object, (*l)->shndx, last_input_offset,
- length, last_output_offset);
+ (*l)->object->add_merge_mapping(this, (*l)->shndx,
+ last_input_offset, length, last_output_offset);
last_input_offset = p->offset;
if (p->stringpool_key != 0)
last_output_offset =
diff --git a/gold/merge.h b/gold/merge.h
index 6318f45..3e500bc 100644
--- a/gold/merge.h
+++ b/gold/merge.h
@@ -33,8 +33,6 @@
namespace gold
{
-class Merge_map;
-
// For each object with merge sections, we store an Object_merge_map.
// This is used to map locations in input sections to a merged output
// section. The output section itself is not recorded here--it can be
@@ -57,8 +55,9 @@ class Object_merge_map
// discarded. OUTPUT_OFFSET is relative to the start of the merged
// data in the output section.
void
- add_mapping(const Merge_map*, unsigned int shndx, section_offset_type offset,
- section_size_type length, section_offset_type output_offset);
+ add_mapping(const Output_section_data*, unsigned int shndx,
+ section_offset_type offset, section_size_type length,
+ section_offset_type output_offset);
// Get the output offset for an input address. MERGE_MAP is the map
// we are looking for, or NULL if we don't care. The input address
@@ -68,13 +67,13 @@ class Object_merge_map
// mapping is known, false otherwise. *OUTPUT_OFFSET is relative to
// the start of the merged data in the output section.
bool
- get_output_offset(const Merge_map*, unsigned int shndx,
+ get_output_offset(unsigned int shndx,
section_offset_type offset,
section_offset_type* output_offset);
// Return whether this is the merge map for section SHNDX.
bool
- is_merge_section_for(const Merge_map*, unsigned int shndx);
+ is_merge_section_for(const Output_section_data*, unsigned int shndx);
// Initialize an mapping from input offsets to output addresses for
// section SHNDX. STARTING_ADDRESS is the output address of the
@@ -133,14 +132,14 @@ class Object_merge_map
// look up information for a different Merge_map, we report that
// we don't have it, rather than trying a lookup and returning an
// answer which will receive the wrong offset.
- const Merge_map* merge_map;
+ const Output_section_data* output_data;
// The list of mappings.
Entries entries;
// Whether the ENTRIES field is sorted by input_offset.
bool sorted;
Input_merge_map()
- : merge_map(NULL), entries(), sorted(true)
+ : output_data(NULL), entries(), sorted(true)
{ }
};
@@ -155,7 +154,8 @@ class Object_merge_map
// Get or make the Input_merge_map to use for the section SHNDX
// with MERGE_MAP.
Input_merge_map*
- get_or_make_input_merge_map(const Merge_map* merge_map, unsigned int shndx);
+ get_or_make_input_merge_map(const Output_section_data* merge_map,
+ unsigned int shndx);
// Any given object file will normally only have a couple of input
// sections with mergeable contents. So we keep the first two input
@@ -169,46 +169,6 @@ class Object_merge_map
Section_merge_maps section_merge_maps_;
};
-// This class manages mappings from input sections to offsets in an
-// output section. This is used where input sections are merged. The
-// actual data is stored in fields in Object.
-
-class Merge_map
-{
- public:
- Merge_map()
- { }
-
- // Add a mapping for the bytes from OFFSET to OFFSET + LENGTH in the
- // input section SHNDX in object OBJECT to OUTPUT_OFFSET in the
- // output section. An OUTPUT_OFFSET of -1 means that the bytes are
- // discarded. OUTPUT_OFFSET is not the offset from the start of the
- // output section, it is the offset from the start of the merged
- // data within the output section.
- void
- add_mapping(Relobj* object, unsigned int shndx,
- section_offset_type offset, section_size_type length,
- section_offset_type output_offset);
-
- // Return the output offset for an input address. The input address
- // is at offset OFFSET in section SHNDX in OBJECT. This sets
- // *OUTPUT_OFFSET to the offset in the output section; this will be
- // -1 if the bytes are not being copied to the output. This returns
- // true if the mapping is known, false otherwise. This returns the
- // value stored by add_mapping, namely the offset from the start of
- // the merged data within the output section.
- bool
- get_output_offset(const Relobj* object, unsigned int shndx,
- section_offset_type offset,
- section_offset_type* output_offset) const;
-
- // Return whether this is the merge mapping for section SHNDX in
- // OBJECT. This should return true when get_output_offset would
- // return true for some input offset.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const;
-};
-
// A general class for SHF_MERGE data, to hold functions shared by
// fixed-size constant data and string data.
@@ -216,7 +176,7 @@ class Output_merge_base : public Output_section_data
{
public:
Output_merge_base(uint64_t entsize, uint64_t addralign)
- : Output_section_data(addralign), merge_map_(), entsize_(entsize),
+ : Output_section_data(addralign), entsize_(entsize),
keeps_input_sections_(false), first_relobj_(NULL), first_shndx_(-1),
input_sections_()
{ }
@@ -285,21 +245,6 @@ class Output_merge_base : public Output_section_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for an input section.
- bool
- do_is_merge_section_for(const Relobj*, unsigned int shndx) const;
-
- // Add a mapping from an OFFSET in input section SHNDX in object
- // OBJECT to an OUTPUT_OFFSET in the output section. OUTPUT_OFFSET
- // is the offset from the start of the merged data in the output
- // section.
- void
- add_mapping(Relobj* object, unsigned int shndx, section_offset_type offset,
- section_size_type length, section_offset_type output_offset)
- {
- this->merge_map_.add_mapping(object, shndx, offset, length, output_offset);
- }
-
// This may be overridden by the child class.
virtual bool
do_is_string()
@@ -315,9 +260,6 @@ class Output_merge_base : public Output_section_data
record_input_section(Relobj* relobj, unsigned int shndx);
private:
- // A mapping from input object/section/offset to offset in output
- // section.
- Merge_map merge_map_;
// The entry size. For fixed-size constants, this is the size of
// the constants. For strings, this is the size of a character.
uint64_t entsize_;
diff --git a/gold/object.cc b/gold/object.cc
index 8f16fe7..92a106a 100644
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -41,6 +41,7 @@
#include "plugin.h"
#include "compressed_output.h"
#include "incremental.h"
+#include "merge.h"
namespace gold
{
@@ -268,6 +269,41 @@ Object::handle_split_stack_section(const char* name)
// Class Relobj
+void
+Relobj::add_merge_mapping(Output_section_data *output_data,
+ unsigned int shndx, section_offset_type offset,
+ section_size_type length,
+ section_offset_type output_offset) {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ {
+ object_merge_map = new Object_merge_map();
+ this->set_merge_map(object_merge_map);
+ }
+
+ object_merge_map->add_mapping(output_data, shndx, offset, length,
+ output_offset);
+}
+
+bool
+Relobj::merge_output_offset(unsigned int shndx, section_offset_type offset,
+ section_offset_type *poutput) const {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ return false;
+ return object_merge_map->get_output_offset(shndx, offset, poutput);
+}
+
+bool
+Relobj::is_merge_section_for(const Output_section_data* output_data,
+ unsigned int shndx) const {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ return false;
+ return object_merge_map->is_merge_section_for(output_data, shndx);
+
+}
+
// To copy the symbols data read from the file to a local data structure.
// This function is called from do_layout only while doing garbage
// collection.
diff --git a/gold/object.h b/gold/object.h
index cce6c8c..400b41c 100644
--- a/gold/object.h
+++ b/gold/object.h
@@ -41,6 +41,7 @@ class Cref;
class Layout;
class Output_data;
class Output_section;
+class Output_section_data;
class Output_file;
class Output_symtab_xindex;
class Pluginobj;
@@ -1213,6 +1214,20 @@ class Relobj : public Object
this->object_merge_map_ = object_merge_map;
}
+ void
+ add_merge_mapping(Output_section_data *output_data,
+ unsigned int shndx, section_offset_type offset,
+ section_size_type length,
+ section_offset_type output_offset);
+
+ bool
+ merge_output_offset(unsigned int shndx, section_offset_type offset,
+ section_offset_type *poutput) const;
+
+ bool
+ is_merge_section_for(const Output_section_data* output_data,
+ unsigned int shndx) const;
+
// Record the relocatable reloc info for an input reloc section.
void
set_relocatable_relocs(unsigned int reloc_shndx, Relocatable_relocs* rr)
diff --git a/gold/output.cc b/gold/output.cc
index e9dd522..5d624ab 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -2210,7 +2210,7 @@ Output_section::Input_section::is_merge_section_for(const Relobj* object,
{
if (this->is_input_section())
return false;
- return this->u2_.posd->is_merge_section_for(object, shndx);
+ return object->is_merge_section_for(this->u2_.posd, shndx);
}
// Write out the data. We don't have to do anything for an input
diff --git a/gold/output.h b/gold/output.h
index 8e0c33a..dc11c8f 100644
--- a/gold/output.h
+++ b/gold/output.h
@@ -675,13 +675,6 @@ class Output_section_data : public Output_data
section_offset_type* poutput) const
{ return this->do_output_offset(object, shndx, offset, poutput); }
- // Return whether this is the merge section for the input section
- // SHNDX in OBJECT. This should return true when output_offset
- // would return true for some values of OFFSET.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const
- { return this->do_is_merge_section_for(object, shndx); }
-
// Write the contents to a buffer. This is used for sections which
// require postprocessing, such as compression.
void
@@ -715,11 +708,6 @@ class Output_section_data : public Output_data
section_offset_type*) const
{ return false; }
- // The child class may implement is_merge_section_for.
- virtual bool
- do_is_merge_section_for(const Relobj*, unsigned int) const
- { return false; }
-
// The child class may implement write_to_buffer. Most child
// classes can not appear in a compressed section, and they do not
// implement this.
diff --git a/gold/reloc.cc b/gold/reloc.cc
index 5906754..251a811 100644
--- a/gold/reloc.cc
+++ b/gold/reloc.cc
@@ -1474,7 +1474,7 @@ Merged_symbol_value<size>::value_from_output_section(
typename elfcpp::Elf_types<size>::Elf_Addr input_offset) const
{
section_offset_type output_offset;
- bool found = object->merge_map()->get_output_offset(NULL, input_shndx,
+ bool found = object->merge_map()->get_output_offset(input_shndx,
input_offset,
&output_offset);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch][gold] Remove empty Merge_map class
2015-02-06 23:32 [patch][gold] Remove empty Merge_map class Rafael Espíndola
@ 2015-02-09 16:30 ` Rafael Espíndola
2015-02-17 15:56 ` Rafael Espíndola
1 sibling, 0 replies; 9+ messages in thread
From: Rafael Espíndola @ 2015-02-09 16:30 UTC (permalink / raw)
To: Binutils; +Cc: Cary Coutant
[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]
Turns out we can remove the key completely by using
find_merge_section in is_merge_section_for.
The attached patch does it.
On 6 February 2015 at 18:32, Rafael Espíndola
<rafael.espindola@gmail.com> wrote:
> When looking at the constant merging code to see if the
> mapping could be reused for gc, the most confusing part for
> me was the empty Merge_map class.
>
> It is only used as a key to implement is_merge_section_for (which is
> only used by find_starting_output_address).
>
> Since it is just a key, we can use the Output_section_data as a key
> instead. This patch does that. IMHO removing the indirection makes
> things a bit easier to understand. It also removes the need for the
> virtual do_is_merge_section_for.
>
> Last but not least, it makes some inefficiencies easier to spot. For
> example, Output_merge_data::do_add_input_section handles only one
> input section and therefore only one merge map, but it does
>
> object->add_merge_mapping(this, shndx, i, entsize, k);
>
> in a loop. It could extract Input_merge_map* before the loop.
>
> Cheers,
> Rafael
[-- Attachment #2: t.patch --]
[-- Type: text/x-patch, Size: 25279 bytes --]
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index faea1a8..ec210c9 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -411,8 +411,7 @@ Cie::~Cie()
section_offset_type
Cie::set_output_offset(section_offset_type output_offset,
- unsigned int addralign,
- Merge_map* merge_map)
+ unsigned int addralign)
{
size_t length = this->contents_.length();
@@ -422,8 +421,9 @@ Cie::set_output_offset(section_offset_type output_offset,
if (this->object_ != NULL)
{
// Add a mapping so that relocations are applied correctly.
- merge_map->add_mapping(this->object_, this->shndx_, this->input_offset_,
- length, output_offset);
+ this->object_->add_merge_mapping(this->shndx_,
+ this->input_offset_, length,
+ output_offset);
}
length = align_address(length, addralign);
@@ -432,7 +432,7 @@ Cie::set_output_offset(section_offset_type output_offset,
p != this->fdes_.end();
++p)
{
- (*p)->add_mapping(output_offset + length, merge_map);
+ (*p)->add_mapping(output_offset + length);
size_t fde_length = (*p)->length();
fde_length = align_address(fde_length, addralign);
@@ -531,7 +531,6 @@ Eh_frame::Eh_frame()
eh_frame_hdr_(NULL),
cie_offsets_(),
unmergeable_cie_offsets_(),
- merge_map_(),
mappings_are_done_(false),
final_data_size_(0)
{
@@ -958,8 +957,8 @@ Eh_frame::read_cie(Sized_relobj_file<size, big_endian>* object,
// know for sure that we are doing a special mapping for this
// input section, but that's OK--if we don't do a special
// mapping, nobody will ever ask for the mapping we add here.
- this->merge_map_.add_mapping(object, shndx, (pcie - 8) - pcontents,
- pcieend - (pcie - 8), -1);
+ object->add_merge_mapping(shndx, (pcie - 8) - pcontents,
+ pcieend - (pcie - 8), -1);
}
// Record this CIE plus the offset in the input section.
@@ -1026,8 +1025,8 @@ Eh_frame::read_fde(Sized_relobj_file<size, big_endian>* object,
{
// This FDE applies to a section which we are discarding. We
// can discard this FDE.
- this->merge_map_.add_mapping(object, shndx, (pfde - 8) - pcontents,
- pfdeend - (pfde - 8), -1);
+ object->add_merge_mapping(shndx, (pfde - 8) - pcontents,
+ pfdeend - (pfde - 8), -1);
return true;
}
@@ -1106,15 +1105,13 @@ Eh_frame::set_final_data_size()
p != this->unmergeable_cie_offsets_.end();
++p)
output_offset = (*p)->set_output_offset(output_offset,
- this->addralign(),
- &this->merge_map_);
+ this->addralign());
for (Cie_offsets::iterator p = this->cie_offsets_.begin();
p != this->cie_offsets_.end();
++p)
output_offset = (*p)->set_output_offset(output_offset,
- this->addralign(),
- &this->merge_map_);
+ this->addralign());
this->mappings_are_done_ = true;
this->final_data_size_ = output_offset - output_start;
@@ -1130,16 +1127,7 @@ Eh_frame::do_output_offset(const Relobj* object, unsigned int shndx,
section_offset_type offset,
section_offset_type* poutput) const
{
- return this->merge_map_.get_output_offset(object, shndx, offset, poutput);
-}
-
-// Return whether this is the merge section for an input section.
-
-bool
-Eh_frame::do_is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- return this->merge_map_.is_merge_section_for(object, shndx);
+ return object->merge_output_offset(shndx, offset, poutput);
}
// Write the data to the output file.
diff --git a/gold/ehframe.h b/gold/ehframe.h
index aa2bd31..31cd0f4 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -192,12 +192,12 @@ class Fde
// Add a mapping for this FDE to MERGE_MAP, so that relocations
// against the FDE are applied to right part of the output file.
void
- add_mapping(section_offset_type output_offset, Merge_map* merge_map) const
+ add_mapping(section_offset_type output_offset) const
{
if (this->object_ != NULL)
- merge_map->add_mapping(this->object_, this->u_.from_object.shndx,
- this->u_.from_object.input_offset, this->length(),
- output_offset);
+ this->object_->add_merge_mapping(this->u_.from_object.shndx,
+ this->u_.from_object.input_offset, this->length(),
+ output_offset);
}
// Return whether this FDE was added after merge mapping.
@@ -307,8 +307,7 @@ class Cie
// alignment, typically 4 or 8. This updates MERGE_MAP with the
// mapping. It returns the new output offset.
section_offset_type
- set_output_offset(section_offset_type output_offset, unsigned int addralign,
- Merge_map*);
+ set_output_offset(section_offset_type output_offset, unsigned int addralign);
// Write the CIE to OVIEW starting at OFFSET. Round up the bytes to
// ADDRALIGN. ADDRESS is the virtual address of OVIEW.
@@ -406,10 +405,6 @@ class Eh_frame : public Output_section_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for an input section.
- bool
- do_is_merge_section_for(const Relobj*, unsigned int shndx) const;
-
// Write the data to the file.
void
do_write(Output_file*);
@@ -504,8 +499,6 @@ class Eh_frame : public Output_section_data
// A mapping from unmergeable CIEs to their offset in the output
// file.
Unmergeable_cie_offsets unmergeable_cie_offsets_;
- // A mapping from input sections to the output section.
- Merge_map merge_map_;
// Whether we have created the mappings to the output section.
bool mappings_are_done_;
// The final data size. This is only set if mappings_are_done_ is
diff --git a/gold/merge.cc b/gold/merge.cc
index f547388..3cce843 100644
--- a/gold/merge.cc
+++ b/gold/merge.cc
@@ -62,34 +62,24 @@ Object_merge_map::get_input_merge_map(unsigned int shndx)
// Get or create the Input_merge_map to use for an input section.
Object_merge_map::Input_merge_map*
-Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
- unsigned int shndx)
-{
+Object_merge_map::get_or_make_input_merge_map(unsigned int shndx) {
Input_merge_map* map = this->get_input_merge_map(shndx);
if (map != NULL)
- {
- // For a given input section in a given object, every mapping
- // must be done with the same Merge_map.
- gold_assert(map->merge_map == merge_map);
- return map;
- }
+ return map;
// We need to create a new entry.
if (this->first_shnum_ == -1U)
{
this->first_shnum_ = shndx;
- this->first_map_.merge_map = merge_map;
return &this->first_map_;
}
if (this->second_shnum_ == -1U)
{
this->second_shnum_ = shndx;
- this->second_map_.merge_map = merge_map;
return &this->second_map_;
}
Input_merge_map* new_map = new Input_merge_map;
- new_map->merge_map = merge_map;
this->section_merge_maps_[shndx] = new_map;
return new_map;
}
@@ -97,12 +87,12 @@ Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
// Add a mapping.
void
-Object_merge_map::add_mapping(const Merge_map* merge_map, unsigned int shndx,
+Object_merge_map::add_mapping(unsigned int shndx,
section_offset_type input_offset,
section_size_type length,
section_offset_type output_offset)
{
- Input_merge_map* map = this->get_or_make_input_merge_map(merge_map, shndx);
+ Input_merge_map* map = this->get_or_make_input_merge_map(shndx);
// Try to merge the new entry in the last one we saw.
if (!map->entries.empty())
@@ -142,14 +132,12 @@ Object_merge_map::add_mapping(const Merge_map* merge_map, unsigned int shndx,
// Get the output offset for an input address.
bool
-Object_merge_map::get_output_offset(const Merge_map* merge_map,
- unsigned int shndx,
+Object_merge_map::get_output_offset(unsigned int shndx,
section_offset_type input_offset,
section_offset_type* output_offset)
{
Input_merge_map* map = this->get_input_merge_map(shndx);
- if (map == NULL
- || (merge_map != NULL && map->merge_map != merge_map))
+ if (map == NULL)
return false;
if (!map->sorted)
@@ -182,16 +170,6 @@ Object_merge_map::get_output_offset(const Merge_map* merge_map,
return true;
}
-// Return whether this is the merge map for section SHNDX.
-
-inline bool
-Object_merge_map::is_merge_section_for(const Merge_map* merge_map,
- unsigned int shndx)
-{
- Input_merge_map* map = this->get_input_merge_map(shndx);
- return map != NULL && map->merge_map == merge_map;
-}
-
// Initialize a mapping from input offsets to output addresses.
template<int size>
@@ -231,57 +209,6 @@ Object_merge_map::initialize_input_to_output_map(
}
}
-// Class Merge_map.
-
-// Add a mapping for the bytes from OFFSET to OFFSET + LENGTH in input
-// section SHNDX in object OBJECT to an OUTPUT_OFFSET in merged data
-// in an output section.
-
-void
-Merge_map::add_mapping(Relobj* object, unsigned int shndx,
- section_offset_type offset, section_size_type length,
- section_offset_type output_offset)
-{
- gold_assert(object != NULL);
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- {
- object_merge_map = new Object_merge_map();
- object->set_merge_map(object_merge_map);
- }
-
- object_merge_map->add_mapping(this, shndx, offset, length, output_offset);
-}
-
-// Return the output offset for an input address. The input address
-// is at offset OFFSET in section SHNDX in OBJECT. This sets
-// *OUTPUT_OFFSET to the offset in the merged data in the output
-// section. This returns true if the mapping is known, false
-// otherwise.
-
-bool
-Merge_map::get_output_offset(const Relobj* object, unsigned int shndx,
- section_offset_type offset,
- section_offset_type* output_offset) const
-{
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- return false;
- return object_merge_map->get_output_offset(this, shndx, offset,
- output_offset);
-}
-
-// Return whether this is the merge section for SHNDX in OBJECT.
-
-bool
-Merge_map::is_merge_section_for(const Relobj* object, unsigned int shndx) const
-{
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- return false;
- return object_merge_map->is_merge_section_for(this, shndx);
-}
-
// Class Output_merge_base.
// Return the output offset for an input offset. The input address is
@@ -294,16 +221,7 @@ Output_merge_base::do_output_offset(const Relobj* object,
section_offset_type offset,
section_offset_type* poutput) const
{
- return this->merge_map_.get_output_offset(object, shndx, offset, poutput);
-}
-
-// Return whether this is the merge section for SHNDX in OBJECT.
-
-bool
-Output_merge_base::do_is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- return this->merge_map_.is_merge_section_for(object, shndx);
+ return object->merge_output_offset(shndx, offset, poutput);
}
// Record a merged input section for script processing.
@@ -439,7 +357,7 @@ Output_merge_data::do_add_input_section(Relobj* object, unsigned int shndx)
}
// Record the offset of this constant in the output section.
- this->add_mapping(object, shndx, i, entsize, k);
+ object->add_merge_mapping(shndx, i, entsize, k);
}
// For script processing, we keep the input sections.
@@ -628,8 +546,8 @@ Output_merge_string<Char_type>::finalize_merged_data()
{
section_size_type length = p->offset - last_input_offset;
if (length > 0)
- this->add_mapping((*l)->object, (*l)->shndx, last_input_offset,
- length, last_output_offset);
+ (*l)->object->add_merge_mapping((*l)->shndx, last_input_offset,
+ length, last_output_offset);
last_input_offset = p->offset;
if (p->stringpool_key != 0)
last_output_offset =
diff --git a/gold/merge.h b/gold/merge.h
index 6318f45..d07c4bd 100644
--- a/gold/merge.h
+++ b/gold/merge.h
@@ -33,8 +33,6 @@
namespace gold
{
-class Merge_map;
-
// For each object with merge sections, we store an Object_merge_map.
// This is used to map locations in input sections to a merged output
// section. The output section itself is not recorded here--it can be
@@ -57,7 +55,7 @@ class Object_merge_map
// discarded. OUTPUT_OFFSET is relative to the start of the merged
// data in the output section.
void
- add_mapping(const Merge_map*, unsigned int shndx, section_offset_type offset,
+ add_mapping(unsigned int shndx, section_offset_type offset,
section_size_type length, section_offset_type output_offset);
// Get the output offset for an input address. MERGE_MAP is the map
@@ -68,14 +66,10 @@ class Object_merge_map
// mapping is known, false otherwise. *OUTPUT_OFFSET is relative to
// the start of the merged data in the output section.
bool
- get_output_offset(const Merge_map*, unsigned int shndx,
+ get_output_offset(unsigned int shndx,
section_offset_type offset,
section_offset_type* output_offset);
- // Return whether this is the merge map for section SHNDX.
- bool
- is_merge_section_for(const Merge_map*, unsigned int shndx);
-
// Initialize an mapping from input offsets to output addresses for
// section SHNDX. STARTING_ADDRESS is the output address of the
// merged section.
@@ -114,33 +108,13 @@ class Object_merge_map
{
typedef std::vector<Input_merge_entry> Entries;
- // We store these with the Relobj, and we look them up by input
- // section. It is possible to have two different merge maps
- // associated with a single output section. For example, this
- // happens routinely with .rodata, when merged string constants
- // and merged fixed size constants are both put into .rodata. The
- // output offset that we store is not the offset from the start of
- // the output section; it is the offset from the start of the
- // merged data in the output section. That means that the caller
- // is going to add the offset of the merged data within the output
- // section, which means that the caller needs to know which set of
- // merged data it found the entry in. So it's not enough to find
- // this data based on the input section and the output section; we
- // also have to find it based on a set of merged data in the
- // output section. In order to verify that we are looking at the
- // right data, we store a pointer to the Merge_map here, and we
- // pass in a pointer when looking at the data. If we are asked to
- // look up information for a different Merge_map, we report that
- // we don't have it, rather than trying a lookup and returning an
- // answer which will receive the wrong offset.
- const Merge_map* merge_map;
// The list of mappings.
Entries entries;
// Whether the ENTRIES field is sorted by input_offset.
bool sorted;
Input_merge_map()
- : merge_map(NULL), entries(), sorted(true)
+ : entries(), sorted(true)
{ }
};
@@ -155,7 +129,7 @@ class Object_merge_map
// Get or make the Input_merge_map to use for the section SHNDX
// with MERGE_MAP.
Input_merge_map*
- get_or_make_input_merge_map(const Merge_map* merge_map, unsigned int shndx);
+ get_or_make_input_merge_map(unsigned int shndx);
// Any given object file will normally only have a couple of input
// sections with mergeable contents. So we keep the first two input
@@ -169,46 +143,6 @@ class Object_merge_map
Section_merge_maps section_merge_maps_;
};
-// This class manages mappings from input sections to offsets in an
-// output section. This is used where input sections are merged. The
-// actual data is stored in fields in Object.
-
-class Merge_map
-{
- public:
- Merge_map()
- { }
-
- // Add a mapping for the bytes from OFFSET to OFFSET + LENGTH in the
- // input section SHNDX in object OBJECT to OUTPUT_OFFSET in the
- // output section. An OUTPUT_OFFSET of -1 means that the bytes are
- // discarded. OUTPUT_OFFSET is not the offset from the start of the
- // output section, it is the offset from the start of the merged
- // data within the output section.
- void
- add_mapping(Relobj* object, unsigned int shndx,
- section_offset_type offset, section_size_type length,
- section_offset_type output_offset);
-
- // Return the output offset for an input address. The input address
- // is at offset OFFSET in section SHNDX in OBJECT. This sets
- // *OUTPUT_OFFSET to the offset in the output section; this will be
- // -1 if the bytes are not being copied to the output. This returns
- // true if the mapping is known, false otherwise. This returns the
- // value stored by add_mapping, namely the offset from the start of
- // the merged data within the output section.
- bool
- get_output_offset(const Relobj* object, unsigned int shndx,
- section_offset_type offset,
- section_offset_type* output_offset) const;
-
- // Return whether this is the merge mapping for section SHNDX in
- // OBJECT. This should return true when get_output_offset would
- // return true for some input offset.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const;
-};
-
// A general class for SHF_MERGE data, to hold functions shared by
// fixed-size constant data and string data.
@@ -216,7 +150,7 @@ class Output_merge_base : public Output_section_data
{
public:
Output_merge_base(uint64_t entsize, uint64_t addralign)
- : Output_section_data(addralign), merge_map_(), entsize_(entsize),
+ : Output_section_data(addralign), entsize_(entsize),
keeps_input_sections_(false), first_relobj_(NULL), first_shndx_(-1),
input_sections_()
{ }
@@ -285,21 +219,6 @@ class Output_merge_base : public Output_section_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for an input section.
- bool
- do_is_merge_section_for(const Relobj*, unsigned int shndx) const;
-
- // Add a mapping from an OFFSET in input section SHNDX in object
- // OBJECT to an OUTPUT_OFFSET in the output section. OUTPUT_OFFSET
- // is the offset from the start of the merged data in the output
- // section.
- void
- add_mapping(Relobj* object, unsigned int shndx, section_offset_type offset,
- section_size_type length, section_offset_type output_offset)
- {
- this->merge_map_.add_mapping(object, shndx, offset, length, output_offset);
- }
-
// This may be overridden by the child class.
virtual bool
do_is_string()
@@ -315,9 +234,6 @@ class Output_merge_base : public Output_section_data
record_input_section(Relobj* relobj, unsigned int shndx);
private:
- // A mapping from input object/section/offset to offset in output
- // section.
- Merge_map merge_map_;
// The entry size. For fixed-size constants, this is the size of
// the constants. For strings, this is the size of a character.
uint64_t entsize_;
diff --git a/gold/object.cc b/gold/object.cc
index 8f16fe7..51ad561 100644
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -41,6 +41,7 @@
#include "plugin.h"
#include "compressed_output.h"
#include "incremental.h"
+#include "merge.h"
namespace gold
{
@@ -268,6 +269,29 @@ Object::handle_split_stack_section(const char* name)
// Class Relobj
+void
+Relobj::add_merge_mapping(unsigned int shndx, section_offset_type offset,
+ section_size_type length,
+ section_offset_type output_offset) {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ {
+ object_merge_map = new Object_merge_map();
+ this->set_merge_map(object_merge_map);
+ }
+
+ object_merge_map->add_mapping(shndx, offset, length, output_offset);
+}
+
+bool
+Relobj::merge_output_offset(unsigned int shndx, section_offset_type offset,
+ section_offset_type *poutput) const {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ return false;
+ return object_merge_map->get_output_offset(shndx, offset, poutput);
+}
+
// To copy the symbols data read from the file to a local data structure.
// This function is called from do_layout only while doing garbage
// collection.
diff --git a/gold/object.h b/gold/object.h
index cce6c8c..f3394d3 100644
--- a/gold/object.h
+++ b/gold/object.h
@@ -41,6 +41,7 @@ class Cref;
class Layout;
class Output_data;
class Output_section;
+class Output_section_data;
class Output_file;
class Output_symtab_xindex;
class Pluginobj;
@@ -1213,6 +1214,15 @@ class Relobj : public Object
this->object_merge_map_ = object_merge_map;
}
+ void
+ add_merge_mapping(unsigned int shndx, section_offset_type offset,
+ section_size_type length,
+ section_offset_type output_offset);
+
+ bool
+ merge_output_offset(unsigned int shndx, section_offset_type offset,
+ section_offset_type *poutput) const;
+
// Record the relocatable reloc info for an input reloc section.
void
set_relocatable_relocs(unsigned int reloc_shndx, Relocatable_relocs* rr)
diff --git a/gold/output.cc b/gold/output.cc
index e9dd522..927bcd7 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -2201,18 +2201,6 @@ Output_section::Input_section::output_offset(
}
}
-// Return whether this is the merge section for the input section
-// SHNDX in OBJECT.
-
-inline bool
-Output_section::Input_section::is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- if (this->is_input_section())
- return false;
- return this->u2_.posd->is_merge_section_for(object, shndx);
-}
-
// Write out the data. We don't have to do anything for an input
// section--they are handled via Object::relocate--but this is where
// we write out the data for an Output_section_data.
@@ -3041,6 +3029,8 @@ Output_section::find_starting_output_address(const Relobj* object,
unsigned int shndx,
uint64_t* paddr) const
{
+ const Output_section_data* data = this->find_merge_section(object, shndx);
+
// FIXME: This becomes a bottle-neck if we have many relaxed sections.
// Looking up the merge section map does not always work as we sometimes
// find a merge section without its address set.
@@ -3055,7 +3045,7 @@ Output_section::find_starting_output_address(const Relobj* object,
// method to get the output offset of input offset 0.
// Unfortunately we don't know for sure that input offset 0 is
// mapped at all.
- if (p->is_merge_section_for(object, shndx))
+ if (!p->is_input_section() && p->output_section_data() == data)
{
*paddr = addr;
return true;
diff --git a/gold/output.h b/gold/output.h
index 8e0c33a..6d0a63b 100644
--- a/gold/output.h
+++ b/gold/output.h
@@ -675,13 +675,6 @@ class Output_section_data : public Output_data
section_offset_type* poutput) const
{ return this->do_output_offset(object, shndx, offset, poutput); }
- // Return whether this is the merge section for the input section
- // SHNDX in OBJECT. This should return true when output_offset
- // would return true for some values of OFFSET.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const
- { return this->do_is_merge_section_for(object, shndx); }
-
// Write the contents to a buffer. This is used for sections which
// require postprocessing, such as compression.
void
@@ -715,11 +708,6 @@ class Output_section_data : public Output_data
section_offset_type*) const
{ return false; }
- // The child class may implement is_merge_section_for.
- virtual bool
- do_is_merge_section_for(const Relobj*, unsigned int) const
- { return false; }
-
// The child class may implement write_to_buffer. Most child
// classes can not appear in a compressed section, and they do not
// implement this.
@@ -3773,11 +3761,6 @@ class Output_section : public Output_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for the input section
- // SHNDX in OBJECT.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const;
-
// Write out the data. This does nothing for an input section.
void
write(Output_file*);
diff --git a/gold/reloc.cc b/gold/reloc.cc
index 5906754..251a811 100644
--- a/gold/reloc.cc
+++ b/gold/reloc.cc
@@ -1474,7 +1474,7 @@ Merged_symbol_value<size>::value_from_output_section(
typename elfcpp::Elf_types<size>::Elf_Addr input_offset) const
{
section_offset_type output_offset;
- bool found = object->merge_map()->get_output_offset(NULL, input_shndx,
+ bool found = object->merge_map()->get_output_offset(input_shndx,
input_offset,
&output_offset);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch][gold] Remove empty Merge_map class
2015-02-06 23:32 [patch][gold] Remove empty Merge_map class Rafael Espíndola
2015-02-09 16:30 ` Rafael Espíndola
@ 2015-02-17 15:56 ` Rafael Espíndola
2015-02-22 3:12 ` Rafael Espíndola
1 sibling, 1 reply; 9+ messages in thread
From: Rafael Espíndola @ 2015-02-17 15:56 UTC (permalink / raw)
To: Binutils; +Cc: Cary Coutant
[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]
Ping. A rebased patch is attached.
On 6 February 2015 at 18:32, Rafael Espíndola
<rafael.espindola@gmail.com> wrote:
> When looking at the constant merging code to see if the
> mapping could be reused for gc, the most confusing part for
> me was the empty Merge_map class.
>
> It is only used as a key to implement is_merge_section_for (which is
> only used by find_starting_output_address).
>
> Since it is just a key, we can use the Output_section_data as a key
> instead. This patch does that. IMHO removing the indirection makes
> things a bit easier to understand. It also removes the need for the
> virtual do_is_merge_section_for.
>
> Last but not least, it makes some inefficiencies easier to spot. For
> example, Output_merge_data::do_add_input_section handles only one
> input section and therefore only one merge map, but it does
>
> object->add_merge_mapping(this, shndx, i, entsize, k);
>
> in a loop. It could extract Input_merge_map* before the loop.
>
> Cheers,
> Rafael
[-- Attachment #2: t.patch --]
[-- Type: text/x-patch, Size: 24206 bytes --]
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index faea1a8..711103b 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -412,7 +412,7 @@ Cie::~Cie()
section_offset_type
Cie::set_output_offset(section_offset_type output_offset,
unsigned int addralign,
- Merge_map* merge_map)
+ Output_section_data *output_data)
{
size_t length = this->contents_.length();
@@ -422,8 +422,9 @@ Cie::set_output_offset(section_offset_type output_offset,
if (this->object_ != NULL)
{
// Add a mapping so that relocations are applied correctly.
- merge_map->add_mapping(this->object_, this->shndx_, this->input_offset_,
- length, output_offset);
+ this->object_->add_merge_mapping(output_data, this->shndx_,
+ this->input_offset_, length,
+ output_offset);
}
length = align_address(length, addralign);
@@ -432,7 +433,7 @@ Cie::set_output_offset(section_offset_type output_offset,
p != this->fdes_.end();
++p)
{
- (*p)->add_mapping(output_offset + length, merge_map);
+ (*p)->add_mapping(output_offset + length, output_data);
size_t fde_length = (*p)->length();
fde_length = align_address(fde_length, addralign);
@@ -531,7 +532,6 @@ Eh_frame::Eh_frame()
eh_frame_hdr_(NULL),
cie_offsets_(),
unmergeable_cie_offsets_(),
- merge_map_(),
mappings_are_done_(false),
final_data_size_(0)
{
@@ -958,8 +958,8 @@ Eh_frame::read_cie(Sized_relobj_file<size, big_endian>* object,
// know for sure that we are doing a special mapping for this
// input section, but that's OK--if we don't do a special
// mapping, nobody will ever ask for the mapping we add here.
- this->merge_map_.add_mapping(object, shndx, (pcie - 8) - pcontents,
- pcieend - (pcie - 8), -1);
+ object->add_merge_mapping(this, shndx, (pcie - 8) - pcontents,
+ pcieend - (pcie - 8), -1);
}
// Record this CIE plus the offset in the input section.
@@ -1026,8 +1026,8 @@ Eh_frame::read_fde(Sized_relobj_file<size, big_endian>* object,
{
// This FDE applies to a section which we are discarding. We
// can discard this FDE.
- this->merge_map_.add_mapping(object, shndx, (pfde - 8) - pcontents,
- pfdeend - (pfde - 8), -1);
+ object->add_merge_mapping(this, shndx, (pfde - 8) - pcontents,
+ pfdeend - (pfde - 8), -1);
return true;
}
@@ -1107,14 +1107,14 @@ Eh_frame::set_final_data_size()
++p)
output_offset = (*p)->set_output_offset(output_offset,
this->addralign(),
- &this->merge_map_);
+ this);
for (Cie_offsets::iterator p = this->cie_offsets_.begin();
p != this->cie_offsets_.end();
++p)
output_offset = (*p)->set_output_offset(output_offset,
this->addralign(),
- &this->merge_map_);
+ this);
this->mappings_are_done_ = true;
this->final_data_size_ = output_offset - output_start;
@@ -1130,16 +1130,7 @@ Eh_frame::do_output_offset(const Relobj* object, unsigned int shndx,
section_offset_type offset,
section_offset_type* poutput) const
{
- return this->merge_map_.get_output_offset(object, shndx, offset, poutput);
-}
-
-// Return whether this is the merge section for an input section.
-
-bool
-Eh_frame::do_is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- return this->merge_map_.is_merge_section_for(object, shndx);
+ return object->merge_output_offset(shndx, offset, poutput);
}
// Write the data to the output file.
diff --git a/gold/ehframe.h b/gold/ehframe.h
index aa2bd31..d7152ad 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -192,12 +192,13 @@ class Fde
// Add a mapping for this FDE to MERGE_MAP, so that relocations
// against the FDE are applied to right part of the output file.
void
- add_mapping(section_offset_type output_offset, Merge_map* merge_map) const
+ add_mapping(section_offset_type output_offset,
+ Output_section_data* output_data) const
{
if (this->object_ != NULL)
- merge_map->add_mapping(this->object_, this->u_.from_object.shndx,
- this->u_.from_object.input_offset, this->length(),
- output_offset);
+ this->object_->add_merge_mapping(output_data, this->u_.from_object.shndx,
+ this->u_.from_object.input_offset, this->length(),
+ output_offset);
}
// Return whether this FDE was added after merge mapping.
@@ -308,7 +309,7 @@ class Cie
// mapping. It returns the new output offset.
section_offset_type
set_output_offset(section_offset_type output_offset, unsigned int addralign,
- Merge_map*);
+ Output_section_data*);
// Write the CIE to OVIEW starting at OFFSET. Round up the bytes to
// ADDRALIGN. ADDRESS is the virtual address of OVIEW.
@@ -406,10 +407,6 @@ class Eh_frame : public Output_section_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for an input section.
- bool
- do_is_merge_section_for(const Relobj*, unsigned int shndx) const;
-
// Write the data to the file.
void
do_write(Output_file*);
@@ -504,8 +501,6 @@ class Eh_frame : public Output_section_data
// A mapping from unmergeable CIEs to their offset in the output
// file.
Unmergeable_cie_offsets unmergeable_cie_offsets_;
- // A mapping from input sections to the output section.
- Merge_map merge_map_;
// Whether we have created the mappings to the output section.
bool mappings_are_done_;
// The final data size. This is only set if mappings_are_done_ is
diff --git a/gold/merge.cc b/gold/merge.cc
index bf00481..53c4883 100644
--- a/gold/merge.cc
+++ b/gold/merge.cc
@@ -62,15 +62,14 @@ Object_merge_map::get_input_merge_map(unsigned int shndx)
// Get or create the Input_merge_map to use for an input section.
Object_merge_map::Input_merge_map*
-Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
- unsigned int shndx)
-{
+Object_merge_map::get_or_make_input_merge_map(
+ const Output_section_data* output_data, unsigned int shndx) {
Input_merge_map* map = this->get_input_merge_map(shndx);
if (map != NULL)
{
// For a given input section in a given object, every mapping
// must be done with the same Merge_map.
- gold_assert(map->merge_map == merge_map);
+ gold_assert(map->output_data == output_data);
return map;
}
@@ -78,18 +77,18 @@ Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
if (this->first_shnum_ == -1U)
{
this->first_shnum_ = shndx;
- this->first_map_.merge_map = merge_map;
+ this->first_map_.output_data = output_data;
return &this->first_map_;
}
if (this->second_shnum_ == -1U)
{
this->second_shnum_ = shndx;
- this->second_map_.merge_map = merge_map;
+ this->second_map_.output_data = output_data;
return &this->second_map_;
}
Input_merge_map* new_map = new Input_merge_map;
- new_map->merge_map = merge_map;
+ new_map->output_data = output_data;
this->section_merge_maps_[shndx] = new_map;
return new_map;
}
@@ -97,12 +96,13 @@ Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
// Add a mapping.
void
-Object_merge_map::add_mapping(const Merge_map* merge_map, unsigned int shndx,
+Object_merge_map::add_mapping(const Output_section_data* output_data,
+ unsigned int shndx,
section_offset_type input_offset,
section_size_type length,
section_offset_type output_offset)
{
- Input_merge_map* map = this->get_or_make_input_merge_map(merge_map, shndx);
+ Input_merge_map* map = this->get_or_make_input_merge_map(output_data, shndx);
// Try to merge the new entry in the last one we saw.
if (!map->entries.empty())
@@ -142,14 +142,12 @@ Object_merge_map::add_mapping(const Merge_map* merge_map, unsigned int shndx,
// Get the output offset for an input address.
bool
-Object_merge_map::get_output_offset(const Merge_map* merge_map,
- unsigned int shndx,
+Object_merge_map::get_output_offset(unsigned int shndx,
section_offset_type input_offset,
section_offset_type* output_offset)
{
Input_merge_map* map = this->get_input_merge_map(shndx);
- if (map == NULL
- || (merge_map != NULL && map->merge_map != merge_map))
+ if (map == NULL)
return false;
if (!map->sorted)
@@ -181,12 +179,12 @@ Object_merge_map::get_output_offset(const Merge_map* merge_map,
// Return whether this is the merge map for section SHNDX.
-inline bool
-Object_merge_map::is_merge_section_for(const Merge_map* merge_map,
+bool
+Object_merge_map::is_merge_section_for(const Output_section_data* output_data,
unsigned int shndx)
{
Input_merge_map* map = this->get_input_merge_map(shndx);
- return map != NULL && map->merge_map == merge_map;
+ return map != NULL && map->output_data == output_data;
}
// Initialize a mapping from input offsets to output addresses.
@@ -228,62 +226,11 @@ Object_merge_map::initialize_input_to_output_map(
}
}
-// Class Merge_map.
-
-// Add a mapping for the bytes from OFFSET to OFFSET + LENGTH in input
-// section SHNDX in object OBJECT to an OUTPUT_OFFSET in merged data
-// in an output section.
-
-void
-Merge_map::add_mapping(Relobj* object, unsigned int shndx,
- section_offset_type offset, section_size_type length,
- section_offset_type output_offset)
-{
- gold_assert(object != NULL);
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- {
- object_merge_map = new Object_merge_map();
- object->set_merge_map(object_merge_map);
- }
-
- object_merge_map->add_mapping(this, shndx, offset, length, output_offset);
-}
-
-// Return the output offset for an input address. The input address
-// is at offset OFFSET in section SHNDX in OBJECT. This sets
-// *OUTPUT_OFFSET to the offset in the merged data in the output
-// section. This returns true if the mapping is known, false
-// otherwise.
-
-bool
-Merge_map::get_output_offset(const Relobj* object, unsigned int shndx,
- section_offset_type offset,
- section_offset_type* output_offset) const
-{
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- return false;
- return object_merge_map->get_output_offset(this, shndx, offset,
- output_offset);
-}
-
-// Return whether this is the merge section for SHNDX in OBJECT.
-
-bool
-Merge_map::is_merge_section_for(const Relobj* object, unsigned int shndx) const
-{
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- return false;
- return object_merge_map->is_merge_section_for(this, shndx);
-}
-
// Class Output_merge_base.
// Return the output offset for an input offset. The input address is
// at offset OFFSET in section SHNDX in OBJECT. If we know the
-// offset, set *POUTPUT and return true. Otherwise return false.
+// offset, set *POUTPUT and return true. Otherwise return false
bool
Output_merge_base::do_output_offset(const Relobj* object,
@@ -291,16 +238,7 @@ Output_merge_base::do_output_offset(const Relobj* object,
section_offset_type offset,
section_offset_type* poutput) const
{
- return this->merge_map_.get_output_offset(object, shndx, offset, poutput);
-}
-
-// Return whether this is the merge section for SHNDX in OBJECT.
-
-bool
-Output_merge_base::do_is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- return this->merge_map_.is_merge_section_for(object, shndx);
+ return object->merge_output_offset(shndx, offset, poutput);
}
// Record a merged input section for script processing.
@@ -436,7 +374,7 @@ Output_merge_data::do_add_input_section(Relobj* object, unsigned int shndx)
}
// Record the offset of this constant in the output section.
- this->add_mapping(object, shndx, i, entsize, k);
+ object->add_merge_mapping(this, shndx, i, entsize, k);
}
// For script processing, we keep the input sections.
@@ -625,8 +563,8 @@ Output_merge_string<Char_type>::finalize_merged_data()
{
section_size_type length = p->offset - last_input_offset;
if (length > 0)
- this->add_mapping((*l)->object, (*l)->shndx, last_input_offset,
- length, last_output_offset);
+ (*l)->object->add_merge_mapping(this, (*l)->shndx,
+ last_input_offset, length, last_output_offset);
last_input_offset = p->offset;
if (p->stringpool_key != 0)
last_output_offset =
diff --git a/gold/merge.h b/gold/merge.h
index 6318f45..3e500bc 100644
--- a/gold/merge.h
+++ b/gold/merge.h
@@ -33,8 +33,6 @@
namespace gold
{
-class Merge_map;
-
// For each object with merge sections, we store an Object_merge_map.
// This is used to map locations in input sections to a merged output
// section. The output section itself is not recorded here--it can be
@@ -57,8 +55,9 @@ class Object_merge_map
// discarded. OUTPUT_OFFSET is relative to the start of the merged
// data in the output section.
void
- add_mapping(const Merge_map*, unsigned int shndx, section_offset_type offset,
- section_size_type length, section_offset_type output_offset);
+ add_mapping(const Output_section_data*, unsigned int shndx,
+ section_offset_type offset, section_size_type length,
+ section_offset_type output_offset);
// Get the output offset for an input address. MERGE_MAP is the map
// we are looking for, or NULL if we don't care. The input address
@@ -68,13 +67,13 @@ class Object_merge_map
// mapping is known, false otherwise. *OUTPUT_OFFSET is relative to
// the start of the merged data in the output section.
bool
- get_output_offset(const Merge_map*, unsigned int shndx,
+ get_output_offset(unsigned int shndx,
section_offset_type offset,
section_offset_type* output_offset);
// Return whether this is the merge map for section SHNDX.
bool
- is_merge_section_for(const Merge_map*, unsigned int shndx);
+ is_merge_section_for(const Output_section_data*, unsigned int shndx);
// Initialize an mapping from input offsets to output addresses for
// section SHNDX. STARTING_ADDRESS is the output address of the
@@ -133,14 +132,14 @@ class Object_merge_map
// look up information for a different Merge_map, we report that
// we don't have it, rather than trying a lookup and returning an
// answer which will receive the wrong offset.
- const Merge_map* merge_map;
+ const Output_section_data* output_data;
// The list of mappings.
Entries entries;
// Whether the ENTRIES field is sorted by input_offset.
bool sorted;
Input_merge_map()
- : merge_map(NULL), entries(), sorted(true)
+ : output_data(NULL), entries(), sorted(true)
{ }
};
@@ -155,7 +154,8 @@ class Object_merge_map
// Get or make the Input_merge_map to use for the section SHNDX
// with MERGE_MAP.
Input_merge_map*
- get_or_make_input_merge_map(const Merge_map* merge_map, unsigned int shndx);
+ get_or_make_input_merge_map(const Output_section_data* merge_map,
+ unsigned int shndx);
// Any given object file will normally only have a couple of input
// sections with mergeable contents. So we keep the first two input
@@ -169,46 +169,6 @@ class Object_merge_map
Section_merge_maps section_merge_maps_;
};
-// This class manages mappings from input sections to offsets in an
-// output section. This is used where input sections are merged. The
-// actual data is stored in fields in Object.
-
-class Merge_map
-{
- public:
- Merge_map()
- { }
-
- // Add a mapping for the bytes from OFFSET to OFFSET + LENGTH in the
- // input section SHNDX in object OBJECT to OUTPUT_OFFSET in the
- // output section. An OUTPUT_OFFSET of -1 means that the bytes are
- // discarded. OUTPUT_OFFSET is not the offset from the start of the
- // output section, it is the offset from the start of the merged
- // data within the output section.
- void
- add_mapping(Relobj* object, unsigned int shndx,
- section_offset_type offset, section_size_type length,
- section_offset_type output_offset);
-
- // Return the output offset for an input address. The input address
- // is at offset OFFSET in section SHNDX in OBJECT. This sets
- // *OUTPUT_OFFSET to the offset in the output section; this will be
- // -1 if the bytes are not being copied to the output. This returns
- // true if the mapping is known, false otherwise. This returns the
- // value stored by add_mapping, namely the offset from the start of
- // the merged data within the output section.
- bool
- get_output_offset(const Relobj* object, unsigned int shndx,
- section_offset_type offset,
- section_offset_type* output_offset) const;
-
- // Return whether this is the merge mapping for section SHNDX in
- // OBJECT. This should return true when get_output_offset would
- // return true for some input offset.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const;
-};
-
// A general class for SHF_MERGE data, to hold functions shared by
// fixed-size constant data and string data.
@@ -216,7 +176,7 @@ class Output_merge_base : public Output_section_data
{
public:
Output_merge_base(uint64_t entsize, uint64_t addralign)
- : Output_section_data(addralign), merge_map_(), entsize_(entsize),
+ : Output_section_data(addralign), entsize_(entsize),
keeps_input_sections_(false), first_relobj_(NULL), first_shndx_(-1),
input_sections_()
{ }
@@ -285,21 +245,6 @@ class Output_merge_base : public Output_section_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for an input section.
- bool
- do_is_merge_section_for(const Relobj*, unsigned int shndx) const;
-
- // Add a mapping from an OFFSET in input section SHNDX in object
- // OBJECT to an OUTPUT_OFFSET in the output section. OUTPUT_OFFSET
- // is the offset from the start of the merged data in the output
- // section.
- void
- add_mapping(Relobj* object, unsigned int shndx, section_offset_type offset,
- section_size_type length, section_offset_type output_offset)
- {
- this->merge_map_.add_mapping(object, shndx, offset, length, output_offset);
- }
-
// This may be overridden by the child class.
virtual bool
do_is_string()
@@ -315,9 +260,6 @@ class Output_merge_base : public Output_section_data
record_input_section(Relobj* relobj, unsigned int shndx);
private:
- // A mapping from input object/section/offset to offset in output
- // section.
- Merge_map merge_map_;
// The entry size. For fixed-size constants, this is the size of
// the constants. For strings, this is the size of a character.
uint64_t entsize_;
diff --git a/gold/object.cc b/gold/object.cc
index c90b67e..cd1365e 100644
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -41,6 +41,7 @@
#include "plugin.h"
#include "compressed_output.h"
#include "incremental.h"
+#include "merge.h"
namespace gold
{
@@ -268,6 +269,41 @@ Object::handle_split_stack_section(const char* name)
// Class Relobj
+void
+Relobj::add_merge_mapping(Output_section_data *output_data,
+ unsigned int shndx, section_offset_type offset,
+ section_size_type length,
+ section_offset_type output_offset) {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ {
+ object_merge_map = new Object_merge_map();
+ this->set_merge_map(object_merge_map);
+ }
+
+ object_merge_map->add_mapping(output_data, shndx, offset, length,
+ output_offset);
+}
+
+bool
+Relobj::merge_output_offset(unsigned int shndx, section_offset_type offset,
+ section_offset_type *poutput) const {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ return false;
+ return object_merge_map->get_output_offset(shndx, offset, poutput);
+}
+
+bool
+Relobj::is_merge_section_for(const Output_section_data* output_data,
+ unsigned int shndx) const {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ return false;
+ return object_merge_map->is_merge_section_for(output_data, shndx);
+
+}
+
// To copy the symbols data read from the file to a local data structure.
// This function is called from do_layout only while doing garbage
// collection.
diff --git a/gold/object.h b/gold/object.h
index cce6c8c..400b41c 100644
--- a/gold/object.h
+++ b/gold/object.h
@@ -41,6 +41,7 @@ class Cref;
class Layout;
class Output_data;
class Output_section;
+class Output_section_data;
class Output_file;
class Output_symtab_xindex;
class Pluginobj;
@@ -1213,6 +1214,20 @@ class Relobj : public Object
this->object_merge_map_ = object_merge_map;
}
+ void
+ add_merge_mapping(Output_section_data *output_data,
+ unsigned int shndx, section_offset_type offset,
+ section_size_type length,
+ section_offset_type output_offset);
+
+ bool
+ merge_output_offset(unsigned int shndx, section_offset_type offset,
+ section_offset_type *poutput) const;
+
+ bool
+ is_merge_section_for(const Output_section_data* output_data,
+ unsigned int shndx) const;
+
// Record the relocatable reloc info for an input reloc section.
void
set_relocatable_relocs(unsigned int reloc_shndx, Relocatable_relocs* rr)
diff --git a/gold/output.cc b/gold/output.cc
index e9dd522..5d624ab 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -2210,7 +2210,7 @@ Output_section::Input_section::is_merge_section_for(const Relobj* object,
{
if (this->is_input_section())
return false;
- return this->u2_.posd->is_merge_section_for(object, shndx);
+ return object->is_merge_section_for(this->u2_.posd, shndx);
}
// Write out the data. We don't have to do anything for an input
diff --git a/gold/output.h b/gold/output.h
index 8e0c33a..dc11c8f 100644
--- a/gold/output.h
+++ b/gold/output.h
@@ -675,13 +675,6 @@ class Output_section_data : public Output_data
section_offset_type* poutput) const
{ return this->do_output_offset(object, shndx, offset, poutput); }
- // Return whether this is the merge section for the input section
- // SHNDX in OBJECT. This should return true when output_offset
- // would return true for some values of OFFSET.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const
- { return this->do_is_merge_section_for(object, shndx); }
-
// Write the contents to a buffer. This is used for sections which
// require postprocessing, such as compression.
void
@@ -715,11 +708,6 @@ class Output_section_data : public Output_data
section_offset_type*) const
{ return false; }
- // The child class may implement is_merge_section_for.
- virtual bool
- do_is_merge_section_for(const Relobj*, unsigned int) const
- { return false; }
-
// The child class may implement write_to_buffer. Most child
// classes can not appear in a compressed section, and they do not
// implement this.
diff --git a/gold/reloc.cc b/gold/reloc.cc
index 5906754..251a811 100644
--- a/gold/reloc.cc
+++ b/gold/reloc.cc
@@ -1474,7 +1474,7 @@ Merged_symbol_value<size>::value_from_output_section(
typename elfcpp::Elf_types<size>::Elf_Addr input_offset) const
{
section_offset_type output_offset;
- bool found = object->merge_map()->get_output_offset(NULL, input_shndx,
+ bool found = object->merge_map()->get_output_offset(input_shndx,
input_offset,
&output_offset);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch][gold] Remove empty Merge_map class
2015-02-17 15:56 ` Rafael Espíndola
@ 2015-02-22 3:12 ` Rafael Espíndola
2015-02-27 17:09 ` Rafael Espíndola
0 siblings, 1 reply; 9+ messages in thread
From: Rafael Espíndola @ 2015-02-22 3:12 UTC (permalink / raw)
To: Binutils; +Cc: Cary Coutant
ping. This is just deleting dead code.
On 17 February 2015 at 10:56, Rafael Espíndola
<rafael.espindola@gmail.com> wrote:
> Ping. A rebased patch is attached.
>
> On 6 February 2015 at 18:32, Rafael Espíndola
> <rafael.espindola@gmail.com> wrote:
>> When looking at the constant merging code to see if the
>> mapping could be reused for gc, the most confusing part for
>> me was the empty Merge_map class.
>>
>> It is only used as a key to implement is_merge_section_for (which is
>> only used by find_starting_output_address).
>>
>> Since it is just a key, we can use the Output_section_data as a key
>> instead. This patch does that. IMHO removing the indirection makes
>> things a bit easier to understand. It also removes the need for the
>> virtual do_is_merge_section_for.
>>
>> Last but not least, it makes some inefficiencies easier to spot. For
>> example, Output_merge_data::do_add_input_section handles only one
>> input section and therefore only one merge map, but it does
>>
>> object->add_merge_mapping(this, shndx, i, entsize, k);
>>
>> in a loop. It could extract Input_merge_map* before the loop.
>>
>> Cheers,
>> Rafael
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch][gold] Remove empty Merge_map class
2015-02-22 3:12 ` Rafael Espíndola
@ 2015-02-27 17:09 ` Rafael Espíndola
2015-03-02 19:08 ` Cary Coutant
0 siblings, 1 reply; 9+ messages in thread
From: Rafael Espíndola @ 2015-02-27 17:09 UTC (permalink / raw)
To: Binutils; +Cc: Cary Coutant
[-- Attachment #1: Type: text/plain, Size: 907 bytes --]
Ping. Lets focus on just this patch first. It is just deleting dead
code. To see it, noticed that:
* Merge_map is empty, so its methods (add_mapping, get_output_offset,
is_merge_section_for) don't use "this" and are really just free
standing functions. In this patch they are converted 1:1 into Relobj
methods since they all currently take a Relobj* as the first argument.
* The only use of a pointer to a Merge_map is in the comparison
"map->merge_map == merge_map" in
Object_merge_map::is_merge_section_for, which indicates that Merge_map
is now really just an ID.
* Merge_map is never allocated on its own, just as a member variable
of Eh_frame and Output_merge_base. Given that, we can just use a
pointer to Eh_frame/Output_merge_base and save the member. The common
base class of Eh_frame and Output_merge_base is Output_section_data,
so this patch s/Merge_map/Output_section_data/.
Cheers,
Rafael
[-- Attachment #2: t.patch --]
[-- Type: text/x-patch, Size: 24206 bytes --]
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index faea1a8..711103b 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -412,7 +412,7 @@ Cie::~Cie()
section_offset_type
Cie::set_output_offset(section_offset_type output_offset,
unsigned int addralign,
- Merge_map* merge_map)
+ Output_section_data *output_data)
{
size_t length = this->contents_.length();
@@ -422,8 +422,9 @@ Cie::set_output_offset(section_offset_type output_offset,
if (this->object_ != NULL)
{
// Add a mapping so that relocations are applied correctly.
- merge_map->add_mapping(this->object_, this->shndx_, this->input_offset_,
- length, output_offset);
+ this->object_->add_merge_mapping(output_data, this->shndx_,
+ this->input_offset_, length,
+ output_offset);
}
length = align_address(length, addralign);
@@ -432,7 +433,7 @@ Cie::set_output_offset(section_offset_type output_offset,
p != this->fdes_.end();
++p)
{
- (*p)->add_mapping(output_offset + length, merge_map);
+ (*p)->add_mapping(output_offset + length, output_data);
size_t fde_length = (*p)->length();
fde_length = align_address(fde_length, addralign);
@@ -531,7 +532,6 @@ Eh_frame::Eh_frame()
eh_frame_hdr_(NULL),
cie_offsets_(),
unmergeable_cie_offsets_(),
- merge_map_(),
mappings_are_done_(false),
final_data_size_(0)
{
@@ -958,8 +958,8 @@ Eh_frame::read_cie(Sized_relobj_file<size, big_endian>* object,
// know for sure that we are doing a special mapping for this
// input section, but that's OK--if we don't do a special
// mapping, nobody will ever ask for the mapping we add here.
- this->merge_map_.add_mapping(object, shndx, (pcie - 8) - pcontents,
- pcieend - (pcie - 8), -1);
+ object->add_merge_mapping(this, shndx, (pcie - 8) - pcontents,
+ pcieend - (pcie - 8), -1);
}
// Record this CIE plus the offset in the input section.
@@ -1026,8 +1026,8 @@ Eh_frame::read_fde(Sized_relobj_file<size, big_endian>* object,
{
// This FDE applies to a section which we are discarding. We
// can discard this FDE.
- this->merge_map_.add_mapping(object, shndx, (pfde - 8) - pcontents,
- pfdeend - (pfde - 8), -1);
+ object->add_merge_mapping(this, shndx, (pfde - 8) - pcontents,
+ pfdeend - (pfde - 8), -1);
return true;
}
@@ -1107,14 +1107,14 @@ Eh_frame::set_final_data_size()
++p)
output_offset = (*p)->set_output_offset(output_offset,
this->addralign(),
- &this->merge_map_);
+ this);
for (Cie_offsets::iterator p = this->cie_offsets_.begin();
p != this->cie_offsets_.end();
++p)
output_offset = (*p)->set_output_offset(output_offset,
this->addralign(),
- &this->merge_map_);
+ this);
this->mappings_are_done_ = true;
this->final_data_size_ = output_offset - output_start;
@@ -1130,16 +1130,7 @@ Eh_frame::do_output_offset(const Relobj* object, unsigned int shndx,
section_offset_type offset,
section_offset_type* poutput) const
{
- return this->merge_map_.get_output_offset(object, shndx, offset, poutput);
-}
-
-// Return whether this is the merge section for an input section.
-
-bool
-Eh_frame::do_is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- return this->merge_map_.is_merge_section_for(object, shndx);
+ return object->merge_output_offset(shndx, offset, poutput);
}
// Write the data to the output file.
diff --git a/gold/ehframe.h b/gold/ehframe.h
index aa2bd31..d7152ad 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -192,12 +192,13 @@ class Fde
// Add a mapping for this FDE to MERGE_MAP, so that relocations
// against the FDE are applied to right part of the output file.
void
- add_mapping(section_offset_type output_offset, Merge_map* merge_map) const
+ add_mapping(section_offset_type output_offset,
+ Output_section_data* output_data) const
{
if (this->object_ != NULL)
- merge_map->add_mapping(this->object_, this->u_.from_object.shndx,
- this->u_.from_object.input_offset, this->length(),
- output_offset);
+ this->object_->add_merge_mapping(output_data, this->u_.from_object.shndx,
+ this->u_.from_object.input_offset, this->length(),
+ output_offset);
}
// Return whether this FDE was added after merge mapping.
@@ -308,7 +309,7 @@ class Cie
// mapping. It returns the new output offset.
section_offset_type
set_output_offset(section_offset_type output_offset, unsigned int addralign,
- Merge_map*);
+ Output_section_data*);
// Write the CIE to OVIEW starting at OFFSET. Round up the bytes to
// ADDRALIGN. ADDRESS is the virtual address of OVIEW.
@@ -406,10 +407,6 @@ class Eh_frame : public Output_section_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for an input section.
- bool
- do_is_merge_section_for(const Relobj*, unsigned int shndx) const;
-
// Write the data to the file.
void
do_write(Output_file*);
@@ -504,8 +501,6 @@ class Eh_frame : public Output_section_data
// A mapping from unmergeable CIEs to their offset in the output
// file.
Unmergeable_cie_offsets unmergeable_cie_offsets_;
- // A mapping from input sections to the output section.
- Merge_map merge_map_;
// Whether we have created the mappings to the output section.
bool mappings_are_done_;
// The final data size. This is only set if mappings_are_done_ is
diff --git a/gold/merge.cc b/gold/merge.cc
index bf00481..53c4883 100644
--- a/gold/merge.cc
+++ b/gold/merge.cc
@@ -62,15 +62,14 @@ Object_merge_map::get_input_merge_map(unsigned int shndx)
// Get or create the Input_merge_map to use for an input section.
Object_merge_map::Input_merge_map*
-Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
- unsigned int shndx)
-{
+Object_merge_map::get_or_make_input_merge_map(
+ const Output_section_data* output_data, unsigned int shndx) {
Input_merge_map* map = this->get_input_merge_map(shndx);
if (map != NULL)
{
// For a given input section in a given object, every mapping
// must be done with the same Merge_map.
- gold_assert(map->merge_map == merge_map);
+ gold_assert(map->output_data == output_data);
return map;
}
@@ -78,18 +77,18 @@ Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
if (this->first_shnum_ == -1U)
{
this->first_shnum_ = shndx;
- this->first_map_.merge_map = merge_map;
+ this->first_map_.output_data = output_data;
return &this->first_map_;
}
if (this->second_shnum_ == -1U)
{
this->second_shnum_ = shndx;
- this->second_map_.merge_map = merge_map;
+ this->second_map_.output_data = output_data;
return &this->second_map_;
}
Input_merge_map* new_map = new Input_merge_map;
- new_map->merge_map = merge_map;
+ new_map->output_data = output_data;
this->section_merge_maps_[shndx] = new_map;
return new_map;
}
@@ -97,12 +96,13 @@ Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
// Add a mapping.
void
-Object_merge_map::add_mapping(const Merge_map* merge_map, unsigned int shndx,
+Object_merge_map::add_mapping(const Output_section_data* output_data,
+ unsigned int shndx,
section_offset_type input_offset,
section_size_type length,
section_offset_type output_offset)
{
- Input_merge_map* map = this->get_or_make_input_merge_map(merge_map, shndx);
+ Input_merge_map* map = this->get_or_make_input_merge_map(output_data, shndx);
// Try to merge the new entry in the last one we saw.
if (!map->entries.empty())
@@ -142,14 +142,12 @@ Object_merge_map::add_mapping(const Merge_map* merge_map, unsigned int shndx,
// Get the output offset for an input address.
bool
-Object_merge_map::get_output_offset(const Merge_map* merge_map,
- unsigned int shndx,
+Object_merge_map::get_output_offset(unsigned int shndx,
section_offset_type input_offset,
section_offset_type* output_offset)
{
Input_merge_map* map = this->get_input_merge_map(shndx);
- if (map == NULL
- || (merge_map != NULL && map->merge_map != merge_map))
+ if (map == NULL)
return false;
if (!map->sorted)
@@ -181,12 +179,12 @@ Object_merge_map::get_output_offset(const Merge_map* merge_map,
// Return whether this is the merge map for section SHNDX.
-inline bool
-Object_merge_map::is_merge_section_for(const Merge_map* merge_map,
+bool
+Object_merge_map::is_merge_section_for(const Output_section_data* output_data,
unsigned int shndx)
{
Input_merge_map* map = this->get_input_merge_map(shndx);
- return map != NULL && map->merge_map == merge_map;
+ return map != NULL && map->output_data == output_data;
}
// Initialize a mapping from input offsets to output addresses.
@@ -228,62 +226,11 @@ Object_merge_map::initialize_input_to_output_map(
}
}
-// Class Merge_map.
-
-// Add a mapping for the bytes from OFFSET to OFFSET + LENGTH in input
-// section SHNDX in object OBJECT to an OUTPUT_OFFSET in merged data
-// in an output section.
-
-void
-Merge_map::add_mapping(Relobj* object, unsigned int shndx,
- section_offset_type offset, section_size_type length,
- section_offset_type output_offset)
-{
- gold_assert(object != NULL);
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- {
- object_merge_map = new Object_merge_map();
- object->set_merge_map(object_merge_map);
- }
-
- object_merge_map->add_mapping(this, shndx, offset, length, output_offset);
-}
-
-// Return the output offset for an input address. The input address
-// is at offset OFFSET in section SHNDX in OBJECT. This sets
-// *OUTPUT_OFFSET to the offset in the merged data in the output
-// section. This returns true if the mapping is known, false
-// otherwise.
-
-bool
-Merge_map::get_output_offset(const Relobj* object, unsigned int shndx,
- section_offset_type offset,
- section_offset_type* output_offset) const
-{
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- return false;
- return object_merge_map->get_output_offset(this, shndx, offset,
- output_offset);
-}
-
-// Return whether this is the merge section for SHNDX in OBJECT.
-
-bool
-Merge_map::is_merge_section_for(const Relobj* object, unsigned int shndx) const
-{
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- return false;
- return object_merge_map->is_merge_section_for(this, shndx);
-}
-
// Class Output_merge_base.
// Return the output offset for an input offset. The input address is
// at offset OFFSET in section SHNDX in OBJECT. If we know the
-// offset, set *POUTPUT and return true. Otherwise return false.
+// offset, set *POUTPUT and return true. Otherwise return false
bool
Output_merge_base::do_output_offset(const Relobj* object,
@@ -291,16 +238,7 @@ Output_merge_base::do_output_offset(const Relobj* object,
section_offset_type offset,
section_offset_type* poutput) const
{
- return this->merge_map_.get_output_offset(object, shndx, offset, poutput);
-}
-
-// Return whether this is the merge section for SHNDX in OBJECT.
-
-bool
-Output_merge_base::do_is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- return this->merge_map_.is_merge_section_for(object, shndx);
+ return object->merge_output_offset(shndx, offset, poutput);
}
// Record a merged input section for script processing.
@@ -436,7 +374,7 @@ Output_merge_data::do_add_input_section(Relobj* object, unsigned int shndx)
}
// Record the offset of this constant in the output section.
- this->add_mapping(object, shndx, i, entsize, k);
+ object->add_merge_mapping(this, shndx, i, entsize, k);
}
// For script processing, we keep the input sections.
@@ -625,8 +563,8 @@ Output_merge_string<Char_type>::finalize_merged_data()
{
section_size_type length = p->offset - last_input_offset;
if (length > 0)
- this->add_mapping((*l)->object, (*l)->shndx, last_input_offset,
- length, last_output_offset);
+ (*l)->object->add_merge_mapping(this, (*l)->shndx,
+ last_input_offset, length, last_output_offset);
last_input_offset = p->offset;
if (p->stringpool_key != 0)
last_output_offset =
diff --git a/gold/merge.h b/gold/merge.h
index 6318f45..3e500bc 100644
--- a/gold/merge.h
+++ b/gold/merge.h
@@ -33,8 +33,6 @@
namespace gold
{
-class Merge_map;
-
// For each object with merge sections, we store an Object_merge_map.
// This is used to map locations in input sections to a merged output
// section. The output section itself is not recorded here--it can be
@@ -57,8 +55,9 @@ class Object_merge_map
// discarded. OUTPUT_OFFSET is relative to the start of the merged
// data in the output section.
void
- add_mapping(const Merge_map*, unsigned int shndx, section_offset_type offset,
- section_size_type length, section_offset_type output_offset);
+ add_mapping(const Output_section_data*, unsigned int shndx,
+ section_offset_type offset, section_size_type length,
+ section_offset_type output_offset);
// Get the output offset for an input address. MERGE_MAP is the map
// we are looking for, or NULL if we don't care. The input address
@@ -68,13 +67,13 @@ class Object_merge_map
// mapping is known, false otherwise. *OUTPUT_OFFSET is relative to
// the start of the merged data in the output section.
bool
- get_output_offset(const Merge_map*, unsigned int shndx,
+ get_output_offset(unsigned int shndx,
section_offset_type offset,
section_offset_type* output_offset);
// Return whether this is the merge map for section SHNDX.
bool
- is_merge_section_for(const Merge_map*, unsigned int shndx);
+ is_merge_section_for(const Output_section_data*, unsigned int shndx);
// Initialize an mapping from input offsets to output addresses for
// section SHNDX. STARTING_ADDRESS is the output address of the
@@ -133,14 +132,14 @@ class Object_merge_map
// look up information for a different Merge_map, we report that
// we don't have it, rather than trying a lookup and returning an
// answer which will receive the wrong offset.
- const Merge_map* merge_map;
+ const Output_section_data* output_data;
// The list of mappings.
Entries entries;
// Whether the ENTRIES field is sorted by input_offset.
bool sorted;
Input_merge_map()
- : merge_map(NULL), entries(), sorted(true)
+ : output_data(NULL), entries(), sorted(true)
{ }
};
@@ -155,7 +154,8 @@ class Object_merge_map
// Get or make the Input_merge_map to use for the section SHNDX
// with MERGE_MAP.
Input_merge_map*
- get_or_make_input_merge_map(const Merge_map* merge_map, unsigned int shndx);
+ get_or_make_input_merge_map(const Output_section_data* merge_map,
+ unsigned int shndx);
// Any given object file will normally only have a couple of input
// sections with mergeable contents. So we keep the first two input
@@ -169,46 +169,6 @@ class Object_merge_map
Section_merge_maps section_merge_maps_;
};
-// This class manages mappings from input sections to offsets in an
-// output section. This is used where input sections are merged. The
-// actual data is stored in fields in Object.
-
-class Merge_map
-{
- public:
- Merge_map()
- { }
-
- // Add a mapping for the bytes from OFFSET to OFFSET + LENGTH in the
- // input section SHNDX in object OBJECT to OUTPUT_OFFSET in the
- // output section. An OUTPUT_OFFSET of -1 means that the bytes are
- // discarded. OUTPUT_OFFSET is not the offset from the start of the
- // output section, it is the offset from the start of the merged
- // data within the output section.
- void
- add_mapping(Relobj* object, unsigned int shndx,
- section_offset_type offset, section_size_type length,
- section_offset_type output_offset);
-
- // Return the output offset for an input address. The input address
- // is at offset OFFSET in section SHNDX in OBJECT. This sets
- // *OUTPUT_OFFSET to the offset in the output section; this will be
- // -1 if the bytes are not being copied to the output. This returns
- // true if the mapping is known, false otherwise. This returns the
- // value stored by add_mapping, namely the offset from the start of
- // the merged data within the output section.
- bool
- get_output_offset(const Relobj* object, unsigned int shndx,
- section_offset_type offset,
- section_offset_type* output_offset) const;
-
- // Return whether this is the merge mapping for section SHNDX in
- // OBJECT. This should return true when get_output_offset would
- // return true for some input offset.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const;
-};
-
// A general class for SHF_MERGE data, to hold functions shared by
// fixed-size constant data and string data.
@@ -216,7 +176,7 @@ class Output_merge_base : public Output_section_data
{
public:
Output_merge_base(uint64_t entsize, uint64_t addralign)
- : Output_section_data(addralign), merge_map_(), entsize_(entsize),
+ : Output_section_data(addralign), entsize_(entsize),
keeps_input_sections_(false), first_relobj_(NULL), first_shndx_(-1),
input_sections_()
{ }
@@ -285,21 +245,6 @@ class Output_merge_base : public Output_section_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for an input section.
- bool
- do_is_merge_section_for(const Relobj*, unsigned int shndx) const;
-
- // Add a mapping from an OFFSET in input section SHNDX in object
- // OBJECT to an OUTPUT_OFFSET in the output section. OUTPUT_OFFSET
- // is the offset from the start of the merged data in the output
- // section.
- void
- add_mapping(Relobj* object, unsigned int shndx, section_offset_type offset,
- section_size_type length, section_offset_type output_offset)
- {
- this->merge_map_.add_mapping(object, shndx, offset, length, output_offset);
- }
-
// This may be overridden by the child class.
virtual bool
do_is_string()
@@ -315,9 +260,6 @@ class Output_merge_base : public Output_section_data
record_input_section(Relobj* relobj, unsigned int shndx);
private:
- // A mapping from input object/section/offset to offset in output
- // section.
- Merge_map merge_map_;
// The entry size. For fixed-size constants, this is the size of
// the constants. For strings, this is the size of a character.
uint64_t entsize_;
diff --git a/gold/object.cc b/gold/object.cc
index c90b67e..cd1365e 100644
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -41,6 +41,7 @@
#include "plugin.h"
#include "compressed_output.h"
#include "incremental.h"
+#include "merge.h"
namespace gold
{
@@ -268,6 +269,41 @@ Object::handle_split_stack_section(const char* name)
// Class Relobj
+void
+Relobj::add_merge_mapping(Output_section_data *output_data,
+ unsigned int shndx, section_offset_type offset,
+ section_size_type length,
+ section_offset_type output_offset) {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ {
+ object_merge_map = new Object_merge_map();
+ this->set_merge_map(object_merge_map);
+ }
+
+ object_merge_map->add_mapping(output_data, shndx, offset, length,
+ output_offset);
+}
+
+bool
+Relobj::merge_output_offset(unsigned int shndx, section_offset_type offset,
+ section_offset_type *poutput) const {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ return false;
+ return object_merge_map->get_output_offset(shndx, offset, poutput);
+}
+
+bool
+Relobj::is_merge_section_for(const Output_section_data* output_data,
+ unsigned int shndx) const {
+ Object_merge_map* object_merge_map = this->merge_map();
+ if (object_merge_map == NULL)
+ return false;
+ return object_merge_map->is_merge_section_for(output_data, shndx);
+
+}
+
// To copy the symbols data read from the file to a local data structure.
// This function is called from do_layout only while doing garbage
// collection.
diff --git a/gold/object.h b/gold/object.h
index cce6c8c..400b41c 100644
--- a/gold/object.h
+++ b/gold/object.h
@@ -41,6 +41,7 @@ class Cref;
class Layout;
class Output_data;
class Output_section;
+class Output_section_data;
class Output_file;
class Output_symtab_xindex;
class Pluginobj;
@@ -1213,6 +1214,20 @@ class Relobj : public Object
this->object_merge_map_ = object_merge_map;
}
+ void
+ add_merge_mapping(Output_section_data *output_data,
+ unsigned int shndx, section_offset_type offset,
+ section_size_type length,
+ section_offset_type output_offset);
+
+ bool
+ merge_output_offset(unsigned int shndx, section_offset_type offset,
+ section_offset_type *poutput) const;
+
+ bool
+ is_merge_section_for(const Output_section_data* output_data,
+ unsigned int shndx) const;
+
// Record the relocatable reloc info for an input reloc section.
void
set_relocatable_relocs(unsigned int reloc_shndx, Relocatable_relocs* rr)
diff --git a/gold/output.cc b/gold/output.cc
index e9dd522..5d624ab 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -2210,7 +2210,7 @@ Output_section::Input_section::is_merge_section_for(const Relobj* object,
{
if (this->is_input_section())
return false;
- return this->u2_.posd->is_merge_section_for(object, shndx);
+ return object->is_merge_section_for(this->u2_.posd, shndx);
}
// Write out the data. We don't have to do anything for an input
diff --git a/gold/output.h b/gold/output.h
index 8e0c33a..dc11c8f 100644
--- a/gold/output.h
+++ b/gold/output.h
@@ -675,13 +675,6 @@ class Output_section_data : public Output_data
section_offset_type* poutput) const
{ return this->do_output_offset(object, shndx, offset, poutput); }
- // Return whether this is the merge section for the input section
- // SHNDX in OBJECT. This should return true when output_offset
- // would return true for some values of OFFSET.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const
- { return this->do_is_merge_section_for(object, shndx); }
-
// Write the contents to a buffer. This is used for sections which
// require postprocessing, such as compression.
void
@@ -715,11 +708,6 @@ class Output_section_data : public Output_data
section_offset_type*) const
{ return false; }
- // The child class may implement is_merge_section_for.
- virtual bool
- do_is_merge_section_for(const Relobj*, unsigned int) const
- { return false; }
-
// The child class may implement write_to_buffer. Most child
// classes can not appear in a compressed section, and they do not
// implement this.
diff --git a/gold/reloc.cc b/gold/reloc.cc
index 5906754..251a811 100644
--- a/gold/reloc.cc
+++ b/gold/reloc.cc
@@ -1474,7 +1474,7 @@ Merged_symbol_value<size>::value_from_output_section(
typename elfcpp::Elf_types<size>::Elf_Addr input_offset) const
{
section_offset_type output_offset;
- bool found = object->merge_map()->get_output_offset(NULL, input_shndx,
+ bool found = object->merge_map()->get_output_offset(input_shndx,
input_offset,
&output_offset);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch][gold] Remove empty Merge_map class
2015-02-27 17:09 ` Rafael Espíndola
@ 2015-03-02 19:08 ` Cary Coutant
2015-03-03 8:28 ` Rafael Espíndola
0 siblings, 1 reply; 9+ messages in thread
From: Cary Coutant @ 2015-03-02 19:08 UTC (permalink / raw)
To: Rafael Espíndola; +Cc: Binutils
> Ping. Lets focus on just this patch first. It is just deleting dead
> code. To see it, noticed that:
>
> * Merge_map is empty, so its methods (add_mapping, get_output_offset,
> is_merge_section_for) don't use "this" and are really just free
> standing functions. In this patch they are converted 1:1 into Relobj
> methods since they all currently take a Relobj* as the first argument.
>
> * The only use of a pointer to a Merge_map is in the comparison
> "map->merge_map == merge_map" in
> Object_merge_map::is_merge_section_for, which indicates that Merge_map
> is now really just an ID.
>
> * Merge_map is never allocated on its own, just as a member variable
> of Eh_frame and Output_merge_base. Given that, we can just use a
> pointer to Eh_frame/Output_merge_base and save the member. The common
> base class of Eh_frame and Output_merge_base is Output_section_data,
> so this patch s/Merge_map/Output_section_data/.
Thanks. This looks good, but let's take it one step further. The only
exposure that Relobj::merge_map_ has outside the class is in reloc.cc:
template<int size>
void
Merged_symbol_value<size>::initialize_input_to_output_map(
const Relobj* object,
unsigned int input_shndx)
{
Object_merge_map* map = object->merge_map();
map->initialize_input_to_output_map<size>(input_shndx,
this->output_start_address_,
&this->output_addresses_);
}
// Get the output value corresponding to an input offset if we
// couldn't find it in the hash table.
template<int size>
typename elfcpp::Elf_types<size>::Elf_Addr
Merged_symbol_value<size>::value_from_output_section(
const Relobj* object,
unsigned int input_shndx,
typename elfcpp::Elf_types<size>::Elf_Addr input_offset) const
{
section_offset_type output_offset;
bool found = object->merge_map()->get_output_offset(input_shndx,
input_offset,
&output_offset);
...
In the first case, let's just add an initialize_input_to_output_map()
method to Relobj, and call it instead of first getting the merge_map.
In the second case, you can now call object->merge_output_offset().
Now you can remove Relobj::merge_map(), and you can also remove
Relobj::set_merge_map(), instead just setting merge_map_ directly in
Relobj::add_merge_mapping().
Also, please write a ChangeLog entry.
(Sorry I've been so slow.)
-cary
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch][gold] Remove empty Merge_map class
2015-03-02 19:08 ` Cary Coutant
@ 2015-03-03 8:28 ` Rafael Espíndola
2015-03-04 23:22 ` Cary Coutant
0 siblings, 1 reply; 9+ messages in thread
From: Rafael Espíndola @ 2015-03-03 8:28 UTC (permalink / raw)
To: Cary Coutant; +Cc: Binutils
[-- Attachment #1: Type: text/plain, Size: 1846 bytes --]
> Thanks. This looks good, but let's take it one step further. The only
> exposure that Relobj::merge_map_ has outside the class is in reloc.cc:
>
> template<int size>
> void
> Merged_symbol_value<size>::initialize_input_to_output_map(
> const Relobj* object,
> unsigned int input_shndx)
> {
> Object_merge_map* map = object->merge_map();
> map->initialize_input_to_output_map<size>(input_shndx,
> this->output_start_address_,
> &this->output_addresses_);
> }
>
> // Get the output value corresponding to an input offset if we
> // couldn't find it in the hash table.
>
> template<int size>
> typename elfcpp::Elf_types<size>::Elf_Addr
> Merged_symbol_value<size>::value_from_output_section(
> const Relobj* object,
> unsigned int input_shndx,
> typename elfcpp::Elf_types<size>::Elf_Addr input_offset) const
> {
> section_offset_type output_offset;
> bool found = object->merge_map()->get_output_offset(input_shndx,
> input_offset,
> &output_offset);
> ...
>
> In the first case, let's just add an initialize_input_to_output_map()
> method to Relobj, and call it instead of first getting the merge_map.
> In the second case, you can now call object->merge_output_offset().
>
> Now you can remove Relobj::merge_map(), and you can also remove
> Relobj::set_merge_map(), instead just setting merge_map_ directly in
> Relobj::add_merge_mapping().
>
> Also, please write a ChangeLog entry.
Hi,
A new patch that includes the above improvements and a ChangeLog entry
is attached. Would you mind pushing it for me? I am at a business trip
this week and just noticed I never copied my ssh key to my laptop :-(
Cheers,
Rafael
[-- Attachment #2: t.patch --]
[-- Type: application/octet-stream, Size: 29647 bytes --]
diff --git a/gold/ChangeLog b/gold/ChangeLog
index d7dd903..1f54323 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,70 @@
+2015-03-02 Rafael Ávila de Espíndola <rafael.espindola@gmail.com>
+
+ * ehframe.cc (Cie::set_output_offset): Pass in and use a
+ Output_section_data instead of a Merge_map.
+ (Eh_frame::Eh_frame): Don't initialize merge_map_.
+ (Eh_frame::read_cie): Use add_merge_mapping instead of
+ Merge_map::add_mapping.
+ (Eh_frame::read_fde): Ditto.
+ (Eh_frame::set_final_data_size): Use this instead of this->merge_map_.
+ (Eh_frame::do_output_offset): Use merge_output_offset istead of
+ merge_map_->get_output_offset.
+ (Eh_frame::do_is_merge_section_for): Delete.
+ * ehframe.h (Fde::add_mapping): Pass in and use a Output_section_data
+ instead of a Merge_map.
+ (Cie::set_output_offset): Pass in a Output_section_data instead of a
+ Merge_map.
+ (Eh_frame::do_is_merge_section_for): Delete.
+ (Eh_frame::merge_map_): Delete.
+ * merge.cc (Object_merge_map::get_or_make_input_merge_map): Pass in
+ and use a Output_section_data instead of a Merge_map.
+ (Object_merge_map::add_mapping): Ditto.
+ (Object_merge_map::get_output_offset): Remove the merge_map argument.
+ (Object_merge_map::is_merge_section_for): Pass in and use a
+ Output_section_data instead of a Merge_map.
+ (Merge_map): Delete.
+ (Output_merge_base::do_output_offset): Use merge_output_offset instead
+ of merge_map_.get_output_offset.
+ (Output_merge_base::do_is_merge_section_for): Delete.
+ (Output_merge_data::do_add_input_section): Use
+ object->add_merge_mapping instead of add_mapping.
+ (Output_merge_string<Char_type>::finalize_merged_data): Ditto.
+ * merge.h (Merge_map): Delete forward declaration.
+ (Object_merge_map::add_mapping): Pass in and use a Output_section_data
+ instead of a Merge_map.
+ (Object_merge_map::get_output_offset): Remove the merge_map argument.
+ (Object_merge_map::is_merge_section_for): Pass in and use a
+ Output_section_data instead of a Merge_map.
+ (Input_merge_map::Object_merge_map::merge_map): Replace with
+ output_data.
+ (Object_merge_map::get_or_make_input_merge_map): Pass in and use a
+ Output_section_data instead of a Merge_map.
+ (Merge_map): Delete.
+ (Output_merge_base::Output_merge_base): Don't initialize merge_map_.
+ (Output_merge_base::do_is_merge_section_for): Delete.
+ (Output_merge_base::add_mapping): Delete.
+ (Output_merge_base::merge_map_): Delete.
+ * object.cc (Relobj::initialize_input_to_output_map): New.
+ (Relobj::initialize_input_to_output_map): New.
+ (Relobj::merge_output_offset): New.
+ (Relobj::is_merge_section_for): New.
+ (Relobj::initialize_input_to_output_map): Instantiate for 32 and 64
+ bits.
+ * object.h (Relobj::merge_map): Delete.
+ (initialize_input_to_output_map): New.
+ (set_merge_map): Delete.
+ (add_merge_mapping): New.
+ (merge_output_offset): New.
+ (is_merge_section_for): New.
+ * output.cc (Output_section::Input_section::is_merge_section_for):
+ Use object->is_merge_section_for.
+ * output.h (Output_section_data::is_merge_section_for): Delete.
+ (Output_section_data::do_is_merge_section_for): Delete.
+ * reloc.cc (Merged_symbol_value<size>::initialize_input_to_output_map):
+ Use object->initialize_input_to_output_map.
+ (Merged_symbol_value<size>::value_from_output_section): Use
+ object->merge_output_offset.
+
2015-02-04 Peter Collingbourne <pcc@google.com>
Cary Coutant <ccoutant@google.com>
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index faea1a8..711103b 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -412,7 +412,7 @@ Cie::~Cie()
section_offset_type
Cie::set_output_offset(section_offset_type output_offset,
unsigned int addralign,
- Merge_map* merge_map)
+ Output_section_data *output_data)
{
size_t length = this->contents_.length();
@@ -422,8 +422,9 @@ Cie::set_output_offset(section_offset_type output_offset,
if (this->object_ != NULL)
{
// Add a mapping so that relocations are applied correctly.
- merge_map->add_mapping(this->object_, this->shndx_, this->input_offset_,
- length, output_offset);
+ this->object_->add_merge_mapping(output_data, this->shndx_,
+ this->input_offset_, length,
+ output_offset);
}
length = align_address(length, addralign);
@@ -432,7 +433,7 @@ Cie::set_output_offset(section_offset_type output_offset,
p != this->fdes_.end();
++p)
{
- (*p)->add_mapping(output_offset + length, merge_map);
+ (*p)->add_mapping(output_offset + length, output_data);
size_t fde_length = (*p)->length();
fde_length = align_address(fde_length, addralign);
@@ -531,7 +532,6 @@ Eh_frame::Eh_frame()
eh_frame_hdr_(NULL),
cie_offsets_(),
unmergeable_cie_offsets_(),
- merge_map_(),
mappings_are_done_(false),
final_data_size_(0)
{
@@ -958,8 +958,8 @@ Eh_frame::read_cie(Sized_relobj_file<size, big_endian>* object,
// know for sure that we are doing a special mapping for this
// input section, but that's OK--if we don't do a special
// mapping, nobody will ever ask for the mapping we add here.
- this->merge_map_.add_mapping(object, shndx, (pcie - 8) - pcontents,
- pcieend - (pcie - 8), -1);
+ object->add_merge_mapping(this, shndx, (pcie - 8) - pcontents,
+ pcieend - (pcie - 8), -1);
}
// Record this CIE plus the offset in the input section.
@@ -1026,8 +1026,8 @@ Eh_frame::read_fde(Sized_relobj_file<size, big_endian>* object,
{
// This FDE applies to a section which we are discarding. We
// can discard this FDE.
- this->merge_map_.add_mapping(object, shndx, (pfde - 8) - pcontents,
- pfdeend - (pfde - 8), -1);
+ object->add_merge_mapping(this, shndx, (pfde - 8) - pcontents,
+ pfdeend - (pfde - 8), -1);
return true;
}
@@ -1107,14 +1107,14 @@ Eh_frame::set_final_data_size()
++p)
output_offset = (*p)->set_output_offset(output_offset,
this->addralign(),
- &this->merge_map_);
+ this);
for (Cie_offsets::iterator p = this->cie_offsets_.begin();
p != this->cie_offsets_.end();
++p)
output_offset = (*p)->set_output_offset(output_offset,
this->addralign(),
- &this->merge_map_);
+ this);
this->mappings_are_done_ = true;
this->final_data_size_ = output_offset - output_start;
@@ -1130,16 +1130,7 @@ Eh_frame::do_output_offset(const Relobj* object, unsigned int shndx,
section_offset_type offset,
section_offset_type* poutput) const
{
- return this->merge_map_.get_output_offset(object, shndx, offset, poutput);
-}
-
-// Return whether this is the merge section for an input section.
-
-bool
-Eh_frame::do_is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- return this->merge_map_.is_merge_section_for(object, shndx);
+ return object->merge_output_offset(shndx, offset, poutput);
}
// Write the data to the output file.
diff --git a/gold/ehframe.h b/gold/ehframe.h
index aa2bd31..bb47381 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -192,10 +192,11 @@ class Fde
// Add a mapping for this FDE to MERGE_MAP, so that relocations
// against the FDE are applied to right part of the output file.
void
- add_mapping(section_offset_type output_offset, Merge_map* merge_map) const
+ add_mapping(section_offset_type output_offset,
+ Output_section_data* output_data) const
{
if (this->object_ != NULL)
- merge_map->add_mapping(this->object_, this->u_.from_object.shndx,
+ this->object_->add_merge_mapping(output_data, this->u_.from_object.shndx,
this->u_.from_object.input_offset, this->length(),
output_offset);
}
@@ -308,7 +309,7 @@ class Cie
// mapping. It returns the new output offset.
section_offset_type
set_output_offset(section_offset_type output_offset, unsigned int addralign,
- Merge_map*);
+ Output_section_data*);
// Write the CIE to OVIEW starting at OFFSET. Round up the bytes to
// ADDRALIGN. ADDRESS is the virtual address of OVIEW.
@@ -406,10 +407,6 @@ class Eh_frame : public Output_section_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for an input section.
- bool
- do_is_merge_section_for(const Relobj*, unsigned int shndx) const;
-
// Write the data to the file.
void
do_write(Output_file*);
@@ -504,8 +501,6 @@ class Eh_frame : public Output_section_data
// A mapping from unmergeable CIEs to their offset in the output
// file.
Unmergeable_cie_offsets unmergeable_cie_offsets_;
- // A mapping from input sections to the output section.
- Merge_map merge_map_;
// Whether we have created the mappings to the output section.
bool mappings_are_done_;
// The final data size. This is only set if mappings_are_done_ is
diff --git a/gold/merge.cc b/gold/merge.cc
index bf00481..5d93c74 100644
--- a/gold/merge.cc
+++ b/gold/merge.cc
@@ -62,15 +62,14 @@ Object_merge_map::get_input_merge_map(unsigned int shndx)
// Get or create the Input_merge_map to use for an input section.
Object_merge_map::Input_merge_map*
-Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
- unsigned int shndx)
-{
+Object_merge_map::get_or_make_input_merge_map(
+ const Output_section_data* output_data, unsigned int shndx) {
Input_merge_map* map = this->get_input_merge_map(shndx);
if (map != NULL)
{
// For a given input section in a given object, every mapping
// must be done with the same Merge_map.
- gold_assert(map->merge_map == merge_map);
+ gold_assert(map->output_data == output_data);
return map;
}
@@ -78,18 +77,18 @@ Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
if (this->first_shnum_ == -1U)
{
this->first_shnum_ = shndx;
- this->first_map_.merge_map = merge_map;
+ this->first_map_.output_data = output_data;
return &this->first_map_;
}
if (this->second_shnum_ == -1U)
{
this->second_shnum_ = shndx;
- this->second_map_.merge_map = merge_map;
+ this->second_map_.output_data = output_data;
return &this->second_map_;
}
Input_merge_map* new_map = new Input_merge_map;
- new_map->merge_map = merge_map;
+ new_map->output_data = output_data;
this->section_merge_maps_[shndx] = new_map;
return new_map;
}
@@ -97,12 +96,13 @@ Object_merge_map::get_or_make_input_merge_map(const Merge_map* merge_map,
// Add a mapping.
void
-Object_merge_map::add_mapping(const Merge_map* merge_map, unsigned int shndx,
+Object_merge_map::add_mapping(const Output_section_data* output_data,
+ unsigned int shndx,
section_offset_type input_offset,
section_size_type length,
section_offset_type output_offset)
{
- Input_merge_map* map = this->get_or_make_input_merge_map(merge_map, shndx);
+ Input_merge_map* map = this->get_or_make_input_merge_map(output_data, shndx);
// Try to merge the new entry in the last one we saw.
if (!map->entries.empty())
@@ -142,14 +142,12 @@ Object_merge_map::add_mapping(const Merge_map* merge_map, unsigned int shndx,
// Get the output offset for an input address.
bool
-Object_merge_map::get_output_offset(const Merge_map* merge_map,
- unsigned int shndx,
+Object_merge_map::get_output_offset(unsigned int shndx,
section_offset_type input_offset,
section_offset_type* output_offset)
{
Input_merge_map* map = this->get_input_merge_map(shndx);
- if (map == NULL
- || (merge_map != NULL && map->merge_map != merge_map))
+ if (map == NULL)
return false;
if (!map->sorted)
@@ -181,12 +179,12 @@ Object_merge_map::get_output_offset(const Merge_map* merge_map,
// Return whether this is the merge map for section SHNDX.
-inline bool
-Object_merge_map::is_merge_section_for(const Merge_map* merge_map,
+bool
+Object_merge_map::is_merge_section_for(const Output_section_data* output_data,
unsigned int shndx)
{
Input_merge_map* map = this->get_input_merge_map(shndx);
- return map != NULL && map->merge_map == merge_map;
+ return map != NULL && map->output_data == output_data;
}
// Initialize a mapping from input offsets to output addresses.
@@ -228,57 +226,6 @@ Object_merge_map::initialize_input_to_output_map(
}
}
-// Class Merge_map.
-
-// Add a mapping for the bytes from OFFSET to OFFSET + LENGTH in input
-// section SHNDX in object OBJECT to an OUTPUT_OFFSET in merged data
-// in an output section.
-
-void
-Merge_map::add_mapping(Relobj* object, unsigned int shndx,
- section_offset_type offset, section_size_type length,
- section_offset_type output_offset)
-{
- gold_assert(object != NULL);
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- {
- object_merge_map = new Object_merge_map();
- object->set_merge_map(object_merge_map);
- }
-
- object_merge_map->add_mapping(this, shndx, offset, length, output_offset);
-}
-
-// Return the output offset for an input address. The input address
-// is at offset OFFSET in section SHNDX in OBJECT. This sets
-// *OUTPUT_OFFSET to the offset in the merged data in the output
-// section. This returns true if the mapping is known, false
-// otherwise.
-
-bool
-Merge_map::get_output_offset(const Relobj* object, unsigned int shndx,
- section_offset_type offset,
- section_offset_type* output_offset) const
-{
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- return false;
- return object_merge_map->get_output_offset(this, shndx, offset,
- output_offset);
-}
-
-// Return whether this is the merge section for SHNDX in OBJECT.
-
-bool
-Merge_map::is_merge_section_for(const Relobj* object, unsigned int shndx) const
-{
- Object_merge_map* object_merge_map = object->merge_map();
- if (object_merge_map == NULL)
- return false;
- return object_merge_map->is_merge_section_for(this, shndx);
-}
-
// Class Output_merge_base.
// Return the output offset for an input offset. The input address is
@@ -291,16 +238,7 @@ Output_merge_base::do_output_offset(const Relobj* object,
section_offset_type offset,
section_offset_type* poutput) const
{
- return this->merge_map_.get_output_offset(object, shndx, offset, poutput);
-}
-
-// Return whether this is the merge section for SHNDX in OBJECT.
-
-bool
-Output_merge_base::do_is_merge_section_for(const Relobj* object,
- unsigned int shndx) const
-{
- return this->merge_map_.is_merge_section_for(object, shndx);
+ return object->merge_output_offset(shndx, offset, poutput);
}
// Record a merged input section for script processing.
@@ -436,7 +374,7 @@ Output_merge_data::do_add_input_section(Relobj* object, unsigned int shndx)
}
// Record the offset of this constant in the output section.
- this->add_mapping(object, shndx, i, entsize, k);
+ object->add_merge_mapping(this, shndx, i, entsize, k);
}
// For script processing, we keep the input sections.
@@ -625,8 +563,8 @@ Output_merge_string<Char_type>::finalize_merged_data()
{
section_size_type length = p->offset - last_input_offset;
if (length > 0)
- this->add_mapping((*l)->object, (*l)->shndx, last_input_offset,
- length, last_output_offset);
+ (*l)->object->add_merge_mapping(this, (*l)->shndx,
+ last_input_offset, length, last_output_offset);
last_input_offset = p->offset;
if (p->stringpool_key != 0)
last_output_offset =
diff --git a/gold/merge.h b/gold/merge.h
index 6318f45..3e500bc 100644
--- a/gold/merge.h
+++ b/gold/merge.h
@@ -33,8 +33,6 @@
namespace gold
{
-class Merge_map;
-
// For each object with merge sections, we store an Object_merge_map.
// This is used to map locations in input sections to a merged output
// section. The output section itself is not recorded here--it can be
@@ -57,8 +55,9 @@ class Object_merge_map
// discarded. OUTPUT_OFFSET is relative to the start of the merged
// data in the output section.
void
- add_mapping(const Merge_map*, unsigned int shndx, section_offset_type offset,
- section_size_type length, section_offset_type output_offset);
+ add_mapping(const Output_section_data*, unsigned int shndx,
+ section_offset_type offset, section_size_type length,
+ section_offset_type output_offset);
// Get the output offset for an input address. MERGE_MAP is the map
// we are looking for, or NULL if we don't care. The input address
@@ -68,13 +67,13 @@ class Object_merge_map
// mapping is known, false otherwise. *OUTPUT_OFFSET is relative to
// the start of the merged data in the output section.
bool
- get_output_offset(const Merge_map*, unsigned int shndx,
+ get_output_offset(unsigned int shndx,
section_offset_type offset,
section_offset_type* output_offset);
// Return whether this is the merge map for section SHNDX.
bool
- is_merge_section_for(const Merge_map*, unsigned int shndx);
+ is_merge_section_for(const Output_section_data*, unsigned int shndx);
// Initialize an mapping from input offsets to output addresses for
// section SHNDX. STARTING_ADDRESS is the output address of the
@@ -133,14 +132,14 @@ class Object_merge_map
// look up information for a different Merge_map, we report that
// we don't have it, rather than trying a lookup and returning an
// answer which will receive the wrong offset.
- const Merge_map* merge_map;
+ const Output_section_data* output_data;
// The list of mappings.
Entries entries;
// Whether the ENTRIES field is sorted by input_offset.
bool sorted;
Input_merge_map()
- : merge_map(NULL), entries(), sorted(true)
+ : output_data(NULL), entries(), sorted(true)
{ }
};
@@ -155,7 +154,8 @@ class Object_merge_map
// Get or make the Input_merge_map to use for the section SHNDX
// with MERGE_MAP.
Input_merge_map*
- get_or_make_input_merge_map(const Merge_map* merge_map, unsigned int shndx);
+ get_or_make_input_merge_map(const Output_section_data* merge_map,
+ unsigned int shndx);
// Any given object file will normally only have a couple of input
// sections with mergeable contents. So we keep the first two input
@@ -169,46 +169,6 @@ class Object_merge_map
Section_merge_maps section_merge_maps_;
};
-// This class manages mappings from input sections to offsets in an
-// output section. This is used where input sections are merged. The
-// actual data is stored in fields in Object.
-
-class Merge_map
-{
- public:
- Merge_map()
- { }
-
- // Add a mapping for the bytes from OFFSET to OFFSET + LENGTH in the
- // input section SHNDX in object OBJECT to OUTPUT_OFFSET in the
- // output section. An OUTPUT_OFFSET of -1 means that the bytes are
- // discarded. OUTPUT_OFFSET is not the offset from the start of the
- // output section, it is the offset from the start of the merged
- // data within the output section.
- void
- add_mapping(Relobj* object, unsigned int shndx,
- section_offset_type offset, section_size_type length,
- section_offset_type output_offset);
-
- // Return the output offset for an input address. The input address
- // is at offset OFFSET in section SHNDX in OBJECT. This sets
- // *OUTPUT_OFFSET to the offset in the output section; this will be
- // -1 if the bytes are not being copied to the output. This returns
- // true if the mapping is known, false otherwise. This returns the
- // value stored by add_mapping, namely the offset from the start of
- // the merged data within the output section.
- bool
- get_output_offset(const Relobj* object, unsigned int shndx,
- section_offset_type offset,
- section_offset_type* output_offset) const;
-
- // Return whether this is the merge mapping for section SHNDX in
- // OBJECT. This should return true when get_output_offset would
- // return true for some input offset.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const;
-};
-
// A general class for SHF_MERGE data, to hold functions shared by
// fixed-size constant data and string data.
@@ -216,7 +176,7 @@ class Output_merge_base : public Output_section_data
{
public:
Output_merge_base(uint64_t entsize, uint64_t addralign)
- : Output_section_data(addralign), merge_map_(), entsize_(entsize),
+ : Output_section_data(addralign), entsize_(entsize),
keeps_input_sections_(false), first_relobj_(NULL), first_shndx_(-1),
input_sections_()
{ }
@@ -285,21 +245,6 @@ class Output_merge_base : public Output_section_data
section_offset_type offset,
section_offset_type* poutput) const;
- // Return whether this is the merge section for an input section.
- bool
- do_is_merge_section_for(const Relobj*, unsigned int shndx) const;
-
- // Add a mapping from an OFFSET in input section SHNDX in object
- // OBJECT to an OUTPUT_OFFSET in the output section. OUTPUT_OFFSET
- // is the offset from the start of the merged data in the output
- // section.
- void
- add_mapping(Relobj* object, unsigned int shndx, section_offset_type offset,
- section_size_type length, section_offset_type output_offset)
- {
- this->merge_map_.add_mapping(object, shndx, offset, length, output_offset);
- }
-
// This may be overridden by the child class.
virtual bool
do_is_string()
@@ -315,9 +260,6 @@ class Output_merge_base : public Output_section_data
record_input_section(Relobj* relobj, unsigned int shndx);
private:
- // A mapping from input object/section/offset to offset in output
- // section.
- Merge_map merge_map_;
// The entry size. For fixed-size constants, this is the size of
// the constants. For strings, this is the size of a character.
uint64_t entsize_;
diff --git a/gold/object.cc b/gold/object.cc
index c90b67e..7286e5a 100644
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -41,6 +41,7 @@
#include "plugin.h"
#include "compressed_output.h"
#include "incremental.h"
+#include "merge.h"
namespace gold
{
@@ -268,6 +269,50 @@ Object::handle_split_stack_section(const char* name)
// Class Relobj
+template<int size>
+void
+Relobj::initialize_input_to_output_map(unsigned int shndx,
+ typename elfcpp::Elf_types<size>::Elf_Addr starting_address,
+ Unordered_map<section_offset_type,
+ typename elfcpp::Elf_types<size>::Elf_Addr>* output_addresses) const {
+ Object_merge_map *map = this->object_merge_map_;
+ map->initialize_input_to_output_map<size>(shndx, starting_address,
+ output_addresses);
+}
+
+void
+Relobj::add_merge_mapping(Output_section_data *output_data,
+ unsigned int shndx, section_offset_type offset,
+ section_size_type length,
+ section_offset_type output_offset) {
+ if (this->object_merge_map_ == NULL)
+ {
+ this->object_merge_map_ = new Object_merge_map();
+ }
+
+ this->object_merge_map_->add_mapping(output_data, shndx, offset, length,
+ output_offset);
+}
+
+bool
+Relobj::merge_output_offset(unsigned int shndx, section_offset_type offset,
+ section_offset_type *poutput) const {
+ Object_merge_map* object_merge_map = this->object_merge_map_;
+ if (object_merge_map == NULL)
+ return false;
+ return object_merge_map->get_output_offset(shndx, offset, poutput);
+}
+
+bool
+Relobj::is_merge_section_for(const Output_section_data* output_data,
+ unsigned int shndx) const {
+ Object_merge_map* object_merge_map = this->object_merge_map_;
+ if (object_merge_map == NULL)
+ return false;
+ return object_merge_map->is_merge_section_for(output_data, shndx);
+
+}
+
// To copy the symbols data read from the file to a local data structure.
// This function is called from do_layout only while doing garbage
// collection.
@@ -3209,6 +3254,24 @@ make_elf_object(const std::string& name, Input_file* input_file, off_t offset,
// Instantiate the templates we need.
+#if defined(HAVE_TARGET_64_LITTLE) || defined(HAVE_TARGET_64_BIG)
+template
+void
+Relobj::initialize_input_to_output_map<64>(unsigned int shndx,
+ typename elfcpp::Elf_types<64>::Elf_Addr starting_address,
+ Unordered_map<section_offset_type,
+ typename elfcpp::Elf_types<64>::Elf_Addr>* output_addresses) const;
+#endif
+
+#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_32_BIG)
+template
+void
+Relobj::initialize_input_to_output_map<32>(unsigned int shndx,
+ typename elfcpp::Elf_types<32>::Elf_Addr starting_address,
+ Unordered_map<section_offset_type,
+ typename elfcpp::Elf_types<32>::Elf_Addr>* output_addresses) const;
+#endif
+
#ifdef HAVE_TARGET_32_LITTLE
template
void
diff --git a/gold/object.h b/gold/object.h
index cce6c8c..126bff5 100644
--- a/gold/object.h
+++ b/gold/object.h
@@ -41,6 +41,7 @@ class Cref;
class Layout;
class Output_data;
class Output_section;
+class Output_section_data;
class Output_file;
class Output_symtab_xindex;
class Pluginobj;
@@ -1200,18 +1201,26 @@ class Relobj : public Object
relocs_must_follow_section_writes() const
{ return this->relocs_must_follow_section_writes_; }
- // Return the object merge map.
- Object_merge_map*
- merge_map() const
- { return this->object_merge_map_; }
+ template<int size>
+ void
+ initialize_input_to_output_map(unsigned int shndx,
+ typename elfcpp::Elf_types<size>::Elf_Addr starting_address,
+ Unordered_map<section_offset_type,
+ typename elfcpp::Elf_types<size>::Elf_Addr>* output_address) const;
- // Set the object merge map.
void
- set_merge_map(Object_merge_map* object_merge_map)
- {
- gold_assert(this->object_merge_map_ == NULL);
- this->object_merge_map_ = object_merge_map;
- }
+ add_merge_mapping(Output_section_data *output_data,
+ unsigned int shndx, section_offset_type offset,
+ section_size_type length,
+ section_offset_type output_offset);
+
+ bool
+ merge_output_offset(unsigned int shndx, section_offset_type offset,
+ section_offset_type *poutput) const;
+
+ bool
+ is_merge_section_for(const Output_section_data* output_data,
+ unsigned int shndx) const;
// Record the relocatable reloc info for an input reloc section.
void
diff --git a/gold/output.cc b/gold/output.cc
index 01838cc..8faa040 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -2210,7 +2210,7 @@ Output_section::Input_section::is_merge_section_for(const Relobj* object,
{
if (this->is_input_section())
return false;
- return this->u2_.posd->is_merge_section_for(object, shndx);
+ return object->is_merge_section_for(this->u2_.posd, shndx);
}
// Write out the data. We don't have to do anything for an input
diff --git a/gold/output.h b/gold/output.h
index 8e0c33a..dc11c8f 100644
--- a/gold/output.h
+++ b/gold/output.h
@@ -675,13 +675,6 @@ class Output_section_data : public Output_data
section_offset_type* poutput) const
{ return this->do_output_offset(object, shndx, offset, poutput); }
- // Return whether this is the merge section for the input section
- // SHNDX in OBJECT. This should return true when output_offset
- // would return true for some values of OFFSET.
- bool
- is_merge_section_for(const Relobj* object, unsigned int shndx) const
- { return this->do_is_merge_section_for(object, shndx); }
-
// Write the contents to a buffer. This is used for sections which
// require postprocessing, such as compression.
void
@@ -715,11 +708,6 @@ class Output_section_data : public Output_data
section_offset_type*) const
{ return false; }
- // The child class may implement is_merge_section_for.
- virtual bool
- do_is_merge_section_for(const Relobj*, unsigned int) const
- { return false; }
-
// The child class may implement write_to_buffer. Most child
// classes can not appear in a compressed section, and they do not
// implement this.
diff --git a/gold/reloc.cc b/gold/reloc.cc
index 5906754..49ab3cb 100644
--- a/gold/reloc.cc
+++ b/gold/reloc.cc
@@ -1457,10 +1457,9 @@ Merged_symbol_value<size>::initialize_input_to_output_map(
const Relobj* object,
unsigned int input_shndx)
{
- Object_merge_map* map = object->merge_map();
- map->initialize_input_to_output_map<size>(input_shndx,
- this->output_start_address_,
- &this->output_addresses_);
+ object->initialize_input_to_output_map<size>(input_shndx,
+ this->output_start_address_,
+ &this->output_addresses_);
}
// Get the output value corresponding to an input offset if we
@@ -1474,9 +1473,8 @@ Merged_symbol_value<size>::value_from_output_section(
typename elfcpp::Elf_types<size>::Elf_Addr input_offset) const
{
section_offset_type output_offset;
- bool found = object->merge_map()->get_output_offset(NULL, input_shndx,
- input_offset,
- &output_offset);
+ bool found = object->merge_output_offset(input_shndx, input_offset,
+ &output_offset);
// If this assertion fails, it means that some relocation was
// against a portion of an input merge section which we didn't map
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch][gold] Remove empty Merge_map class
2015-03-03 8:28 ` Rafael Espíndola
@ 2015-03-04 23:22 ` Cary Coutant
2015-03-05 15:56 ` H.J. Lu
0 siblings, 1 reply; 9+ messages in thread
From: Cary Coutant @ 2015-03-04 23:22 UTC (permalink / raw)
To: Rafael Espíndola; +Cc: Binutils
I've committed this patch on your behalf. Thanks!
-cary
commit dbe40a889191708b6e32441b1c64937844645574
Author: Rafael Ávila de Espíndola <rafael.espindola@gmail.com>
Date: Wed Mar 4 15:10:18 2015 -0800
Remove empty class Merge_map.
2015-03-02 Rafael Ávila de Espíndola <rafael.espindola@gmail.com>
* ehframe.cc (Cie::set_output_offset): Pass in and use a
Output_section_data instead of a Merge_map.
(Eh_frame::Eh_frame): Don't initialize merge_map_.
(Eh_frame::read_cie): Use add_merge_mapping instead of
Merge_map::add_mapping.
(Eh_frame::read_fde): Ditto.
(Eh_frame::set_final_data_size): Use this instead of this->merge_map_.
(Eh_frame::do_output_offset): Use merge_output_offset istead of
merge_map_->get_output_offset.
(Eh_frame::do_is_merge_section_for): Delete.
* ehframe.h (Fde::add_mapping): Pass in and use a Output_section_data
instead of a Merge_map.
(Cie::set_output_offset): Pass in a Output_section_data instead of a
Merge_map.
(Eh_frame::do_is_merge_section_for): Delete.
(Eh_frame::merge_map_): Delete.
* merge.cc (Object_merge_map::get_or_make_input_merge_map): Pass in
and use a Output_section_data instead of a Merge_map.
(Object_merge_map::add_mapping): Ditto.
(Object_merge_map::get_output_offset): Remove the merge_map argument.
(Object_merge_map::is_merge_section_for): Pass in and use a
Output_section_data instead of a Merge_map.
(Merge_map): Delete.
(Output_merge_base::do_output_offset): Use merge_output_offset instead
of merge_map_.get_output_offset.
(Output_merge_base::do_is_merge_section_for): Delete.
(Output_merge_data::do_add_input_section): Use
object->add_merge_mapping instead of add_mapping.
(Output_merge_string<Char_type>::finalize_merged_data): Ditto.
* merge.h (Merge_map): Delete forward declaration.
(Object_merge_map::add_mapping): Pass in and use a Output_section_data
instead of a Merge_map.
(Object_merge_map::get_output_offset): Remove the merge_map argument.
(Object_merge_map::is_merge_section_for): Pass in and use a
Output_section_data instead of a Merge_map.
(Input_merge_map::Object_merge_map::merge_map): Replace with
output_data.
(Object_merge_map::get_or_make_input_merge_map): Pass in and use a
Output_section_data instead of a Merge_map.
(Merge_map): Delete.
(Output_merge_base::Output_merge_base): Don't initialize merge_map_.
(Output_merge_base::do_is_merge_section_for): Delete.
(Output_merge_base::add_mapping): Delete.
(Output_merge_base::merge_map_): Delete.
* object.cc (Relobj::initialize_input_to_output_map): New.
(Relobj::initialize_input_to_output_map): New.
(Relobj::merge_output_offset): New.
(Relobj::is_merge_section_for): New.
(Relobj::initialize_input_to_output_map): Instantiate for 32 and 64
bits.
* object.h (Relobj::merge_map): Delete.
(initialize_input_to_output_map): New.
(set_merge_map): Delete.
(add_merge_mapping): New.
(merge_output_offset): New.
(is_merge_section_for): New.
* output.cc (Output_section::Input_section::is_merge_section_for):
Use object->is_merge_section_for.
* output.h (Output_section_data::is_merge_section_for): Delete.
(Output_section_data::do_is_merge_section_for): Delete.
* reloc.cc (Merged_symbol_value<size>::initialize_input_to_output_map):
Use object->initialize_input_to_output_map.
(Merged_symbol_value<size>::value_from_output_section): Use
object->merge_output_offset.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch][gold] Remove empty Merge_map class
2015-03-04 23:22 ` Cary Coutant
@ 2015-03-05 15:56 ` H.J. Lu
0 siblings, 0 replies; 9+ messages in thread
From: H.J. Lu @ 2015-03-05 15:56 UTC (permalink / raw)
To: Cary Coutant; +Cc: Rafael Espíndola, Binutils
[-- Attachment #1: Type: text/plain, Size: 4465 bytes --]
On Wed, Mar 4, 2015 at 3:22 PM, Cary Coutant <ccoutant@google.com> wrote:
> I've committed this patch on your behalf. Thanks!
>
> -cary
>
> commit dbe40a889191708b6e32441b1c64937844645574
> Author: Rafael Ávila de Espíndola <rafael.espindola@gmail.com>
> Date: Wed Mar 4 15:10:18 2015 -0800
>
> Remove empty class Merge_map.
>
> 2015-03-02 Rafael Ávila de Espíndola <rafael.espindola@gmail.com>
>
> * ehframe.cc (Cie::set_output_offset): Pass in and use a
> Output_section_data instead of a Merge_map.
> (Eh_frame::Eh_frame): Don't initialize merge_map_.
> (Eh_frame::read_cie): Use add_merge_mapping instead of
> Merge_map::add_mapping.
> (Eh_frame::read_fde): Ditto.
> (Eh_frame::set_final_data_size): Use this instead of this->merge_map_.
> (Eh_frame::do_output_offset): Use merge_output_offset istead of
> merge_map_->get_output_offset.
> (Eh_frame::do_is_merge_section_for): Delete.
> * ehframe.h (Fde::add_mapping): Pass in and use a Output_section_data
> instead of a Merge_map.
> (Cie::set_output_offset): Pass in a Output_section_data instead of a
> Merge_map.
> (Eh_frame::do_is_merge_section_for): Delete.
> (Eh_frame::merge_map_): Delete.
> * merge.cc (Object_merge_map::get_or_make_input_merge_map): Pass in
> and use a Output_section_data instead of a Merge_map.
> (Object_merge_map::add_mapping): Ditto.
> (Object_merge_map::get_output_offset): Remove the merge_map argument.
> (Object_merge_map::is_merge_section_for): Pass in and use a
> Output_section_data instead of a Merge_map.
> (Merge_map): Delete.
> (Output_merge_base::do_output_offset): Use merge_output_offset instead
> of merge_map_.get_output_offset.
> (Output_merge_base::do_is_merge_section_for): Delete.
> (Output_merge_data::do_add_input_section): Use
> object->add_merge_mapping instead of add_mapping.
> (Output_merge_string<Char_type>::finalize_merged_data): Ditto.
> * merge.h (Merge_map): Delete forward declaration.
> (Object_merge_map::add_mapping): Pass in and use a Output_section_data
> instead of a Merge_map.
> (Object_merge_map::get_output_offset): Remove the merge_map argument.
> (Object_merge_map::is_merge_section_for): Pass in and use a
> Output_section_data instead of a Merge_map.
> (Input_merge_map::Object_merge_map::merge_map): Replace with
> output_data.
> (Object_merge_map::get_or_make_input_merge_map): Pass in and use a
> Output_section_data instead of a Merge_map.
> (Merge_map): Delete.
> (Output_merge_base::Output_merge_base): Don't initialize merge_map_.
> (Output_merge_base::do_is_merge_section_for): Delete.
> (Output_merge_base::add_mapping): Delete.
> (Output_merge_base::merge_map_): Delete.
> * object.cc (Relobj::initialize_input_to_output_map): New.
> (Relobj::initialize_input_to_output_map): New.
> (Relobj::merge_output_offset): New.
> (Relobj::is_merge_section_for): New.
> (Relobj::initialize_input_to_output_map): Instantiate for 32 and 64
> bits.
> * object.h (Relobj::merge_map): Delete.
> (initialize_input_to_output_map): New.
> (set_merge_map): Delete.
> (add_merge_mapping): New.
> (merge_output_offset): New.
> (is_merge_section_for): New.
> * output.cc (Output_section::Input_section::is_merge_section_for):
> Use object->is_merge_section_for.
> * output.h (Output_section_data::is_merge_section_for): Delete.
> (Output_section_data::do_is_merge_section_for): Delete.
> * reloc.cc (Merged_symbol_value<size>::initialize_input_to_output_map):
> Use object->initialize_input_to_output_map.
> (Merged_symbol_value<size>::value_from_output_section): Use
> object->merge_output_offset.
GCC 4.2 complains:
binutils/gold/object.cc:3261: error: using ‘typename’ outside of template
This patch removes typename. It works with both GCC 4.2 and 4.8.
I checked it in as an obvious fix.
* output.cc (Relobj::initialize_input_to_output_map<size>):
Remove typename on elfcpp::Elf_types<size>::Elf_Addr.
--
H.J.
[-- Attachment #2: 0001-Remove-typename-on-elfcpp-Elf_types-size-Elf_Addr.patch --]
[-- Type: text/x-patch, Size: 2224 bytes --]
From beb8418f4799b50ce414b7a63ac7a2a363dc8a05 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 5 Mar 2015 07:52:41 -0800
Subject: [PATCH] Remove typename on elfcpp::Elf_types<size>::Elf_Addr
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
GCC 4.2 complains:
binutils/gold/object.cc:3261: error: using ‘typename’ outside of template
This patch removes typename. It works with both GCC 4.2 and 4.8.
* output.cc (Relobj::initialize_input_to_output_map<size>):
Remove typename on elfcpp::Elf_types<size>::Elf_Addr.
---
gold/ChangeLog | 5 +++++
gold/object.cc | 8 ++++----
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/gold/ChangeLog b/gold/ChangeLog
index fe6a56b..24d3560 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,8 @@
+2015-03-05 H.J. Lu <hongjiu.lu@intel.com>
+
+ * output.cc (Relobj::initialize_input_to_output_map<size>):
+ Remove typename on elfcpp::Elf_types<size>::Elf_Addr.
+
2015-03-04 Cary Coutant <ccoutant@google.com>
* parameters.cc (Parameters::set_target_once): Call
diff --git a/gold/object.cc b/gold/object.cc
index 7286e5a..84e4568 100644
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -3258,18 +3258,18 @@ make_elf_object(const std::string& name, Input_file* input_file, off_t offset,
template
void
Relobj::initialize_input_to_output_map<64>(unsigned int shndx,
- typename elfcpp::Elf_types<64>::Elf_Addr starting_address,
+ elfcpp::Elf_types<64>::Elf_Addr starting_address,
Unordered_map<section_offset_type,
- typename elfcpp::Elf_types<64>::Elf_Addr>* output_addresses) const;
+ elfcpp::Elf_types<64>::Elf_Addr>* output_addresses) const;
#endif
#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_32_BIG)
template
void
Relobj::initialize_input_to_output_map<32>(unsigned int shndx,
- typename elfcpp::Elf_types<32>::Elf_Addr starting_address,
+ elfcpp::Elf_types<32>::Elf_Addr starting_address,
Unordered_map<section_offset_type,
- typename elfcpp::Elf_types<32>::Elf_Addr>* output_addresses) const;
+ elfcpp::Elf_types<32>::Elf_Addr>* output_addresses) const;
#endif
#ifdef HAVE_TARGET_32_LITTLE
--
1.9.3
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-03-05 15:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-06 23:32 [patch][gold] Remove empty Merge_map class Rafael Espíndola
2015-02-09 16:30 ` Rafael Espíndola
2015-02-17 15:56 ` Rafael Espíndola
2015-02-22 3:12 ` Rafael Espíndola
2015-02-27 17:09 ` Rafael Espíndola
2015-03-02 19:08 ` Cary Coutant
2015-03-03 8:28 ` Rafael Espíndola
2015-03-04 23:22 ` Cary Coutant
2015-03-05 15:56 ` H.J. Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).