* [GOLD] PowerPC TLS fixes
@ 2012-09-09 14:43 Alan Modra
2012-09-10 17:05 ` Ian Lance Taylor
2012-09-12 16:59 ` H.J. Lu
0 siblings, 2 replies; 9+ messages in thread
From: Alan Modra @ 2012-09-09 14:43 UTC (permalink / raw)
To: binutils
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.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.139
diff -u -p -r1.139 output.h
--- gold/output.h 5 Sep 2012 00:34:20 -0000 1.139
+++ gold/output.h 9 Sep 2012 13:44:56 -0000
@@ -2246,6 +2246,12 @@ class Output_data_got : public Output_da
bool
add_global_plt(Symbol* gsym, unsigned int got_type);
+ // Like add_global, but for a TLS symbol where the value will be
+ // offset using Target::tls_offset_for_global
+ bool
+ add_global_tls(Symbol* gsym, unsigned int got_type)
+ { return add_global_plt(gsym, got_type); }
+
// Add an entry for a global symbol to the GOT, and add a dynamic
// relocation of type R_TYPE for the GOT entry.
void
@@ -2270,6 +2276,12 @@ class Output_data_got : public Output_da
bool
add_local_plt(Relobj* object, unsigned int sym_index, unsigned int got_type);
+ // Like add_local, but for a TLS symbol where the value will be
+ // offset using Target::tls_offset_for_local
+ bool
+ add_local_tls(Relobj* object, unsigned int sym_index, unsigned int got_type)
+ { return add_local_plt(object, sym_index, got_type); }
+
// Add an entry for a local symbol to the GOT, and add a dynamic
// relocation of type R_TYPE for the GOT entry.
void
@@ -2278,12 +2290,25 @@ class Output_data_got : public Output_da
unsigned int r_type);
// 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.
void
add_local_pair_with_rel(Relobj* object, unsigned int sym_index,
unsigned int shndx, unsigned int got_type,
Output_data_reloc_generic* rel_dyn,
- unsigned int r_type_1, unsigned int r_type_2);
+ unsigned int r_type);
+
+ // Add a pair of entries for a local symbol to the GOT, and add
+ // a dynamic relocation of type R_TYPE using STN_UNDEF on the first.
+ // The first got entry will have a value of zero, the second the
+ // value of the local symbol offset by Target::tls_offset_for_local.
+ void
+ add_local_tls_pair(Relobj* object, unsigned int sym_index,
+ unsigned int got_type,
+ Output_data_reloc_generic* rel_dyn,
+ unsigned int r_type);
// Add a constant to the GOT. This returns the offset of the new
// entry from the start of the GOT.
@@ -2370,7 +2395,7 @@ class Output_data_got : public Output_da
// Write the GOT entry to an output view.
void
- write(unsigned char* pov) const;
+ write(unsigned int got_indx, unsigned char* pov) const;
private:
enum
@@ -2393,6 +2418,7 @@ class Output_data_got : public Output_da
// for a global symbol, or CONSTANT_CODE for a constant.
unsigned int local_sym_index_ : 31;
// 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;
};
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_;
- if (!this->use_plt_offset_)
- {
- uint64_t lval = object->local_symbol_value(lsi, 0);
- val = convert_types<Valtype, uint64_t>(lval);
- }
- else
+ bool is_tls = object->local_symbol(lsi)->is_tls_symbol();
+ if (this->use_plt_offset_ && !is_tls)
{
uint64_t plt_address =
parameters->target().plt_address_for_local(object, lsi);
val = plt_address + object->local_plt_offset(lsi);
}
+ else
+ {
+ uint64_t lval = object->local_symbol_value(lsi, 0);
+ val = convert_types<Valtype, uint64_t>(lval);
+ if (this->use_plt_offset_ && is_tls)
+ val += parameters->target().tls_offset_for_local(object, lsi,
+ got_indx);
+ }
}
break;
}
@@ -1566,8 +1576,10 @@ Output_data_got<size, big_endian>::add_l
}
// 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.
template<int size, bool big_endian>
void
Output_data_got<size, big_endian>::add_local_pair_with_rel(
@@ -1576,8 +1588,7 @@ Output_data_got<size, big_endian>::add_l
unsigned int shndx,
unsigned int got_type,
Output_data_reloc_generic* rel_dyn,
- unsigned int r_type_1,
- unsigned int r_type_2)
+ unsigned int r_type)
{
if (object->local_has_got_offset(symndx, got_type))
return;
@@ -1587,11 +1598,30 @@ Output_data_got<size, big_endian>::add_l
Got_entry(object, symndx, false));
object->set_local_got_offset(symndx, got_type, got_offset);
Output_section* os = object->output_section(shndx);
- rel_dyn->add_output_section_generic(os, r_type_1, this, got_offset, 0);
+ rel_dyn->add_output_section_generic(os, r_type, this, got_offset, 0);
+}
- if (r_type_2 != 0)
- rel_dyn->add_output_section_generic(os, r_type_2, this,
- got_offset + size / 8, 0);
+// Add a pair of entries for a local symbol to the GOT, and add
+// a dynamic relocation of type R_TYPE using STN_UNDEF on the first.
+// The first got entry will have a value of zero, the second the
+// value of the local symbol offset by Target::tls_offset_for_local.
+template<int size, bool big_endian>
+void
+Output_data_got<size, big_endian>::add_local_tls_pair(
+ Relobj* object,
+ unsigned int symndx,
+ unsigned int got_type,
+ Output_data_reloc_generic* rel_dyn,
+ unsigned int r_type)
+{
+ if (object->local_has_got_offset(symndx, got_type))
+ return;
+
+ unsigned int got_offset
+ = this->add_got_entry_pair(Got_entry(),
+ Got_entry(object, symndx, true));
+ object->set_local_got_offset(symndx, got_type, got_offset);
+ rel_dyn->add_local_generic(object, 0, r_type, this, got_offset, 0);
}
// Reserve a slot in the GOT for a local symbol or the second slot of a pair.
@@ -1634,11 +1664,9 @@ Output_data_got<size, big_endian>::do_wr
unsigned char* const oview = of->get_output_view(off, oview_size);
unsigned char* pov = oview;
- for (typename Got_entries::const_iterator p = this->entries_.begin();
- p != this->entries_.end();
- ++p)
+ for (unsigned int i = 0; i < this->entries_.size(); ++i)
{
- p->write(pov);
+ this->entries_[i].write(i, pov);
pov += add;
}
Index: gold/target.h
===================================================================
RCS file: /cvs/src/src/gold/target.h,v
retrieving revision 1.69
diff -u -p -r1.69 target.h
--- gold/target.h 9 Sep 2012 03:43:51 -0000 1.69
+++ gold/target.h 9 Sep 2012 11:45:27 -0000
@@ -270,6 +270,20 @@ class Target
plt_address_for_local(const Relobj* object, unsigned int symndx) const
{ return this->do_plt_address_for_local(object, symndx); }
+ // Return the offset to use for the GOT_INDX'th got entry which is
+ // for a local tls symbol specified by OBJECT, SYMNDX
+ int64_t
+ tls_offset_for_local(const Relobj* object,
+ unsigned int symndx,
+ unsigned int got_indx) const
+ { return do_tls_offset_for_local(object, symndx, got_indx); }
+
+ // Return the offset to use for the GOT_INDX'th got entry which is
+ // for global tls symbol GSYM
+ int64_t
+ tls_offset_for_global(Symbol* gsym, unsigned int got_indx) const
+ { return do_tls_offset_for_global(gsym, got_indx); }
+
// Return whether this target can use relocation types to determine
// if a function's address is taken.
bool
@@ -546,6 +560,14 @@ class Target
do_plt_address_for_local(const Relobj*, unsigned int) const
{ gold_unreachable(); }
+ virtual int64_t
+ do_tls_offset_for_local(const Relobj*, unsigned int, unsigned int) const
+ { gold_unreachable(); }
+
+ virtual int64_t
+ do_tls_offset_for_global(Symbol*, unsigned int) const
+ { gold_unreachable(); }
+
// Virtual function which may be overriden by the child class.
virtual bool
do_can_check_for_function_pointers() const
Index: gold/arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.153
diff -u -p -r1.153 arm.cc
--- gold/arm.cc 5 Sep 2012 00:34:20 -0000 1.153
+++ gold/arm.cc 9 Sep 2012 11:45:27 -0000
@@ -8068,7 +8068,7 @@ Target_arm<big_endian>::Scan::local(Symb
got->add_local_pair_with_rel(object, r_sym, shndx,
GOT_TYPE_TLS_PAIR,
target->rel_dyn_section(layout),
- elfcpp::R_ARM_TLS_DTPMOD32, 0);
+ elfcpp::R_ARM_TLS_DTPMOD32);
else
got->add_tls_gd32_with_static_reloc(GOT_TYPE_TLS_PAIR,
object, r_sym);
Index: gold/i386.cc
===================================================================
RCS file: /cvs/src/src/gold/i386.cc,v
retrieving revision 1.146
diff -u -p -r1.146 i386.cc
--- gold/i386.cc 5 Sep 2012 00:34:20 -0000 1.146
+++ gold/i386.cc 9 Sep 2012 11:45:27 -0000
@@ -1874,7 +1874,7 @@ Target_i386::Scan::local(Symbol_table* s
got->add_local_pair_with_rel(object, r_sym, shndx,
GOT_TYPE_TLS_PAIR,
target->rel_dyn_section(layout),
- elfcpp::R_386_TLS_DTPMOD32, 0);
+ elfcpp::R_386_TLS_DTPMOD32);
}
else if (optimized_type != tls::TLSOPT_TO_LE)
unsupported_reloc_local(object, r_type);
Index: gold/sparc.cc
===================================================================
RCS file: /cvs/src/src/gold/sparc.cc,v
retrieving revision 1.59
diff -u -p -r1.59 sparc.cc
--- gold/sparc.cc 5 Sep 2012 00:34:20 -0000 1.59
+++ gold/sparc.cc 9 Sep 2012 11:45:27 -0000
@@ -2429,8 +2429,7 @@ Target_sparc<size, big_endian>::Scan::lo
target->rela_dyn_section(layout),
(size == 64
? elfcpp::R_SPARC_TLS_DTPMOD64
- : elfcpp::R_SPARC_TLS_DTPMOD32),
- 0);
+ : elfcpp::R_SPARC_TLS_DTPMOD32));
if (r_type == elfcpp::R_SPARC_TLS_GD_CALL)
generate_tls_call(symtab, layout, target);
}
Index: gold/x86_64.cc
===================================================================
RCS file: /cvs/src/src/gold/x86_64.cc,v
retrieving revision 1.155
diff -u -p -r1.155 x86_64.cc
--- gold/x86_64.cc 5 Sep 2012 00:34:20 -0000 1.155
+++ gold/x86_64.cc 9 Sep 2012 11:45:27 -0000
@@ -2477,7 +2477,7 @@ Target_x86_64<size>::Scan::local(Symbol_
shndx,
GOT_TYPE_TLS_PAIR,
target->rela_dyn_section(layout),
- elfcpp::R_X86_64_DTPMOD64, 0);
+ elfcpp::R_X86_64_DTPMOD64);
}
else if (optimized_type != tls::TLSOPT_TO_LE)
unsupported_reloc_local(object, r_type);
Index: gold/powerpc.cc
===================================================================
RCS file: /cvs/src/src/gold/powerpc.cc,v
retrieving revision 1.56
diff -u -p -r1.56 powerpc.cc
--- gold/powerpc.cc 9 Sep 2012 03:43:51 -0000 1.56
+++ gold/powerpc.cc 9 Sep 2012 11:45:27 -0000
@@ -270,6 +270,18 @@ class Target_powerpc : public Sized_targ
uint64_t
do_dynsym_value(const Symbol*) const;
+ // Return the offset to use for the GOT_INDX'th got entry which is
+ // for a local tls symbol specified by OBJECT, SYMNDX
+ int64_t
+ do_tls_offset_for_local(const Relobj* object,
+ unsigned int symndx,
+ unsigned int got_indx) const;
+
+ // Return the offset to use for the GOT_INDX'th got entry which is
+ // for global tls symbol GSYM
+ int64_t
+ do_tls_offset_for_global(Symbol* gsym, unsigned int got_indx) const;
+
// Relocate a section.
void
relocate_section(const Relocate_info<size, big_endian>*,
@@ -2349,7 +2361,7 @@ Target_powerpc<size, big_endian>::Scan::
Output_section* output_section,
const elfcpp::Rela<size, big_endian>& reloc,
unsigned int r_type,
- const elfcpp::Sym<size, big_endian>& lsym)
+ const elfcpp::Sym<size, big_endian>& /* lsym */)
{
Powerpc_relobj<size, big_endian>* ppc_object
= static_cast<Powerpc_relobj<size, big_endian>*>(object);
@@ -2523,16 +2535,9 @@ Target_powerpc<size, big_endian>::Scan::
Output_data_got_powerpc<size, big_endian>* got
= target->got_section(symtab, layout);
unsigned int r_sym = elfcpp::elf_r_sym<size>(reloc.get_r_info());
- unsigned int shndx = lsym.get_st_shndx();
- bool is_ordinary;
- shndx = object->adjust_sym_shndx(r_sym, shndx, &is_ordinary);
- gold_assert(is_ordinary);
- got->add_local_pair_with_rel(object, r_sym,
- shndx,
- GOT_TYPE_TLSGD,
- target->rela_dyn_section(layout),
- elfcpp::R_POWERPC_DTPMOD,
- elfcpp::R_POWERPC_DTPREL);
+ Reloc_section* rela_dyn = target->rela_dyn_section(layout);
+ got->add_local_tls_pair(object, r_sym, GOT_TYPE_TLSGD,
+ rela_dyn, elfcpp::R_POWERPC_DTPMOD);
}
else if (tls_type == tls::TLSOPT_TO_LE)
{
@@ -2574,9 +2579,7 @@ Target_powerpc<size, big_endian>::Scan::
Output_data_got_powerpc<size, big_endian>* got
= target->got_section(symtab, layout);
unsigned int r_sym = elfcpp::elf_r_sym<size>(reloc.get_r_info());
- got->add_local_with_rel(object, r_sym, GOT_TYPE_DTPREL,
- target->rela_dyn_section(layout),
- elfcpp::R_POWERPC_DTPREL);
+ got->add_local_tls(object, r_sym, GOT_TYPE_DTPREL);
}
break;
@@ -2591,9 +2594,7 @@ Target_powerpc<size, big_endian>::Scan::
Output_data_got_powerpc<size, big_endian>* got
= target->got_section(symtab, layout);
unsigned int r_sym = elfcpp::elf_r_sym<size>(reloc.get_r_info());
- got->add_local_with_rel(object, r_sym, GOT_TYPE_TPREL,
- target->rela_dyn_section(layout),
- elfcpp::R_POWERPC_TPREL);
+ got->add_local_tls(object, r_sym, GOT_TYPE_TPREL);
}
else if (tls_type == tls::TLSOPT_TO_LE)
{
@@ -2913,9 +2914,15 @@ Target_powerpc<size, big_endian>::Scan::
{
Output_data_got_powerpc<size, big_endian>* got
= target->got_section(symtab, layout);
- got->add_global_with_rel(gsym, GOT_TYPE_DTPREL,
- target->rela_dyn_section(layout),
- elfcpp::R_POWERPC_DTPREL);
+ if (!gsym->final_value_is_known()
+ && (gsym->is_from_dynobj()
+ || gsym->is_undefined()
+ || gsym->is_preemptible()))
+ got->add_global_with_rel(gsym, GOT_TYPE_DTPREL,
+ target->rela_dyn_section(layout),
+ elfcpp::R_POWERPC_DTPREL);
+ else
+ got->add_global_tls(gsym, GOT_TYPE_DTPREL);
}
break;
@@ -2930,9 +2937,15 @@ Target_powerpc<size, big_endian>::Scan::
{
Output_data_got_powerpc<size, big_endian>* got
= target->got_section(symtab, layout);
- got->add_global_with_rel(gsym, GOT_TYPE_TPREL,
- target->rela_dyn_section(layout),
- elfcpp::R_POWERPC_TPREL);
+ if (!gsym->final_value_is_known()
+ && (gsym->is_from_dynobj()
+ || gsym->is_undefined()
+ || gsym->is_preemptible()))
+ got->add_global_with_rel(gsym, GOT_TYPE_TPREL,
+ target->rela_dyn_section(layout),
+ elfcpp::R_POWERPC_TPREL);
+ else
+ got->add_global_tls(gsym, GOT_TYPE_TPREL);
}
else if (tls_type == tls::TLSOPT_TO_LE)
{
@@ -4421,6 +4434,69 @@ Target_powerpc<size, big_endian>::do_dyn
gold_unreachable();
}
+// Return the offset to use for the GOT_INDX'th got entry which is
+// for a local tls symbol specified by OBJECT, SYMNDX
+template<int size, bool big_endian>
+int64_t
+Target_powerpc<size, big_endian>::do_tls_offset_for_local(
+ const Relobj* object,
+ unsigned int symndx,
+ unsigned int got_indx) const
+{
+ const Powerpc_relobj<size, big_endian>* ppc_object
+ = static_cast<const Powerpc_relobj<size, big_endian>*>(object);
+ if (ppc_object->local_symbol(symndx)->is_tls_symbol())
+ {
+ for (Got_type got_type = GOT_TYPE_TLSGD;
+ got_type <= GOT_TYPE_TPREL;
+ got_type = Got_type(got_type + 1))
+ if (ppc_object->local_has_got_offset(symndx, got_type))
+ {
+ unsigned int off = ppc_object->local_got_offset(symndx, got_type);
+ if (got_type == GOT_TYPE_TLSGD)
+ off += size / 8;
+ if (off == got_indx * (size / 8))
+ {
+ if (got_type == GOT_TYPE_TPREL)
+ return -tp_offset;
+ else
+ return -dtp_offset;
+ }
+ }
+ }
+ gold_unreachable();
+}
+
+// Return the offset to use for the GOT_INDX'th got entry which is
+// for global tls symbol GSYM
+template<int size, bool big_endian>
+int64_t
+Target_powerpc<size, big_endian>::do_tls_offset_for_global(
+ Symbol* gsym,
+ unsigned int got_indx) const
+{
+ if (gsym->type() == elfcpp::STT_TLS)
+ {
+ for (Got_type got_type = GOT_TYPE_TLSGD;
+ got_type <= GOT_TYPE_TPREL;
+ got_type = Got_type(got_type + 1))
+ if (gsym->has_got_offset(got_type))
+ {
+ unsigned int off = gsym->got_offset(got_type);
+ if (got_type == GOT_TYPE_TLSGD)
+ off += size / 8;
+ if (off == got_indx * (size / 8))
+ {
+ if (got_type == GOT_TYPE_TPREL)
+ return -tp_offset;
+ else
+ return -dtp_offset;
+ }
+ }
+ }
+ gold_unreachable();
+}
+
// The selector for powerpc object files.
template<int size, bool big_endian>
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GOLD] PowerPC TLS fixes
2012-09-09 14:43 [GOLD] PowerPC TLS fixes Alan Modra
@ 2012-09-10 17:05 ` Ian Lance Taylor
2012-09-12 16:25 ` H.J. Lu
2012-09-12 16:59 ` H.J. Lu
1 sibling, 1 reply; 9+ messages in thread
From: Ian Lance Taylor @ 2012-09-10 17:05 UTC (permalink / raw)
To: binutils
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GOLD] PowerPC TLS fixes
2012-09-10 17:05 ` Ian Lance Taylor
@ 2012-09-12 16:25 ` H.J. Lu
0 siblings, 0 replies; 9+ messages in thread
From: H.J. Lu @ 2012-09-12 16:25 UTC (permalink / raw)
To: Ian Lance Taylor, Alan Modra; +Cc: binutils
On Mon, Sep 10, 2012 at 10:04 AM, Ian Lance Taylor <iant@google.com> wrote:
> 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.
>
This breaks native x32 gold:
gcc -mx32 -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow
-Werror -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -fmerge-constants
-g -O2 -o ifuncmain7pic -Bgcctestdir/ ifuncmain7pic.o
gcctestdir/ld: internal error in do_tls_offset_for_local, at
/export/gnu/import/git/binutils/gold/target.h:565
collect2: error: ld returned 1 exit status
make[3]: *** [ifuncmain7pic] Error 1
make[3]: *** Waiting for unfinished jobs....
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GOLD] PowerPC TLS fixes
2012-09-09 14:43 [GOLD] PowerPC TLS fixes Alan Modra
2012-09-10 17:05 ` Ian Lance Taylor
@ 2012-09-12 16:59 ` H.J. Lu
2012-09-12 17:07 ` H.J. Lu
1 sibling, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2012-09-12 16:59 UTC (permalink / raw)
To: binutils
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GOLD] PowerPC TLS fixes
2012-09-12 16:59 ` H.J. Lu
@ 2012-09-12 17:07 ` H.J. Lu
2012-09-12 17:11 ` H.J. Lu
2012-09-12 18:29 ` Ian Lance Taylor
0 siblings, 2 replies; 9+ messages in thread
From: H.J. Lu @ 2012-09-12 17:07 UTC (permalink / raw)
To: binutils, Alan Modra, Ian Lance Taylor
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GOLD] PowerPC TLS fixes
2012-09-12 17:07 ` H.J. Lu
@ 2012-09-12 17:11 ` H.J. Lu
2012-09-12 18:29 ` Ian Lance Taylor
1 sibling, 0 replies; 9+ messages in thread
From: H.J. Lu @ 2012-09-12 17:11 UTC (permalink / raw)
To: binutils, Alan Modra, Ian Lance Taylor
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GOLD] PowerPC TLS fixes
2012-09-12 17:07 ` H.J. Lu
2012-09-12 17:11 ` H.J. Lu
@ 2012-09-12 18:29 ` Ian Lance Taylor
2012-09-12 22:20 ` Alan Modra
1 sibling, 1 reply; 9+ messages in thread
From: Ian Lance Taylor @ 2012-09-12 18:29 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils, Alan Modra
[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]
On Wed, Sep 12, 2012 at 10:06 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
> 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 committed this patch. I think this is the right fix. Please close
the PR if it is fixed for you.
Ian
2012-09-12 Ian Lance Taylor <iant@google.com>
PR gold/14570
* output.cc: Rename Output_data_got template parameter from size
to got_size for all functions. Compile all variants of
Output_data_got.
(Output_data_got::Got_entry::write): Correct use of size for
symbol value. Use local_is_tls rather than casting to
Sized_relobj_file.
* object.h (class Object): Add local_is_tls and do_local_is_tls.
(class Sized_relobj_file): Add do_local_is_tls.
* incremental.h (class Sized_relobj_incr): Add do_local_is_tls.
[-- Attachment #2: foo.patch --]
[-- Type: application/octet-stream, Size: 13742 bytes --]
Index: incremental.h
===================================================================
RCS file: /cvs/src/src/gold/incremental.h,v
retrieving revision 1.32
diff -u -p -r1.32 incremental.h
--- incremental.h 24 Apr 2012 22:05:28 -0000 1.32
+++ incremental.h 12 Sep 2012 18:23:10 -0000
@@ -1962,6 +1962,10 @@ class Sized_relobj_incr : public Sized_r
do_local_plt_offset(unsigned int) const
{ gold_unreachable(); }
+ bool
+ do_local_is_tls(unsigned int) const
+ { gold_unreachable(); }
+
// Return the number of local symbols.
unsigned int
do_local_symbol_count() const
Index: object.h
===================================================================
RCS file: /cvs/src/src/gold/object.h,v
retrieving revision 1.119
diff -u -p -r1.119 object.h
--- object.h 5 Sep 2012 00:34:20 -0000 1.119
+++ object.h 12 Sep 2012 18:23:10 -0000
@@ -1079,6 +1079,11 @@ class Relobj : public Object
unsigned int got_offset)
{ this->do_set_local_got_offset(symndx, got_type, got_offset); }
+ // Return whether the local symbol SYMNDX is a TLS symbol.
+ bool
+ local_is_tls(unsigned int symndx) const
+ { return this->do_local_is_tls(symndx); }
+
// The number of local symbols in the input symbol table.
virtual unsigned int
local_symbol_count() const
@@ -1259,6 +1264,10 @@ class Relobj : public Object
do_set_local_got_offset(unsigned int symndx, unsigned int got_type,
unsigned int got_offset) = 0;
+ // Return whether local symbol SYMNDX is a TLS symbol.
+ virtual bool
+ do_local_is_tls(unsigned int symndx) const = 0;
+
// Return the number of local symbols--implemented by child class.
virtual unsigned int
do_local_symbol_count() const = 0;
@@ -2166,6 +2175,11 @@ class Sized_relobj_file : public Sized_r
unsigned int
do_local_plt_offset(unsigned int symndx) const;
+ // Return whether local symbol SYMNDX is a TLS symbol.
+ bool
+ do_local_is_tls(unsigned int symndx) const
+ { return this->local_symbol(symndx)->is_tls_symbol(); }
+
// Return the number of local symbols.
unsigned int
do_local_symbol_count() const
Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.173
diff -u -p -r1.173 output.cc
--- output.cc 10 Sep 2012 23:10:41 -0000 1.173
+++ output.cc 12 Sep 2012 18:23:10 -0000
@@ -1367,9 +1367,9 @@ Output_data_group<size, big_endian>::do_
// Write out the entry.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
void
-Output_data_got<size, big_endian>::Got_entry::write(
+Output_data_got<got_size, big_endian>::Got_entry::write(
unsigned int got_indx,
unsigned char* pov) const
{
@@ -1388,13 +1388,36 @@ Output_data_got<size, big_endian>::Got_e
+ gsym->plt_offset());
else
{
- Sized_symbol<size>* sgsym;
- // This cast is a bit ugly. We don't want to put a
- // virtual method in Symbol, because we want Symbol to be
- // as small as possible.
- sgsym = static_cast<Sized_symbol<size>*>(gsym);
- val = sgsym->value();
- if (this->use_plt_or_tls_offset_ && gsym->type() == elfcpp::STT_TLS)
+ switch (parameters->size_and_endianness())
+ {
+#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_32_BIG)
+ case Parameters::TARGET_32_LITTLE:
+ case Parameters::TARGET_32_BIG:
+ {
+ // This cast is ugly. We don't want to put a
+ // virtual method in Symbol, because we want Symbol
+ // to be as small as possible.
+ Sized_symbol<32>::Value_type v;
+ v = static_cast<Sized_symbol<32>*>(gsym)->value();
+ val = convert_types<Valtype, Sized_symbol<32>::Value_type>(v);
+ }
+ break;
+#endif
+#if defined(HAVE_TARGET_64_LITTLE) || defined(HAVE_TARGET_64_BIG)
+ case Parameters::TARGET_64_LITTLE:
+ case Parameters::TARGET_64_BIG:
+ {
+ Sized_symbol<64>::Value_type v;
+ v = static_cast<Sized_symbol<64>*>(gsym)->value();
+ val = convert_types<Valtype, Sized_symbol<64>::Value_type>(v);
+ }
+ break;
+#endif
+ default:
+ gold_unreachable();
+ }
+ if (this->use_plt_or_tls_offset_
+ && gsym->type() == elfcpp::STT_TLS)
val += parameters->target().tls_offset_for_global(gsym,
got_indx);
}
@@ -1414,10 +1437,9 @@ Output_data_got<size, big_endian>::Got_e
default:
{
- const Sized_relobj_file<size, big_endian>* object
- = static_cast<Sized_relobj_file<size, big_endian>*>(this->u_.object);
+ const Relobj* object = this->u_.object;
const unsigned int lsi = this->local_sym_index_;
- bool is_tls = object->local_symbol(lsi)->is_tls_symbol();
+ bool is_tls = object->local_is_tls(lsi);
if (this->use_plt_or_tls_offset_ && !is_tls)
{
uint64_t plt_address =
@@ -1436,7 +1458,7 @@ Output_data_got<size, big_endian>::Got_e
break;
}
- elfcpp::Swap<size, big_endian>::writeval(pov, val);
+ elfcpp::Swap<got_size, big_endian>::writeval(pov, val);
}
// Output_data_got methods.
@@ -1445,9 +1467,9 @@ Output_data_got<size, big_endian>::Got_e
// this is a new GOT entry, false if the symbol already had a GOT
// entry.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
bool
-Output_data_got<size, big_endian>::add_global(
+Output_data_got<got_size, big_endian>::add_global(
Symbol* gsym,
unsigned int got_type)
{
@@ -1461,10 +1483,10 @@ Output_data_got<size, big_endian>::add_g
// Like add_global, but use the PLT offset.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
bool
-Output_data_got<size, big_endian>::add_global_plt(Symbol* gsym,
- unsigned int got_type)
+Output_data_got<got_size, big_endian>::add_global_plt(Symbol* gsym,
+ unsigned int got_type)
{
if (gsym->has_got_offset(got_type))
return false;
@@ -1477,9 +1499,9 @@ Output_data_got<size, big_endian>::add_g
// Add an entry for a global symbol to the GOT, and add a dynamic
// relocation of type R_TYPE for the GOT entry.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
void
-Output_data_got<size, big_endian>::add_global_with_rel(
+Output_data_got<got_size, big_endian>::add_global_with_rel(
Symbol* gsym,
unsigned int got_type,
Output_data_reloc_generic* rel_dyn,
@@ -1496,9 +1518,9 @@ Output_data_got<size, big_endian>::add_g
// Add a pair of entries for a global 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.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
void
-Output_data_got<size, big_endian>::add_global_pair_with_rel(
+Output_data_got<got_size, big_endian>::add_global_pair_with_rel(
Symbol* gsym,
unsigned int got_type,
Output_data_reloc_generic* rel_dyn,
@@ -1514,16 +1536,16 @@ Output_data_got<size, big_endian>::add_g
if (r_type_2 != 0)
rel_dyn->add_global_generic(gsym, r_type_2, this,
- got_offset + size / 8, 0);
+ got_offset + got_size / 8, 0);
}
// Add an entry for a local symbol to the GOT. This returns true if
// this is a new GOT entry, false if the symbol already has a GOT
// entry.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
bool
-Output_data_got<size, big_endian>::add_local(
+Output_data_got<got_size, big_endian>::add_local(
Relobj* object,
unsigned int symndx,
unsigned int got_type)
@@ -1539,9 +1561,9 @@ Output_data_got<size, big_endian>::add_l
// Like add_local, but use the PLT offset.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
bool
-Output_data_got<size, big_endian>::add_local_plt(
+Output_data_got<got_size, big_endian>::add_local_plt(
Relobj* object,
unsigned int symndx,
unsigned int got_type)
@@ -1558,9 +1580,9 @@ Output_data_got<size, big_endian>::add_l
// Add an entry for a local symbol to the GOT, and add a dynamic
// relocation of type R_TYPE for the GOT entry.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
void
-Output_data_got<size, big_endian>::add_local_with_rel(
+Output_data_got<got_size, big_endian>::add_local_with_rel(
Relobj* object,
unsigned int symndx,
unsigned int got_type,
@@ -1580,9 +1602,9 @@ Output_data_got<size, big_endian>::add_l
// 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.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
void
-Output_data_got<size, big_endian>::add_local_pair_with_rel(
+Output_data_got<got_size, big_endian>::add_local_pair_with_rel(
Relobj* object,
unsigned int symndx,
unsigned int shndx,
@@ -1605,9 +1627,9 @@ Output_data_got<size, big_endian>::add_l
// a dynamic relocation of type R_TYPE using STN_UNDEF on the first.
// The first got entry will have a value of zero, the second the
// value of the local symbol offset by Target::tls_offset_for_local.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
void
-Output_data_got<size, big_endian>::add_local_tls_pair(
+Output_data_got<got_size, big_endian>::add_local_tls_pair(
Relobj* object,
unsigned int symndx,
unsigned int got_type,
@@ -1626,9 +1648,9 @@ Output_data_got<size, big_endian>::add_l
// Reserve a slot in the GOT for a local symbol or the second slot of a pair.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
void
-Output_data_got<size, big_endian>::reserve_local(
+Output_data_got<got_size, big_endian>::reserve_local(
unsigned int i,
Relobj* object,
unsigned int sym_index,
@@ -1640,9 +1662,9 @@ Output_data_got<size, big_endian>::reser
// Reserve a slot in the GOT for a global symbol.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
void
-Output_data_got<size, big_endian>::reserve_global(
+Output_data_got<got_size, big_endian>::reserve_global(
unsigned int i,
Symbol* gsym,
unsigned int got_type)
@@ -1653,11 +1675,11 @@ Output_data_got<size, big_endian>::reser
// Write out the GOT.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
void
-Output_data_got<size, big_endian>::do_write(Output_file* of)
+Output_data_got<got_size, big_endian>::do_write(Output_file* of)
{
- const int add = size / 8;
+ const int add = got_size / 8;
const off_t off = this->offset();
const off_t oview_size = this->data_size();
@@ -1680,9 +1702,9 @@ Output_data_got<size, big_endian>::do_wr
// Create a new GOT entry and return its offset.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
unsigned int
-Output_data_got<size, big_endian>::add_got_entry(Got_entry got_entry)
+Output_data_got<got_size, big_endian>::add_got_entry(Got_entry got_entry)
{
if (!this->is_data_size_valid())
{
@@ -1693,11 +1715,12 @@ Output_data_got<size, big_endian>::add_g
else
{
// For an incremental update, find an available slot.
- off_t got_offset = this->free_list_.allocate(size / 8, size / 8, 0);
+ off_t got_offset = this->free_list_.allocate(got_size / 8,
+ got_size / 8, 0);
if (got_offset == -1)
gold_fallback(_("out of patch space (GOT);"
" relink with --incremental-full"));
- unsigned int got_index = got_offset / (size / 8);
+ unsigned int got_index = got_offset / (got_size / 8);
gold_assert(got_index < this->entries_.size());
this->entries_[got_index] = got_entry;
return static_cast<unsigned int>(got_offset);
@@ -1706,10 +1729,11 @@ Output_data_got<size, big_endian>::add_g
// Create a pair of new GOT entries and return the offset of the first.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
unsigned int
-Output_data_got<size, big_endian>::add_got_entry_pair(Got_entry got_entry_1,
- Got_entry got_entry_2)
+Output_data_got<got_size, big_endian>::add_got_entry_pair(
+ Got_entry got_entry_1,
+ Got_entry got_entry_2)
{
if (!this->is_data_size_valid())
{
@@ -1723,11 +1747,12 @@ Output_data_got<size, big_endian>::add_g
else
{
// For an incremental update, find an available pair of slots.
- off_t got_offset = this->free_list_.allocate(2 * size / 8, size / 8, 0);
+ off_t got_offset = this->free_list_.allocate(2 * got_size / 8,
+ got_size / 8, 0);
if (got_offset == -1)
gold_fallback(_("out of patch space (GOT);"
" relink with --incremental-full"));
- unsigned int got_index = got_offset / (size / 8);
+ unsigned int got_index = got_offset / (got_size / 8);
gold_assert(got_index < this->entries_.size());
this->entries_[got_index] = got_entry_1;
this->entries_[got_index + 1] = got_entry_2;
@@ -1737,9 +1762,9 @@ Output_data_got<size, big_endian>::add_g
// Replace GOT entry I with a new value.
-template<int size, bool big_endian>
+template<int got_size, bool big_endian>
void
-Output_data_got<size, big_endian>::replace_got_entry(
+Output_data_got<got_size, big_endian>::replace_got_entry(
unsigned int i,
Got_entry got_entry)
{
@@ -5442,24 +5467,16 @@ template
class Output_data_group<64, true>;
#endif
-#ifdef HAVE_TARGET_32_LITTLE
template
class Output_data_got<32, false>;
-#endif
-#ifdef HAVE_TARGET_32_BIG
template
class Output_data_got<32, true>;
-#endif
-#ifdef HAVE_TARGET_64_LITTLE
template
class Output_data_got<64, false>;
-#endif
-#ifdef HAVE_TARGET_64_BIG
template
class Output_data_got<64, true>;
-#endif
} // End namespace gold.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GOLD] PowerPC TLS fixes
2012-09-12 18:29 ` Ian Lance Taylor
@ 2012-09-12 22:20 ` Alan Modra
2012-09-12 23:12 ` Ian Lance Taylor
0 siblings, 1 reply; 9+ messages in thread
From: Alan Modra @ 2012-09-12 22:20 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: H.J. Lu, binutils
On Wed, Sep 12, 2012 at 11:29:41AM -0700, Ian Lance Taylor wrote:
> On Wed, Sep 12, 2012 at 10:06 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > The template size parameter is for GOT size, which
> > is independent of the template size parameter for
Ouch.
> I committed this patch. I think this is the right fix. Please close
> the PR if it is fixed for you.
Thanks Ian!
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GOLD] PowerPC TLS fixes
2012-09-12 22:20 ` Alan Modra
@ 2012-09-12 23:12 ` Ian Lance Taylor
0 siblings, 0 replies; 9+ messages in thread
From: Ian Lance Taylor @ 2012-09-12 23:12 UTC (permalink / raw)
To: Ian Lance Taylor, H.J. Lu, binutils
On Wed, Sep 12, 2012 at 3:19 PM, Alan Modra <amodra@gmail.com> wrote:
> On Wed, Sep 12, 2012 at 11:29:41AM -0700, Ian Lance Taylor wrote:
>> On Wed, Sep 12, 2012 at 10:06 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> >
>> > The template size parameter is for GOT size, which
>> > is independent of the template size parameter for
>
> Ouch.
It's my fault for leaving the template parameter name as size rather
than got_size in output.cc.
Ian
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-09-12 23:12 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-09 14:43 [GOLD] PowerPC TLS fixes Alan Modra
2012-09-10 17:05 ` Ian Lance Taylor
2012-09-12 16:25 ` H.J. Lu
2012-09-12 16:59 ` H.J. Lu
2012-09-12 17:07 ` H.J. Lu
2012-09-12 17:11 ` H.J. Lu
2012-09-12 18:29 ` Ian Lance Taylor
2012-09-12 22:20 ` Alan Modra
2012-09-12 23:12 ` Ian Lance Taylor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).