Bug 71537 - GCC rejects consetxpr boolean conversions and comparisons on the result of pointer arithmetic.
Summary: GCC rejects consetxpr boolean conversions and comparisons on the result of po...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 7.0
: P3 normal
Target Milestone: ---
Assignee: Jakub Jelinek
URL:
Keywords: rejects-valid
: 71457 (view as bug list)
Depends on:
Blocks: constexpr
  Show dependency treegraph
 
Reported: 2016-06-14 20:30 UTC by Eric Fiselier
Modified: 2017-02-15 23:41 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 4.9.3, 5.3.0, 6.1.0, 7.0
Last reconfirmed: 2016-06-14 00:00:00


Attachments
gcc7-pr71537-strchr.patch (833 bytes, patch)
2016-12-05 12:09 UTC, Jakub Jelinek
Details | Diff
gcc7-pr71537-memchr.patch (1.38 KB, patch)
2016-12-05 13:36 UTC, Jakub Jelinek
Details | Diff
gcc7-pr71537.patch (972 bytes, patch)
2016-12-05 14:38 UTC, Jakub Jelinek
Details | Diff
gcc7-pr71537-2.patch (755 bytes, patch)
2017-01-10 11:25 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Fiselier 2016-06-14 20:30:51 UTC
For example:

constexpr int n[42] = {1};
constexpr int x = n ? 1 : 0; // Accepted.
constexpr int xx = n + 1 ? 1 : 0; // Rejected
constexpr int xxx = "abc" + 1 ? 1 : 0; // Also Rejected

This has a significant impact on constexpr algorithms which operate over raw pointers since pointer comparisons are not considered constant expressions.

For example this bug breaks libc++'s implementation of string_view::find and renders the "char_traits<char>::find(...)" interface useless since it returns a possibly null pointer.
Comment 1 Martin Sebor 2016-06-14 23:27:31 UTC
Confirmed, though only the xxx initialization is rejected with GCC 5 and later, the array case works (4.9.3 rejects both xx and xxx).  It looks like the fold_comparison function that handles these operations for arrays isn't prepared to do the same for a string literal.  Below is the test with the failing case simplified to converting a POINTER_PLUS expression directly to bool.  (The -Waddress warning should probably get its own bug.)

I CC Jakub who has been making improvements in this area lately (bug 71448 and bug 67376 before it).

$ cat t.C && /home/msebor/build/gcc-trunk-svn/gcc/xgcc -B /home/msebor/build/gcc-trunk-svn/gcc -Wall -Wextra -Wpedantic t.C
constexpr int n[42] = {1};
constexpr int x = n ? 1 : 0;           // Accepted
constexpr int xx = n + 1 ? 1 : 0;      // Accepted 
constexpr int xxx = "abc" + 1 ? 1 : 0; // Rejected

constexpr bool b = "abc" + 1;          // Rejected (#2)
t.C:2:27: warning: the address of ‘n’ will always evaluate as ‘true’ [-Waddress]
 constexpr int x = n ? 1 : 0; // Accepted.
                           ^
t.C:4:37: error: ‘((((const char*)"abc") + 1u) != 0u)’ is not a constant expression
 constexpr int xxx = "abc" + 1 ? 1 : 0; // Rejected
                                     ^
t.C:6:28: error: ‘((((const char*)"abc") + 1u) != 0u)’ is not a constant expression
 constexpr bool b = "abc" + 1;          // Rejected (#2)
                            ^
Comment 2 Eric Fiselier 2016-06-15 00:05:31 UTC
Hi Martin,

The  'xx' case is only accepted if it occurs at a global scope. If it appears in a function body it is still rejected.
Comment 3 Eric Fiselier 2016-12-03 06:13:25 UTC
Ping and CC Jason the Wizard. This is needed to implement C++17 constexpr char_traits (See wg21.link/P0426R1).
Comment 4 Eric Fiselier 2016-12-03 13:36:36 UTC
> Ping and CC Jason the Wizard. This is needed to implement C++17 constexpr 
> char_traits (See wg21.link/P0426R1).

Same as above but correctly CC Jason.
Comment 5 Jonathan Wakely 2016-12-03 13:42:49 UTC
N.B. for char_traits::char::find() we almost certainly want to use __builtin_strchr and have the FE expand it in-place in constant expressions, as the performance of our current code sucks, see PR 66414.
Comment 6 Jonathan Wakely 2016-12-03 14:13:43 UTC
(In reply to Jonathan Wakely from comment #5)
> N.B. for char_traits::char::find() we almost certainly want to use
> __builtin_strchr and have the FE expand it in-place in constant expressions,
> as the performance of our current code sucks, see PR 66414.

Oops, faulty memory - that bug's about string::find not char_traits::find. Our char_traits<char>::find already uses __builtin_memchr (and should continue to do so, which means we need that to be usable in a constexpr function if it isn't already).

