This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [GOLD] Reduce size of class Symbol
- From: Cary Coutant <ccoutant at gmail dot com>
- To: Alan Modra <amodra at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Thu, 3 Aug 2017 22:12:52 -0700
- Subject: Re: [GOLD] Reduce size of class Symbol
- Authentication-results: sourceware.org; auth=none
- References: <20170801041614.GN1956@bubble.grove.modra.org>
> bool
> has_plt_offset() const
> - { return this->plt_offset_ != -1U; }
> + { return this->u_.from_object.plt_offset != -1U; }
>
> // Return the offset into the PLT section of this symbol.
> unsigned int
> plt_offset() const
> {
> gold_assert(this->has_plt_offset());
> - return this->plt_offset_;
> + return this->u_.from_object.plt_offset;
> }
>
> // Set the PLT offset of this symbol.
> @@ -463,7 +463,7 @@ class Symbol
> set_plt_offset(unsigned int plt_offset)
> {
> gold_assert(plt_offset != -1U);
> - this->plt_offset_ = plt_offset;
> + this->u_.from_object.plt_offset = plt_offset;
> }
I like the goal, but the problem is that you're always accessing the
plt_offset through the u_.from_object variant, no matter what variant
is actually there. While you've been careful to make sure that the
plt_offset field has the same offset in each variant, it's still both
fragile and technically illegal.
Since the hole is caused by each struct being padded to a multiple of
the largest member, it seems to me that a better strategy would be to
split the union into two separate ones, like so:
union
{
// This is used if SOURCE_ == FROM_OBJECT.
// Object in which symbol is defined, or in which it was first
// seen.
Object* object;
// This is used if SOURCE_ == IN_OUTPUT_DATA.
// Output_data in which symbol is defined. Before
// Layout::finalize the symbol's value is an offset within the
// Output_data.
Output_data* output_data;
// This is used if SOURCE_ == IN_OUTPUT_SEGMENT.
// Output_segment in which the symbol is defined. Before
// Layout::finalize the symbol's value is an offset.
Output_segment* output_segment;
} u1_;
union
{
// This is used if SOURCE_ == FROM_OBJECT.
// Section number in object_ in which symbol is defined.
unsigned int shndx;
// This is used if SOURCE_ == IN_OUTPUT_DATA.
// True if the offset is from the end, false if the offset is
// from the beginning.
bool offset_is_from_end;
// This is used if SOURCE_ == IN_OUTPUT_SEGMENT.
// The base to use for the offset before Layout::finalize.
Segment_offset_base offset_base;
} u2_;
Now on 64-bit, u1_ will be 64 bits, but u2_ will be 32 bits, and
shouldn't force a hole (since the next member is also 32 bits).
-cary