diff mbox series

[03/12] enum: fix UB when rshifting by full width

Message ID 20180908000046.65842-4-luc.vanoostenryck@gmail.com (mailing list archive)
State Mainlined, archived
Headers show
Series fixes for enum's base type | expand

Commit Message

Luc Van Oostenryck Sept. 8, 2018, midnight UTC
Shifting by an amount greater or equal than the width
of the type is Undefined Behaviour. In the present case,
when type_is_ok() is called with a type as wide as an
ullong (64 bits here), the bounds are shifted by 64
which is UB and at execution (on x86) the value is simply
unchanged (since the shift is done with the amount modulo 63).

This, of course, doesn't give the expected result and
as consequence valid enums can have an invalid base type
(bad_ctype).

Fix this by doing the shift with a small helper which
return 0 if the amount is equal to the maximum width.

NB. Doing the shift in two steps could also be a solution,
  as maybe some clever trick, but since this code is in
  no way critical performance-wise, the solution here
  has the merit to be very explicit.

Fixes: b598c1d75a9c455c85a894172329941300fcfb9f
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c                    | 17 +++++++++++++++--
 validation/bug-rshift-ub.c |  1 -
 2 files changed, 15 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/parse.c b/parse.c
index 00b8ebbd1..560c582f4 100644
--- a/parse.c
+++ b/parse.c
@@ -805,6 +805,19 @@  static void lower_boundary(Num *n, Num *v)
 		n->y = v->y;
 }
 
+///
+// safe right shift
+//
+// This allow to use a shift amount as big (or bigger)
+// than the width of the value to be shifted, in which case
+// the result is, of course, 0.
+static unsigned long long rshift(unsigned long long val, unsigned int n)
+{
+	if (n >= (sizeof(val) * 8))
+		return 0;
+	return val >> n;
+}
+
 static int type_is_ok(struct symbol *type, Num *upper, Num *lower)
 {
 	int shift = type->bit_size;
@@ -812,9 +825,9 @@  static int type_is_ok(struct symbol *type, Num *upper, Num *lower)
 
 	if (!is_unsigned)
 		shift--;
-	if (upper->x == 0 && upper->y >> shift)
+	if (upper->x == 0 && rshift(upper->y, shift))
 		return 0;
-	if (lower->x == 0 || (!is_unsigned && (~lower->y >> shift) == 0))
+	if (lower->x == 0 || (!is_unsigned && rshift(~lower->y, shift) == 0))
 		return 1;
 	return 0;
 }
diff --git a/validation/bug-rshift-ub.c b/validation/bug-rshift-ub.c
index 3a5f3a577..7654abbd8 100644
--- a/validation/bug-rshift-ub.c
+++ b/validation/bug-rshift-ub.c
@@ -6,7 +6,6 @@  static enum a a = A;
 
 /*
  * check-name: bug-rshift-ub
- * check-known-to-fail
  * check-description:
  *	This test trigger(ed) a bug on x86 caused by a
  *	full width shift (which is UB), expecting to get