This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RFC: Unify overflow checking
- From: Daniel Jacobowitz <drow at false dot org>
- To: binutils at sourceware dot org
- Date: Tue, 22 Apr 2008 10:07:55 -0400
- Subject: RFC: Unify overflow checking
Does anyone remember why bfd_check_overflow and _bfd_relocate_contents
have different overflow checks? It looks to me as if the ones in
_bfd_relocate_contents are always appropriate, and bfd_check_overflow
is simply stale. This (untested so far) patch unifies them.
--
Daniel Jacobowitz
CodeSourcery
2008-04-22 Daniel Jacobowitz <dan@codesourcery.com>
* reloc.c (bfd_check_overflow): Update from _bfd_relocate_contents.
(_bfd_relocate_contents): Use bfd_check_overflow.
Index: reloc.c
===================================================================
RCS file: /cvs/src/src/bfd/reloc.c,v
retrieving revision 1.174
diff -u -p -r1.174 reloc.c
--- reloc.c 16 Apr 2008 08:51:18 -0000 1.174
+++ reloc.c 22 Apr 2008 14:06:27 -0000
@@ -495,7 +495,7 @@ bfd_check_overflow (enum complain_overfl
unsigned int addrsize,
bfd_vma relocation)
{
- bfd_vma fieldmask, addrmask, signmask, ss, a;
+ bfd_vma fieldmask, addrmask, signmask, ss, a, b, sum;
bfd_reloc_status_type flag = bfd_reloc_ok;
/* Note: BITSIZE should always be <= ADDRSIZE, but in case it's not,
@@ -506,6 +506,7 @@ bfd_check_overflow (enum complain_overfl
signmask = ~fieldmask;
addrmask = N_ONES (addrsize) | fieldmask;
a = (relocation & addrmask) >> rightshift;;
+ b = (x & howto->src_mask & addrmask) >> bitpos;
switch (how)
{
@@ -527,11 +528,53 @@ bfd_check_overflow (enum complain_overfl
ss = a & signmask;
if (ss != 0 && ss != ((addrmask >> rightshift) & signmask))
flag = bfd_reloc_overflow;
+
+ /* We only need this next bit of code if the sign bit of B
+ is below the sign bit of A. This would only happen if
+ SRC_MASK had fewer bits than BITSIZE. Note that if
+ SRC_MASK has more bits than BITSIZE, we can get into
+ trouble; we would need to verify that B is in range, as
+ we do for A above. */
+ ss = ((~howto->src_mask) >> 1) & howto->src_mask;
+ ss >>= bitpos;
+
+ /* Set all the bits above the sign bit. */
+ b = (b ^ ss) - ss;
+
+ /* Now we can do the addition. */
+ sum = a + b;
+
+ /* See if the result has the correct sign. Bits above the
+ sign bit are junk now; ignore them. If the sum is
+ positive, make sure we did not have all negative inputs;
+ if the sum is negative, make sure we did not have all
+ positive inputs. The test below looks only at the sign
+ bits, and it really just
+ SIGN (A) == SIGN (B) && SIGN (A) != SIGN (SUM)
+
+ We mask with addrmask here to explicitly allow an address
+ wrap-around. The Linux kernel relies on it, and it is
+ the only way to write assembler code which can run when
+ loaded at a location 0x80000000 away from the location at
+ which it is linked. */
+ if (((~(a ^ b)) & (a ^ sum)) & signmask & addrmask)
+ flag = bfd_reloc_overflow;
break;
case complain_overflow_unsigned:
- /* We have an overflow if the address does not fit in the field. */
- if ((a & signmask) != 0)
+ /* Checking for an unsigned overflow is relatively easy:
+ trim the addresses and add, and trim the result as well.
+ Overflow is normally indicated when the result does not
+ fit in the field. However, we also need to consider the
+ case when, e.g., fieldmask is 0x7fffffff or smaller, an
+ input is 0x80000000, and bfd_vma is only 32 bits; then we
+ will get sum == 0, but there is an overflow, since the
+ inputs did not fit in the field. Instead of doing a
+ separate test, we can check for this by or-ing in the
+ operands when testing for the sum overflowing its final
+ field. */
+ sum = (a + b) & addrmask;
+ if ((a | b | sum) & signmask)
flag = bfd_reloc_overflow;
break;
@@ -1424,92 +1467,10 @@ _bfd_relocate_contents (reloc_howto_type
in a type larger than bfd_vma, which would be inefficient. */
flag = bfd_reloc_ok;
if (howto->complain_on_overflow != complain_overflow_dont)
- {
- bfd_vma addrmask, fieldmask, signmask, ss;
- bfd_vma a, b, sum;
-
- /* Get the values to be added together. For signed and unsigned
- relocations, we assume that all values should be truncated to
- the size of an address. For bitfields, all the bits matter.
- See also bfd_check_overflow. */
- fieldmask = N_ONES (howto->bitsize);
- signmask = ~fieldmask;
- addrmask = N_ONES (bfd_arch_bits_per_address (input_bfd)) | fieldmask;
- a = (relocation & addrmask) >> rightshift;
- b = (x & howto->src_mask & addrmask) >> bitpos;
-
- switch (howto->complain_on_overflow)
- {
- case complain_overflow_signed:
- /* If any sign bits are set, all sign bits must be set.
- That is, A must be a valid negative address after
- shifting. */
- signmask = ~(fieldmask >> 1);
- /* Fall thru */
-
- case complain_overflow_bitfield:
- /* Much like the signed check, but for a field one bit
- wider. We allow a bitfield to represent numbers in the
- range -2**n to 2**n-1, where n is the number of bits in the
- field. Note that when bfd_vma is 32 bits, a 32-bit reloc
- can't overflow, which is exactly what we want. */
- ss = a & signmask;
- if (ss != 0 && ss != ((addrmask >> rightshift) & signmask))
- flag = bfd_reloc_overflow;
-
- /* We only need this next bit of code if the sign bit of B
- is below the sign bit of A. This would only happen if
- SRC_MASK had fewer bits than BITSIZE. Note that if
- SRC_MASK has more bits than BITSIZE, we can get into
- trouble; we would need to verify that B is in range, as
- we do for A above. */
- ss = ((~howto->src_mask) >> 1) & howto->src_mask;
- ss >>= bitpos;
-
- /* Set all the bits above the sign bit. */
- b = (b ^ ss) - ss;
-
- /* Now we can do the addition. */
- sum = a + b;
-
- /* See if the result has the correct sign. Bits above the
- sign bit are junk now; ignore them. If the sum is
- positive, make sure we did not have all negative inputs;
- if the sum is negative, make sure we did not have all
- positive inputs. The test below looks only at the sign
- bits, and it really just
- SIGN (A) == SIGN (B) && SIGN (A) != SIGN (SUM)
-
- We mask with addrmask here to explicitly allow an address
- wrap-around. The Linux kernel relies on it, and it is
- the only way to write assembler code which can run when
- loaded at a location 0x80000000 away from the location at
- which it is linked. */
- if (((~(a ^ b)) & (a ^ sum)) & signmask & addrmask)
- flag = bfd_reloc_overflow;
- break;
-
- case complain_overflow_unsigned:
- /* Checking for an unsigned overflow is relatively easy:
- trim the addresses and add, and trim the result as well.
- Overflow is normally indicated when the result does not
- fit in the field. However, we also need to consider the
- case when, e.g., fieldmask is 0x7fffffff or smaller, an
- input is 0x80000000, and bfd_vma is only 32 bits; then we
- will get sum == 0, but there is an overflow, since the
- inputs did not fit in the field. Instead of doing a
- separate test, we can check for this by or-ing in the
- operands when testing for the sum overflowing its final
- field. */
- sum = (a + b) & addrmask;
- if ((a | b | sum) & signmask)
- flag = bfd_reloc_overflow;
- break;
-
- default:
- abort ();
- }
- }
+ flag = bfd_check_overflow (howto->complain_on_overflow, howto->bitsize,
+ rightshift,
+ bfd_arch_bits_per_address (input_bfd),
+ relocation);
/* Put RELOCATION in the right bits. */
relocation >>= (bfd_vma) rightshift;