Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

2015-02-11 Thread Mike Stump
On Feb 11, 2015, at 4:24 AM, Marek Polacek pola...@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  pola...@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.

Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

2015-02-11 Thread Marek Polacek
On Wed, Feb 11, 2015 at 09:44:24AM -0800, Mike Stump wrote:
 On Feb 11, 2015, at 4:24 AM, Marek Polacek pola...@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  pola...@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.

Yeah, it works, I don't know why I changed it for the C test case, but
not for the C++ one.

 Ok with the above fixes, if applicable.

Thanks.


2015-02-11  Marek Polacek  pola...@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..05e049e 100644
--- gcc/testsuite/g++.dg/ubsan/shift-1.C
+++ gcc/testsuite/g++.dg/ubsan/shift-1.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 is not a constant expression  { xfail { 
*-*-* } } } */
+case -1  -1: /* { dg-error is not a constant expression  { xfail { 
*-*-* } } } */
+case 1  -1: /* { dg-error is not a constant expression  { xfail { 
*-*-* } } } */
+case -1  -1: /* { dg-error is not a constant expression  { xfail { 
*-*-* } } } */
+  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 { 
*-*-* } } } */
+case 1  200: /* { dg-error is not a constant expression  { xfail { 
*-*-* } } } */
+  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;
+}

Marek


Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

2015-02-11 Thread Marek Polacek
On Sun, Jan 25, 2015 at 12:07:46PM -0800, Mike Stump wrote:
 On Dec 1, 2014, at 2:52 AM, Marek Polacek pola...@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  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: 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  pola...@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 } */
+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;
+}

Marek


Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

2015-01-25 Thread Mike Stump
On Dec 1, 2014, at 2:52 AM, Marek Polacek pola...@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  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: 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.

Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

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

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 } */

Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

2014-12-01 Thread Jason Merrill

OK, thanks.

Jason


Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

2014-11-30 Thread Jason Merrill

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.

Jason



Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

2014-11-27 Thread Marek Polacek
On Wed, Nov 26, 2014 at 12:03:45PM -0500, Jason Merrill wrote:
 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.

Ok.
 
 -/* { 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.

Just to check that I understand correctly, the point is that with
delayed folding we'd give an error here?

I added the xfails.

 +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.

Fixed.

Bootstrapped/regtested on ppc64-linux.

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

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

Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

2014-11-26 Thread Jason Merrill

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



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

2014-11-20 Thread Marek Polacek
This patch fixes a bunch of ICEs related to C++14 constexprs and
-fsanitize=undefined.  We should ignore ubsan internal functions
and ubsan builtins in constexpr functions in cxx_eval_call_expression.

Also add proper printing of internal functions into the C++ printer.

Bootstrapped/regtested on ppc64-linux, ok for trunk?

2014-11-20  Marek Polacek  pola...@redhat.com

PR sanitizer/63956
* constexpr.c: Include ubsan.h.
(cxx_eval_call_expression): Bail out for IFN_UBSAN_{NULL,BOUNDS}
internal functions and for ubsan builtins in constexpr functions.
* error.c: Include internal-fn.h.
(dump_expr): Add printing of internal functions.

* g++.dg/ubsan/pr63956.C: New test.

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 2678223..684e36f 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 (!current_function_decl  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/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 

Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

2014-11-20 Thread Jakub Jelinek
On Thu, Nov 20, 2014 at 06:14:52PM +0100, Marek Polacek wrote:
 This patch fixes a bunch of ICEs related to C++14 constexprs and
 -fsanitize=undefined.  We should ignore ubsan internal functions
 and ubsan builtins in constexpr functions in cxx_eval_call_expression.
 
 Also add proper printing of internal functions into the C++ printer.
 
 Bootstrapped/regtested on ppc64-linux, ok for trunk?

I'd like Jason to review this.  But a few nits:

 @@ -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 (!current_function_decl  is_ubsan_builtin_p (fun))
 +return void_node;
 +

I don't understand the !current_function_decl here.

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.

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.
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?

Jakub


Re: [PATCH] Fix ubsan and C++14 constexpr ICEs (PR sanitizer/63956)

2014-11-20 Thread Marek Polacek
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;