| Message ID | 20101223204951.GJ16156@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Dec 23, 2010 at 2:49 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > The testcase in the patch is miscompiled. The problem seems to be > that the B::B() ctor contains > { > <<cleanup_point <<< Unknown tree: expr_stmt > A::A (&((struct B *) this)->D.1396) >>>>>; > <<< Unknown tree: cleanup_stmt > <<cleanup_point <<< Unknown tree: expr_stmt > (void) (((struct B *) this)->D.1396._vptr.A = &_ZTV1B + 16) >>>>>; > A::~A ((struct A *) this) > >>>; > } > and build_data_member_initialization ignores CLEANUP_STMT, which means > the constexpr ctor is expanded as setting _vptr.A field to &_ZTV1A + 16 > instead of &_ZTV1B + 16. The following patch ignores the cleanup (A::~A > dtor in this case), but handles CLEANUP_BODY. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Jason had worked several times and made many improvements in this particular area, so I'll him approve (or disapprove.) One thing that `constexpr' has shown is our lack of systematic documentation of what kind of trees can appear where, and definite unambiguous mapping of C++ souce-level constructs to internal GCC trees... :-/
OK. Jason
--- gcc/cp/semantics.c.jj 2010-12-16 10:55:31.000000000 +0100 +++ gcc/cp/semantics.c 2010-12-23 13:02:11.000000000 +0100 @@ -5440,11 +5440,25 @@ build_data_member_initialization (tree t if (t == error_mark_node) return false; if (TREE_CODE (t) == CLEANUP_STMT) - /* We can't see a CLEANUP_STMT in a constructor for a literal class, - but we can in a constexpr constructor for a non-literal class. Just - ignore it; either all the initialization will be constant, in which - case the cleanup can't run, or it can't be constexpr. */ - return true; + { + /* We can't see a CLEANUP_STMT in a constructor for a literal class, + but we can in a constexpr constructor for a non-literal class. Just + ignore it; either all the initialization will be constant, in which + case the cleanup can't run, or it can't be constexpr. + Still recurse into CLEANUP_BODY. */ + t = CLEANUP_BODY (t); + if (TREE_CODE (t) == STATEMENT_LIST) + { + tree_stmt_iterator i; + for (i = tsi_start (t); !tsi_end_p (i); tsi_next (&i)) + { + if (! build_data_member_initialization (tsi_stmt (i), vec)) + return false; + } + return true; + } + return build_data_member_initialization (t, vec); + } if (TREE_CODE (t) == CONVERT_EXPR) t = TREE_OPERAND (t, 0); if (TREE_CODE (t) == INIT_EXPR --- gcc/testsuite/g++.dg/cpp0x/constexpr-base4.C.jj 2010-12-23 13:04:46.000000000 +0100 +++ gcc/testsuite/g++.dg/cpp0x/constexpr-base4.C 2010-12-23 13:03:48.000000000 +0100 @@ -0,0 +1,28 @@ +// PR c++/46626 +// { dg-do run } +// { dg-options "-std=c++0x" } + +struct A +{ + virtual void f () = 0; + virtual ~A () { } +}; + +struct B : A +{ + virtual void f () { } +}; + +static void +foo (A *a) +{ + a->f (); +} + +int +main () +{ + B b; + foo (&b); + return 0; +}
Hi! The testcase in the patch is miscompiled. The problem seems to be that the B::B() ctor contains { <<cleanup_point <<< Unknown tree: expr_stmt A::A (&((struct B *) this)->D.1396) >>>>>; <<< Unknown tree: cleanup_stmt <<cleanup_point <<< Unknown tree: expr_stmt (void) (((struct B *) this)->D.1396._vptr.A = &_ZTV1B + 16) >>>>>; A::~A ((struct A *) this) >>>; } and build_data_member_initialization ignores CLEANUP_STMT, which means the constexpr ctor is expanded as setting _vptr.A field to &_ZTV1A + 16 instead of &_ZTV1B + 16. The following patch ignores the cleanup (A::~A dtor in this case), but handles CLEANUP_BODY. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2010-12-23 Jakub Jelinek <jakub@redhat.com> PR c++/46626 * semantics.c (build_data_member_initialization): For CLEANUP_STMT recurse into CLEANUP_BODY. * g++.dg/cpp0x/constexpr-base4.C: New test. Jakub