Re: [GOLD] PowerPC TLS fixes

[Ian Lance Taylor <iant at google dot com>]

On Sun, Sep 9, 2012 at 7:42 AM, Alan Modra <amodra@gmail.com> wrote:
>
>         * 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.

Unimportant, but I've always been of the opinion that it's OK to just
write "Update all calls" in the change to the function definition.

>         * 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.

>    // Add a pair of entries for a local symbol to the GOT, and add
> -  // dynamic relocations of type R_TYPE_1 and R_TYPE_2, respectively.
> +  // a dynamic relocations of type R_TYPE using the section symbol of
> +  // the output section to which input section SHNDX maps, on the first.
> +  // The first got entry will have a value of zero, the second the
> +  // value of the local symbol.

s/dynamic relocations/dynamic relocation/


>      // Whether to use the PLT offset of the symbol if it has one.
> +    // For TLS symbols, whether to offset the symbol value.
>      bool use_plt_offset_ : 1;

If we're going to change the meaning of the field, let's rename it.
Otherwise the uses can be very confusing.  I suggest simply
use_plt_or_tls_offset_.


>  // Add a pair of entries for a local symbol to the GOT, and add
> -// dynamic relocations of type R_TYPE_1 and R_TYPE_2, respectively.
> -// If R_TYPE_2 == 0, add the second entry with no relocation.
> +// a dynamic relocations of type R_TYPE using the section symbol of
> +// the output section to which input section SHNDX maps, on the first.
> +// The first got entry will have a value of zero, the second the
> +// value of the local symbol.

s/dynamic relocations/dynamic relocation/

> +  // Return the offset to use for the GOT_INDX'th got entry which is
> +  // for a local tls symbol specified by OBJECT, SYMNDX

Missing period at end of comment.

> +  // Return the offset to use for the GOT_INDX'th got entry which is
> +  // for global tls symbol GSYM

Missing period here too.

This is OK with those changes.

Thanks.

Ian