[PATCH] Add -fsanitize=shift{,-base,-exponent} support
Jakub Jelinek
jakub@redhat.com
Tue Nov 8 06:54:00 GMT 2016
Hi!
In the libsanitizer merge Maxim posted I've noticed that llvm split
the -fsanitize=shift option into suboption (at least one really weirdly
named, I have never seen the shift count being called exponent), this
patch implements the same with the same option names for gcc.
The interesting part is if the two suboptions disagree on
-fsanitize-recover, then we need to emit two checks and two calls (one with
_abort, one without).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-11-08 Jakub Jelinek <jakub@redhat.com>
* flag-types.h (enum sanitize_code): Add SANITIZE_SHIFT_BASE
and SANITIZE_SHIFT_EXPONENT, change SANITIZE_SHIFT to bitwise
or of them, renumber other enumerators.
* opts.c (sanitizer_opts): Add shift-base and shift-exponent.
* doc/invoke.texi: Document -fsanitize=shift-base and
-fsanitize-shift-exponent, document -fsanitize=shift as
having those 2 suboptions.
c-family/
* c-ubsan.c (ubsan_instrument_shift): Handle split
-fsanitize=shift-base and -fsanitize=shift-exponent.
testsuite/
* gcc.dg/ubsan/c99-shift-3.c: New test.
* gcc.dg/ubsan/c99-shift-4.c: New test.
* gcc.dg/ubsan/c99-shift-5.c: New test.
* gcc.dg/ubsan/c99-shift-6.c: New test.
--- gcc/flag-types.h.jj 2016-11-07 14:04:29.129755670 +0100
+++ gcc/flag-types.h 2016-11-07 18:50:54.989652650 +0100
@@ -211,24 +211,26 @@ enum sanitize_code {
/* LeakSanitizer. */
SANITIZE_LEAK = 1UL << 4,
/* UndefinedBehaviorSanitizer. */
- SANITIZE_SHIFT = 1UL << 5,
- SANITIZE_DIVIDE = 1UL << 6,
- SANITIZE_UNREACHABLE = 1UL << 7,
- SANITIZE_VLA = 1UL << 8,
- SANITIZE_NULL = 1UL << 9,
- SANITIZE_RETURN = 1UL << 10,
- SANITIZE_SI_OVERFLOW = 1UL << 11,
- SANITIZE_BOOL = 1UL << 12,
- SANITIZE_ENUM = 1UL << 13,
- SANITIZE_FLOAT_DIVIDE = 1UL << 14,
- SANITIZE_FLOAT_CAST = 1UL << 15,
- SANITIZE_BOUNDS = 1UL << 16,
- SANITIZE_ALIGNMENT = 1UL << 17,
- SANITIZE_NONNULL_ATTRIBUTE = 1UL << 18,
- SANITIZE_RETURNS_NONNULL_ATTRIBUTE = 1UL << 19,
- SANITIZE_OBJECT_SIZE = 1UL << 20,
- SANITIZE_VPTR = 1UL << 21,
- SANITIZE_BOUNDS_STRICT = 1UL << 22,
+ SANITIZE_SHIFT_BASE = 1UL << 5,
+ SANITIZE_SHIFT_EXPONENT = 1UL << 6,
+ SANITIZE_DIVIDE = 1UL << 7,
+ SANITIZE_UNREACHABLE = 1UL << 8,
+ SANITIZE_VLA = 1UL << 9,
+ SANITIZE_NULL = 1UL << 10,
+ SANITIZE_RETURN = 1UL << 11,
+ SANITIZE_SI_OVERFLOW = 1UL << 12,
+ SANITIZE_BOOL = 1UL << 13,
+ SANITIZE_ENUM = 1UL << 14,
+ SANITIZE_FLOAT_DIVIDE = 1UL << 15,
+ SANITIZE_FLOAT_CAST = 1UL << 16,
+ SANITIZE_BOUNDS = 1UL << 17,
+ SANITIZE_ALIGNMENT = 1UL << 18,
+ SANITIZE_NONNULL_ATTRIBUTE = 1UL << 19,
+ SANITIZE_RETURNS_NONNULL_ATTRIBUTE = 1UL << 20,
+ SANITIZE_OBJECT_SIZE = 1UL << 21,
+ SANITIZE_VPTR = 1UL << 22,
+ SANITIZE_BOUNDS_STRICT = 1UL << 23,
+ SANITIZE_SHIFT = SANITIZE_SHIFT_BASE | SANITIZE_SHIFT_EXPONENT,
SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE
| SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN
| SANITIZE_SI_OVERFLOW | SANITIZE_BOOL | SANITIZE_ENUM
--- gcc/opts.c.jj 2016-11-07 14:04:29.131755645 +0100
+++ gcc/opts.c 2016-11-07 18:50:54.990652637 +0100
@@ -1477,6 +1477,8 @@ const struct sanitizer_opts_s sanitizer_
SANITIZER_OPT (thread, SANITIZE_THREAD, false),
SANITIZER_OPT (leak, SANITIZE_LEAK, false),
SANITIZER_OPT (shift, SANITIZE_SHIFT, true),
+ SANITIZER_OPT (shift-base, SANITIZE_SHIFT_BASE, true),
+ SANITIZER_OPT (shift-exponent, SANITIZE_SHIFT_EXPONENT, true),
SANITIZER_OPT (integer-divide-by-zero, SANITIZE_DIVIDE, true),
SANITIZER_OPT (undefined, SANITIZE_UNDEFINED, true),
SANITIZER_OPT (unreachable, SANITIZE_UNREACHABLE, false),
--- gcc/doc/invoke.texi.jj 2016-11-07 14:04:28.853759109 +0100
+++ gcc/doc/invoke.texi 2016-11-07 18:50:54.998652536 +0100
@@ -10555,6 +10555,21 @@ at runtime. Current suboptions are:
This option enables checking that the result of a shift operation is
not undefined. Note that what exactly is considered undefined differs
slightly between C and C++, as well as between ISO C90 and C99, etc.
+This option has two suboptions, @option{-fsanitize=shift-base} and
+@option{-fsanitize=shift-exponent}.
+
+@item -fsanitize=shift-exponent
+@opindex fsanitize=shift-exponent
+This option enables checking that the second argument of a shift operation
+is not negative and is smaller than the precision of the promoted first
+argument.
+
+@item -fsanitize=shift-base
+@opindex fsanitize=shift-base
+If the second argument of a shift operation is within range, check that the
+result of a shift operation is not undefined. Note that what exactly is
+considered undefined differs slightly between C and C++, as well as between
+ISO C90 and C99, etc.
@item -fsanitize=integer-divide-by-zero
@opindex fsanitize=integer-divide-by-zero
--- gcc/c-family/c-ubsan.c.jj 2016-11-07 14:04:29.102756006 +0100
+++ gcc/c-family/c-ubsan.c 2016-11-07 18:50:54.991652625 +0100
@@ -130,7 +130,8 @@ ubsan_instrument_shift (location_t loc,
/* If this is not a signed operation, don't perform overflow checks.
Also punt on bit-fields. */
if (TYPE_OVERFLOW_WRAPS (type0)
- || GET_MODE_BITSIZE (TYPE_MODE (type0)) != TYPE_PRECISION (type0))
+ || GET_MODE_BITSIZE (TYPE_MODE (type0)) != TYPE_PRECISION (type0)
+ || (flag_sanitize & SANITIZE_SHIFT_BASE) == 0)
;
/* For signed x << y, in C99/C11, the following:
@@ -171,8 +172,27 @@ ubsan_instrument_shift (location_t loc,
/* In case we have a SAVE_EXPR in a conditional context, we need to
make sure it gets evaluated before the condition. */
t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op0), t);
- t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t,
- tt ? tt : integer_zero_node);
+
+ enum sanitize_code recover_kind = SANITIZE_SHIFT_EXPONENT;
+ tree else_t = void_node;
+ if (tt)
+ {
+ if ((flag_sanitize & SANITIZE_SHIFT_EXPONENT) == 0)
+ {
+ t = fold_build1 (TRUTH_NOT_EXPR, boolean_type_node, t);
+ t = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, t, tt);
+ recover_kind = SANITIZE_SHIFT_BASE;
+ }
+ else
+ {
+ if (flag_sanitize_undefined_trap_on_error
+ || ((!(flag_sanitize_recover & SANITIZE_SHIFT_EXPONENT))
+ == (!(flag_sanitize_recover & SANITIZE_SHIFT_BASE))))
+ t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, tt);
+ else
+ else_t = tt;
+ }
+ }
if (flag_sanitize_undefined_trap_on_error)
tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0);
@@ -185,7 +205,7 @@ ubsan_instrument_shift (location_t loc,
data = build_fold_addr_expr_loc (loc, data);
enum built_in_function bcode
- = (flag_sanitize_recover & SANITIZE_SHIFT)
+ = (flag_sanitize_recover & recover_kind)
? BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS
: BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS_ABORT;
tt = builtin_decl_explicit (bcode);
@@ -193,8 +213,22 @@ ubsan_instrument_shift (location_t loc,
op1 = unshare_expr (op1);
tt = build_call_expr_loc (loc, tt, 3, data, ubsan_encode_value (op0),
ubsan_encode_value (op1));
+ if (else_t != void_node)
+ {
+ bcode = (flag_sanitize_recover & SANITIZE_SHIFT_BASE)
+ ? BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS
+ : BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS_ABORT;
+ tree else_tt = builtin_decl_explicit (bcode);
+ op0 = unshare_expr (op0);
+ op1 = unshare_expr (op1);
+ else_tt = build_call_expr_loc (loc, else_tt, 3, data,
+ ubsan_encode_value (op0),
+ ubsan_encode_value (op1));
+ else_t = fold_build3 (COND_EXPR, void_type_node, else_t,
+ else_tt, void_node);
+ }
}
- t = fold_build3 (COND_EXPR, void_type_node, t, tt, void_node);
+ t = fold_build3 (COND_EXPR, void_type_node, t, tt, else_t);
return t;
}
--- gcc/testsuite/gcc.dg/ubsan/c99-shift-3.c.jj 2016-11-07 18:54:33.916880052 +0100
+++ gcc/testsuite/gcc.dg/ubsan/c99-shift-3.c 2016-11-07 19:08:08.318566032 +0100
@@ -0,0 +1,18 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=shift-base -fno-sanitize=shift-exponent -w -std=c99" } */
+
+int
+main (void)
+{
+ int a = -42;
+ int b = -43;
+ volatile int c = 129;
+ int d = 1;
+ a << 1;
+ b << c;
+ a << 1;
+ d <<= 31;
+}
+/* { dg-output "left shift of negative value -42\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*left shift of negative value -42\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*left shift of 1 by 31 places cannot be represented in type 'int'" } */
--- gcc/testsuite/gcc.dg/ubsan/c99-shift-4.c.jj 2016-11-07 18:55:20.786286472 +0100
+++ gcc/testsuite/gcc.dg/ubsan/c99-shift-4.c 2016-11-07 19:16:59.049847278 +0100
@@ -0,0 +1,18 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=shift-exponent -fno-sanitize=shift-base -w -std=c99" } */
+
+int
+main (void)
+{
+ int a = -42;
+ int b = -43;
+ volatile int c = 129;
+ int d = 1;
+ b << c;
+ a << 1;
+ a << 1;
+ d <<= 31;
+ b << (c + 1);
+}
+/* { dg-output "shift exponent 129 is too large for \[^\n\r]*-bit type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*shift exponent 130 is too large for \[^\n\r]*-bit type 'int'" } */
--- gcc/testsuite/gcc.dg/ubsan/c99-shift-5.c.jj 2016-11-07 19:11:29.554018440 +0100
+++ gcc/testsuite/gcc.dg/ubsan/c99-shift-5.c 2016-11-07 19:16:11.676446989 +0100
@@ -0,0 +1,21 @@
+/* { dg-do run } */
+/* { dg-shouldfail "ubsan" } */
+/* { dg-options "-fsanitize=shift -fsanitize-recover=shift-base -fno-sanitize-recover=shift-exponent -w -std=c99" } */
+
+int
+main (void)
+{
+ int a = -42;
+ int b = -43;
+ volatile int c = 129;
+ int d = 1;
+ a << 1;
+ a << 1;
+ d <<= 31;
+ b << c;
+ a << 1;
+}
+/* { dg-output "left shift of negative value -42\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*left shift of negative value -42\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*left shift of 1 by 31 places cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*shift exponent 129 is too large for \[^\n\r]*-bit type 'int'" } */
--- gcc/testsuite/gcc.dg/ubsan/c99-shift-6.c.jj 2016-11-07 19:14:21.190845651 +0100
+++ gcc/testsuite/gcc.dg/ubsan/c99-shift-6.c 2016-11-07 19:15:53.807673194 +0100
@@ -0,0 +1,18 @@
+/* { dg-do run } */
+/* { dg-shouldfail "ubsan" } */
+/* { dg-options "-fsanitize=shift -fsanitize-recover=shift-exponent -fno-sanitize-recover=shift-base -w -std=c99" } */
+
+int
+main (void)
+{
+ int a = -42;
+ int b = -43;
+ volatile int c = 129;
+ 1 << c;
+ 1 << (c + 1);
+ a << 1;
+ b << 1;
+}
+/* { dg-output "shift exponent 129 is too large for \[^\n\r]*-bit type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*shift exponent 130 is too large for \[^\n\r]*-bit type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*left shift of negative value -42\[^\n\r]*(\n|\r\n|\r)" } */
Jakub
More information about the Gcc-patches
mailing list