We still need the generic char_traits<CharT>::find to be constexpr too though.
Comment 7 Martin Sebor 2016-12-03 16:59:03 UTC
Unfortunately, __builtin_memchr is not usable in constexpr contexts.  Hardly any of these builtins are.  I think the only exception is __builtin_strlen but only to a limited extent (bug 77357 has some background).  There's a general problem with folding it and other functions like it when the argument is an array (i.e., a CONSTRUCTOR in GCC, as opposed to a STRING_CST).  Bug 78257 has a test case involving memcmp but the same problem applies to memchr.  Once that is solved all of this should just work both in C and in C++ (i.e., even beyond constexpr).
Comment 8 Jakub Jelinek 2016-12-04 13:21:00 UTC
(In reply to Martin Sebor from comment #7)
> Unfortunately, __builtin_memchr is not usable in constexpr contexts.  Hardly
> any of these builtins are.

Can you explain why?  Most of the builtins that can fold on GENERIC should be usable in constexpr contexts.  The constexpr folder invokes the middle-end builtin folder.
Comment 9 Jonathan Wakely 2016-12-04 18:44:32 UTC
I think we need __builtin_memchr et al to be usable in constant expressions, because replacing them with hand-written loops would be a step backwards. For uses outside constant expressions we want to go to libc and benefit from the optimization work put into the libc routines.
Comment 10 Jakub Jelinek 2016-12-05 12:09:29 UTC
Created attachment 40249 [details]
gcc7-pr71537-strchr.patch

For the builtins, it seems to be just recent regressions, people are moving builtins folding out of builtins.c into gimple-fold.c and totally forgetting constexpr.  Here is a patch to handle strchr/strrchr.  I'll look at memchr next.
Comment 11 Jakub Jelinek 2016-12-05 13:36:07 UTC
Created attachment 40250 [details]
gcc7-pr71537-memchr.patch

And this patch handles memchr.
Comment 12 Jakub Jelinek 2016-12-05 14:38:44 UTC
Created attachment 40251 [details]
gcc7-pr71537.patch

Patch for the comparison of string_cst + const offset with nullptr.
Comment 13 Jakub Jelinek 2016-12-06 09:23:08 UTC
Author: jakub
Date: Tue Dec  6 09:22:36 2016
New Revision: 243284

URL: https://gcc.gnu.org/viewcvs?rev=243284&root=gcc&view=rev
Log:
	PR c++/71537
	* fold-const-call.c (fold_const_call): Handle
	CFN_BUILT_IN_{INDEX,STRCHR,RINDEX,STRRCHR}.

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

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/constexpr-strchr.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const-call.c
    trunk/gcc/testsuite/ChangeLog
Comment 14 Jakub Jelinek 2016-12-06 09:24:23 UTC
Author: jakub
Date: Tue Dec  6 09:23:51 2016
New Revision: 243285

URL: https://gcc.gnu.org/viewcvs?rev=243285&root=gcc&view=rev
Log:
2016-12-06  Jakub Jelinek  <jakub@redhat.com>

	PR c++/71537
	* fold-const-call.c (fold_const_call_1): Remove memchr handling here.
	(fold_const_call) <case CFN_BUILT_IN_STRNCMP,
	case CFN_BUILT_IN_STRNCASECMP>: Formatting improvements.
	(fold_const_call) <case CFN_BUILT_IN_MEMCMP>: Likewise.  If s2 is 0
	and arguments have no side-effects, return 0.
	(fold_const_call): Handle CFN_BUILT_IN_MEMCHR.

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

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/constexpr-memchr.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const-call.c
    trunk/gcc/testsuite/ChangeLog
Comment 15 Jakub Jelinek 2016-12-06 09:25:08 UTC
Author: jakub
Date: Tue Dec  6 09:24:36 2016
New Revision: 243286

URL: https://gcc.gnu.org/viewcvs?rev=243286&root=gcc&view=rev
Log:
	PR c++/71537
	* fold-const.c (fold_comparison): Assume CONSTANT_CLASS_P (base0)
	plus offset is non-zero.  For maybe_nonzero_address decl base0,
	require indirect_base0.

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

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/constexpr-71537.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog
Comment 16 Jakub Jelinek 2016-12-07 09:05:24 UTC
Fixed.
Comment 17 Eric Fiselier 2016-12-07 21:20:08 UTC
This has not been fixed. As mentioned the original reproducer still fails to compile when it appears at block scope:

// g++ -std=c++1z test.cpp
constexpr bool foo() {
  constexpr int n[42] = {1};
  constexpr int x = n ? 1 : 0; // Accepted.
  constexpr int xx = n + 1 ? 1 : 0; // Rejected
  constexpr int xxx = "abc" + 1 ? 1 : 0; // Newly accepted.
  return true;
}

int main() { static_assert(foo(), ""); }
Comment 18 Jakub Jelinek 2017-01-10 11:25:38 UTC
Created attachment 40488 [details]
gcc7-pr71537-2.patch

Untested fix for that.
Comment 19 Jakub Jelinek 2017-01-11 20:11:10 UTC
Author: jakub
Date: Wed Jan 11 20:10:36 2017
New Revision: 244333

URL: https://gcc.gnu.org/viewcvs?rev=244333&root=gcc&view=rev
Log:
	PR c++/71537
	* fold-const.c (maybe_nonzero_address): Return 1 for function
	local objects.
	(tree_single_nonzero_warnv_p): Don't handle function local objects
	here.

	* g++.dg/cpp1y/constexpr-71537.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/cpp1y/constexpr-71537.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog
Comment 20 Jakub Jelinek 2017-01-11 20:29:38 UTC
Fixed.
Comment 21 Martin Sebor 2017-02-15 23:41:34 UTC
*** Bug 71457 has been marked as a duplicate of this bug. ***