diff mbox

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

Message ID 20141201105255.GK15555@redhat.com
State New
Headers show

Commit Message

Marek Polacek Dec. 1, 2014, 10:52 a.m. UTC
On Sun, Nov 30, 2014 at 11:00:12PM -0500, Jason Merrill wrote:
> On 11/27/2014 08:57 AM, Marek Polacek wrote:
> >-/* { dg-error "is not a constant expression" "" { target c++ } 12 } */
> >+/* { dg-error "" "" { xfail { *-*-* } } 11 } */
> 
> Please keep the expected message.

Done in the below.

2014-12-01  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: Add xfails.
	* g++.dg/ubsan/div-by-zero-1.C: Don't use -w.  Add xfail.
	* g++.dg/ubsan/pr63956.C: New test.


	Marek

Comments

Jason Merrill Dec. 1, 2014, 2:39 p.m. UTC | #1
OK, thanks.

Jason
Mike Stump Jan. 25, 2015, 8:07 p.m. UTC | #2
On Dec 1, 2014, at 2:52 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Sun, Nov 30, 2014 at 11:00:12PM -0500, Jason Merrill wrote:
>> On 11/27/2014 08:57 AM, Marek Polacek wrote:
>>> -/* { dg-error "is not a constant expression" "" { target c++ } 12 } */
>>> +/* { dg-error "" "" { xfail { *-*-* } } 11 } */
>> 
>> Please keep the expected message.
> 
> Done in the below.
> 
> 2014-12-01  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: Add fails.

Do you see:

PASS: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 11)
XFAIL: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 11)
PASS: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 14)
XFAIL: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 14)
PASS: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 17)
XFAIL: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 17)
PASS: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 20)
XFAIL: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 20)
PASS: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 34)
XFAIL: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 34)
PASS: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 37)
XFAIL: c-c++-common/ubsan/shift-5.c   -O0   (test for errors, line 37)

on x86_64 on linux?

If so, this really isn’t cool.  You cannot have the same name as both pass and fail.  At the heart of regression analysis is the notion that no test that passed before now fails.  One can run contrib/compare_tests to see if a patch one is working on has any regressions in it, the beauty of the script is it will tell you in plain language if there are any regressions or not.  The standard for gcc is, no regressions.

Could you find a way to fix this?  Splitting into C and C++ test cases might be one way.  Fixing any expected failures might be another.
diff mbox

Patch

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index ef9ef70..45d5959 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,19 @@  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:
+	if (!ctx->quiet)
+	  error_at (loc, "call to internal function");
+	*non_constant_p = true;
+	return t;
+      }
+
   if (TREE_CODE (fun) != FUNCTION_DECL)
     {
       /* Might be a constexpr function pointer.  */
@@ -1171,6 +1185,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 5dcc149..ff26fb9 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.
 
@@ -2039,6 +2040,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..d779571 100644
--- gcc/testsuite/c-c++-common/ubsan/shift-5.c
+++ gcc/testsuite/c-c++-common/ubsan/shift-5.c
@@ -2,31 +2,41 @@ 
 /* { dg-options "-fsanitize=shift -w" } */
 /* { 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 } 11 } */
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 11 } */
     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 } 14 } */
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 14 } */
     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 } 17 } */
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 17 } */
     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 } 20 } */
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 20 } */
+      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 } 34 } */
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 34 } */
     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 } 37 } */
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 37 } */
       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..769cea6 100644
--- gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C
+++ gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C
@@ -1,10 +1,14 @@ 
 /* { dg-do compile } */
-/* { dg-options "-fsanitize=integer-divide-by-zero -w" } */
+/* { dg-options "-fsanitize=integer-divide-by-zero" } */
+
+/* TODO: We expect an error on the invalid case here, because that
+   must be a constant-expression.  This will be fixed when we have
+   proper delayed folding.  */
 
 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" } */
+    ;  /* { dg-error "division by zero" "" { xfail *-*-* } 10 } */
 }
diff --git gcc/testsuite/g++.dg/ubsan/pr63956.C gcc/testsuite/g++.dg/ubsan/pr63956.C
index e69de29..adfb55f 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 int k3 = fn3 (INT_MIN, -1); // { dg-error "overflow in constant expression" }
+
+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 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 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 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 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);
+
+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;
 }