Re: [PATCH] avoid undefined behavior due to oversized shifts

[nick clifton <nickc at redhat dot com>]

Hi Jan, Hi Nikolai,

> But that wouldn't eliminate the need for fixing the shifts, as the
> checks above don't entirely exclude the chunksz == sizeof(x) case.

Agreed.  How about this version instead ?


@@ -7917,31 +7917,46 @@ get_value (bfd_vma size,
           bfd *input_bfd,
           bfd_byte *location)
 {
+  int shift;
   bfd_vma x = 0;

+  /* Sanity checks.  */
+  if (chunksz > sizeof (x)
+      || size < chunksz
+      || size % chunksz != 0
+      || input_bfd == NULL
+      || location == NULL)
+    abort ();
+
+  if (chunksz == sizeof (x))
+    {
+      if (size > chunksz)
+       abort ();
+      shift = 0;
+    }
+  else
+    shift = 8 * chunksz;
+
   for (; size; size -= chunksz, location += chunksz)
     {
       switch (chunksz)
        {
-       default:
-       case 0:
-         abort ();
        case 1:
-         x = (x << (8 * chunksz)) | bfd_get_8 (input_bfd, location);
+         x = (x << shift) | bfd_get_8 (input_bfd, location);
          break;
        case 2:
-         x = (x << (8 * chunksz)) | bfd_get_16 (input_bfd, location);
+         x = (x << shift) | bfd_get_16 (input_bfd, location);
          break;
        case 4:
-         x = (x << (8 * chunksz)) | bfd_get_32 (input_bfd, location);
+         x = (x << shift) | bfd_get_32 (input_bfd, location);
          break;
-       case 8:
 #ifdef BFD64
-         x = (x << (8 * chunksz)) | bfd_get_64 (input_bfd, location);
-#else
-         abort ();
-#endif
+       case 8:
+         x = (x << shift) | bfd_get_64 (input_bfd, location);
          break;
+#endif
+       default:
+         abort ();
        }
     }
   return x;