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>
- Cc: Binutils <binutils at sourceware dot org>, "H.J. Lu" <hjl dot tools at gmail dot com>
- Date: Tue, 14 Jul 2015 18:48:06 +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>
2015-07-09 20:49 GMT+03:00 Cary Coutant <ccoutant@gmail.com>:
>> 2015-07-09 Andrew Senkevich <andrew.senkevich@intel.com>
>>
>> * gold/x86_64.cc: Added overflow check for converting
>> R_X86_64_GOTPCREL to R_X86_64_PC32.
>
> Factor out "gold/" from the filename, add the PR number, and prefer
> present tense:
>
> gold/
> PR gold/18609
> * x86_64.cc: Add overflow check for converting
> R_X86_64_GOTPCREL to R_X86_64_PC32.
>
>
>> @@ -3543,7 +3511,9 @@ Target_x86_64<size>::Relocate::relocate(
>> // mov foo@GOTPCREL(%rip), %reg
>> // to lea foo(%rip), %reg.
>> // if possible.
>> - if (rela.get_r_offset() >= 2
>> + int64_t x = psymval->value(object, addend) - address;
>> + if (x == static_cast<int64_t>(static_cast<int32_t>(x))
>> + && rela.get_r_offset() >= 2
>
> Please use Bits<32>::has_overflow(x) to check for overflow (from reloc.h).
Here is corrected patch (we need to overload has_overflow() to handle x32 case):
diff --git a/gold/ChangeLog b/gold/ChangeLog
index a8c2507..7218f6f 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,9 @@
+2015-07-14 Andrew Senkevich <andrew.senkevich@intel.com>
+
+ PR gold/18609
+ * x86_64.cc: Overflow check for converting R_X86_64_GOTPCREL
+ to R_X86_64_PC32.
+
2015-07-12 H.J. Lu <hongjiu.lu@intel.com>
PR gold/18322
diff --git a/gold/reloc.h b/gold/reloc.h
index 559206e..fd95238 100644
--- a/gold/reloc.h
+++ b/gold/reloc.h
@@ -807,6 +807,12 @@ class Bits
return as_signed > max || as_signed < min;
}
+ static inline bool
+ has_overflow(uint32_t val)
+ {
+ return has_overflow32(val);
+ }
+
// Return true if VAL (stored in a uint64_t) has overflowed both a
// signed and an unsigned value. E.g.,
// Bits<8>::has_signed_unsigned_overflow would check -128 <= VAL <
diff --git a/gold/x86_64.cc b/gold/x86_64.cc
index 007af1d..8f53bfc 100644
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@@ -2480,23 +2480,6 @@ 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
- // 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)
- {
- section_size_type stype;
- const unsigned char* view = object->section_contents(data_shndx,
- &stype, true);
- if (view[reloc.get_r_offset() - 2] == 0x8b)
- break;
- }
-
-
// The symbol requires a GOT entry.
unsigned int r_sym = elfcpp::elf_r_sym<size>(reloc.get_r_info());
@@ -2906,21 +2889,6 @@ 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);
- // 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.
- if (r_type == elfcpp::R_X86_64_GOTPCREL
- && reloc.get_r_offset() >= 2
- && Target_x86_64<size>::can_convert_mov_to_lea(gsym))
- {
- section_size_type stype;
- const unsigned char* view = object->section_contents(data_shndx,
- &stype, true);
- if (view[reloc.get_r_offset() - 2] == 0x8b)
- break;
- }
-
if (gsym->final_value_is_known())
{
// For a STT_GNU_IFUNC symbol we want the PLT address.
@@ -3543,7 +3511,8 @@ Target_x86_64<size>::Relocate::relocate(
// mov foo@GOTPCREL(%rip), %reg
// to lea foo(%rip), %reg.
// if possible.
- if (rela.get_r_offset() >= 2
+ if (!Bits<32>::has_overflow(psymval->value(object, addend) - address)
+ && rela.get_r_offset() >= 2
&& view[-2] == 0x8b
&& ((gsym == NULL && !psymval->is_ifunc_symbol())
|| (gsym != NULL
Is it ok?
--
WBR,
Andrew