[PATCH] PR gold/18609

H.J. Lu hjl.tools@gmail.com
Mon Jul 20 12:38:00 GMT 2015


On Tue, Jul 14, 2015 at 8:48 AM, Andrew Senkevich
<andrew.n.senkevich@gmail.com> wrote:
> 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.

Please list names of the functions you changed and describe changes you made.

>  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 you remove those changes, won't it generate an unused GOT slot
when GOTPCREL relocation is converted to PC-relative relocation?

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

There are

  // Return true if VAL (stored in a uint32_t) has overflowed a signed
  // value with BITS bits.
  static inline bool
  has_overflow32(uint32_t val)
  {
    gold_assert(bits > 0 && bits <= 32);
    if (bits == 32)
      return false;
    int32_t max = (1 << (bits - 1)) - 1;
    int32_t min = -(1 << (bits - 1));
    int32_t as_signed = static_cast<int32_t>(val);
    return as_signed > max || as_signed < min;
  }

Bits<32>::has_overflow will always return false for x32.  I think we
need has_overflow_xxx similar to power.cc and use them to check
relocation overflow instead of the ones in reloc.h.



-- 
H.J.



More information about the Binutils mailing list