Bug 70323 - [6 regression] missing error on integer overflow in constexpr function result converted to bool
Summary: [6 regression] missing error on integer overflow in constexpr function result...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 6.0
: P1 normal
Target Milestone: 6.0
Assignee: Jason Merrill
URL:
Keywords: accepts-invalid
Depends on:
Blocks: constexpr
  Show dependency treegraph
 
Reported: 2016-03-20 17:31 UTC by Martin Sebor
Modified: 2016-03-24 18:06 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.9.3, 5.3.0
Known to fail: 6.0
Last reconfirmed: 2016-03-21 00:00:00


Attachments
gcc6-pr70323.patch (590 bytes, patch)
2016-03-22 10:09 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2016-03-20 17:31:04 UTC
The following invalid program is rejected by GCC 5 with two errors, one due to each of the two erroneous calls to the constexpr functions.  The program is now accepted by GCC 6.0.  The errors are restored when bool is replaced with int.

The first invalid call became accepted as a result of r233878.  The second one sometime last week (after r234169).

$ cat t.c && /build/gcc-trunk/gcc/xgcc -B/build/gcc-trunk/gcc -S -Wall -Wextra -Wpedantic -o/dev/null -xc++ t.c
constexpr int overflow_if_0 (int i) { return __INT_MAX__ + !i; }
constexpr int overflow_if_1 (int i) { return __INT_MAX__ + i; }

constexpr bool i0_0 = overflow_if_0 (0);   // error expected
constexpr bool i0_1 = overflow_if_0 (1);
constexpr bool i1_0 = overflow_if_1 (0);
constexpr bool i1_1 = overflow_if_1 (1);   // error expected
$
Comment 1 Jakub Jelinek 2016-03-22 09:09:59 UTC
I'm still getting the second error even with latest trunk, so can only partially reproduce it:
pr70323.C:7:37:   in constexpr expansion of ‘overflow_if_1(1)’
pr70323.C:7:39: error: overflow in constant expression [-fpermissive]
 constexpr bool i1_1 = overflow_if_1 (1);   // error expected
                                       ^
I agree that r233878 introduced the only regression I can reproduce here.  No revision between r234100 and r234359 emits no errors (checked all of them), so you must have had some local changes in.
Comment 2 Jakub Jelinek 2016-03-22 09:44:03 UTC
As for the missing first error, the function is cp_folded into
return <retval> = i == 0 ? -2147483648(OVF) : 2147483647;
Thus, I wonder if VERIFY_CONSTANT or something similar shouldn't be done either
in cxx_eval_constant_expression's
  if (CONSTANT_CLASS_P (t))
    {
      if (TREE_OVERFLOW (t) && (!flag_permissive || ctx->quiet))
        *overflow_p = true;
      return t;
    }
or on the other side, when evaluating the whole constexpr function call (and if there shouldn't be some way to report it only once, not many times).
I think there is some other PR that it would be nice to just remember what kind of errors were seen during constexpr evaluation and at what locus and let the caller emit the diagnostics.
Comment 3 Jakub Jelinek 2016-03-22 10:09:41 UTC
Created attachment 38054 [details]
gcc6-pr70323.patch

This seems like it would work, but haven't bootstrapped/regtested it yet.
Comment 4 Martin Sebor 2016-03-22 15:21:13 UTC
That's odd.  I do have local changes in my tree but I verified it on three other machines.  I've retested with today's pristine top of trunk on powerpc64le, still with no errors.
Comment 5 Martin Sebor 2016-03-22 15:34:25 UTC
(In reply to Jakub Jelinek from comment #3)
> Created attachment 38054 [details]

Hmm.  Something else must be going on.  I've applied your patch on powerpc64le but it hasn't changed anything.
Comment 6 Martin Sebor 2016-03-22 15:42:24 UTC
I see the problem: It's -Wall that suppresses the error.
Comment 7 Jakub Jelinek 2016-03-22 15:52:59 UTC
(In reply to Martin Sebor from comment #6)
> I see the problem: It's -Wall that suppresses the error.

Yeah, seems with -Wall ctx->quiet is true (probably desirable, for some kind of warning we don't want to warn multiple times), but likely we then cache the results in a hash table.
So we either need to cache also the ctx->quiet flag along with the from and to
trees and only use cache if either ctx->quiet or if the cached quiet value matches the remembered value, or not cache ctx->quiet lookups (bad), or cache the warnings/errors we need to emit along with the cached values.
Comment 8 Jakub Jelinek 2016-03-23 18:45:58 UTC
Author: jakub
Date: Wed Mar 23 18:45:26 2016
New Revision: 234438

URL: https://gcc.gnu.org/viewcvs?rev=234438&root=gcc&view=rev
Log:
	PR c++/70323
	* constexpr.c (cxx_eval_constant_expression): Diagnose overflow
	on TREE_OVERFLOW constants.

	* g++.dg/cpp0x/constexpr-70323.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/constexpr-70323.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/constexpr.c
    trunk/gcc/testsuite/ChangeLog
Comment 9 Jakub Jelinek 2016-03-23 18:50:34 UTC
One part fixed, the -Wall part still broken.
Comment 10 Jason Merrill 2016-03-24 18:00:31 UTC
Author: jason
Date: Thu Mar 24 17:59:58 2016
New Revision: 234463

URL: https://gcc.gnu.org/viewcvs?rev=234463&root=gcc&view=rev
Log:
	PR c++/70323

	* constexpr.c (cxx_eval_call_expression): Don't cache result if
	*overflow_p.

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/constexpr-70323a.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/constexpr.c
Comment 11 Jason Merrill 2016-03-24 18:06:58 UTC
Fixed.