diff mbox

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

Message ID 20150211122414.GQ28333@redhat.com
State New
Headers show

Commit Message

Marek Polacek Feb. 11, 2015, 12:24 p.m. UTC 
On Sun, Jan 25, 2015 at 12:07:46PM -0800, Mike Stump wrote:
> 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.

Sorry for not replying sooner.

The following patch splits the test into C and C++ test cases, so
hopefully fixing the issue.  Ok for trunk?

2015-02-11  Marek Polacek  <polacek@redhat.com>

	* g++.dg/ubsan/shift-1.C: New test.
	* gcc.dg/ubsan/c-shift-2.c: New test.
	* c-c++-common/ubsan/shift-5.c: Remove file.


	Marek

Comments

Mike Stump Feb. 11, 2015, 5:44 p.m. UTC | #1 
On Feb 11, 2015, at 4:24 AM, Marek Polacek <polacek@redhat.com> wrote:
> 
> The following patch splits the test into C and C++ test cases, so
> hopefully fixing the issue.  Ok for trunk?
> 
> 2015-02-11  Marek Polacek  <polacek@redhat.com>
> 
> 	* g++.dg/ubsan/shift-1.C: New test.
> 	* gcc.dg/ubsan/c-shift-2.c: New test.
> 	* c-c++-common/ubsan/shift-5.c: Remove file.
> 
> diff --git gcc/testsuite/g++.dg/ubsan/shift-1.C gcc/testsuite/g++.dg/ubsan/shift-1.C
> index e69de29..8a71279 100644
> --- gcc/testsuite/g++.dg/ubsan/shift-1.C
> +++ gcc/testsuite/g++.dg/ubsan/shift-1.C
> @@ -0,0 +1,37 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=shift -w" } */
> +/* { dg-shouldfail "ubsan" } */
> +
> +int
> +foo (int x)
> +{
> +  /* None of the following should pass.  */
> +  switch (x)
> +    {
> +    case 1 >> -1:
> +/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 11 } */

Never include line numbers, unless there is no other way.  Here, I think you can drop it, and merely ensure this is on the right line?

An example from gcc.dg:

int g2(int a; __attribute__((unused))); /* { dg-error "just a forward declaration" "no parms" { xfail *-*-* } } */

I’m hoping that style will work.

> +    case -1 >> -1:
> +/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 13 } */

Likewise.

> +    case 1 << -1:
> +/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 15 } */

Likewise.

> +    case -1 << -1:
> +/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 17 } */

Likewise.

> +      return 1;
> +    }
> +  return 0;
> +}
> +
> +int
> +bar (int x)
> +{
> +  /* None of the following should pass.  */
> +  switch (x)
> +    {
> +    case -1 >> 200:
> +/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 30 } */

Likewise.

> +    case 1 << 200:
> +/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 32 } */

Likewise.

Ok with the above fixes, if applicable.
diff mbox

Patch

diff --git gcc/testsuite/g++.dg/ubsan/shift-1.C gcc/testsuite/g++.dg/ubsan/shift-1.C
index e69de29..8a71279 100644
--- gcc/testsuite/g++.dg/ubsan/shift-1.C
+++ gcc/testsuite/g++.dg/ubsan/shift-1.C
@@ -0,0 +1,37 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift -w" } */
+/* { dg-shouldfail "ubsan" } */
+
+int
+foo (int x)
+{
+  /* None of the following should pass.  */
+  switch (x)
+    {
+    case 1 >> -1:
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 11 } */
+    case -1 >> -1:
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 13 } */
+    case 1 << -1:
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 15 } */
+    case -1 << -1:
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 17 } */
+      return 1;
+    }
+  return 0;
+}
+
+int
+bar (int x)
+{
+  /* None of the following should pass.  */
+  switch (x)
+    {
+    case -1 >> 200:
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 30 } */
+    case 1 << 200:
+/* { dg-error "is not a constant expression" "" { xfail { *-*-* } } 32 } */
+      return 1;
+    }
+  return 0;
+}
diff --git gcc/testsuite/gcc.dg/ubsan/c-shift-2.c gcc/testsuite/gcc.dg/ubsan/c-shift-2.c
index e69de29..beb0dbe 100644
--- gcc/testsuite/gcc.dg/ubsan/c-shift-2.c
+++ gcc/testsuite/gcc.dg/ubsan/c-shift-2.c
@@ -0,0 +1,31 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift -w" } */
+/* { dg-shouldfail "ubsan" } */
+
+int
+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" } */
+    case -1 >> -1: /* { dg-error "case label does not reduce to an integer constant" } */
+    case 1 << -1: /* { dg-error "case label does not reduce to an integer constant" } */
+    case -1 << -1: /* { dg-error "case label does not reduce to an integer constant" } */
+      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" } */
+    case 1 << 200: /* { dg-error "case label does not reduce to an integer constant" } */
+      return 1;
+    }
+  return 0;
+}