diff mbox

Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

Message ID 20141120190404.GH29446@redhat.com
State New
Headers show

Commit Message

Marek Polacek Nov. 20, 2014, 7:04 p.m. UTC
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  <polacek@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.


	Marek

Comments

Jason Merrill Nov. 26, 2014, 5:03 p.m. UTC | #1
On 11/20/2014 02:04 PM, Marek Polacek wrote:
> +  if (fun == NULL_TREE)
> +    switch (CALL_EXPR_IFN (t))
> +      {
> +      case IFN_UBSAN_NULL:
> +      case IFN_UBSAN_BOUNDS:
> +	return void_node;
> +      default:
> +	break;
> +      }

Other IFNs should make the call non-constant.

> -/* { dg-error "is not a constant expression" "" { target c++ } 12 } */
> +/* { dg-warning "right shift count is negative" "" { target c++ } 12 } */

This should be an xfail (pending the delayed folding work) instead of a 
different test.

> +constexpr int n3 = fn7 ((const int *) 0, 8); // { dg-error "is not a constant expression|constexpr call flows off" }

The "flows off the end" error is a bug and should not be tested for. 
I'm going to check in a fix.

Jason
diff mbox

Patch

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;
 }