On Fri, Feb 24, 2017 at 11:11:05AM -0800, Jason Merrill wrote:
> On Fri, Feb 24, 2017 at 8:22 AM, Marek Polacek <pola...@redhat.com> wrote:
> > I had an interesting time tracking down some of the problems with this code.
> > Hopefully I've sussed out now how this stuff works.
> >
> > We've got
> >
> > struct A { char c; };
> > char A::*p = &A::c;
> > static char A::*const q = p;
> > and then
> > &(a.*q) - &a.c
> > which should evaluate to 0. Here "p" will be 0, that's the offset from the
> > start of the struct to "c". "q" is const-qualified and static and
> > initialized
> > with "p", so we get to cp_fold_maybe_rvalue -> decl_constant_value ->
> > constant_value_1. Now, NULL pointer-to-data-members are represented by -1,
> > so
> > that a null pointer is distinguishable from an offset of the first member
> > of a
> > struct (0). So constant_value_1 looks at the DECL_INITIAL of "q", which is
> > -1,
> > a constant, we fold "q" to -1, and sadness ensues. I believe the -1 value
> > is
> > only an internal representation and shouldn't be used like that.
>
> Since q is initialized from p, it shouldn't have a DECL_INITIAL of -1;
> that sounds like the bug.
The DECL_INITIAL of -1 comes from cp_finish_decl:
7038 The memory occupied by any object of static storage
7039 duration is zero-initialized at program startup before
7040 any other initialization takes place.
7041
7042 We cannot create an appropriate initializer until after
7043 the type of DECL is finalized. If DECL_INITIAL is set,
7044 then the DECL is statically initialized, and any
7045 necessary zero-initialization has already been performed. */
7046 if (TREE_STATIC (decl) && !DECL_INITIAL (decl))
7047 DECL_INITIAL (decl) = build_zero_init (TREE_TYPE (decl),
7048 /*nelts=*/NULL_TREE,
7049
/*static_storage_p=*/true);
Later on, we reach expand_static_init which creates an initializer for q (to
set it to p) guarded with __cxa_guard_acquire, but that DECL_INITIAL is left
untouched. My first attempts to clear the DECL_INITIAL had flopped and I
decided to do just this, but I can't see how this could be correct :(.
Any better ideas?
Bootstrapped/regtested on x86_64-linux.
2017-02-28 Marek Polacek <pola...@redhat.com>
PR c++/79687
* decl.c (expand_static_init): Clear DECL_INITIAL for
static pointer-to-data-members.
* g++.dg/expr/ptrmem8.C: New test.
* g++.dg/expr/ptrmem9.C: New test.
diff --git gcc/cp/decl.c gcc/cp/decl.c
index 828359f..9cfeaa1d 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -8139,6 +8139,14 @@ expand_static_init (tree decl, tree init)
finish_expr_stmt (init);
+ /* A static pointer-to-data-member had been initialized to -1,
+ and then initialized again with INIT. Remove its DECL_INITIAL
+ so that it won't confuse us later when folding. */
+ if (TYPE_PTRDATAMEM_P (TREE_TYPE (decl))
+ && DECL_INITIAL (decl)
+ && null_member_pointer_value_p (DECL_INITIAL (decl)))
+ DECL_INITIAL (decl) = NULL_TREE;
+
if (thread_guard)
{
finish_compound_stmt (inner_then_clause);
diff --git gcc/testsuite/g++.dg/expr/ptrmem8.C
gcc/testsuite/g++.dg/expr/ptrmem8.C
index e69de29..c5a766a 100644
--- gcc/testsuite/g++.dg/expr/ptrmem8.C
+++ gcc/testsuite/g++.dg/expr/ptrmem8.C
@@ -0,0 +1,15 @@
+// PR c++/79687
+// { dg-do run }
+
+struct A
+{
+ char c;
+};
+
+int main()
+{
+ char A::* p = &A::c;
+ static char A::* const q = p;
+ A a;
+ return &(a.*q) - &a.c;
+}
diff --git gcc/testsuite/g++.dg/expr/ptrmem9.C
gcc/testsuite/g++.dg/expr/ptrmem9.C
index e69de29..32ce777 100644
--- gcc/testsuite/g++.dg/expr/ptrmem9.C
+++ gcc/testsuite/g++.dg/expr/ptrmem9.C
@@ -0,0 +1,19 @@
+// PR c++/79687
+// { dg-do run }
+
+struct A
+{
+ char c;
+};
+
+int main()
+{
+ static char A::* p1 = &A::c;
+ char A::* const q1 = p1;
+
+ char A::* p2 = &A::c;
+ static char A::* const q2 = p2;
+
+ A a;
+ return (&(a.*q1) - &a.c) || (&(a.*q2) - &a.c);
+}
Marek
