[PATCH][GOLD] Fix lower bound for number of bits.

[PATCH][GOLD] Fix lower bound for number of bits.

[Doug Kwan (關振德)]

[-- Attachment #1: Type: text/plain, Size: 811 bytes --]

Hi Ian,

    I noticed that you move the Bits class template  from arm.cc to
reloc.h recently.  I found that the minimum number of bits is 0 in
various methods, this is mostly likely my bug.  If bits == 0,
expressions like "1U << (bits - 1)" are undefined.  If changed lower
bound from 0 to 1 in all assertions in the Bits class template.  In
addition, I also change some related code in arm.cc.  This is tested
on both x86_64 and ARM.

-Doug


2012-02-02  Doug Kwan  <dougkwan@google.com>

        * arm.cc (Arm_relocate_functions::abs8,
        Arm_relocate_functions::abs16): Use
        Bits::has_signed_unsigned_overflow32.
        (Arm_relocate_functions::thm_abs8): Correct range of
        overflow check.
        * reloc.h (Bits class): Change minimum number of bits from 0 to 1
        in assertions.

[-- Attachment #2: patch-bits.txt --]
[-- Type: text/plain, Size: 3985 bytes --]

? gold/autom4te.cache
Index: gold/arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.145
diff -u -u -p -r1.145 arm.cc
--- gold/arm.cc	2 Feb 2012 20:21:17 -0000	1.145
+++ gold/arm.cc	2 Feb 2012 22:06:43 -0000
@@ -3183,8 +3183,7 @@ class Arm_relocate_functions : public Re
     elfcpp::Swap<8, big_endian>::writeval(wv, val);
 
     // R_ARM_ABS8 permits signed or unsigned results.
-    int signed_x = static_cast<int32_t>(x);
-    return ((signed_x < -128 || signed_x > 255)
+    return (Bits<8>::has_signed_unsigned_overflow32(x)
 	    ? This::STATUS_OVERFLOW
 	    : This::STATUS_OKAY);
   }
@@ -3203,10 +3202,7 @@ class Arm_relocate_functions : public Re
     Reltype x = psymval->value(object, addend);
     val = Bits<32>::bit_select32(val, x << 6, 0x7e0U);
     elfcpp::Swap<16, big_endian>::writeval(wv, val);
-
-    // R_ARM_ABS16 permits signed or unsigned results.
-    int signed_x = static_cast<int32_t>(x);
-    return ((signed_x < -32768 || signed_x > 65535)
+    return (Bits<5>::has_overflow32(x)
 	    ? This::STATUS_OVERFLOW
 	    : This::STATUS_OKAY);
   }
@@ -3245,8 +3241,7 @@ class Arm_relocate_functions : public Re
     elfcpp::Swap_unaligned<16, big_endian>::writeval(view, val);
 
     // R_ARM_ABS16 permits signed or unsigned results.
-    int signed_x = static_cast<int32_t>(x);
-    return ((signed_x < -32768 || signed_x > 65536)
+    return (Bits<16>::has_signed_unsigned_overflow32(x)
 	    ? This::STATUS_OVERFLOW
 	    : This::STATUS_OKAY);
   }
Index: gold/reloc.h
===================================================================
RCS file: /cvs/src/src/gold/reloc.h,v
retrieving revision 1.34
diff -u -u -p -r1.34 reloc.h
--- gold/reloc.h	28 Jan 2012 01:47:01 -0000	1.34
+++ gold/reloc.h	2 Feb 2012 22:06:43 -0000
@@ -724,11 +724,11 @@ class Bits
 {
  public:
   // Sign extend an n-bit unsigned integer stored in a uint32_t into
-  // an int32_t.  BITS must be between 0 and 32.
+  // an int32_t.  BITS must be between 1 and 32.
   static inline int32_t
   sign_extend32(uint32_t val)
   {
-    gold_assert(bits >= 0 && bits <= 32);
+    gold_assert(bits > 0 && bits <= 32);
     if (bits == 32)
       return static_cast<int32_t>(val);
     uint32_t mask = (~static_cast<uint32_t>(0)) >> (32 - bits);
@@ -745,7 +745,7 @@ class Bits
   static inline bool
   has_overflow32(uint32_t val)
   {
-    gold_assert(bits >= 0 && bits <= 32);
+    gold_assert(bits > 0 && bits <= 32);
     if (bits == 32)
       return false;
     int32_t max = (1 << (bits - 1)) - 1;
@@ -761,7 +761,7 @@ class Bits
   static inline bool
   has_signed_unsigned_overflow32(uint32_t val)
   {
-    gold_assert(bits >= 0 && bits <= 32);
+    gold_assert(bits > 0 && bits <= 32);
     if (bits == 32)
       return false;
     int32_t max = static_cast<int32_t>((1U << bits) - 1);
@@ -778,11 +778,11 @@ class Bits
   { return (a & ~mask) | (b & mask); }
 
   // Sign extend an n-bit unsigned integer stored in a uint64_t into
-  // an int64_t.  BITS must be between 0 and 64.
+  // an int64_t.  BITS must be between 1 and 64.
   static inline int64_t
   sign_extend(uint64_t val)
   {
-    gold_assert(bits >= 0 && bits <= 64);
+    gold_assert(bits > 0 && bits <= 64);
     if (bits == 64)
       return static_cast<int64_t>(val);
     uint64_t mask = (~static_cast<uint64_t>(0)) >> (64 - bits);
@@ -799,7 +799,7 @@ class Bits
   static inline bool
   has_overflow(uint64_t val)
   {
-    gold_assert(bits >= 0 && bits <= 64);
+    gold_assert(bits > 0 && bits <= 64);
     if (bits == 64)
       return false;
     int64_t max = (static_cast<int64_t>(1) << (bits - 1)) - 1;
@@ -815,7 +815,7 @@ class Bits
   static inline bool
   has_signed_unsigned_overflow64(uint64_t val)
   {
-    gold_assert(bits >= 0 && bits <= 64);
+    gold_assert(bits > 0 && bits <= 64);
     if (bits == 64)
       return false;
     int64_t max = static_cast<int64_t>((static_cast<uint64_t>(1) << bits) - 1);

Re: [PATCH][GOLD] Fix lower bound for number of bits.

[Ian Lance Taylor]

"Doug Kwan (關振德)" <dougkwan@google.com> writes:

> 2012-02-02  Doug Kwan  <dougkwan@google.com>
>
>         * arm.cc (Arm_relocate_functions::abs8,
>         Arm_relocate_functions::abs16): Use
>         Bits::has_signed_unsigned_overflow32.
>         (Arm_relocate_functions::thm_abs8): Correct range of
>         overflow check.
>         * reloc.h (Bits class): Change minimum number of bits from 0 to 1
>         in assertions.

This is OK.

Thanks.

Ian