On Wed, Feb 19, 2025 at 10:14:21AM -0500, Patrick Palka wrote:
> On Wed, 19 Feb 2025, Marek Polacek wrote:
>
> > I suppose it's safer to leave this for GCC 16, but anyway:
> >
> > Bootstrapped/regtested on x86_64-pc-linux-gnu.
> >
> > -- >8 --
> > Since r10-7718 the attached tests produce an ICE in verify_address:
> >
> > error: constant not recomputed when 'ADDR_EXPR' changed
> >
> > but before that we wrongly rejected the tests with "is not a constant
> > expression". This patch fixes both problems.
> >
> > Since r10-7718 replace_decl_r can replace
> >
> > {._M_dataplus=&<retval>._M_local_buf, ._M_local_buf=0}
> >
> > with
> >
> > {._M_dataplus=&HelloWorld._M_local_buf, ._M_local_buf=0}
> >
> > The initial &<retval>._M_local_buf was not constant, but since
> > HelloWorld is a static VAR_DECL, the resulting &HelloWorld._M_local_buf
> > should have been marked as TREE_CONSTANT. And since we're taking
> > its address, the whole thing should be TREE_ADDRESSABLE.
> >
> > PR c++/114913
> > PR c++/110822
> >
> > gcc/cp/ChangeLog:
> >
> > * constexpr.cc (replace_decl_r): If we've replaced something
> > inside of an ADDR_EXPR, call cxx_mark_addressable and
> > recompute_tree_invariant_for_addr_expr on the resulting ADDR_EXPR.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp0x/constexpr-nsdmi4.C: New test.
> > * g++.dg/cpp0x/constexpr-nsdmi5.C: New test.
> > ---
> > gcc/cp/constexpr.cc | 17 ++++++++++++++++-
> > gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi4.C | 19 +++++++++++++++++++
> > gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi5.C | 15 +++++++++++++++
> > 3 files changed, 50 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi4.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi5.C
> >
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index 59dd0668af3..c41454057cb 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -2707,7 +2707,22 @@ replace_decl_r (tree *tp, int *walk_subtrees, void
> > *data)
> > {
> > replace_decl_data *d = (replace_decl_data *) data;
> >
> > - if (*tp == d->decl)
> > + /* We could be replacing
> > + &<retval>.bar -> &foo.bar
> > + where foo is a static VAR_DECL, so we need to recompute TREE_CONSTANT
> > + on the ADDR_EXPR around it. */
> > + if (TREE_CODE (*tp) == ADDR_EXPR)
> > + {
> > + d->pset->add (*tp);
> > + cp_walk_tree (&TREE_OPERAND (*tp, 0), replace_decl_r, d, nullptr);
> > + if (d->changed)
>
> d->changed could already be true if something before the ADDR_EXPR was
> replaced. I think we should clear/restore d->changed around the
> cp_walk_tree call to accurately track whether anything inside the
> ADDR_EXPR was replaced. Otherwise LGTM
You're right! I believe I have to restore d->changed like this. Thanks.
Tested on x86_64-pc-linux-gnu.
-- >8 --
Since r10-7718 the attached tests produce an ICE in verify_address:
error: constant not recomputed when 'ADDR_EXPR' changed
but before that we wrongly rejected the tests with "is not a constant
expression". This patch fixes both problems.
Since r10-7718 replace_decl_r can replace
{._M_dataplus=&<retval>._M_local_buf, ._M_local_buf=0}
with
{._M_dataplus=&HelloWorld._M_local_buf, ._M_local_buf=0}
The initial &<retval>._M_local_buf was not constant, but since
HelloWorld is a static VAR_DECL, the resulting &HelloWorld._M_local_buf
should have been marked as TREE_CONSTANT. And since we're taking
its address, the whole thing should be TREE_ADDRESSABLE.
PR c++/114913
PR c++/110822
gcc/cp/ChangeLog:
* constexpr.cc (replace_decl_r): If we've replaced something
inside of an ADDR_EXPR, call cxx_mark_addressable and
recompute_tree_invariant_for_addr_expr on the resulting ADDR_EXPR.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-nsdmi4.C: New test.
* g++.dg/cpp0x/constexpr-nsdmi5.C: New test.
---
gcc/cp/constexpr.cc | 21 ++++++++++++++++++-
gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi4.C | 19 +++++++++++++++++
gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi5.C | 15 +++++++++++++
3 files changed, 54 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi4.C
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi5.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 59dd0668af3..a478faddbb5 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2707,7 +2707,26 @@ replace_decl_r (tree *tp, int *walk_subtrees, void *data)
{
replace_decl_data *d = (replace_decl_data *) data;
- if (*tp == d->decl)
+ /* We could be replacing
+ &<retval>.bar -> &foo.bar
+ where foo is a static VAR_DECL, so we need to recompute TREE_CONSTANT
+ on the ADDR_EXPR around it. */
+ if (TREE_CODE (*tp) == ADDR_EXPR)
+ {
+ d->pset->add (*tp);
+ auto save_changed = d->changed;
+ d->changed = false;
+ cp_walk_tree (&TREE_OPERAND (*tp, 0), replace_decl_r, d, nullptr);
+ if (d->changed)
+ {
+ cxx_mark_addressable (*tp);
+ recompute_tree_invariant_for_addr_expr (*tp);
+ }
+ else
+ d->changed = save_changed;
+ *walk_subtrees = 0;
+ }
+ else if (*tp == d->decl)
{
*tp = unshare_expr (d->replacement);
d->changed = true;
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi4.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi4.C
new file mode 100644
index 00000000000..360470dbccb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi4.C
@@ -0,0 +1,19 @@
+// PR c++/114913
+// { dg-do compile { target c++11 } }
+
+struct strt {
+ char *_M_dataplus;
+ char _M_local_buf = 0;
+ constexpr strt()
+ : _M_dataplus(&_M_local_buf) {}
+ constexpr strt(const strt &)
+ : _M_dataplus(&_M_local_buf) {}
+};
+
+constexpr strt
+f ()
+{
+ return {};
+}
+constexpr strt HelloWorld = f();
+const char *a() { return HelloWorld._M_dataplus; }
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi5.C
b/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi5.C
new file mode 100644
index 00000000000..0a0acaa9fdf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi5.C
@@ -0,0 +1,15 @@
+// PR c++/110822
+// { dg-do compile { target c++11 } }
+
+void __ostream_insert(const char*);
+struct basic_string {
+ const char* _M_p;
+ char _M_local_buf[16] = {};
+ constexpr basic_string() : _M_p(_M_local_buf) {}
+ const char *data() const { return _M_p; }
+};
+constexpr basic_string f() { return {}; }
+constexpr basic_string text = f();
+int main() {
+ __ostream_insert(text._M_p);
+}
base-commit: ee6619b1246b38cfb36f6efd931a6f475a9033c7
--
2.48.1
