On Thu, Nov 20, 2014 at 06:27:25PM +0100, Jakub Jelinek wrote: > On Thu, Nov 20, 2014 at 06:14:52PM +0100, Marek Polacek wrote: > > + if (!current_function_decl && is_ubsan_builtin_p (fun)) > > + return void_node; > > + > > I don't understand the !current_function_decl here. That is because I only wanted to ignore ubsan builtins in constexpr functions (in constexpr context), to not to break shift-5.c, where for C++ we expect error: '<ubsan routine call>' is not a constant expression
But that sounds dubious to me now, so I went ahead and did away with that. Which means that compile-time diagnostics is now the same no matter whether -fsanitize=undefined is on. > Also, looking at is_ubsan_builtin_p definition, I'd say > it should IMHO at least test DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL > before comparing the function name, you can declare > __builtin_ubsan_foobarbaz () and use it and it won't be a builtin. Um, ok. > As for the testcase, I'd like to understand if C++ FE should reject > the constexpr functions when used with arguments that trigger undefined > behavior. But certainly the behavior should not depend on whether > -fsanitize=undefined or not. With this patch we generate the same compile-time diagnostics with -fsanitize=undefined as without it in the new testcase. > Also, what is the reason for constexpr call flows off the end errors? > Shouldn't that be avoided if any error is found while interpreting the > function? Maybe. Short testcase: constexpr int foo (int i, int j) { if (i != 42) i /= j; return i; } constexpr int i = foo (2, 0); The reason is that we issue "flows off the end" if the result of a constexpr function is NULL_TREE - and in this case it was. Perhaps we should set ctx->quiet if an error is encountered during evaluation of a constexpr function? Here's updated patch. Thanks. 2014-11-20 Marek Polacek <pola...@redhat.com> PR sanitizer/63956 * ubsan.c (is_ubsan_builtin_p): Check also built-in class. cp/ * constexpr.c: Include ubsan.h. (cxx_eval_call_expression): Bail out for IFN_UBSAN_{NULL,BOUNDS} internal functions and for ubsan builtins. * error.c: Include internal-fn.h. (dump_expr): Add printing of internal functions. testsuite/ * c-c++-common/ubsan/shift-5.c: Don't use -w for C++. Change dg-error to dg-warning for C++. * g++.dg/ubsan/div-by-zero-1.C: Don't use -w. Change dg-error to dg-warning. * g++.dg/ubsan/pr63956.C: New test. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 2678223..32f07be 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. If not see #include "gimplify.h" #include "builtins.h" #include "tree-inline.h" +#include "ubsan.h" static bool verify_constant (tree, bool, bool *, bool *); #define VERIFY_CONSTANT(X) \ @@ -1151,6 +1152,16 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, constexpr_call *entry; bool depth_ok; + if (fun == NULL_TREE) + switch (CALL_EXPR_IFN (t)) + { + case IFN_UBSAN_NULL: + case IFN_UBSAN_BOUNDS: + return void_node; + default: + break; + } + if (TREE_CODE (fun) != FUNCTION_DECL) { /* Might be a constexpr function pointer. */ @@ -1171,6 +1182,10 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, } if (DECL_CLONED_FUNCTION_P (fun)) fun = DECL_CLONED_FUNCTION (fun); + + if (is_ubsan_builtin_p (fun)) + return void_node; + if (is_builtin_fn (fun)) return cxx_eval_builtin_function_call (ctx, t, addr, non_constant_p, overflow_p); diff --git gcc/cp/error.c gcc/cp/error.c index 76f86cb..09789ad 100644 --- gcc/cp/error.c +++ gcc/cp/error.c @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see #include "tree-pretty-print.h" #include "c-family/c-objc.h" #include "ubsan.h" +#include "internal-fn.h" #include <new> // For placement-new. @@ -2037,6 +2038,14 @@ dump_expr (cxx_pretty_printer *pp, tree t, int flags) tree fn = CALL_EXPR_FN (t); bool skipfirst = false; + /* Deal with internal functions. */ + if (fn == NULL_TREE) + { + pp_string (pp, internal_fn_name (CALL_EXPR_IFN (t))); + dump_call_expr_args (pp, t, flags, skipfirst); + break; + } + if (TREE_CODE (fn) == ADDR_EXPR) fn = TREE_OPERAND (fn, 0); diff --git gcc/testsuite/c-c++-common/ubsan/shift-5.c gcc/testsuite/c-c++-common/ubsan/shift-5.c index 6f9c52a..f8a88e0 100644 --- gcc/testsuite/c-c++-common/ubsan/shift-5.c +++ gcc/testsuite/c-c++-common/ubsan/shift-5.c @@ -1,32 +1,43 @@ /* { dg-do compile } */ -/* { dg-options "-fsanitize=shift -w" } */ +/* { dg-options "-fsanitize=shift -w" { target c } } */ +/* { dg-options "-fsanitize=shift" { target c++ } } */ /* { dg-shouldfail "ubsan" } */ -int x; int -foo (void) +foo (int x) { /* None of the following should pass. */ switch (x) { case 1 >> -1: -/* { dg-error "case label does not reduce to an integer constant" "" {target c } 12 } */ -/* { dg-error "is not a constant expression" "" { target c++ } 12 } */ +/* { dg-error "case label does not reduce to an integer constant" "" { target c } 12 } */ +/* { dg-warning "right shift count is negative" "" { target c++ } 12 } */ case -1 >> -1: -/* { dg-error "case label does not reduce to an integer constant" "" {target c } 15 } */ -/* { dg-error "is not a constant expression" "" { target c++ } 15 } */ +/* { dg-error "case label does not reduce to an integer constant" "" { target c } 15 } */ +/* { dg-warning "right shift count is negative" "" { target c++ } 15 } */ case 1 << -1: -/* { dg-error "case label does not reduce to an integer constant" "" {target c } 18 } */ -/* { dg-error "is not a constant expression" "" { target c++ } 18 } */ +/* { dg-error "case label does not reduce to an integer constant" "" { target c } 18 } */ +/* { dg-warning "left shift count is negative" "" { target c++ } 18 } */ case -1 << -1: -/* { dg-error "case label does not reduce to an integer constant" "" {target c } 21 } */ -/* { dg-error "is not a constant expression" "" { target c++ } 21 } */ +/* { dg-error "case label does not reduce to an integer constant" "" { target c } 21 } */ +/* { dg-warning "left shift count is negative" "" { target c++ } 21 } */ + return 1; + } + return 0; +} + +int +bar (int x) +{ + /* None of the following should pass. */ + switch (x) + { case -1 >> 200: -/* { dg-error "case label does not reduce to an integer constant" "" {target c } 24 } */ -/* { dg-error "is not a constant expression" "" { target c++ } 24 } */ +/* { dg-error "case label does not reduce to an integer constant" "" { target c } 35 } */ +/* { dg-warning "right shift count >= width of type" "" { target c++ } 35 } */ case 1 << 200: -/* { dg-error "case label does not reduce to an integer constant" "" {target c } 27 } */ -/* { dg-error "is not a constant expression" "" { target c++ } 27 } */ +/* { dg-error "case label does not reduce to an integer constant" "" { target c } 38 } */ +/* { dg-warning "left shift count >= width of type " "" { target c++ } 38 } */ return 1; } return 0; diff --git gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C index 88acfa1..369be2c 100644 --- gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C +++ gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C @@ -1,10 +1,10 @@ /* { dg-do compile } */ -/* { dg-options "-fsanitize=integer-divide-by-zero -w" } */ +/* { dg-options "-fsanitize=integer-divide-by-zero" } */ void foo (int i) { switch (i) - case 0 * (1 / 0): /* { dg-error "is not a constant expression" } */ + case 0 * (1 / 0): /* { dg-warning "division by zero" } */ ; } diff --git gcc/testsuite/g++.dg/ubsan/pr63956.C gcc/testsuite/g++.dg/ubsan/pr63956.C index e69de29..7bc0b77 100644 --- gcc/testsuite/g++.dg/ubsan/pr63956.C +++ gcc/testsuite/g++.dg/ubsan/pr63956.C @@ -0,0 +1,172 @@ +// PR sanitizer/63956 +// { dg-do compile } +// { dg-options "-std=c++14 -fsanitize=undefined,float-divide-by-zero,float-cast-overflow" } + +#define SA(X) static_assert((X),#X) +#define INT_MIN (-__INT_MAX__ - 1) + +constexpr int +fn1 (int a, int b) +{ + if (b != 2) + a <<= b; + return a; +} + +constexpr int i1 = fn1 (5, 3); +constexpr int i2 = fn1 (5, -2); +constexpr int i3 = fn1 (5, sizeof (int) * __CHAR_BIT__); +constexpr int i4 = fn1 (5, 256); +constexpr int i5 = fn1 (5, 2); +constexpr int i6 = fn1 (-2, 4); +constexpr int i7 = fn1 (0, 2); + +SA (i1 == 40); +SA (i5 == 5); +SA (i7 == 0); + +constexpr int +fn2 (int a, int b) +{ + if (b != 2) + a >>= b; + return a; +} + +constexpr int j1 = fn2 (4, 1); +constexpr int j2 = fn2 (4, -1); +constexpr int j3 = fn2 (10, sizeof (int) * __CHAR_BIT__); +constexpr int j4 = fn2 (1, 256); +constexpr int j5 = fn2 (5, 2); +constexpr int j6 = fn2 (-2, 4); +constexpr int j7 = fn2 (0, 4); + +SA (j1 == 2); +SA (j5 == 5); +SA (j7 == 0); + +constexpr int +fn3 (int a, int b) +{ + if (b != 2) + a = a / b; + return a; +} + +constexpr int k1 = fn3 (8, 4); +constexpr int k2 = fn3 (7, 0); // { dg-error "is not a constant expression|constexpr call flows off" } +constexpr int k3 = fn3 (INT_MIN, -1); // { dg-error "overflow in constant expression|constexpr call flows off" } + +SA (k1 == 2); + +constexpr float +fn4 (float a, float b) +{ + if (b != 2.0) + a = a / b; + return a; +} + +constexpr float l1 = fn4 (5.0, 3.0); +constexpr float l2 = fn4 (7.0, 0.0); // { dg-error "is not a constant expression|constexpr call flows off" } + +constexpr int +fn5 (const int *a, int b) +{ + if (b != 2) + b = a[b]; + return b; +} + +constexpr int m1[4] = { 1, 2, 3, 4 }; +constexpr int m2 = fn5 (m1, 3); +constexpr int m3 = fn5 (m1, 4); // { dg-error "array subscript out of bound|constexpr call flows off" } + +constexpr int +fn6 (const int &a, int b) +{ + if (b != 2) + b = a; + return b; +} + +constexpr int +fn7 (const int *a, int b) +{ + if (b != 3) + return fn6 (*a, b); + return 7; +} + +constexpr int n1 = 7; +constexpr int n2 = fn7 (&n1, 5); +constexpr int n3 = fn7 ((const int *) 0, 8); // { dg-error "is not a constant expression|constexpr call flows off" } + +constexpr int +fn8 (int i) +{ + constexpr int g[10] = { }; + return g[i]; +} + +constexpr int o1 = fn8 (9); +constexpr int o2 = fn8 (10); // { dg-error "array subscript out of bound" } + +constexpr int +fn9 (int a, int b) +{ + if (b != 0) + return a + b; + return a; +} + +constexpr int p1 = fn9 (42, 7); +constexpr int p2 = fn9 (__INT_MAX__, 1); // { dg-error "overflow in constant expression" } +constexpr int p3 = fn9 (__INT_MAX__, -1); +constexpr int p4 = fn9 (INT_MIN, 1); +constexpr int p5 = fn9 (INT_MIN, -1); // { dg-error "overflow in constant expression" } + +SA (p1 == 49); +SA (p3 == __INT_MAX__ - 1); +SA (p4 == INT_MIN + 1); + +constexpr int +fn10 (int a, int b) +{ + if (b != 0) + return a * b; + return a; +} + +constexpr int q1 = fn10 (10, 10); +constexpr int q2 = fn10 (__INT_MAX__, 2); // { dg-error "overflow in constant expression" } +constexpr int q3 = fn10 (INT_MIN, 2); // { dg-error "overflow in constant expression" } +constexpr int q4 = fn10 (-1, -1); + +SA (q1 == 100); +SA (q4 == 1); + +constexpr int +fn11 (double d) +{ + int i = d; + if (i != 0) + return i; + return i * 2; +} + +constexpr int r1 = fn11 (3.4); +constexpr int r2 = fn11 (__builtin_inf ()); // { dg-error "overflow in constant expression|constexpr call flows off" } + +constexpr int +fn12 (int i) +{ + if (i == 42) + __builtin_unreachable (); // { dg-error "is not a constant expression" } + return i + 10; +} + +constexpr int s1 = fn12 (1); +constexpr int s2 = fn12 (42); // { dg-error "constexpr call flows off the end of the function" } + +SA (s1 == 11); diff --git gcc/ubsan.c gcc/ubsan.c index b3d5343..91a1382 100644 --- gcc/ubsan.c +++ gcc/ubsan.c @@ -653,6 +653,7 @@ bool is_ubsan_builtin_p (tree t) { return TREE_CODE (t) == FUNCTION_DECL + && DECL_BUILT_IN_CLASS (t) == BUILT_IN_NORMAL && strncmp (IDENTIFIER_POINTER (DECL_NAME (t)), "__builtin___ubsan_", 18) == 0; } Marek