This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR gold/18609
- From: Andrew Senkevich <andrew dot n dot senkevich at gmail dot com>
- To: Cary Coutant <ccoutant at gmail dot com>, Binutils <binutils at sourceware dot org>
- Date: Thu, 27 Aug 2015 15:12:38 +0300
- Subject: Re: [PATCH] PR gold/18609
- Authentication-results: sourceware.org; auth=none
- References: <CAMXFM3ufsJmH8trBQ6GJ5ZWBRxEpXbTTr-cDY_X=hxrkEsK=pQ at mail dot gmail dot com> <CAJimCsGgpjpAZdmwZF+kviPANMWnS_Fi6+hPtKftN82nJOLT7A at mail dot gmail dot com> <CAMXFM3tDV0tELGu6iiLop6gBO2c0S48GYTDaMet-wHXbQxGF6g at mail dot gmail dot com> <CAMe9rOrGp-KGrRyqH6TNsMJfsAOS3ze=e=O1SAOgd0GQhQqBMA at mail dot gmail dot com> <CAMXFM3vmaJNcg_Kz5tMUoR7jsJABiF2qVYRrXqg1p=qok_g5aQ at mail dot gmail dot com> <CAMe9rOrKALc8Q363j28vz3BjyBHXXN2vRwFUy2LEj-k6vrtuzQ at mail dot gmail dot com> <CAMXFM3un-Sj0r4p22pSn9op23ZUp9kfK9Gb2HQeSKkNY_OWx-w at mail dot gmail dot com> <CAMe9rOpf=+KzO0GSgTB7fwS6f6j0-X+di5eTOp+c7uRQ2oqBOw at mail dot gmail dot com> <CAJimCsH_M7BWXZOvu89iwJY0XR5G-hG976rgsiNhgD7V0dKhgg at mail dot gmail dot com>
2015-07-22 4:06 GMT+03:00 Cary Coutant <ccoutant@gmail.com>:
>>>>>> If you remove those changes, won't it generate an unused GOT slot
>>>>>> when GOTPCREL relocation is converted to PC-relative relocation?
>>>>>
>>>>> Yes, it can generate unused GOT slots.
>>>>
>>>> I think we should add a testcase to check for the unused GOT slot.
>>>> Please check if you can implement similar heuristic in gold:
>>>>
>>>> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=59cab532835904f368b0aa99267afba5fda5ded2
>>>
>>> No addresses available at the time of Target_x86_64<size>::Scan::local
>>> and *::global work, so not clear how to use some heuristics here...
>>
>> There are no addresses available in ld.bfd neither. An estimate was
>> used in ld.bfd.
>
> In gold, I think the best you can do is check to see if the branch and
> its target are in the same output section, then use
> output_section_offset() plus offset within the input section to check
> distance between the two. At the time you're scanning relocations, we
> haven't even done a tentative layout of output sections, so you'll
> have to be pessimistic if the two aren't in the same output section.
Do you mean uint64_t Object::output_section_offset(unsigned int shndx)?
Why we should use it if checked what branch and target are in the same
section so distance depends on offsets in that section only?
I propose the following patch, comments are welcome.
2015-08-27 Andrew Senkevich <andrew.senkevich@intel.com>
gold/
PR gold/18609
* x86_64.cc (Target_x86_64::Relocate::relocate): Added overflow check
for convert mov foo@GOTPCREL(%rip), %reg to lea foo(%rip), %reg.
(Target_x86_64::Scan::global, Target_x86_64::Scan::local): Added check
to avoid unused PLT slots.
diff --git a/gold/x86_64.cc b/gold/x86_64.cc
index 007af1d..db17443 100644
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@@ -2344,6 +2344,32 @@ Target_x86_64<size>::Scan::reloc_needs_plt_for_ifunc(
return flags != 0;
}
+template<int size, int valsize>
+class x86_64_overflow_check
+{
+public:
+ typedef typename elfcpp::Elf_types<size>::Elf_Addr Address;
+
+ static inline bool
+ has_overflow_signed(Address value)
+ {
+ // limit = 1 << (valsize - 1) without shift count exceeding size of type
+ Address limit = static_cast<Address>(1) << ((valsize - 1) >> 1);
+ limit <<= ((valsize - 1) >> 1);
+ limit <<= ((valsize - 1) - 2 * ((valsize - 1) >> 1));
+ return value + limit > (limit << 1) - 1;
+ }
+
+ static inline bool
+ has_overflow_unsigned(Address value)
+ {
+ Address limit = static_cast<Address>(1) << ((valsize - 1) >> 1);
+ limit <<= ((valsize - 1) >> 1);
+ limit <<= ((valsize - 1) - 2 * ((valsize - 1) >> 1));
+ return value > (limit << 1) - 1;
+ }
+};
+
// Scan a relocation for a local symbol.
template<int size>
@@ -2480,14 +2506,21 @@ Target_x86_64<size>::Scan::local(Symbol_table* symtab,
// The symbol requires a GOT section.
Output_data_got<64, false>* got = target->got_section(symtab, layout);
- // If the relocation symbol isn't IFUNC,
- // and is local, then we will convert
+ typename elfcpp::Elf_types<size>::Elf_Addr value
+ = reloc.get_r_offset() + reloc.get_r_addend() - lsym.get_st_value();
+
+ // If the relocation symbol isn't IFUNC, and is local,
+ // and branch and its target are in the same output section,
+ // and distance isn't overflow, then we will convert
// mov foo@GOTPCREL(%rip), %reg
// to lea foo(%rip), %reg.
// in Relocate::relocate.
if (r_type == elfcpp::R_X86_64_GOTPCREL
&& reloc.get_r_offset() >= 2
- && !is_ifunc)
+ && !is_ifunc
+ && strcmp(object->section_name(lsym.get_st_shndx()).c_str(),
+ ".text") == 0
+ && !x86_64_overflow_check<size, 32>::has_overflow_signed(value))
{
section_size_type stype;
const unsigned char* view = object->section_contents(data_shndx,
@@ -2496,7 +2529,6 @@ Target_x86_64<size>::Scan::local(Symbol_table* symtab,
break;
}
-
// The symbol requires a GOT entry.
unsigned int r_sym = elfcpp::elf_r_sym<size>(reloc.get_r_info());
@@ -2905,14 +2937,21 @@ Target_x86_64<size>::Scan::global(Symbol_table* symtab,
{
// The symbol requires a GOT entry.
Output_data_got<64, false>* got = target->got_section(symtab, layout);
+ Sized_symbol<size>* ssym = static_cast<Sized_symbol<size>*>(gsym);
+ typename elfcpp::Elf_types<size>::Elf_Addr value
+ = reloc.get_r_offset() + reloc.get_r_addend() - ssym->value();
// If we convert this from
// mov foo@GOTPCREL(%rip), %reg
// to lea foo(%rip), %reg.
- // in Relocate::relocate, then there is nothing to do here.
+ // in Relocate::relocate and
+ // branch and its target are in the same output section and
+ // distance isn't overflow, then there is nothing to do here.
if (r_type == elfcpp::R_X86_64_GOTPCREL
&& reloc.get_r_offset() >= 2
- && Target_x86_64<size>::can_convert_mov_to_lea(gsym))
+ && Target_x86_64<size>::can_convert_mov_to_lea(gsym)
+ && gsym->output_section() == output_section
+ && !x86_64_overflow_check<size, 32>::has_overflow_signed(value))
{
section_size_type stype;
const unsigned char* view = object->section_contents(data_shndx,
@@ -3543,7 +3582,10 @@ Target_x86_64<size>::Relocate::relocate(
// mov foo@GOTPCREL(%rip), %reg
// to lea foo(%rip), %reg.
// if possible.
- if (rela.get_r_offset() >= 2
+ typename elfcpp::Elf_types<size>::Elf_Addr value;
+ value = psymval->value(object, addend) - address;
+ if (!x86_64_overflow_check<size, 32>::has_overflow_signed(value)
+ && rela.get_r_offset() >= 2
&& view[-2] == 0x8b
&& ((gsym == NULL && !psymval->is_ifunc_symbol())
|| (gsym != NULL
--
WBR,
Andrew