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