Re: [GOLD] PowerPC TLS fixes

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

On Wed, Sep 12, 2012 at 10:06 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> 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?
>

I opened:

http://www.sourceware.org/bugzilla/show_bug.cgi?id=14570

-- 
H.J.