This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PING^2] [PATCH] PR gold/18609
- From: Cary Coutant <ccoutant at gmail dot com>
- To: Andrew Senkevich <andrew dot n dot senkevich at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Fri, 5 Feb 2016 09:38:51 -0800
- Subject: Re: [PING^2] [PATCH] PR gold/18609
- Authentication-results: sourceware.org; auth=none
- References: <CAMXFM3vBLkgF71DWiA42pxg4FLq8Te5JyOCr2Nwk8XmSEhmdVg at mail dot gmail dot com>
> +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;
> + }
> +};
Use the functions from the Bits class in reloc.h.
> - // 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))
We shouldn't be looking at the section name at all. That doesn't
really implement what the comment says, anyway.
The overflow check is only necessary for size == 64, right? You can
use Bits<32>::has_overflow(value).
> @@ -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))
Likewise.
> - 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
Likewise.
-cary