diff options
author | Luc Van Oostenryck <luc.vanoostenryck@gmail.com> | 2018-05-01 23:23:45 +0200 |
---|---|---|
committer | Luc Van Oostenryck <luc.vanoostenryck@gmail.com> | 2018-10-05 03:29:19 +0200 |
commit | bff011270111c3fcdc0b7a5490d7d0c92c0c50ea (patch) | |
tree | 0cdcac91a2a0151f513fa8a941dc609e4c903b71 | |
parent | a4d38cf8543f93d2456e19fc8b603d5843bf054a (diff) | |
download | sparse-bff011270111c3fcdc0b7a5490d7d0c92c0c50ea.tar.gz |
enum: fix UB when rshifting by full width
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>
-rw-r--r-- | parse.c | 17 | ||||
-rw-r--r-- | validation/bug-rshift-ub.c | 1 |
2 files changed, 15 insertions, 3 deletions
@@ -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 3a5f3a57..7654abbd 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 |