On Tue, Feb 28, 2017 at 01:12:38PM -1000, Jason Merrill wrote:
> On Tue, Feb 28, 2017 at 10:10 AM, Marek Polacek <pola...@redhat.com> wrote:
> > 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);
>
> Ah, that makes sense. We do want to do constant-initialization with
> -1 before we do dynamic initialization with p.
>
> So we need to detect in constant_value_1 that the variable has a
> dynamic initializer and therefore return the variable rather than -1.
> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P seems useful, perhaps in
> combination with DECL_NONTRIVIALLY_INITIALIZED.
Got it. I think the following should be the real fix. I ran g++ dg.exp
with some logging to see how often the new check triggers, and it only
triggered in the two new tests, so I'm fairly happy with that.
Bootstrapped/regtested on x86_64-linux, ok for trunk and 6?
2017-03-01 Marek Polacek <pola...@redhat.com>
PR c++/79687
* init.c (constant_value_1): Break if the variable has a dynamic
initializer.
* g++.dg/expr/ptrmem8.C: New test.
* g++.dg/expr/ptrmem9.C: New test.
diff --git gcc/cp/init.c gcc/cp/init.c
index 7ded37e..12e6bf4 100644
--- gcc/cp/init.c
+++ gcc/cp/init.c
@@ -2193,6 +2193,13 @@ constant_value_1 (tree decl, bool strict_p, bool
return_aggregate_cst_ok_p)
if (TREE_CODE (init) == CONSTRUCTOR
&& !DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl))
break;
+ /* If the variable has a dynamic initializer, don't use its
+ DECL_INITIAL which doesn't reflect the real value. */
+ if (VAR_P (decl)
+ && TREE_STATIC (decl)
+ && !DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl)
+ && DECL_NONTRIVIALLY_INITIALIZED_P (decl))
+ break;
decl = unshare_expr (init);
}
return decl;
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
