Re: [GOLD] PowerPC TLS fixes

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Sep 12, 2012 at 9:58 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Sep 9, 2012 at 7:42 AM, Alan Modra <amodra@gmail.com> wrote:
>> This patch corrects TLS GOT entries for powerpc.  We were generating
>> unnecessary relocations for symbols that resolve locally, and TLSGD
>> module/offset GOT entries were just plain wrong.  TLSGD entries for
>> local symbols consist of two words, the first value doesn't matter but
>> this word needs a DTPMOD dynamic relocation against any local tls
>> symbol or even the zero index symbol.  The second word is the DTPREL
>> offset for the local symbol, a constant, but powerpc differs from
>> other targets in that the dynamic thread pointer is offset by 0x8000.
>> A DTPREL relocation thus evaluates to the symbol value minus 0x8000.
>> If we remove all the unnecessary relocations then we also need to
>> support TPREL GOT entries.  These also are offset, but by 0x7000.
>>
>> Offsets from the symbol are not supported by current gold GOT handling
>> infrastructure, and trying to use the typical BFD scheme of
>> initializing the .got section entries while relocating other sections
>> isn't a good idea when section relocation happens in parallel.  I
>> originally thought I'd need to crib another bit from Got_entry
>> local_sym_index_, but then realized a tls symbol will never need a plt
>> offset, so we can use that bit to signify a target dependent offset.
>>
>> I also tidy the existing add_local_pair_with_rel.  No target uses the
>> second reloc, and I can't see any possible use of a DTP relative reloc
>> against an output section symbol.  A DTP relative reloc in a tls_index
>> (module/offset) entry always refers to the symbol of interest in the
>> associated __tls_get_addr call (contrasted to the module reloc which
>> ought to be happy with any symbol in the same object).
>>
>> OK, to apply?
>>
>>         * output.h (Output_data_got::add_global_tls, add_local_tls,
>>         add_local_tls_pair): New functions.
>>         (Output_data_got::add_local_pair_with_rel): Remove second
>>         reloc param.  Expand comment.
>>         (Output_data_got::Got_entry::write): Add got_index param.
>>         * output.cc (Output_data_got::add_global_tls, add_local_tls,
>>         add_local_tls_pair): New functions.
>>         (Output_data_got::Got_entry::write): Handle tls symbols
>>         with use_plt_offset_ set specially.
>>         (Output_data_got::add_local_pair_with_rel): Only one reloc.
>>         (Output_data_got::do_write): Replace iterator with index, pass
>>         index to entry write function.
>>         * target.h (Target::tls_offset_for_local, tls_offset_for_global,
>>         do_tls_offset_for_local, do_tls_offset_for_global): New functions.
>>         * arm.cc (Target_arm::Scan::local): Update add_local_pair_with_rel
>>         call.
>>         * i386.cc (Target_i386::Scan::local): Likewise.
>>         * sparc.cc (Target_sparc::Scan::local): Likewise.
>>         * x86_64.cc (Target_x86_64::Scan::local): Likewise.
>>         * powerpc.cc (Target_powerpc::do_tls_offset_for_local,
>>         do_tls_offset_for_global): New functions.
>>         (Target_powerpc::Scan::local): Correct TLS relocations and got
>>         entry values.
>>         (Target_powerpc::Scan::global): Don't emit unnecessary
>>         dynamic relocations on TLS GOT entries.
>>
>
>>
>> Index: gold/output.cc
>> ===================================================================
>> RCS file: /cvs/src/src/gold/output.cc,v
>> retrieving revision 1.171
>> diff -u -p -r1.171 output.cc
>> --- gold/output.cc      5 Sep 2012 00:34:20 -0000       1.171
>> +++ gold/output.cc      9 Sep 2012 11:45:27 -0000
>> @@ -1369,7 +1369,9 @@ Output_data_group<size, big_endian>::do_
>>
>>  template<int size, bool big_endian>
>>  void
>> -Output_data_got<size, big_endian>::Got_entry::write(unsigned char* pov) const
>> +Output_data_got<size, big_endian>::Got_entry::write(
>> +    unsigned int got_indx,
>> +    unsigned char* pov) const
>>  {
>>    Valtype val = 0;
>>
>> @@ -1392,6 +1394,9 @@ Output_data_got<size, big_endian>::Got_e
>>             // as small as possible.
>>             sgsym = static_cast<Sized_symbol<size>*>(gsym);
>>             val = sgsym->value();
>> +           if (this->use_plt_offset_ && gsym->type() == elfcpp::STT_TLS)
>> +             val += parameters->target().tls_offset_for_global(gsym,
>> +                                                               got_indx);
>>           }
>>        }
>>        break;
>> @@ -1409,19 +1414,24 @@ Output_data_got<size, big_endian>::Got_e
>>
>>      default:
>>        {
>> -       const Relobj* object = this->u_.object;
>> +       const Sized_relobj_file<size, big_endian>* object
>> +         = static_cast<Sized_relobj_file<size, big_endian>*>(this->u_.object);
>>          const unsigned int lsi = this->local_sym_index_;
>
> This breaks x32.  X32 uses 32-bit ELF format with 64-bit GOT entry.
>
> H.J.

Output_data_got is declared as:

template<int got_size, bool big_endian>
class Output_data_got : public Output_data_got_base
{
 public:
  typedef typename elfcpp::Elf_types<got_size>::Elf_Addr Valtype;

The template size parameter is for GOT size, which
is independent of the template size parameter for

// A regular object file.  This is size and endian specific.

template<int size, bool big_endian>
class Sized_relobj_file : public Sized_relobj<size, big_endian>
{

Ian, how should we fix it?

Thanks.

-- 
H.J.