This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR gold/18695
- From: Andrew Senkevich <andrew dot n dot senkevich at gmail dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Thu, 27 Aug 2015 21:30:52 +0300
- Subject: Re: [PATCH] PR gold/18695
- Authentication-results: sourceware.org; auth=none
- References: <CAMXFM3t6BrUdey=bSO0XKVsSGuDs5uM9sEywyCGV1rm_hk9Tmw at mail dot gmail dot com> <CAMe9rOoPftkXp_47m4XD9iN0K7gbhGJ6XocBqcDCwVLWHue_cQ at mail dot gmail dot com>
> Shouldn't there be generic address overflow checks which can be used
> by all targets?
As for me I am not sure it is really needed.
Here is new version of the patch.
2015-08-27 Andrew Senkevich <andrew.senkevich@intel.com>
gold/
PR gold/18695
* x86_64.cc (Target_x86_64::Relocate::relocate): Added overflow checks
for 32-bit relocations.
(x86_64_overflow_check): New class.
diff --git a/gold/x86_64.cc b/gold/x86_64.cc
old mode 100644
new mode 100755
index 007af1d..0d0928f
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@@ -3320,6 +3320,32 @@ Target_x86_64<size>::do_finalize_sections(
}
}
+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;
+ }
+};
+
// Perform a relocation.
template<int size>
@@ -3431,24 +3457,38 @@ Target_x86_64<size>::Relocate::relocate(
break;
case elfcpp::R_X86_64_32:
- // FIXME: we need to verify that value + addend fits into 32 bits:
- // uint64_t x = value + addend;
- // x == static_cast<uint64_t>(static_cast<uint32_t>(x))
- // Likewise for other <=32-bit relocations (but see R_X86_64_32S).
- Relocate_functions<size, false>::rela32(view, object, psymval, addend);
+ {
+ Relocate_functions<size, false>::rela32(view, object, psymval, addend);
+ typename elfcpp::Elf_types<size>::Elf_Addr value;
+ value = psymval->value(object, addend) + addend;
+ if (x86_64_overflow_check<size, 32>::has_overflow_unsigned(value))
+ gold_error_at_location(relinfo, relnum, rela.get_r_offset(),
+ _("relocation overflow"));
+ }
break;
case elfcpp::R_X86_64_32S:
- // FIXME: we need to verify that value + addend fits into 32 bits:
- // int64_t x = value + addend; // note this quantity is signed!
- // x == static_cast<int64_t>(static_cast<int32_t>(x))
- Relocate_functions<size, false>::rela32(view, object, psymval, addend);
+ {
+ Relocate_functions<size, false>::rela32(view, object, psymval, addend);
+ typename elfcpp::Elf_types<size>::Elf_Addr value;
+ value = psymval->value(object, addend) + addend;
+ if (x86_64_overflow_check<size, 32>::has_overflow_signed(value))
+ gold_error_at_location(relinfo, relnum, rela.get_r_offset(),
+ _("relocation overflow"));
+ }
break;
case elfcpp::R_X86_64_PC32:
case elfcpp::R_X86_64_PC32_BND:
- Relocate_functions<size, false>::pcrela32(view, object, psymval, addend,
- address);
+ {
+ Relocate_functions<size, false>::pcrela32(view, object, psymval, addend,
+ address);
+ 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))
+ gold_error_at_location(relinfo, relnum, rela.get_r_offset(),
+ _("relocation overflow"));
+ }
break;
case elfcpp::R_X86_64_16:
@@ -3471,17 +3511,24 @@ Target_x86_64<size>::Relocate::relocate(
case elfcpp::R_X86_64_PLT32:
case elfcpp::R_X86_64_PLT32_BND:
- gold_assert(gsym == NULL
- || gsym->has_plt_offset()
- || gsym->final_value_is_known()
- || (gsym->is_defined()
- && !gsym->is_from_dynobj()
- && !gsym->is_preemptible()));
- // Note: while this code looks the same as for R_X86_64_PC32, it
- // behaves differently because psymval was set to point to
- // the PLT entry, rather than the symbol, in Scan::global().
- Relocate_functions<size, false>::pcrela32(view, object, psymval, addend,
- address);
+ {
+ gold_assert(gsym == NULL
+ || gsym->has_plt_offset()
+ || gsym->final_value_is_known()
+ || (gsym->is_defined()
+ && !gsym->is_from_dynobj()
+ && !gsym->is_preemptible()));
+ // Note: while this code looks the same as for R_X86_64_PC32, it
+ // behaves differently because psymval was set to point to
+ // the PLT entry, rather than the symbol, in Scan::global().
+ Relocate_functions<size, false>::pcrela32(view, object, psymval, addend,
+ address);
+ 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))
+ gold_error_at_location(relinfo, relnum, rela.get_r_offset(),
+ _("relocation overflow"));
+ }
break;
case elfcpp::R_X86_64_PLTOFF64:
--
WBR,
Andrew