Message ID | 20170228201021.GC3172@redhat.com |
---|---|
State | New |
Headers | show |
On Tue, Feb 28, 2017 at 10:10 AM, Marek Polacek <polacek@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 <polacek@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. Jason
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); +}