This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [GOLD] Reduce size of class Symbol


>    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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]