Home Original page

RFC: Unify overflow checking

Daniel Jacobowitz drow@false.org
Tue Apr 22 18:33:00 GMT 2008 
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;
More information about the Binutils mailing list