Bug 68120 - can't easily deal with integer overflow at compile time
Summary: can't easily deal with integer overflow at compile time
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: unknown
: P3 enhancement
Target Milestone: ---
Assignee: Martin Sebor
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-27 20:15 UTC by Paul Eggert
Modified: 2017-02-16 04:41 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 4.9.3, 5.3.0, 6.0
Last reconfirmed: 2015-10-28 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Eggert 2015-10-27 20:15:13 UTC
This is a followup to Bug#61129, which asked for integer-overflow-detecting arithmetic intrinsics. We have them now (thanks!) but I'm running into some problems using them.

The Gnulib intprops module defines macros to check for integer overflow, e.g., INT_ADD_OVERFLOW (a, b) returns 1 if the C expression A+B would overflow. These macros are implemented in portable C. I'm trying to use GCC 5's new __builtin_add_overflow etc. functions to speed up their runtime execution. This turns out to be a pain because these macros are used in integer constant expressions, where __builtin_add_overflow etc. cannot be used.

I am working around the problem with macro definitions like this:

 # define INT_ADD_OVERFLOW(a, b) \
   (__builtin_constant_p ((a) == (b)) \
    ? _GL_INT_ADD_OVERFLOW (a, b) \
    : __builtin_add_overflow (a, b, &(__typeof__ ((a) + (b))) {0}))

where _GL_INT_ADD_OVERFLOW is a complex macro implemented in Standard C and is a constant expression when its arguments are constant expressions. Although I think this will work, it is ugly, and the macro _GL_INT_ADD_OVERFLOW is complicated.

To simplify use of __builtin_add_overflow etc., I suggest extending these builtins as follows.

First, allow the 3rd argument to be a null pointer constant of type pointer-to-integer, meaning that the caller only wants to know if the overflow occurred and does not want the low-order bits of the result. E.g., __builtin_add_overflow (a, b, (int *) 0) returns 1 if the mathematical value of A + B does not fit in 'int'.  This would allow the above macro to be simplified to:

 # define INT_ADD_OVERFLOW(a, b) \
   __builtin_add_overflow (a, b, (__typeof__ ((a) + (b)) *) 0)

Second, allow the 3rd argument to be omitted, as a shorthand for a null pointer to the integer type of A+B. This further simplification would allow the above macro to be simplified to:

  # define INT_ADD_OVERFLOW(a, b) __builtin_add_overflow (a, b)

Third, add a function __builtin_add_wrapv (a, b, c) that acts like __builtin_add_overflow (a, b, c) except that it returns the value that would be stored if C were a non-null pointer.  The 3rd argument C is optional here too, with the same semantics as for __builtin_add_overflow.  This will let the user get the bottom-order bits of an overflowed value in a constant expression, something that can't be done with the current primitives.

The basic idea here is that we need intrinsics that deal with values and that do not require non-null addresses, so that we can use these intrinsics in constant expressions.
Comment 1 Richard Biener 2015-10-28 12:08:33 UTC
Maybe the FE constant folding code simply needs to handle them specially?  But overloads sound possible.
Comment 2 Paul Eggert 2015-12-18 17:41:52 UTC
(In reply to Paul Eggert from comment #0)
> I am working around the problem with macro definitions like this:
> 
>  # define INT_ADD_OVERFLOW(a, b) \
>    (__builtin_constant_p ((a) == (b)) \
>     ? _GL_INT_ADD_OVERFLOW (a, b) \
>     : __builtin_add_overflow (a, b, &(__typeof__ ((a) + (b))) {0}))

Just to follow up on my own bug report, the above approach turned out not to work, because the resulting macro expands to an expression that is not an integer constant expression according to GCC's current rules. I eventually gave up on this approach entirely, and INT_ADD_OVERFLOW (a, b) now expands to this:

  (((((0 * (0 * (b) + (a)) - (1)) < 0) ? - (~ (0 * (0 * (b) + (a)) + (0)) == -1) - ((((0 * (0 * (b) + (a)) + (1)) << (sizeof ((0 * (b) + (a)) + 0) * 8 - 2)) - 1) * 2 + 1) : (0 * (0 * (b) + (a)) + (0)))) < 0 ? ((b) < 0 ? (a) < ((((0 * (0 * (b) + (a)) - (1)) < 0) ? - (~ (0 * (0 * (b) + (a)) + (0)) == -1) - ((((0 * (0 * (b) + (a)) + (1)) << (sizeof ((0 * (b) + (a)) + 0) * 8 - 2)) - 1) * 2 + 1) : (0 * (0 * (b) + (a)) + (0)))) - (b) : ((((0 * (0 * (b) + (a)) - (1)) < 0) ? ((((0 * (0 * (b) + (a)) + (1)) << (sizeof ((0 * (b) + (a)) + 0) * 8 - 2)) - 1) * 2 + 1) : (0 * (0 * (b) + (a)) - (1)))) - (b) < (a)) : (a) < 0 ? (b) <= (a) + (b) : (b) < 0 ? (a) <= (a) + (b) : (a) + (b) < (b))

Although this *is* an integer constant expression, it is less nice than a simple call to __builtin_add_overflow would be, and it generates less-efficient code in some cases when A and B are not constants.
Comment 3 Martin Sebor 2016-04-01 20:32:03 UTC
I raised c++/70507 to make a record of another problem caused by the built-ins not being treated as constant expressions.
Comment 4 Martin Sebor 2016-04-29 00:42:10 UTC
Testing a patch for this bug as well as bug 70507.
Comment 5 Martin Sebor 2016-05-01 16:41:10 UTC
Patch posted for review:
https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00013.html
Comment 6 Paul Eggert 2016-05-01 18:15:09 UTC
(In reply to Martin Sebor from comment #5)
> Patch posted for review:
> https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00013.html

Thanks. This patch appears to address almost all the request, and it is definitely a step forward.

Even with the change, I see no easy way in a constant expression to compute the bottom-order bits (ignoring overflow) of a signed integer value, but I can raise this issue as a separate enhancement request if it turns into a problem in practice.
Comment 7 Jakub Jelinek 2016-06-08 19:03:49 UTC
Author: jakub
Date: Wed Jun  8 19:03:17 2016
New Revision: 237238

URL: https://gcc.gnu.org/viewcvs?rev=237238&root=gcc&view=rev
Log:
	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.

Added:
    trunk/gcc/testsuite/c-c++-common/builtin-arith-overflow-2.c
    trunk/gcc/testsuite/g++.dg/cpp0x/constexpr-arith-overflow.C
    trunk/gcc/testsuite/g++.dg/cpp1y/constexpr-arith-overflow.C
    trunk/gcc/testsuite/g++.dg/ext/builtin-arith-overflow-1.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/builtins.c
    trunk/gcc/builtins.def
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-typeck.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/constexpr.c
    trunk/gcc/cp/tree.c
    trunk/gcc/doc/extend.texi
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/builtin-arith-overflow-1.c
Comment 8 Martin Sebor 2016-06-13 15:57:41 UTC
r237238 lets GCC 7 accept the integer overflow built-ins in constant expressions whenever their arguments are.