Re: [PATCH] integer overflow checking builtins in constant expressions

[Jason Merrill]

OK.

Jason

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Tue, Jun 07, 2016 at 03:56:43PM -0600, Martin Sebor wrote:
> >+The built-in functions promote the first two operands into infinite 
> >precision signed type
> >+and perform addition on those promoted operands. The result is then
> >+cast to the type the third argument.
> 
> The above is missing an "of" (it should read "type of the third
> argument".)

Thanks for spotting that, fixed below.
Additionally, I've noticed that ATTR_NOTHROW_TYPEGENERIC_LEAF
for the 3 new builtins is wrong, unlike the other overflow builtins,
the *_p ones are also const.  It shouldn't change much, because we lower it
very early to the internal fns that are const, but at least from the POV of
the FEs it describes the builtins properly.

Ok for trunk?

2016-06-08  Martin Sebor  
Jakub Jelinek  

PR c++/70507
PR c/68120
* builtins.def (BUILT_IN_ADD_OVERFLOW_P, BUILT_IN_SUB_OVERFLOW_P,
BUILT_IN_MUL_OVERFLOW_P): New builtins.
* builtins.c: Include gimple-fold.h.
(fold_builtin_arith_overflow): Handle
BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
(fold_builtin_3): Likewise.
* doc/extend.texi (Integer Overflow Builtins): Document
__builtin_{add,sub,mul}_overflow_p.
gcc/c/
* c-typeck.c (convert_arguments): Don't promote last argument
of BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
gcc/cp/
* constexpr.c: Include gimple-fold.h.
(cxx_eval_internal_function): New function.
(cxx_eval_call_expression): Call it.
(potential_constant_expression_1): Handle integer arithmetic
overflow built-ins.
* tree.c (builtin_valid_in_constant_expr_p): Handle
BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
gcc/c-family/
* c-common.c (check_builtin_function_arguments): Handle
BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
gcc/testsuite/
* c-c++-common/builtin-arith-overflow-1.c: Add test cases.
* c-c++-common/builtin-arith-overflow-2.c: New test.
* g++.dg/ext/builtin-arith-overflow-1.C: New test.
* g++.dg/cpp0x/constexpr-arith-overflow.C: New test.
* g++.dg/cpp1y/constexpr-arith-overflow.C: New test.

--- gcc/builtins.def.jj 2016-06-06 14:40:35.619347198 +0200
+++ gcc/builtins.def2016-06-07 17:46:52.034206039 +0200
@@ -710,6 +710,9 @@ DEF_C94_BUILTIN(BUILT_IN_TOWUPPE
 DEF_GCC_BUILTIN(BUILT_IN_ADD_OVERFLOW, "add_overflow", BT_FN_BOOL_VAR, 
ATTR_NOTHROW_TYPEGENERIC_LEAF)
 DEF_GCC_BUILTIN(BUILT_IN_SUB_OVERFLOW, "sub_overflow", BT_FN_BOOL_VAR, 
ATTR_NOTHROW_TYPEGENERIC_LEAF)
 DEF_GCC_BUILTIN(BUILT_IN_MUL_OVERFLOW, "mul_overflow", BT_FN_BOOL_VAR, 
ATTR_NOTHROW_TYPEGENERIC_LEAF)
+DEF_GCC_BUILTIN(BUILT_IN_ADD_OVERFLOW_P, "add_overflow_p", 
BT_FN_BOOL_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
+DEF_GCC_BUILTIN(BUILT_IN_SUB_OVERFLOW_P, "sub_overflow_p", 
BT_FN_BOOL_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
+DEF_GCC_BUILTIN(BUILT_IN_MUL_OVERFLOW_P, "mul_overflow_p", 
BT_FN_BOOL_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
 /* Clang compatibility.  */
 DEF_GCC_BUILTIN(BUILT_IN_SADD_OVERFLOW, "sadd_overflow", 
BT_FN_BOOL_INT_INT_INTPTR, ATTR_NOTHROW_LEAF_LIST)
 DEF_GCC_BUILTIN(BUILT_IN_SADDL_OVERFLOW, "saddl_overflow", 
BT_FN_BOOL_LONG_LONG_LONGPTR, ATTR_NOTHROW_LEAF_LIST)
--- gcc/builtins.c.jj   2016-06-06 14:40:35.601347427 +0200
+++ gcc/builtins.c  2016-06-07 17:46:51.958207014 +0200
@@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.
 #include "rtl-chkp.h"
 #include "internal-fn.h"
 #include "case-cfn-macros.h"
+#include "gimple-fold.h"
 
 
 struct target_builtins default_target_builtins;
@@ -7943,18 +7944,28 @@ fold_builtin_unordered_cmp (location_t l
 /* Fold __builtin_{,s,u}{add,sub,mul}{,l,ll}_overflow, either into normal
arithmetics if it can never overflow, or into internal functions that
return both result of arithmetics and overflowed boolean flag in
-   a complex integer result, or some other check for overflow.  */
+   a complex integer result, or some other check for overflow.
+   Similarly fold __builtin_{add,sub,mul}_overflow_p to just the overflow
+   checking part of that.  */
 
 static tree
 fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 tree arg0, tree arg1, tree arg2)
 {
   enum internal_fn ifn = IFN_LAST;
-  tree type = TREE_TYPE (TREE_TYPE (arg2));
-  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
+  /* The code of the expression corresponding to the type-generic
+ built-in, or ERROR_MARK for the type-specific ones.  */
+  enum tree_code opcode = ERROR_MARK;
+  bool ovf_only = false;
+
   switch (fcode)
 {
+case BUILT_IN_ADD_OVERFLOW_P:
+  ovf_only = true;
+  /* FALLTHRU */
 case BUILT_IN_ADD_OVERFLOW:
+  opcode = PLUS_EXPR;
+  /* FALLTHRU */
 case BUILT_IN_SADD_OVERFLOW:
 case BUILT_IN_SADDL_OVERFLOW:
 case BUILT_IN_SADDLL_OVERFLOW:
@@ -7963,7 

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

+The built-in functions promote the first two operands into infinite precision 
signed type
+and perform addition on those promoted operands. The result is then
+cast to the type the third argument.


The above is missing an "of" (it should read "type of the third
argument".)

Martin

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Tue, Jun 07, 2016 at 02:30:06PM -0600, Martin Sebor wrote:
> I've also since discovered that the handling of (at least)
> BUILTIN_LINE is important in C++ 98 mode and if I had run
> the c-c++-common tests instead of just the strict C++ subset
> I would have seen the c-c++-common/builtin_location.c test
> fail with the same error.
> 
> I suspect I added the other built-ins to the function to get
> the Clang-compatibility typed built-ins to work in C++ 98
> (with the default argument).  When it was decided that
> the Clang built-ins shouldn't change I removed the tests
> but not this change.

Here is the updated patch, with the extra testcase (the previous C++ only
testcases were only c++11 and c++14 effective targets) and
the BUILTIN_{ADD,SUB,MUL}_OVERFLOW_P kept in cp/tree.c.

Tested on x86_64-linux, ok for trunk?

2016-06-07  Martin Sebor  
Jakub Jelinek  

PR c++/70507
PR c/68120
* builtins.def (BUILT_IN_ADD_OVERFLOW_P, BUILT_IN_SUB_OVERFLOW_P,
BUILT_IN_MUL_OVERFLOW_P): New builtins.
* builtins.c: Include gimple-fold.h.
(fold_builtin_arith_overflow): Handle
BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
(fold_builtin_3): Likewise.
* doc/extend.texi (Integer Overflow Builtins): Document
__builtin_{add,sub,mul}_overflow_p.
gcc/c/
* c-typeck.c (convert_arguments): Don't promote last argument
of BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
gcc/cp/
* constexpr.c: Include gimple-fold.h.
(cxx_eval_internal_function): New function.
(cxx_eval_call_expression): Call it.
(potential_constant_expression_1): Handle integer arithmetic
overflow built-ins.
* tree.c (builtin_valid_in_constant_expr_p): Handle
BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
gcc/c-family/
* c-common.c (check_builtin_function_arguments): Handle
BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
gcc/testsuite/
* c-c++-common/builtin-arith-overflow-1.c: Add test cases.
* c-c++-common/builtin-arith-overflow-2.c: New test.
* g++.dg/ext/builtin-arith-overflow-1.C: New test.
* g++.dg/cpp0x/constexpr-arith-overflow.C: New test.
* g++.dg/cpp1y/constexpr-arith-overflow.C: New test.

--- gcc/builtins.def.jj 2016-06-06 14:40:35.619347198 +0200
+++ gcc/builtins.def2016-06-07 17:46:52.034206039 +0200
@@ -710,6 +710,9 @@ DEF_C94_BUILTIN(BUILT_IN_TOWUPPE
 DEF_GCC_BUILTIN(BUILT_IN_ADD_OVERFLOW, "add_overflow", BT_FN_BOOL_VAR, 
ATTR_NOTHROW_TYPEGENERIC_LEAF)
 DEF_GCC_BUILTIN(BUILT_IN_SUB_OVERFLOW, "sub_overflow", BT_FN_BOOL_VAR, 
ATTR_NOTHROW_TYPEGENERIC_LEAF)
 DEF_GCC_BUILTIN(BUILT_IN_MUL_OVERFLOW, "mul_overflow", BT_FN_BOOL_VAR, 
ATTR_NOTHROW_TYPEGENERIC_LEAF)
+DEF_GCC_BUILTIN(BUILT_IN_ADD_OVERFLOW_P, "add_overflow_p", 
BT_FN_BOOL_VAR, ATTR_NOTHROW_TYPEGENERIC_LEAF)
+DEF_GCC_BUILTIN(BUILT_IN_SUB_OVERFLOW_P, "sub_overflow_p", 
BT_FN_BOOL_VAR, ATTR_NOTHROW_TYPEGENERIC_LEAF)
+DEF_GCC_BUILTIN(BUILT_IN_MUL_OVERFLOW_P, "mul_overflow_p", 
BT_FN_BOOL_VAR, ATTR_NOTHROW_TYPEGENERIC_LEAF)
 /* Clang compatibility.  */
 DEF_GCC_BUILTIN(BUILT_IN_SADD_OVERFLOW, "sadd_overflow", 
BT_FN_BOOL_INT_INT_INTPTR, ATTR_NOTHROW_LEAF_LIST)
 DEF_GCC_BUILTIN(BUILT_IN_SADDL_OVERFLOW, "saddl_overflow", 
BT_FN_BOOL_LONG_LONG_LONGPTR, ATTR_NOTHROW_LEAF_LIST)
--- gcc/builtins.c.jj   2016-06-06 14:40:35.601347427 +0200
+++ gcc/builtins.c  2016-06-07 17:46:51.958207014 +0200
@@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.
 #include "rtl-chkp.h"
 #include "internal-fn.h"
 #include "case-cfn-macros.h"
+#include "gimple-fold.h"
 
 
 struct target_builtins default_target_builtins;
@@ -7943,18 +7944,28 @@ fold_builtin_unordered_cmp (location_t l
 /* Fold __builtin_{,s,u}{add,sub,mul}{,l,ll}_overflow, either into normal
arithmetics if it can never overflow, or into internal functions that
return both result of arithmetics and overflowed boolean flag in
-   a complex integer result, or some other check for overflow.  */
+   a complex integer result, or some other check for overflow.
+   Similarly fold __builtin_{add,sub,mul}_overflow_p to just the overflow
+   checking part of that.  */
 
 static tree
 fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 tree arg0, tree arg1, tree arg2)
 {
   enum internal_fn ifn = IFN_LAST;
-  tree type = TREE_TYPE (TREE_TYPE (arg2));
-  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
+  /* The code of the expression corresponding to the type-generic
+ built-in, or ERROR_MARK for the type-specific ones.  */
+  enum tree_code opcode = ERROR_MARK;
+  bool ovf_only = false;
+
   switch (fcode)
 {
+case BUILT_IN_ADD_OVERFLOW_P:
+  ovf_only = true;
+  /* FALLTHRU */
 case BUILT_IN_ADD_OVERFLOW:
+  opcode = PLUS_EXPR;
+  /* FALLTHRU */
 case BUILT_IN_SADD_OVERFLOW:
 case 

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

On 06/07/2016 01:42 PM, Jakub Jelinek wrote:

On Tue, Jun 07, 2016 at 03:35:28PM -0400, Jason Merrill wrote:

Indeed, confirming removal of the cp/tree.c hunk doesn't affect anything in
the testsuite, nor e.g.
enum A {
  B = 1,
  C = 2,
  D = __builtin_add_overflow_p (B, C, C)
};
int e[__builtin_add_overflow_p (B, C, C) + 1];
template  int foo (int);
void
bar ()
{
  foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
}
That said, builtin_valid_in_constant_expr_p is used in lots of spots, and in
various of them I see that without the change it actually sets
*non_integral_constant_expression_p = true;
or something similar, but I have no idea why it still works despite of that.


Ah, that's the C++98 constant expression handling, which is a lot more
restricted.  C++11 and up don't care about that flag.


Oops, actually, it seems even the cp/tree.c hunks are significant:

I've only compiled the above testcase with the default -std=c++14, with
-std=c++98 without the cp/tree.c bits I get:

a.C:4:7: error: ‘bool __builtin_add_overflow_p(...)’ cannot appear in a
constant-expression
D = __builtin_add_overflow_p (B, C, C)
^~~~
a.C:4:40: error: a function call cannot appear in a constant-expression
D = __builtin_add_overflow_p (B, C, C)
 ^
a.C:6:45: error: array bound is not an integer constant before ‘]’ token
  int e[__builtin_add_overflow_p (B, C, C) + 1];
  ^
a.C: In function ‘void bar()’:
a.C:11:8: error: ‘bool __builtin_add_overflow_p(...)’ cannot appear in a
constant-expression
foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
 ^~~~
a.C:11:41: error: a function call cannot appear in a constant-expression
foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
  ^
a.C:11:50: error: no matching function for call to ‘foo(int)’
foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
   ^
a.C:7:22: note: candidate: template int foo(int)
  template  int foo (int);
   ^~~
a.C:7:22: note:   template argument deduction/substitution failed:
a.C:11:50: error: template argument 1 is invalid
foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
   ^

Though, maybe it is only worth supporting the __builtin_*_overflow_p
builtins for C++98 integer constant expression contexts and
thus only handle there the 3 builtins instead of all the others.

I guess I should add this testcase to the testsuite then.


I've also since discovered that the handling of (at least)
BUILTIN_LINE is important in C++ 98 mode and if I had run
the c-c++-common tests instead of just the strict C++ subset
I would have seen the c-c++-common/builtin_location.c test
fail with the same error.

I suspect I added the other built-ins to the function to get
the Clang-compatibility typed built-ins to work in C++ 98
(with the default argument).  When it was decided that
the Clang built-ins shouldn't change I removed the tests
but not this change.

Martin

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Tue, Jun 07, 2016 at 03:35:28PM -0400, Jason Merrill wrote:
> >Indeed, confirming removal of the cp/tree.c hunk doesn't affect anything in
> >the testsuite, nor e.g.
> >enum A {
> >  B = 1,
> >  C = 2,
> >  D = __builtin_add_overflow_p (B, C, C)
> >};
> >int e[__builtin_add_overflow_p (B, C, C) + 1];
> >template  int foo (int);
> >void
> >bar ()
> >{
> >  foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
> >}
> >That said, builtin_valid_in_constant_expr_p is used in lots of spots, and in
> >various of them I see that without the change it actually sets
> >*non_integral_constant_expression_p = true;
> >or something similar, but I have no idea why it still works despite of that.
> 
> Ah, that's the C++98 constant expression handling, which is a lot more
> restricted.  C++11 and up don't care about that flag.

Oops, actually, it seems even the cp/tree.c hunks are significant:

I've only compiled the above testcase with the default -std=c++14, with
-std=c++98 without the cp/tree.c bits I get:

a.C:4:7: error: ‘bool __builtin_add_overflow_p(...)’ cannot appear in a
constant-expression
   D = __builtin_add_overflow_p (B, C, C)
   ^~~~
a.C:4:40: error: a function call cannot appear in a constant-expression
   D = __builtin_add_overflow_p (B, C, C)
^
a.C:6:45: error: array bound is not an integer constant before ‘]’ token
 int e[__builtin_add_overflow_p (B, C, C) + 1];
 ^
a.C: In function ‘void bar()’:
a.C:11:8: error: ‘bool __builtin_add_overflow_p(...)’ cannot appear in a
constant-expression
   foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
^~~~
a.C:11:41: error: a function call cannot appear in a constant-expression
   foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
 ^
a.C:11:50: error: no matching function for call to ‘foo(int)’
   foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
  ^
a.C:7:22: note: candidate: template int foo(int)
 template  int foo (int);
  ^~~
a.C:7:22: note:   template argument deduction/substitution failed:
a.C:11:50: error: template argument 1 is invalid
   foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
  ^

Though, maybe it is only worth supporting the __builtin_*_overflow_p
builtins for C++98 integer constant expression contexts and
thus only handle there the 3 builtins instead of all the others.

I guess I should add this testcase to the testsuite then.

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jason Merrill]

On 06/07/2016 11:51 AM, Martin Sebor wrote:

Unless I hear otherwise I'll submit a separate patch removing
the BUILTIN_FILE, FUNCTION, and LINE labels (and adjust the
comment to more closely reflect the function's purpose).


Since Jakub pointed out that this function is used by the parser for 
C++98 constant-expressions, I think we can leave these labels alone.  A 
comment tweak wouldn't hurt, though.


Jason

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jason Merrill]

On 06/07/2016 12:34 PM, Jakub Jelinek wrote:

On Tue, Jun 07, 2016 at 09:51:05AM -0600, Martin Sebor wrote:

On 06/07/2016 08:32 AM, Jason Merrill wrote:

On 06/06/2016 08:36 AM, Jakub Jelinek wrote:

@@ -352,6 +352,35 @@ builtin_valid_in_constant_expr_p (const_
case BUILT_IN_FUNCTION:
case BUILT_IN_LINE:

+  /* The following built-ins are valid in constant expressions
+ when their arguments are.  */
+case BUILT_IN_ADD_OVERFLOW:
+case BUILT_IN_ADD_OVERFLOW_P:



Why is this necessary?  builtin_valid_in_constant_expr_p is only needed
for builtins that can have constant values even if their operands are
non-constant, which doesn't apply here.

For that matter, I don't see why we needed to add _FUNCTION et al, either.


I don't remember what prompted me to change the function in either
patch.  Perhaps it was the comment above the function that says
this:

/* Test whether DECL is a builtin that may appear in a
   constant-expression. */

But removing the change doesn't seem to affect the C++ test results
for the two features so it looks like the change may not be needed.


Indeed, confirming removal of the cp/tree.c hunk doesn't affect anything in
the testsuite, nor e.g.
enum A {
  B = 1,
  C = 2,
  D = __builtin_add_overflow_p (B, C, C)
};
int e[__builtin_add_overflow_p (B, C, C) + 1];
template  int foo (int);
void
bar ()
{
  foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
}
That said, builtin_valid_in_constant_expr_p is used in lots of spots, and in
various of them I see that without the change it actually sets
*non_integral_constant_expression_p = true;
or something similar, but I have no idea why it still works despite of that.


Ah, that's the C++98 constant expression handling, which is a lot more 
restricted.  C++11 and up don't care about that flag.



Is the patch ok for trunk without the cp/tree.c hunk?


Yes.

Jason

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Tue, Jun 07, 2016 at 09:51:05AM -0600, Martin Sebor wrote:
> On 06/07/2016 08:32 AM, Jason Merrill wrote:
> >On 06/06/2016 08:36 AM, Jakub Jelinek wrote:
> >>@@ -352,6 +352,35 @@ builtin_valid_in_constant_expr_p (const_
> >> case BUILT_IN_FUNCTION:
> >> case BUILT_IN_LINE:
> >>
> >>+  /* The following built-ins are valid in constant expressions
> >>+ when their arguments are.  */
> >>+case BUILT_IN_ADD_OVERFLOW:
> >>+case BUILT_IN_ADD_OVERFLOW_P:
> >
> >
> >Why is this necessary?  builtin_valid_in_constant_expr_p is only needed
> >for builtins that can have constant values even if their operands are
> >non-constant, which doesn't apply here.
> >
> >For that matter, I don't see why we needed to add _FUNCTION et al, either.
> 
> I don't remember what prompted me to change the function in either
> patch.  Perhaps it was the comment above the function that says
> this:
> 
> /* Test whether DECL is a builtin that may appear in a
>constant-expression. */
> 
> But removing the change doesn't seem to affect the C++ test results
> for the two features so it looks like the change may not be needed.

Indeed, confirming removal of the cp/tree.c hunk doesn't affect anything in
the testsuite, nor e.g.
enum A {
  B = 1,
  C = 2,
  D = __builtin_add_overflow_p (B, C, C)
};
int e[__builtin_add_overflow_p (B, C, C) + 1];
template  int foo (int);
void
bar ()
{
  foo <__builtin_add_overflow_p (B, C, C) + 1> (0);
}
That said, builtin_valid_in_constant_expr_p is used in lots of spots, and in
various of them I see that without the change it actually sets
*non_integral_constant_expression_p = true;
or something similar, but I have no idea why it still works despite of that.

Is the patch ok for trunk without the cp/tree.c hunk?

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

On 06/07/2016 08:32 AM, Jason Merrill wrote:

On 06/06/2016 08:36 AM, Jakub Jelinek wrote:

@@ -352,6 +352,35 @@ builtin_valid_in_constant_expr_p (const_
 case BUILT_IN_FUNCTION:
 case BUILT_IN_LINE:

+  /* The following built-ins are valid in constant expressions
+ when their arguments are.  */
+case BUILT_IN_ADD_OVERFLOW:
+case BUILT_IN_ADD_OVERFLOW_P:



Why is this necessary?  builtin_valid_in_constant_expr_p is only needed
for builtins that can have constant values even if their operands are
non-constant, which doesn't apply here.

For that matter, I don't see why we needed to add _FUNCTION et al, either.


I don't remember what prompted me to change the function in either
patch.  Perhaps it was the comment above the function that says
this:

/* Test whether DECL is a builtin that may appear in a
   constant-expression. */

But removing the change doesn't seem to affect the C++ test results
for the two features so it looks like the change may not be needed.

Unless I hear otherwise I'll submit a separate patch removing
the BUILTIN_FILE, FUNCTION, and LINE labels (and adjust the
comment to more closely reflect the function's purpose).  I'll
leave it to Jakub to tweak this patch.

Martin

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jason Merrill]

On 06/06/2016 08:36 AM, Jakub Jelinek wrote:

@@ -352,6 +352,35 @@ builtin_valid_in_constant_expr_p (const_
 case BUILT_IN_FUNCTION:
 case BUILT_IN_LINE:

+  /* The following built-ins are valid in constant expressions
+when their arguments are.  */
+case BUILT_IN_ADD_OVERFLOW:
+case BUILT_IN_ADD_OVERFLOW_P:



Why is this necessary?  builtin_valid_in_constant_expr_p is only needed 
for builtins that can have constant values even if their operands are 
non-constant, which doesn't apply here.


For that matter, I don't see why we needed to add _FUNCTION et al, either.

Jason

Re: [PATCH] integer overflow checking builtins in constant expressions

[Marek Polacek]

On Mon, Jun 06, 2016 at 09:44:08PM +0200, Jakub Jelinek wrote:
> Hi!
> 
> On Mon, Jun 06, 2016 at 02:36:17PM +0200, Jakub Jelinek wrote:
> > 2016-06-06  Martin Sebor  
> > Jakub Jelinek  
> > 
> > PR c++/70507
> > PR c/68120
> > * builtins.def (BUILT_IN_ADD_OVERFLOW_P, BUILT_IN_SUB_OVERFLOW_P,
> > BUILT_IN_MUL_OVERFLOW_P): New builtins.
> > * builtins.c: Include gimple-fold.h.
> > (fold_builtin_arith_overflow): Handle
> > BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
> > (fold_builtin_3): Likewise.
> > * doc/extend.texi (Integer Overflow Builtins): Document
> > __builtin_{add,sub,mul}_overflow_p.
> > gcc/c/
> > * c-typeck.c (convert_arguments): Don't promote last argument
> > of BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
> > gcc/cp/
> > * constexpr.c: Include gimple-fold.h.
> > (cxx_eval_internal_function): New function.
> > (cxx_eval_call_expression): Call it.
> > (potential_constant_expression_1): Handle integer arithmetic
> > overflow built-ins.
> > * tree.c (builtin_valid_in_constant_expr_p): Likewise.
> > gcc/c-family/
> > * c-common.c (check_builtin_function_arguments): Handle
> > BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
> > gcc/testsuite/
> > * c-c++-common/builtin-arith-overflow-1.c: Add test cases.
> > * c-c++-common/builtin-arith-overflow-2.c: New test.
> > * g++.dg/cpp0x/constexpr-arith-overflow.C: New test.
> > * g++.dg/cpp1y/constexpr-arith-overflow.C: New test.
> 
> Now successfully bootstrapped/regtested on x86_64-linux and i686-linux, ok
> for trunk?

I just played with this a bit -- nice.  The c/ and c-family/ parts are OK.

Marek

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

Hi!

On Mon, Jun 06, 2016 at 02:36:17PM +0200, Jakub Jelinek wrote:
> 2016-06-06  Martin Sebor  
>   Jakub Jelinek  
> 
>   PR c++/70507
>   PR c/68120
>   * builtins.def (BUILT_IN_ADD_OVERFLOW_P, BUILT_IN_SUB_OVERFLOW_P,
>   BUILT_IN_MUL_OVERFLOW_P): New builtins.
>   * builtins.c: Include gimple-fold.h.
>   (fold_builtin_arith_overflow): Handle
>   BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
>   (fold_builtin_3): Likewise.
>   * doc/extend.texi (Integer Overflow Builtins): Document
>   __builtin_{add,sub,mul}_overflow_p.
> gcc/c/
>   * c-typeck.c (convert_arguments): Don't promote last argument
>   of BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
> gcc/cp/
>   * constexpr.c: Include gimple-fold.h.
>   (cxx_eval_internal_function): New function.
>   (cxx_eval_call_expression): Call it.
>   (potential_constant_expression_1): Handle integer arithmetic
>   overflow built-ins.
>   * tree.c (builtin_valid_in_constant_expr_p): Likewise.
> gcc/c-family/
>   * c-common.c (check_builtin_function_arguments): Handle
>   BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
> gcc/testsuite/
>   * c-c++-common/builtin-arith-overflow-1.c: Add test cases.
>   * c-c++-common/builtin-arith-overflow-2.c: New test.
>   * g++.dg/cpp0x/constexpr-arith-overflow.C: New test.
>   * g++.dg/cpp1y/constexpr-arith-overflow.C: New test.

Now successfully bootstrapped/regtested on x86_64-linux and i686-linux, ok
for trunk?

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Fri, Jun 03, 2016 at 02:09:44PM -0600, Martin Sebor wrote:
> I see.  I've made the change in the latest update to the patch
> but I wasn't able to create a test case to verify it.  Maybe
> that's because this is constexpr the COMPLEX_EXPR doesn't make
> it far enough to trigger a problem.  If there is a way to test
> it I'd appreciate a suggestion for how (otherwise, if not caught
> in a code review like in this case, it would be a ticking time
> bomb).

As you haven't been willing to try the __builtin_*_overflow_p
way, I've done that myself, it hasn't been really that hard.

I've also found another bug, when computing the value that is stored
in the constexpr function, you weren't converting the INTEGER_CST
args to type, so you could get incorrect type of the result or
other issues.

> It also occurred to me that a more robust solution might be to
> change build_complex to either enforce as a precondition that
> the members have a type that matches the complex type.  I've
> taken the liberty of making this change as part of this patch.

IMHO it doesn't belong to this patch, feel free to submit the tree.c
change separately.

> (It seems that an even better solution would be to have
> build_complex convert the arguments to the expected type
> so that callers don't have to worry about it.)

I diagree with that, build_complex is a low-level function, it shouldn't
perform such conversions.

2016-06-06  Martin Sebor  
Jakub Jelinek  

PR c++/70507
PR c/68120
* builtins.def (BUILT_IN_ADD_OVERFLOW_P, BUILT_IN_SUB_OVERFLOW_P,
BUILT_IN_MUL_OVERFLOW_P): New builtins.
* builtins.c: Include gimple-fold.h.
(fold_builtin_arith_overflow): Handle
BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
(fold_builtin_3): Likewise.
* doc/extend.texi (Integer Overflow Builtins): Document
__builtin_{add,sub,mul}_overflow_p.
gcc/c/
* c-typeck.c (convert_arguments): Don't promote last argument
of BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
gcc/cp/
* constexpr.c: Include gimple-fold.h.
(cxx_eval_internal_function): New function.
(cxx_eval_call_expression): Call it.
(potential_constant_expression_1): Handle integer arithmetic
overflow built-ins.
* tree.c (builtin_valid_in_constant_expr_p): Likewise.
gcc/c-family/
* c-common.c (check_builtin_function_arguments): Handle
BUILT_IN_{ADD,SUB,MUL}_OVERFLOW_P.
gcc/testsuite/
* c-c++-common/builtin-arith-overflow-1.c: Add test cases.
* c-c++-common/builtin-arith-overflow-2.c: New test.
* g++.dg/cpp0x/constexpr-arith-overflow.C: New test.
* g++.dg/cpp1y/constexpr-arith-overflow.C: New test.

--- gcc/builtins.c.jj   2016-06-03 21:25:16.595678286 +0200
+++ gcc/builtins.c  2016-06-06 11:48:06.800100603 +0200
@@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.
 #include "rtl-chkp.h"
 #include "internal-fn.h"
 #include "case-cfn-macros.h"
+#include "gimple-fold.h"
 
 
 struct target_builtins default_target_builtins;
@@ -7943,18 +7944,28 @@ fold_builtin_unordered_cmp (location_t l
 /* Fold __builtin_{,s,u}{add,sub,mul}{,l,ll}_overflow, either into normal
arithmetics if it can never overflow, or into internal functions that
return both result of arithmetics and overflowed boolean flag in
-   a complex integer result, or some other check for overflow.  */
+   a complex integer result, or some other check for overflow.
+   Similarly fold __builtin_{add,sub,mul}_overflow_p to just the overflow
+   checking part of that.  */
 
 static tree
 fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 tree arg0, tree arg1, tree arg2)
 {
   enum internal_fn ifn = IFN_LAST;
-  tree type = TREE_TYPE (TREE_TYPE (arg2));
-  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
+  /* The code of the expression corresponding to the type-generic
+ built-in, or ERROR_MARK for the type-specific ones.  */
+  enum tree_code opcode = ERROR_MARK;
+  bool ovf_only = false;
+
   switch (fcode)
 {
+case BUILT_IN_ADD_OVERFLOW_P:
+  ovf_only = true;
+  /* FALLTHRU */
 case BUILT_IN_ADD_OVERFLOW:
+  opcode = PLUS_EXPR;
+  /* FALLTHRU */
 case BUILT_IN_SADD_OVERFLOW:
 case BUILT_IN_SADDL_OVERFLOW:
 case BUILT_IN_SADDLL_OVERFLOW:
@@ -7963,7 +7974,12 @@ fold_builtin_arith_overflow (location_t
 case BUILT_IN_UADDLL_OVERFLOW:
   ifn = IFN_ADD_OVERFLOW;
   break;
+case BUILT_IN_SUB_OVERFLOW_P:
+  ovf_only = true;
+  /* FALLTHRU */
 case BUILT_IN_SUB_OVERFLOW:
+  opcode = MINUS_EXPR;
+  /* FALLTHRU */
 case BUILT_IN_SSUB_OVERFLOW:
 case BUILT_IN_SSUBL_OVERFLOW:
 case BUILT_IN_SSUBLL_OVERFLOW:
@@ -7972,7 +7988,12 @@ fold_builtin_arith_overflow (location_t
 case BUILT_IN_USUBLL_OVERFLOW:
   ifn = IFN_SUB_OVERFLOW;
   break;
+case 

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

On 06/03/2016 09:32 AM, Jakub Jelinek wrote:

On Fri, Jun 03, 2016 at 09:29:48AM -0600, Martin Sebor wrote:

+   {
+ tree type = TREE_TYPE (TREE_TYPE (t));
+ tree vflow = arith_overflowed_p (opcode, type, arg0, arg1)
+  ? integer_one_node : integer_zero_node;


This looks incorrect, the return type is TREE_TYPE (t), some complex integer
type, therefore vflow needs to be
   tree vflow = build_int_cst (TREE_TYPE (TREE_TYPE (t)),
  arith_overflowed_p (opcode, type, arg0, arg1)
  ? 1 : 0);
no?


I guess it didn't think it mattered since the complex type specifies
the types of the two members.  I don't mind changing it if it does


Sure, it does.  But if there are any differences between the lhs and rhs
type (or e.g. in COMPLEX_EXPR args etc. in GENERIC), then it is invalid IL,
or for GIMPLE if the two types aren't compatible according to the GIMPLE
rules (useless conversion).


I see.  I've made the change in the latest update to the patch
but I wasn't able to create a test case to verify it.  Maybe
that's because this is constexpr the COMPLEX_EXPR doesn't make
it far enough to trigger a problem.  If there is a way to test
it I'd appreciate a suggestion for how (otherwise, if not caught
in a code review like in this case, it would be a ticking time
bomb).

It also occurred to me that a more robust solution might be to
change build_complex to either enforce as a precondition that
the members have a type that matches the complex type.  I've
taken the liberty of making this change as part of this patch.
(It seems that an even better solution would be to have
build_complex convert the arguments to the expected type
so that callers don't have to worry about it.)

Martin
PR c++/70507 - integer overflow builtins not constant expressions
PR c/68120 - can't easily deal with integer overflow at compile time

gcc/cp/ChangeLog:
2016-06-03  Martin Sebor  

	PR c++/70507
	PR c/68120
	* constexpr.c (cxx_eval_internal_function): New function.
	(cxx_eval_call_expression): Call it.
	(potential_constant_expression_1): Handle integer arithmetic
	overflow built-ins.
	* tree.c (builtin_valid_in_constant_expr_p): Same.

gcc/ChangeLog:
2016-06-03  Martin Sebor  

	PR c++/70507
	PR c/68120
	* builtins.c (fold_builtin_unordered_cmp): Handle integer arithmetic
	overflow built-ins.
	* tree.c (build_complex): Assert preconditions.
	* doc/extend.texi (Integer Overflow Builtins): Update.

gcc/testsuite/ChangeLog:
2016-06-03  Martin Sebor  

	PR c++/70507
	PR c/68120
	* c-c++-common/builtin-arith-overflow-1.c: Add test cases.
	* c-c++-common/builtin-arith-overflow-2.c: New test.
	* g++.dg/cpp0x/constexpr-arith-overflow.C: New test.
	* g++.dg/cpp1y/constexpr-arith-overflow.C: New test.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 931d4a6..ada1904 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "rtl-chkp.h"
 #include "internal-fn.h"
 #include "case-cfn-macros.h"
+#include "gimple-fold.h"
 
 
 struct target_builtins default_target_builtins;
@@ -7957,11 +7958,14 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 			 tree arg0, tree arg1, tree arg2)
 {
   enum internal_fn ifn = IFN_LAST;
-  tree type = TREE_TYPE (TREE_TYPE (arg2));
-  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
+  /* The code of the expression corresponding to the type-generic
+ built-in, or ERROR_MARK for the type-specific ones.  */
+  enum tree_code opcode = ERROR_MARK;
+
   switch (fcode)
 {
 case BUILT_IN_ADD_OVERFLOW:
+  opcode = PLUS_EXPR;
 case BUILT_IN_SADD_OVERFLOW:
 case BUILT_IN_SADDL_OVERFLOW:
 case BUILT_IN_SADDLL_OVERFLOW:
@@ -7971,6 +7975,7 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
   ifn = IFN_ADD_OVERFLOW;
   break;
 case BUILT_IN_SUB_OVERFLOW:
+  opcode = MINUS_EXPR;
 case BUILT_IN_SSUB_OVERFLOW:
 case BUILT_IN_SSUBL_OVERFLOW:
 case BUILT_IN_SSUBLL_OVERFLOW:
@@ -7980,6 +7985,7 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
   ifn = IFN_SUB_OVERFLOW;
   break;
 case BUILT_IN_MUL_OVERFLOW:
+  opcode = MULT_EXPR;
 case BUILT_IN_SMUL_OVERFLOW:
 case BUILT_IN_SMULL_OVERFLOW:
 case BUILT_IN_SMULLL_OVERFLOW:
@@ -7991,6 +7997,26 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 default:
   gcc_unreachable ();
 }
+
+  /* For the "generic" overloads, the first two arguments can have different
+ types and the last argument determines the target type to use to check
+ for overflow.  The arguments of the other overloads all have the same
+ type.  */
+  bool isnullp = integer_zerop (arg2);
+  tree type = TREE_TYPE (TREE_TYPE (arg2));
+
+  /* When the last argument to the 

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

On 06/03/2016 09:23 AM, Jakub Jelinek wrote:

On Fri, Jun 03, 2016 at 09:07:09AM -0600, Martin Sebor wrote:

I'm not sure I understand your concern but at this point I would
prefer to keep things as they are.  I like that the functionality


My concern is that if you declare that NULL is valid third argument for e.g.
__builtin_add_overflow, then unless we add some complicated wording into the
docs,


Thanks the clarification.


Trying to document that the third argument may be NULL, but only if it is
constant NULL pointer expression or something like that (what exactly?)
might not be very easily understandable and clear to users.


I think the updated wording is quite clear:

  The first of the functions accepts either a pointer to an integer
  object or a null pointer constant.

/A null pointer constant/ is a basic term defined by both C and C++
so it should be familiar to all C++ programmers.  (To make it 100%
correct we should perhaps say:

  "...or a null pointer constant cast to a pointer to integer."

even though that should be obvious since the functions won't
accept a bare NULL.)


Which is why I've suggested just not allowing (like before) the third
argument to be NULL, and just add new 3 builtins for the test for overflow,
but don't store it anywhere.  They would just be folded early to the same
internal function.  And when adding the 3 new builtins, we can also choose
a different calling convention that would allow the C++98/C constant
expressions, by not having the third argument a pointer (we don't need to
dereference anything), but e.g. just any integer where we ignore the value
(well, evaluate for side-effects) and only care about the type.


I understand what you're suggesting and it's something I could
have easily done when I started on it a few weeks ago but I'm
afraid really out of cycles to continue to tweak the patch.
I think it's good enough as it is for now, gives the requester
what they asked for and lets me finish the C++ VLA bounds
checking, which is the main reason why I did this work to begin
with.  We can revisit this idea when we get the requester's
feedback (and after I've made some headway on my pending tasks).
Does that sound reasonable?

Martin

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Fri, Jun 03, 2016 at 09:29:48AM -0600, Martin Sebor wrote:
> >>+   {
> >>+ tree type = TREE_TYPE (TREE_TYPE (t));
> >>+ tree vflow = arith_overflowed_p (opcode, type, arg0, arg1)
> >>+  ? integer_one_node : integer_zero_node;
> >
> >This looks incorrect, the return type is TREE_TYPE (t), some complex integer
> >type, therefore vflow needs to be
> >   tree vflow = build_int_cst (TREE_TYPE (TREE_TYPE (t)),
> >   arith_overflowed_p (opcode, type, arg0, arg1)
> >   ? 1 : 0);
> >no?
> 
> I guess it didn't think it mattered since the complex type specifies
> the types of the two members.  I don't mind changing it if it does

Sure, it does.  But if there are any differences between the lhs and rhs
type (or e.g. in COMPLEX_EXPR args etc. in GENERIC), then it is invalid IL,
or for GIMPLE if the two types aren't compatible according to the GIMPLE
rules (useless conversion).

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

+   {
+ tree type = TREE_TYPE (TREE_TYPE (t));
+ tree vflow = arith_overflowed_p (opcode, type, arg0, arg1)
+  ? integer_one_node : integer_zero_node;


This looks incorrect, the return type is TREE_TYPE (t), some complex integer
type, therefore vflow needs to be
   tree vflow = build_int_cst (TREE_TYPE (TREE_TYPE (t)),
  arith_overflowed_p (opcode, type, arg0, arg1)
  ? 1 : 0);
no?


I guess it didn't think it mattered since the complex type specifies
the types of the two members.  I don't mind changing it if it does
but I'd like to have a test case to go with it.  Maybe Jason can
help with that.

Martin

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Fri, Jun 03, 2016 at 09:07:09AM -0600, Martin Sebor wrote:
> I'm not sure I understand your concern but at this point I would
> prefer to keep things as they are.  I like that the functionality

My concern is that if you declare that NULL is valid third argument for e.g.
__builtin_add_overflow, then unless we add some complicated wording into the
docs, we have to penalize the code we emit for e.g.
bool
foo (int a, int b, int *p)
{
  return __builtin_add_overflow (a, b, p);
}
While we could previously emit
addl%edi, %esi
movl%esi, (%rdx)
seto%al
retq
you suddenly have to emit (and your patch doesn't do that):
addl%edi, %esi
seto%al
testq   %rdx, %rdx
je  .L1
movl%esi, (%rdx)
.L1:
retq
because you can't at compile time prove if p is NULL (then you wouldn't
store the result) or not.

Trying to document that the third argument may be NULL, but only if it is
constant NULL pointer expression or something like that (what exactly?)
might not be very easily understandable and clear to users.

Which is why I've suggested just not allowing (like before) the third
argument to be NULL, and just add new 3 builtins for the test for overflow,
but don't store it anywhere.  They would just be folded early to the same
internal function.  And when adding the 3 new builtins, we can also choose
a different calling convention that would allow the C++98/C constant
expressions, by not having the third argument a pointer (we don't need to
dereference anything), but e.g. just any integer where we ignore the value
(well, evaluate for side-effects) and only care about the type.

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

On 06/02/2016 01:34 AM, Jakub Jelinek wrote:

On Thu, Jun 02, 2016 at 09:23:16AM +0200, Jakub Jelinek wrote:

Also, perhaps just a documentation thing, it would be good to clarify the
NULL last argument.  From the POV of generating efficient code, I think we
should say something that the last argument (for the GNU builtins) must be
either a pointer to a valid object, or NULL/nullptr constant cast expression
cast to a pointer type, but nothing else.  That is actually what your patch
implements.  But, I'd like to make sure that
   int *p = NULL;
   if (__builtin_add_overflow (a, b, p))
 ...
is actually not valid, otherwise we unnecessarily pessimize many of the GNU
style calls (those that don't pass ), where instead of
   tem = ADD_OVERFLOW (a, b);
   *p = REALPART_EXPR (tem);
   ovf = IMAGPART_EXPR (tem);
we'd need to emit instead
   tem = ADD_OVERFLOW (a, b);
   ovf = IMAGPART_EXPR (tem);
   if (p != NULL)
 *p = REALPART_EXPR (tem);


Though, perhaps that is too ugly, that it has different semantics for
   __builtin_add_overflow (a, b, (int *) NULL)
and for
   int *p = NULL;
   __builtin_add_overflow (a, b, p)
Maybe the cleanest would be to just add 3 extra builtins, again,
typegeneric,
   __builtin_{add,sub,mul}_overflow_p
where either the arguments would be instead of integertype1, integertype2,
integertype3 * rather integertype1, integertype2, integertype3
and we'd only care about the type, not value, of the last argument,
so use it like __builtin_add_overflow_p (a, b, (__typeof ((a) + (b))) 0)
or handle those 3 similarly to __builtin_va_arg, and use
__builtin_add_overflow_p (a, b, int);
I think I prefer the former though.


I'm not sure I understand your concern but at this point I would
prefer to keep things as they are.  I like that the functionality
that was requested in c/68120 could be provided under the existing
interface, and I'm not fond of the idea of adding yet another set
of built-ins just to get at a bit that's already available via
the existing ones (in C++ 11, even in constexpr contexts, provided
the built-ins were allowed there).  I've also spent far more time
on this patch than I had planned and I'm way behind on the tasks
I was asked to work on.

That said, in c/68120 the requester commented that he's considering
submitting a request for yet another enhancement in this area, so
I think letting him play with what we've got now for a bit will be
an opportunity to get his feedback and tweak the API further based
on it.

Martin

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

Hi!

On Thu, Jun 02, 2016 at 06:28:21PM -0600, Martin Sebor wrote:

First of all, can you please respond to the mail I've sent about
NULL argument issues (and proposal for __builtin_*_overflow_p)?

This patch as well as the nonnull attribute patch then depends on that
decision...

> + {
> +   tree type = TREE_TYPE (TREE_TYPE (t));
> +   tree vflow = arith_overflowed_p (opcode, type, arg0, arg1)
> +? integer_one_node : integer_zero_node;

This looks incorrect, the return type is TREE_TYPE (t), some complex integer
type, therefore vflow needs to be
  tree vflow = build_int_cst (TREE_TYPE (TREE_TYPE (t)),
  arith_overflowed_p (opcode, type, arg0, arg1)
  ? 1 : 0);
no?

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

+{
+  /* Perform the computation in the target type and check for
overflow.  */
+  arg0 = fold_convert (type, arg0);
+  arg1 = fold_convert (type, arg1);
+
+  if (tree result = size_binop_loc (loc, opcode, arg0, arg1))
+return TREE_OVERFLOW (result) ? build_one_cst (boolean_type_node)
+  : build_zero_cst (boolean_type_node);


This is wrong, it is not the computation of overflow that is intended.
The documentation says that we compute
(infinite_precision_signed_type) arg0
op (infinite_precision_signed_type) arg1 and then cast it to type, extend
again to infinite_precision_signed_type and compare.  And we have a
helper
function for that already.


Thank you for the suggestion.  Using the helper instead is simpler
(though my tests don't show any difference in the computed result
so I'm not sure I see in what way the original code was wrong).

...


Again, this is wrong, it should have used arith_overflowed_p.


No.  As is apparent from the rest of hunk, we need the result or
the computation here, not just the overflow bit.


It just dawned on me what the problem was and why both places
need arith_overflowed_p:  Converting the arguments to the result
type and doing the arithmetic and checking for overflow in it
would mask it in cases like:

  __builtin_add_overflow (SCHAR_MAX + 1, 1, (signed char*)0)

I've fixed it in the attached patch (and added test cases to
verify it).

Martin
PR c++/70507 - integer overflow builtins not constant expressions
PR c/68120 - can't easily deal with integer overflow at compile time

gcc/cp/ChangeLog:
2016-06-02  Martin Sebor  

	PR c++/70507
	PR c/68120
	* constexpr.c (cxx_eval_internal_function): New function.
	(cxx_eval_call_expression): Call it.
	(potential_constant_expression_1): Handle integer arithmetic
	overflow built-ins.
	* tree.c (builtin_valid_in_constant_expr_p): Same.

gcc/ChangeLog:
2016-06-02  Martin Sebor  

	PR c++/70507
	PR c/68120
	* builtins.c (fold_builtin_unordered_cmp): Handle integer arithmetic
	overflow built-ins.
	* doc/extend.texi (Integer Overflow Builtins): Update.

gcc/testsuite/ChangeLog:
2016-06-02  Martin Sebor  

	PR c++/70507
	PR c/68120
	* c-c++-common/builtin-arith-overflow-1.c: Add test cases.
	* c-c++-common/builtin-arith-overflow-2.c: New test.
	* g++.dg/cpp0x/constexpr-arith-overflow.C: New test.
	* g++.dg/cpp1y/constexpr-arith-overflow.C: New test.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 931d4a6..ada1904 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "rtl-chkp.h"
 #include "internal-fn.h"
 #include "case-cfn-macros.h"
+#include "gimple-fold.h"
 
 
 struct target_builtins default_target_builtins;
@@ -7957,11 +7958,14 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 			 tree arg0, tree arg1, tree arg2)
 {
   enum internal_fn ifn = IFN_LAST;
-  tree type = TREE_TYPE (TREE_TYPE (arg2));
-  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
+  /* The code of the expression corresponding to the type-generic
+ built-in, or ERROR_MARK for the type-specific ones.  */
+  enum tree_code opcode = ERROR_MARK;
+
   switch (fcode)
 {
 case BUILT_IN_ADD_OVERFLOW:
+  opcode = PLUS_EXPR;
 case BUILT_IN_SADD_OVERFLOW:
 case BUILT_IN_SADDL_OVERFLOW:
 case BUILT_IN_SADDLL_OVERFLOW:
@@ -7971,6 +7975,7 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
   ifn = IFN_ADD_OVERFLOW;
   break;
 case BUILT_IN_SUB_OVERFLOW:
+  opcode = MINUS_EXPR;
 case BUILT_IN_SSUB_OVERFLOW:
 case BUILT_IN_SSUBL_OVERFLOW:
 case BUILT_IN_SSUBLL_OVERFLOW:
@@ -7980,6 +7985,7 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
   ifn = IFN_SUB_OVERFLOW;
   break;
 case BUILT_IN_MUL_OVERFLOW:
+  opcode = MULT_EXPR;
 case BUILT_IN_SMUL_OVERFLOW:
 case BUILT_IN_SMULL_OVERFLOW:
 case BUILT_IN_SMULLL_OVERFLOW:
@@ -7991,6 +7997,26 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 default:
   gcc_unreachable ();
 }
+
+  /* For the "generic" overloads, the first two arguments can have different
+ types and the last argument determines the target type to use to check
+ for overflow.  The arguments of the other overloads all have the same
+ type.  */
+  bool isnullp = integer_zerop (arg2);
+  tree type = TREE_TYPE (TREE_TYPE (arg2));
+
+  /* When the last argument to the type-generic built-in is a null pointer
+ and the first two arguments are constant, attempt to fold the built-in
+ call into a constant expression indicating whether or not it detected
+ an overflow but without storing the result.  */
+  if (opcode != ERROR_MARK && isnullp
+  && TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
+{
+  /* Perform the 

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

+  /* For the "generic" overloads, the first two arguments can have different
+ types and the last argument determines the target type to use to check
+ for overflow.  The arguments of the other overloads all have the same
+ type.  */
+  tree type = TREE_TYPE (TREE_TYPE (arg2));
+  bool isnullp = integer_zerop (arg2);
+
+  /* When the last argument is a null pointer and the first two arguments
+ are constant, attempt to fold the built-in call into a constant
+ expression indicating whether or not it detected an overflow but
+ without storing the result.  */
+  if (isnullp
+  && TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)


Doesn't this mean you allow NULL arg2 even for BUILT_IN_ADDL_OVERFLOW and
similar clang builtins?


Yes, that's possible, and always has been.  I've raised a new bug
for it: c/71392 - SEGV calling integer overflow built-ins with
a null pointer.  The built-ins are missing the non-null attribute.
I've assigned the bug to myself and will submit a separate patch
for it in a minute.


+{
+  /* Perform the computation in the target type and check for overflow.  */
+  arg0 = fold_convert (type, arg0);
+  arg1 = fold_convert (type, arg1);
+
+  if (tree result = size_binop_loc (loc, opcode, arg0, arg1))
+   return TREE_OVERFLOW (result) ? build_one_cst (boolean_type_node)
+ : build_zero_cst (boolean_type_node);


This is wrong, it is not the computation of overflow that is intended.
The documentation says that we compute (infinite_precision_signed_type) arg0
op (infinite_precision_signed_type) arg1 and then cast it to type, extend
again to infinite_precision_signed_type and compare.  And we have a helper
function for that already.


Thank you for the suggestion.  Using the helper instead is simpler
(though my tests don't show any difference in the computed result
so I'm not sure I see in what way the original code was wrong).


+  tree arg0 = cxx_eval_constant_expression (ctx, CALL_EXPR_ARG (t, 0), lval,
+   non_constant_p, overflow_p);
+  tree arg1 = cxx_eval_constant_expression (ctx, CALL_EXPR_ARG (t, 1), lval,
+   non_constant_p, overflow_p);
+
+  if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
+{
+  if (tree result = size_binop_loc (EXPR_LOC_OR_LOC (t, input_location),
+   opcode, arg0, arg1))
+   {
+ if (TREE_OVERFLOW (result))
+   {
+ /* Reset TREE_OVERFLOW to avoid warnings for the overflow.  */
+ TREE_OVERFLOW (result) = 0;


Again, this is wrong, it should have used arith_overflowed_p.


No.  As is apparent from the rest of hunk, we need the result or
the computation here, not just the overflow bit.


AFAIK the docs use either C++98, C++11 (without space), or
ISO/IEC 14882:2011 etc., but not C++ 98 or C++ 11.

Also, perhaps just a documentation thing, it would be good to clarify the
NULL last argument.  From the POV of generating efficient code, I think we
should say something that the last argument (for the GNU builtins) must be
either a pointer to a valid object, or NULL/nullptr constant cast expression
cast to a pointer type, but nothing else.  That is actually what your patch
implements.


I've updated the manual to make it clear that the null pointer
must be a constant.

While testing the updated patch I noticed that I had neglected
to update potential_constant_expression_1 in constexpr.c to handle
the internal functions corresponding to the built-ins, so I've
made that change and added test cases to exercise it.  (In the
process, I also uncovered a new constexpr bug and raised
c++/71391 - error on aggregate initialization with side-effects
in a constexpr function.

Attached is an updated patch with these changes.

Martin
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 931d4a6..ada1904 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "rtl-chkp.h"
 #include "internal-fn.h"
 #include "case-cfn-macros.h"
+#include "gimple-fold.h"
 
 
 struct target_builtins default_target_builtins;
@@ -7957,11 +7958,14 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 			 tree arg0, tree arg1, tree arg2)
 {
   enum internal_fn ifn = IFN_LAST;
-  tree type = TREE_TYPE (TREE_TYPE (arg2));
-  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
+  /* The code of the expression corresponding to the type-generic
+ built-in, or ERROR_MARK for the type-specific ones.  */
+  enum tree_code opcode = ERROR_MARK;
+
   switch (fcode)
 {
 case BUILT_IN_ADD_OVERFLOW:
+  opcode = PLUS_EXPR;
 case BUILT_IN_SADD_OVERFLOW:
 case BUILT_IN_SADDL_OVERFLOW:
 case BUILT_IN_SADDLL_OVERFLOW:
@@ -7971,6 +7975,7 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function 

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Thu, Jun 02, 2016 at 09:23:16AM +0200, Jakub Jelinek wrote:
> Also, perhaps just a documentation thing, it would be good to clarify the
> NULL last argument.  From the POV of generating efficient code, I think we
> should say something that the last argument (for the GNU builtins) must be
> either a pointer to a valid object, or NULL/nullptr constant cast expression
> cast to a pointer type, but nothing else.  That is actually what your patch
> implements.  But, I'd like to make sure that
>   int *p = NULL;
>   if (__builtin_add_overflow (a, b, p))
> ...
> is actually not valid, otherwise we unnecessarily pessimize many of the GNU
> style calls (those that don't pass ), where instead of
>   tem = ADD_OVERFLOW (a, b);
>   *p = REALPART_EXPR (tem);
>   ovf = IMAGPART_EXPR (tem);
> we'd need to emit instead
>   tem = ADD_OVERFLOW (a, b);
>   ovf = IMAGPART_EXPR (tem);
>   if (p != NULL)
> *p = REALPART_EXPR (tem);

Though, perhaps that is too ugly, that it has different semantics for
  __builtin_add_overflow (a, b, (int *) NULL)
and for
  int *p = NULL;
  __builtin_add_overflow (a, b, p)
Maybe the cleanest would be to just add 3 extra builtins, again,
typegeneric,
  __builtin_{add,sub,mul}_overflow_p
where either the arguments would be instead of integertype1, integertype2,
integertype3 * rather integertype1, integertype2, integertype3
and we'd only care about the type, not value, of the last argument,
so use it like __builtin_add_overflow_p (a, b, (__typeof ((a) + (b))) 0)
or handle those 3 similarly to __builtin_va_arg, and use
__builtin_add_overflow_p (a, b, int);
I think I prefer the former though.

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Wed, Jun 01, 2016 at 09:10:52PM -0600, Martin Sebor wrote:
> @@ -7957,8 +7957,8 @@ fold_builtin_arith_overflow (location_t loc, enum 
> built_in_function fcode,
>tree arg0, tree arg1, tree arg2)
>  {
>enum internal_fn ifn = IFN_LAST;
> -  tree type = TREE_TYPE (TREE_TYPE (arg2));
> -  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
> +  enum tree_code opcode;
> +
>switch (fcode)
>  {
>  case BUILT_IN_ADD_OVERFLOW:
> @@ -7969,6 +7969,7 @@ fold_builtin_arith_overflow (location_t loc, enum 
> built_in_function fcode,
>  case BUILT_IN_UADDL_OVERFLOW:
>  case BUILT_IN_UADDLL_OVERFLOW:
>ifn = IFN_ADD_OVERFLOW;
> +  opcode = PLUS_EXPR;
>break;
>  case BUILT_IN_SUB_OVERFLOW:
>  case BUILT_IN_SSUB_OVERFLOW:
> @@ -7978,6 +7979,7 @@ fold_builtin_arith_overflow (location_t loc, enum 
> built_in_function fcode,
>  case BUILT_IN_USUBL_OVERFLOW:
>  case BUILT_IN_USUBLL_OVERFLOW:
>ifn = IFN_SUB_OVERFLOW;
> +  opcode = MINUS_EXPR;
>break;
>  case BUILT_IN_MUL_OVERFLOW:
>  case BUILT_IN_SMUL_OVERFLOW:
> @@ -7987,10 +7989,35 @@ fold_builtin_arith_overflow (location_t loc, enum 
> built_in_function fcode,
>  case BUILT_IN_UMULL_OVERFLOW:
>  case BUILT_IN_UMULLL_OVERFLOW:
>ifn = IFN_MUL_OVERFLOW;
> +  opcode = MULT_EXPR;
>break;
>  default:
>gcc_unreachable ();
>  }
> +
> +  /* For the "generic" overloads, the first two arguments can have different
> + types and the last argument determines the target type to use to check
> + for overflow.  The arguments of the other overloads all have the same
> + type.  */
> +  tree type = TREE_TYPE (TREE_TYPE (arg2));
> +  bool isnullp = integer_zerop (arg2);
> +
> +  /* When the last argument is a null pointer and the first two arguments
> + are constant, attempt to fold the built-in call into a constant
> + expression indicating whether or not it detected an overflow but
> + without storing the result.  */
> +  if (isnullp
> +  && TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)

Doesn't this mean you allow NULL arg2 even for BUILT_IN_ADDL_OVERFLOW and
similar clang builtins?  I'd set opcode to ERROR_MARK first, and then do:
  switch (fcode)
{
case BUILT_IN_ADD_OVERFLOW:
  opcode = PLUS_EXPR;
  /* FALLTHROUGH */
case BUILT_IN_SADD_OVERFLOW:
case BUILT_IN_SADDL_OVERFLOW:
case BUILT_IN_SADDLL_OVERFLOW:
case BUILT_IN_UADD_OVERFLOW:
case BUILT_IN_UADDL_OVERFLOW:
case BUILT_IN_UADDLL_OVERFLOW:
  ifn = IFN_ADD_OVERFLOW;
  break;
and similarly, then
  bool isnullp = opcode != ERROR_MARK ? integer_zerop (arg2) : false;
or so.

> +{
> +  /* Perform the computation in the target type and check for overflow.  
> */
> +  arg0 = fold_convert (type, arg0);
> +  arg1 = fold_convert (type, arg1);
> +
> +  if (tree result = size_binop_loc (loc, opcode, arg0, arg1))
> + return TREE_OVERFLOW (result) ? build_one_cst (boolean_type_node)
> +   : build_zero_cst (boolean_type_node);

This is wrong, it is not the computation of overflow that is intended.
The documentation says that we compute (infinite_precision_signed_type) arg0
op (infinite_precision_signed_type) arg1 and then cast it to type, extend
again to infinite_precision_signed_type and compare.  And we have a helper
function for that already.

Furthermore, we have boolean_true_node and boolean_false_node.

Thus, I'd expect here:
return (arith_overflowed_p (opcode, type, arg0, arg1)
? boolean_true_node : boolean_false_node);

> +  tree arg0 = cxx_eval_constant_expression (ctx, CALL_EXPR_ARG (t, 0), lval,
> + non_constant_p, overflow_p);
> +  tree arg1 = cxx_eval_constant_expression (ctx, CALL_EXPR_ARG (t, 1), lval,
> + non_constant_p, overflow_p);
> +
> +  if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
> +{
> +  if (tree result = size_binop_loc (EXPR_LOC_OR_LOC (t, input_location),
> + opcode, arg0, arg1))
> + {
> +   if (TREE_OVERFLOW (result))
> + {
> +   /* Reset TREE_OVERFLOW to avoid warnings for the overflow.  */
> +   TREE_OVERFLOW (result) = 0;

Again, this is wrong, it should have used arith_overflowed_p.

> @@ -1270,18 +1332,8 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, 
> tree t,
>bool depth_ok;
>  
>if (fun == NULL_TREE)
> -switch (CALL_EXPR_IFN (t))
> -  {
> -  case IFN_UBSAN_NULL:
> -  case IFN_UBSAN_BOUNDS:
> -  case IFN_UBSAN_VPTR:
> - return void_node;
> -  default:
> - if (!ctx->quiet)
> -   error_at (loc, "call to internal function");
> - *non_constant_p = true;
> - return t;
> -  }
> +return cxx_eval_internal_function (ctx, t, lval,
> +  

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

Unless I hear otherwise, I'll go ahead and change the patch
to have only the type-generic built-ins accept a null pointer
as an argument.


Since Jason doesn't think making it work in C++ 98 is important
I modified the patch to keep the Clang-compatibility built-ins
unchanged and to remove the default argument from the others.
Attached is an updated version intended to be applied on top
of the c/70883 patch.

Martin
PR c++/70507 - integer overflow builtins not constant expressions
PR c/68120 - can't easily deal with integer overflow at compile time

gcc/testsuite/ChangeLog:
2016-06-01  Martin Sebor  

	PR c++/70507
	PR c/68120
	* c-c++-common/builtin-arith-overflow-1.c: Add test cases.
	* c-c++-common/builtin-arith-overflow-2.c: New test.
	* g++.dg/cpp0x/constexpr-arith-overflow.C: New test.

gcc/cp/ChangeLog:
2016-06-01  Martin Sebor  

	PR c++/70507
	PR c/68120
	* constexpr.c (cxx_eval_internal_function): New function.
	(cxx_eval_call_expression): Call it.
	* tree.c (builtin_valid_in_constant_expr_p): Handle integer
	arithmetic overflow built-ins.

gcc/c-family/ChangeLog:
2016-06-01  Martin Sebor  

	PR c/68120
	(check_builtin_function_arguments): Allow type-specific integer
	arithmetic overflow built-ins to take either 2 or three arguments.

gcc/ChangeLog:
2016-06-01  Martin Sebor  

	PR c++/70507
	PR c/68120
	* builtins.c (fold_builtin_unordered_cmp): Handle integer arithmetic
	overflow built-ins.
	* doc/extend.texi (Integer Overflow Builtins): Update.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 931d4a6..4620553 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -7957,8 +7957,8 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 			 tree arg0, tree arg1, tree arg2)
 {
   enum internal_fn ifn = IFN_LAST;
-  tree type = TREE_TYPE (TREE_TYPE (arg2));
-  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
+  enum tree_code opcode;
+
   switch (fcode)
 {
 case BUILT_IN_ADD_OVERFLOW:
@@ -7969,6 +7969,7 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 case BUILT_IN_UADDL_OVERFLOW:
 case BUILT_IN_UADDLL_OVERFLOW:
   ifn = IFN_ADD_OVERFLOW;
+  opcode = PLUS_EXPR;
   break;
 case BUILT_IN_SUB_OVERFLOW:
 case BUILT_IN_SSUB_OVERFLOW:
@@ -7978,6 +7979,7 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 case BUILT_IN_USUBL_OVERFLOW:
 case BUILT_IN_USUBLL_OVERFLOW:
   ifn = IFN_SUB_OVERFLOW;
+  opcode = MINUS_EXPR;
   break;
 case BUILT_IN_MUL_OVERFLOW:
 case BUILT_IN_SMUL_OVERFLOW:
@@ -7987,10 +7989,35 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
 case BUILT_IN_UMULL_OVERFLOW:
 case BUILT_IN_UMULLL_OVERFLOW:
   ifn = IFN_MUL_OVERFLOW;
+  opcode = MULT_EXPR;
   break;
 default:
   gcc_unreachable ();
 }
+
+  /* For the "generic" overloads, the first two arguments can have different
+ types and the last argument determines the target type to use to check
+ for overflow.  The arguments of the other overloads all have the same
+ type.  */
+  tree type = TREE_TYPE (TREE_TYPE (arg2));
+  bool isnullp = integer_zerop (arg2);
+
+  /* When the last argument is a null pointer and the first two arguments
+ are constant, attempt to fold the built-in call into a constant
+ expression indicating whether or not it detected an overflow but
+ without storing the result.  */
+  if (isnullp
+  && TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
+{
+  /* Perform the computation in the target type and check for overflow.  */
+  arg0 = fold_convert (type, arg0);
+  arg1 = fold_convert (type, arg1);
+
+  if (tree result = size_binop_loc (loc, opcode, arg0, arg1))
+	return TREE_OVERFLOW (result) ? build_one_cst (boolean_type_node)
+	  : build_zero_cst (boolean_type_node);
+}
+
   tree ctype = build_complex_type (type);
   tree call = build_call_expr_internal_loc (loc, ifn, ctype,
 	2, arg0, arg1);
@@ -7998,6 +8025,11 @@ fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
   tree intres = build1_loc (loc, REALPART_EXPR, type, tgt);
   tree ovfres = build1_loc (loc, IMAGPART_EXPR, type, tgt);
   ovfres = fold_convert_loc (loc, boolean_type_node, ovfres);
+
+  if (isnullp)
+  return ovfres;
+
+  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
   tree store
 = fold_build2_loc (loc, MODIFY_EXPR, void_type_node, mem_arg2, intres);
   return build2_loc (loc, COMPOUND_EXPR, boolean_type_node, store, ovfres);
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 482f8af..c77e939 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1255,6 +1255,68 @@ cx_error_context (void)
   return r;
 }
 
+/* Evaluate a call T to a GCC internal function when possible and return
+   the evaluated 

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jason Merrill]

On 05/31/2016 06:44 PM, Martin Sebor wrote:

Without this change, C or C++ 98 users wouldn't be able to use the
built-ins in constant expressions (C++ 98 doesn't allow
casting a null pointer in those contexts).


I can't imagine that C++98 users care about using these built-ins in 
constant expressions.


Jason

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

C++ 98.  But those might be acceptable limitations (at least
in C there's a workaround).


__builtin_* is an extension.  Perhaps for C (any) and C++ (< C++14)
you could require that in constant expressions the last argument
of the builtin has to be a NULL pointer cast to some pointer type
(not necessarily a constant expression)?  This can be handled
simply by saving/restoring the constant expression context around
the parsing of the last argument of the builtin, and afterwards
verifying it has the required properties.


I'll see if there's an easy way to make it work.



For C++14 and later, it would be nice if one could also do:
struct S { int val; bool ovf; };
constexpr S foo (int x, int y)
{
   S ret = { 0, false };
   ret.ovf = __builtin_add_overflow (x, y, );
   return ret;
}
instead of doing:
   ret.ovf = __builtin_add_overflow (x, y, (int *) 0);
   ret.val = (unsigned) x + y;
or similar.
Does that work with your patch?


Yes, it does (it's exercised by the constexpr-arith-overflow.C
test).  The patch is in part motivated by GCC needing to make
use of the overflow built-ins in constexpr functions (to detect
VLA bounds overflow).

Martin

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Wed, Jun 01, 2016 at 09:17:35AM -0600, Martin Sebor wrote:
> I see.  You meant that only the clang compatibility built-ins
> (i.e. the typed ones like  __builtin_uadd_overflow) shouldn't
> be allowed to take null pointer as the last argument, but the
> type-generic ones should.  That would work for C, though it
> won't satisfy the feature request in c/68120 which asks for
> the last argument to be optional (it can't be there since it
> determines the type of the operation).  It will not work for

I don't think we should make it optional.

> C++ 98.  But those might be acceptable limitations (at least
> in C there's a workaround).

__builtin_* is an extension.  Perhaps for C (any) and C++ (< C++14)
you could require that in constant expressions the last argument
of the builtin has to be a NULL pointer cast to some pointer type
(not necessarily a constant expression)?  This can be handled
simply by saving/restoring the constant expression context around
the parsing of the last argument of the builtin, and afterwards
verifying it has the required properties.

For C++14 and later, it would be nice if one could also do:
struct S { int val; bool ovf; };
constexpr S foo (int x, int y)
{
  S ret = { 0, false };
  ret.ovf = __builtin_add_overflow (x, y, );
  return ret;
}
instead of doing:
  ret.ovf = __builtin_add_overflow (x, y, (int *) 0);
  ret.val = (unsigned) x + y;
or similar.
Does that work with your patch?

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

As I said in another mail, IMNSHO you don't want to change the clang
compatibility builtins, therefore arg2 should never be NULL.


Allowing the last argument to be null is the subject of
the enhancement request in c/68120 (one of the two PRs
driving this enhancement).  The argument can only be null
for these overloads and not the type-generic ones because
the latter use the type to which the pointer points to
determine the type in which to do the arithmetic.  Without
this change, C or C++ 98 users wouldn't be able to use the
built-ins in constant expressions (C++ 98 doesn't allow
casting a null pointer in those contexts).


Sorry, I don't understand this argument.  What is wrong on using
const int x = __builtin_add_overflow (1234567, 123456, (int *) NULL);


I see.  You meant that only the clang compatibility built-ins
(i.e. the typed ones like  __builtin_uadd_overflow) shouldn't
be allowed to take null pointer as the last argument, but the
type-generic ones should.  That would work for C, though it
won't satisfy the feature request in c/68120 which asks for
the last argument to be optional (it can't be there since it
determines the type of the operation).  It will not work for
C++ 98.  But those might be acceptable limitations (at least
in C there's a workaround).


?
I don't get any warning/error on
#include 
int *const p = (int *) NULL;
in C89 nor C++98 with -pedantic-errors,


That's because the above is not a constant expression.  If such
a cast were to be used in one like the enum below, GCC in C++ 98
mode would give:

  error: a cast to a type other than an integral or enumeration type 
cannot appear in a constant-expression

   enum { e = (long)(int*)0 };
  ^

(Curiously, this is accepted with a pedantic warning in C mode.)


not to mention that the
builtins are extensions anyway, so what do you accept/reject in its
arguments doesn't need to honor any specific rules.  Even if (int *) NULL
is not allowed for whatever reason, is (int *) 0 disallowed too?  It is
just a matter of what we document.


Since GCC accepts the above as an extension it was tempting
to change G++ to accept it as well.  But such a change seemed
beyond the scope of this patch.  Applying different type system
rules in a set of built-ins than in the rest of the language
wouldn't seem appropriate to me.  I would rather accept the
limitation that the typed built-ins won't be usable in C++ 98
unless Jason recommends to go down that route.

Unless I hear otherwise, I'll go ahead and change the patch
to have only the type-generic built-ins accept a null pointer
as an argument.


I'm not sure I know what part of the patch you're referring
to.  Are you suggesting to submit a separate patch for the
change from "not enough arguments" to "too few arguments"
below?

-   "not enough arguments to function %qE", fndecl);
+   "too few arguments to function %qE", fndecl);


Yeah.  IMO that is an independent change, you can/should add a testcase for
it, and it can go in before or after the patch.


I agree that independent substantive changes are best made
separately.

I made this one part of the patch because it affects the tests
that go with it (that's how I found the inconsistency, by my
tests failing), and because changing the spelling of a warning
seemed too trivial of a change to justify a patch of its own
and taking up all of your time to review and approve it.  But
since you insist I will submit it separately ahead of this
updated patch.

Martin

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Tue, May 31, 2016 at 04:44:45PM -0600, Martin Sebor wrote:
> >I fail to understand this.  You set type to NULL_TREE, and then (not in any
> >kind of loop, nor any label you could goto to) do:
> >   type = type ? type : xxx;
> >That is necessarily equal to type = xxx;, isn't it?
> 
> Each label falls through to the next, so the test prevents
> the subsequent labels from overwriting the value assigned
> to type by the label that matched the switch expression.
> I'm not exactly enamored with these repetitive tests but
> the only alternative that occurred to me was to duplicate
> the whole switch statement, and this seemed like the lesser
> of the two evils.  If you have a suggestion for something
> better I'd be happy to change it.

Ugh, I've missed missing break; Anyway, IMHO we shouldn't change
the old style builtins and therefore you really don't need this.

> >>+  /* When the last argument is a NULL pointer and the first two arguments
> >>+ are constant, attempt to fold the built-in call into a constant
> >>+ expression indicating whether or not it detected an overflow but
> >>+ without storing the result.  */
> >>+  if ((!arg2 || zerop (arg2))
> >
> >As I said in another mail, IMNSHO you don't want to change the clang
> >compatibility builtins, therefore arg2 should never be NULL.
> 
> Allowing the last argument to be null is the subject of
> the enhancement request in c/68120 (one of the two PRs
> driving this enhancement).  The argument can only be null
> for these overloads and not the type-generic ones because
> the latter use the type to which the pointer points to
> determine the type in which to do the arithmetic.  Without
> this change, C or C++ 98 users wouldn't be able to use the
> built-ins in constant expressions (C++ 98 doesn't allow
> casting a null pointer in those contexts).

Sorry, I don't understand this argument.  What is wrong on using
const int x = __builtin_add_overflow (1234567, 123456, (int *) NULL);
?
I don't get any warning/error on
#include 
int *const p = (int *) NULL;
in C89 nor C++98 with -pedantic-errors, not to mention that the
builtins are extensions anyway, so what do you accept/reject in its
arguments doesn't need to honor any specific rules.  Even if (int *) NULL
is not allowed for whatever reason, is (int *) 0 disallowed too?  It is
just a matter of what we document.

The clang compatibility builtins are severely limited and too ugly to live
with, they don't allow different types of the arguments, have just a very
limited set of accepted types, don't allow different result
type, how do you even handle a type for which you don't know if it is
unsigned, unsigned long or unsigned long long?
  size_t a, b, c, d;
...
  if (sizeof (size_t) == sizeof (unsigned long long))
a = __builtin_uaddll_overflow (b, c, );
  else if (sizeof (size_t) == sizeof (unsigned long))
a = __builtin_uaddl_overflow (b, c, );
  else if (sizeof (size_t) == sizeof (unsigned int))
a = __builtin_uadd_overflow (b, c, );
  else
abort ();
?  We really shouldn't encourage these at all.

> I suspect I didn't use integer_zerop because the argument
> is a pointer and not integer, but I'll change it before
> I commit the patch.

Pointers are treated the same as integers, they are represented as
INTEGER_CSTs too.
> 
> >Regarding the varargs vs. named args only different diagnostics, if you
> >think it should change, then IMHO you should submit as independent patch
> >a change to the diagnostics wording, so that the wording is the same, rather
> >than changing functions from fixed arguments to varargs (/ typegeneric).
> 
> I'm not sure I know what part of the patch you're referring
> to.  Are you suggesting to submit a separate patch for the
> change from "not enough arguments" to "too few arguments"
> below?
> 
> - "not enough arguments to function %qE", fndecl);
> + "too few arguments to function %qE", fndecl);

Yeah.  IMO that is an independent change, you can/should add a testcase for
it, and it can go in before or after the patch.

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

On 05/31/2016 03:50 PM, Jakub Jelinek wrote:

On Sun, May 01, 2016 at 10:39:48AM -0600, Martin Sebor wrote:

--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -7878,50 +7878,101 @@ fold_builtin_unordered_cmp (location_t loc, tree 
fndecl, tree arg0, tree arg1,

  static tree
  fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
-tree arg0, tree arg1, tree arg2)
+tree arg0, tree arg1, tree arg2 = NULL_TREE)
  {
enum internal_fn ifn = IFN_LAST;
-  tree type = TREE_TYPE (TREE_TYPE (arg2));
-  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
+  enum tree_code opcode;
+
+  /* For the "generic" overloads, the first two arguments can have different
+ types and the last argument determines the target type to use to check
+ for overflow.  The names of each of the other overloads determines
+ the target type and so the last argument can be omitted.  */
+  tree type = NULL_TREE;
+
switch (fcode)
  {
  case BUILT_IN_ADD_OVERFLOW:
+  type = type ? type : TREE_TYPE (TREE_TYPE (arg2));
  case BUILT_IN_SADD_OVERFLOW:
+  type = type ? type : integer_type_node;
  case BUILT_IN_SADDL_OVERFLOW:
+  type = type ? type : long_integer_type_node;
  case BUILT_IN_SADDLL_OVERFLOW:
+  type = type ? type : long_long_integer_type_node;
  case BUILT_IN_UADD_OVERFLOW:
+  type = type ? type : unsigned_type_node;
  case BUILT_IN_UADDL_OVERFLOW:
+  type = type ? type : long_unsigned_type_node;
  case BUILT_IN_UADDLL_OVERFLOW:
+  type = type ? type : long_long_unsigned_type_node;
ifn = IFN_ADD_OVERFLOW;
+  opcode = PLUS_EXPR;


...

I fail to understand this.  You set type to NULL_TREE, and then (not in any
kind of loop, nor any label you could goto to) do:
   type = type ? type : xxx;
That is necessarily equal to type = xxx;, isn't it?


Each label falls through to the next, so the test prevents
the subsequent labels from overwriting the value assigned
to type by the label that matched the switch expression.
I'm not exactly enamored with these repetitive tests but
the only alternative that occurred to me was to duplicate
the whole switch statement, and this seemed like the lesser
of the two evils.  If you have a suggestion for something
better I'd be happy to change it.


+  /* When the last argument is a NULL pointer and the first two arguments
+ are constant, attempt to fold the built-in call into a constant
+ expression indicating whether or not it detected an overflow but
+ without storing the result.  */
+  if ((!arg2 || zerop (arg2))


As I said in another mail, IMNSHO you don't want to change the clang
compatibility builtins, therefore arg2 should never be NULL.


Allowing the last argument to be null is the subject of
the enhancement request in c/68120 (one of the two PRs
driving this enhancement).  The argument can only be null
for these overloads and not the type-generic ones because
the latter use the type to which the pointer points to
determine the type in which to do the arithmetic.  Without
this change, C or C++ 98 users wouldn't be able to use the
built-ins in constant expressions (C++ 98 doesn't allow
casting a null pointer in those contexts).


As it is a pointer, shouldn't the above be integer_zerop instead?


I suspect I didn't use integer_zerop because the argument
is a pointer and not integer, but I'll change it before
I commit the patch.


Regarding the varargs vs. named args only different diagnostics, if you
think it should change, then IMHO you should submit as independent patch
a change to the diagnostics wording, so that the wording is the same, rather
than changing functions from fixed arguments to varargs (/ typegeneric).


I'm not sure I know what part of the patch you're referring
to.  Are you suggesting to submit a separate patch for the
change from "not enough arguments" to "too few arguments"
below?

-   "not enough arguments to function %qE", fndecl);
+   "too few arguments to function %qE", fndecl);

Martin

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Sun, May 01, 2016 at 10:39:48AM -0600, Martin Sebor wrote:
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -7878,50 +7878,101 @@ fold_builtin_unordered_cmp (location_t loc, tree 
> fndecl, tree arg0, tree arg1,
>  
>  static tree
>  fold_builtin_arith_overflow (location_t loc, enum built_in_function fcode,
> -  tree arg0, tree arg1, tree arg2)
> +  tree arg0, tree arg1, tree arg2 = NULL_TREE)
>  {
>enum internal_fn ifn = IFN_LAST;
> -  tree type = TREE_TYPE (TREE_TYPE (arg2));
> -  tree mem_arg2 = build_fold_indirect_ref_loc (loc, arg2);
> +  enum tree_code opcode;
> +
> +  /* For the "generic" overloads, the first two arguments can have different
> + types and the last argument determines the target type to use to check
> + for overflow.  The names of each of the other overloads determines
> + the target type and so the last argument can be omitted.  */
> +  tree type = NULL_TREE;
> +
>switch (fcode)
>  {
>  case BUILT_IN_ADD_OVERFLOW:
> +  type = type ? type : TREE_TYPE (TREE_TYPE (arg2));
>  case BUILT_IN_SADD_OVERFLOW:
> +  type = type ? type : integer_type_node;
>  case BUILT_IN_SADDL_OVERFLOW:
> +  type = type ? type : long_integer_type_node;
>  case BUILT_IN_SADDLL_OVERFLOW:
> +  type = type ? type : long_long_integer_type_node;
>  case BUILT_IN_UADD_OVERFLOW:
> +  type = type ? type : unsigned_type_node;
>  case BUILT_IN_UADDL_OVERFLOW:
> +  type = type ? type : long_unsigned_type_node;
>  case BUILT_IN_UADDLL_OVERFLOW:
> +  type = type ? type : long_long_unsigned_type_node;
>ifn = IFN_ADD_OVERFLOW;
> +  opcode = PLUS_EXPR;

...

I fail to understand this.  You set type to NULL_TREE, and then (not in any
kind of loop, nor any label you could goto to) do:
  type = type ? type : xxx;
That is necessarily equal to type = xxx;, isn't it?
>break;
>  case BUILT_IN_SUB_OVERFLOW:
> +  type = type ? type : TREE_TYPE (TREE_TYPE (arg2));
>  case BUILT_IN_SSUB_OVERFLOW:
> +  type = type ? type : integer_type_node;
>  case BUILT_IN_SSUBL_OVERFLOW:
> +  type = type ? type : long_integer_type_node;
>  case BUILT_IN_SSUBLL_OVERFLOW:
> +  type = type ? type : long_long_integer_type_node;
>  case BUILT_IN_USUB_OVERFLOW:
> +  type = type ? type : unsigned_type_node;
>  case BUILT_IN_USUBL_OVERFLOW:
> +  type = type ? type : long_unsigned_type_node;
>  case BUILT_IN_USUBLL_OVERFLOW:
> +  type = type ? type : long_long_unsigned_type_node;
>ifn = IFN_SUB_OVERFLOW;
> +  opcode = MINUS_EXPR;
>break;
>  case BUILT_IN_MUL_OVERFLOW:
> +  type = type ? type : TREE_TYPE (TREE_TYPE (arg2));
>  case BUILT_IN_SMUL_OVERFLOW:
> +  type = type ? type : integer_type_node;
>  case BUILT_IN_SMULL_OVERFLOW:
> +  type = type ? type : long_integer_type_node;
>  case BUILT_IN_SMULLL_OVERFLOW:
> +  type = type ? type : long_long_integer_type_node;
>  case BUILT_IN_UMUL_OVERFLOW:
> +  type = type ? type : unsigned_type_node;
>  case BUILT_IN_UMULL_OVERFLOW:
> +  type = type ? type : long_unsigned_type_node;
>  case BUILT_IN_UMULLL_OVERFLOW:
> +  type = type ? type : long_long_unsigned_type_node;
>ifn = IFN_MUL_OVERFLOW;
> +  opcode = MULT_EXPR;
>break;
>  default:
>gcc_unreachable ();
>  }
> +
> +  /* When the last argument is a NULL pointer and the first two arguments
> + are constant, attempt to fold the built-in call into a constant
> + expression indicating whether or not it detected an overflow but
> + without storing the result.  */
> +  if ((!arg2 || zerop (arg2))

As I said in another mail, IMNSHO you don't want to change the clang
compatibility builtins, therefore arg2 should never be NULL.
As it is a pointer, shouldn't the above be integer_zerop instead?

Regarding the varargs vs. named args only different diagnostics, if you
think it should change, then IMHO you should submit as independent patch
a change to the diagnostics wording, so that the wording is the same, rather
than changing functions from fixed arguments to varargs (/ typegeneric).

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jakub Jelinek]

On Tue, May 31, 2016 at 05:31:48PM -0400, Jason Merrill wrote:
> >I'm not quite sure where to move this hunk so that it could be
> >shared or with what.
> >
> >With the patch, fold_builtin_arith_overflow returns the overflow
> >bit alone when the last argument is null.  Otherwise it returns
> >a complex number with both the overflow bit and the result.  Here
> >we need both parts of the result.  I suppose I could factor out
> >the call to size_binop_loc into its own function, have it return
> >the overflow bit, and set a by-reference argument to the arithmetic
> >result so that it could be built into a complex result here but
> >that doesn't seem like it would gain us much, if anything.
> 
> Yeah, I'm not sure what I was thinking.  The patch is OK.

Sorry for not paying attention, but I think it is wrong to change the clang
compatibility builtins at all.  They are provided for clang compatibility,
nothing else, therefore we shouldn't change anything on them.
It is reasonable to extend the GNU builtins.

Jakub

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jason Merrill]

On 05/31/2016 04:48 PM, Martin Sebor wrote:

On 05/17/2016 12:54 PM, Jason Merrill wrote:

On 05/01/2016 12:39 PM, Martin Sebor wrote:

+  if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) ==
INTEGER_CST)
+{
+  if (tree result = size_binop_loc (EXPR_LOC_OR_LOC (t,
input_location),
+opcode, arg0, arg1))
+{
+  if (TREE_OVERFLOW (result))
+{
+  /* Reset TREE_OVERFLOW to avoid warnings for the
overflow.  */
+  TREE_OVERFLOW (result) = 0;
+
+  return build_complex (TREE_TYPE (t), result,
integer_one_node);
+}
+
+  return build_complex (TREE_TYPE (t), result, integer_zero_node);
+}
+}


Should this be in the middle-end somewhere, perhaps shared with
fold_builtin_arith_overflow?  I notice that the comment for that
function says that it folds into normal arithmetic if the operation can
never overflow, but I don't see any code that would accomplish that.


I'm not quite sure where to move this hunk so that it could be
shared or with what.

With the patch, fold_builtin_arith_overflow returns the overflow
bit alone when the last argument is null.  Otherwise it returns
a complex number with both the overflow bit and the result.  Here
we need both parts of the result.  I suppose I could factor out
the call to size_binop_loc into its own function, have it return
the overflow bit, and set a by-reference argument to the arithmetic
result so that it could be built into a complex result here but
that doesn't seem like it would gain us much, if anything.


Yeah, I'm not sure what I was thinking.  The patch is OK.

Jason

Re: [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

On 05/17/2016 12:54 PM, Jason Merrill wrote:

On 05/01/2016 12:39 PM, Martin Sebor wrote:

+  if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) ==
INTEGER_CST)
+{
+  if (tree result = size_binop_loc (EXPR_LOC_OR_LOC (t,
input_location),
+opcode, arg0, arg1))
+{
+  if (TREE_OVERFLOW (result))
+{
+  /* Reset TREE_OVERFLOW to avoid warnings for the overflow.  */
+  TREE_OVERFLOW (result) = 0;
+
+  return build_complex (TREE_TYPE (t), result,
integer_one_node);
+}
+
+  return build_complex (TREE_TYPE (t), result, integer_zero_node);
+}
+}


Should this be in the middle-end somewhere, perhaps shared with
fold_builtin_arith_overflow?  I notice that the comment for that
function says that it folds into normal arithmetic if the operation can
never overflow, but I don't see any code that would accomplish that.


I'm not quite sure where to move this hunk so that it could be
shared or with what.

With the patch, fold_builtin_arith_overflow returns the overflow
bit alone when the last argument is null.  Otherwise it returns
a complex number with both the overflow bit and the result.  Here
we need both parts of the result.  I suppose I could factor out
the call to size_binop_loc into its own function, have it return
the overflow bit, and set a by-reference argument to the arithmetic
result so that it could be built into a complex result here but
that doesn't seem like it would gain us much, if anything.

Do you have any suggestions?

Martin

Re: [PATCH] integer overflow checking builtins in constant expressions

[Jason Merrill]

On 05/01/2016 12:39 PM, Martin Sebor wrote:

+  if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
+{
+  if (tree result = size_binop_loc (EXPR_LOC_OR_LOC (t, input_location),
+   opcode, arg0, arg1))
+   {
+ if (TREE_OVERFLOW (result))
+   {
+ /* Reset TREE_OVERFLOW to avoid warnings for the overflow.  */
+ TREE_OVERFLOW (result) = 0;
+
+ return build_complex (TREE_TYPE (t), result, integer_one_node);
+   }
+
+ return build_complex (TREE_TYPE (t), result, integer_zero_node);
+   }
+}


Should this be in the middle-end somewhere, perhaps shared with 
fold_builtin_arith_overflow?  I notice that the comment for that 
function says that it folds into normal arithmetic if the operation can 
never overflow, but I don't see any code that would accomplish that.


Jason

PING 2 [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

Ping 2 of the following patch:
  https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00013.html

On 05/09/2016 10:38 AM, Martin Sebor wrote:

Pinging the following patch:
   https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00013.html

On 05/01/2016 10:39 AM, Martin Sebor wrote:

c/68120 - can't easily deal with integer overflow at compile time,
is an enhancement request to make the integer overflow intrinsics
usable in constant expressions in C (in addition to letting them
be invoked with just two arguments).

The inability to use the built-ins in constant expressions also
limited to non-constexpr the contexts in which the patch for c++/
69517 - SEGV on a VLA with excess initializer elements, was able
to prevent the SEGV.  This limitation is noted in c++/70507 -
integer overflow builtins not constant expressions.

The attached patch implements the request in c/68120 for both
the C and C++ front-ends.  It stops short of providing the new
__builtin_add_wrapv function requested in c/68120.  It doesn't
seem necessary since the same functionality is available with
the patch via the existing built-ins.

With this enhancement in place it will be possible to add the
C++ VLA checking to constexpr functions and fully resolve c++/
69517 (which I plan to do next).

While testing the patch, I also noticed an minor inconsistency
in the text of the diagnostic GCC issues for invalid calls to
the built-ins with insufficient numbers of arguments:  for one
set of built-ins the error says: "not enough arguments," while
for another it says: "too few arguments."  I raised PR c/70883
- inconsistent error message for calls to __builtin_add_overflow
with too few arguments, for this and include a fix in this patch
as well.

Martin

PS The enhancement to call the type-generic built-ins with a null
pointer is not available in C++ 98 mode because GCC doesn't allow
null pointers in constant expressions.  Since C and later versions
of C++ do, it seems that it might be worthwhile to relax the rules
and accept them in C++ 98 as well so that the built-ins can be used
portably across all versions of C++.

PING [PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

Pinging the following patch:
  https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00013.html

On 05/01/2016 10:39 AM, Martin Sebor wrote:

c/68120 - can't easily deal with integer overflow at compile time,
is an enhancement request to make the integer overflow intrinsics
usable in constant expressions in C (in addition to letting them
be invoked with just two arguments).

The inability to use the built-ins in constant expressions also
limited to non-constexpr the contexts in which the patch for c++/
69517 - SEGV on a VLA with excess initializer elements, was able
to prevent the SEGV.  This limitation is noted in c++/70507 -
integer overflow builtins not constant expressions.

The attached patch implements the request in c/68120 for both
the C and C++ front-ends.  It stops short of providing the new
__builtin_add_wrapv function requested in c/68120.  It doesn't
seem necessary since the same functionality is available with
the patch via the existing built-ins.

With this enhancement in place it will be possible to add the
C++ VLA checking to constexpr functions and fully resolve c++/
69517 (which I plan to do next).

While testing the patch, I also noticed an minor inconsistency
in the text of the diagnostic GCC issues for invalid calls to
the built-ins with insufficient numbers of arguments:  for one
set of built-ins the error says: "not enough arguments," while
for another it says: "too few arguments."  I raised PR c/70883
- inconsistent error message for calls to __builtin_add_overflow
with too few arguments, for this and include a fix in this patch
as well.

Martin

PS The enhancement to call the type-generic built-ins with a null
pointer is not available in C++ 98 mode because GCC doesn't allow
null pointers in constant expressions.  Since C and later versions
of C++ do, it seems that it might be worthwhile to relax the rules
and accept them in C++ 98 as well so that the built-ins can be used
portably across all versions of C++.

[PATCH] integer overflow checking builtins in constant expressions

[Martin Sebor]

c/68120 - can't easily deal with integer overflow at compile time,
is an enhancement request to make the integer overflow intrinsics
usable in constant expressions in C (in addition to letting them
be invoked with just two arguments).

The inability to use the built-ins in constant expressions also
limited to non-constexpr the contexts in which the patch for c++/
69517 - SEGV on a VLA with excess initializer elements, was able
to prevent the SEGV.  This limitation is noted in c++/70507 -
integer overflow builtins not constant expressions.

The attached patch implements the request in c/68120 for both
the C and C++ front-ends.  It stops short of providing the new
__builtin_add_wrapv function requested in c/68120.  It doesn't
seem necessary since the same functionality is available with
the patch via the existing built-ins.

With this enhancement in place it will be possible to add the
C++ VLA checking to constexpr functions and fully resolve c++/
69517 (which I plan to do next).

While testing the patch, I also noticed an minor inconsistency
in the text of the diagnostic GCC issues for invalid calls to
the built-ins with insufficient numbers of arguments:  for one
set of built-ins the error says: "not enough arguments," while
for another it says: "too few arguments."  I raised PR c/70883
- inconsistent error message for calls to __builtin_add_overflow
with too few arguments, for this and include a fix in this patch
as well.

Martin

PS The enhancement to call the type-generic built-ins with a null
pointer is not available in C++ 98 mode because GCC doesn't allow
null pointers in constant expressions.  Since C and later versions
of C++ do, it seems that it might be worthwhile to relax the rules
and accept them in C++ 98 as well so that the built-ins can be used
portably across all versions of C++.

PR c++/70507 - integer overflow builtins not constant expressions
PR c/68120 - can't easily deal with integer overflow at compile time
PR c/70883 - inconsistent error message for calls to __builtin_add_overflow
	with too few arguments

gcc/testsuite/ChangeLog:
2016-04-30  Martin Sebor  

	PR c++/70507
	PR c/68120
	PR c/70883
	* c-c++-common/builtin-arith-overflow-1.c: Add test cases.
	* c-c++-common/builtin-arith-overflow-2.c: New test.
	* g++.dg/cpp0x/constexpr-arith-overflow.C: New test.
	* gcc.dg/builtin-constant_p-1.c: Adjust text of diagnostic.
	* gcc.dg/builtins-error.c:  Same.

gcc/cp/ChangeLog:
2016-04-30  Martin Sebor  

	PR c++/70507
	PR c/68120
	PR c/70883
	* constexpr.c (cxx_eval_internal_function): New function.
	(cxx_eval_call_expression): Call it.
	* tree.c (builtin_valid_in_constant_expr_p): Handle integer
	arithmetic overflow built-ins.

gcc/c-family/ChangeLog:
2016-04-30  Martin Sebor  

	PR c/68120
	PR c/70883
	* c-common.c (builtin_function_validate_nargs): Adjust text
	of diagnostic.
	(check_builtin_function_arguments): Allow type-specific integer
	arithmetic overflow built-ins to take either 2 or three arguments.

gcc/ChangeLog:
2016-04-30  Martin Sebor  

	PR c++/70507
	PR c/68120
	PR c/70883
	* builtin-types.def (BT_FN_BOOL_INT_INT_VAR, BT_FN_BOOL_LONG_LONG_VAR)
	(BT_FN_BOOL_LONGLONG_LONGLONG_VAR, BT_FN_BOOL_UINT_UINT_VAR)
	(BT_FN_BOOL_UINT_UINT_VAR, BT_FN_BOOL_ULONG_ULONG_VAR)
	(BT_FN_BOOL_ULONGLONG_ULONGLONG_VAR): New.
	* builtins.c (fold_builtin_unordered_cmp): Handle integer arithmetic
	overflow built-ins.
	(fold_builtin_2): Handle invocations of type-specific integer arithmetic
	overflow built-ins with two arguments.
	* builtins.def (BUILT_IN_SADD_OVERFLOW, BUILT_IN_SADDL_OVERFLOW):
	Declare built-ins to take two or more arguments (constraining them
	to take two or three in c-common.c).
	(BUILT_IN_SADDLL_OVERFLOW, BUILT_IN_SSUB_OVERFLOW): Same.
	(BUILT_IN_SSUBL_OVERFLOW, BUILT_IN_SSUBLL_OVERFLOW): Same.
	(BUILT_IN_SMUL_OVERFLOW, BUILT_IN_SMULL_OVERFLOW): Same.
	(BUILT_IN_SMULLL_OVERFLOW, BUILT_IN_UADD_OVERFLOW): Same.
	(BUILT_IN_UADDL_OVERFLOW, BUILT_IN_UADDLL_OVERFLOW): Same.
	(BUILT_IN_USUB_OVERFLOW, BUILT_IN_USUBL_OVERFLOW): Same.
	(BUILT_IN_USUBLL_OVERFLOW, BUILT_IN_UMUL_OVERFLOW): Same.
	(BUILT_IN_UMULL_OVERFLOW, BUILT_IN_UMULLL_OVERFLOW): Same.
	* doc/extend.texi (Integer Overflow Builtins): Update.

diff --git a/gcc/builtin-types.def b/gcc/builtin-types.def
index 7fab9f8..ee63284 100644
--- a/gcc/builtin-types.def
+++ b/gcc/builtin-types.def
@@ -451,6 +451,16 @@ DEF_FUNCTION_TYPE_3 (BT_FN_BOOL_ULONG_ULONG_ULONGPTR, BT_BOOL, BT_ULONG,
 DEF_FUNCTION_TYPE_3 (BT_FN_BOOL_ULONGLONG_ULONGLONG_ULONGLONGPTR, BT_BOOL,
 		 BT_ULONGLONG, BT_ULONGLONG, BT_PTR_ULONGLONG)
 
+DEF_FUNCTION_TYPE_VAR_2 (BT_FN_BOOL_INT_INT_VAR, BT_BOOL, BT_INT, BT_INT)
+DEF_FUNCTION_TYPE_VAR_2 (BT_FN_BOOL_LONG_LONG_VAR, BT_BOOL, BT_LONG, BT_LONG)
+DEF_FUNCTION_TYPE_VAR_2 (BT_FN_BOOL_LONGLONG_LONGLONG_VAR, BT_BOOL,
+			 BT_LONGLONG, BT_LONGLONG)
+DEF_FUNCTION_TYPE_VAR_2 (BT_FN_BOOL_UINT_UINT_VAR, BT_BOOL, BT_UINT, BT_UINT)