diff mbox

[C++] c++/78495 inherited ctor and inivis-ref parm

Message ID c4cfe14b-34c2-67f6-4954-85cc5e347ba5@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Jan. 20, 2017, 1:15 p.m. UTC
Bug 78495 is a wrong code bug caused by an invisiref parm.  When 
building the inherited ctor, we use forward_parm to create a forwarding 
reference.  That code generation considers the type of the parm to be 
'T'.  We thus end up with a tree that looks like *&PARM

The subsequent call gets 'CALL_FROM_THUNK' set.  We need that set so 
code generation doesn't copy pass-by-invisiref objects that are 
specified by the C abi (but not specified so by the C++ abi) (POD 
structs, commonly).

cp_genericize is used to mutate invisiref parm_decls from T to &T and 
bash their uses from PARM to *PARM (with suitable checking of &PARM 
turning into plain PARM).  However it explicitly skips genericizing the 
args of a CALL_FROM_THUNK tree. Thus we end up taking the address of the 
reference parm itself.

That appears to be incorrect now we have forward_parm.  This patch 
deletes that skipping.

ok?

nathan

Comments

Jason Merrill Jan. 20, 2017, 1:45 p.m. UTC | #1
On Fri, Jan 20, 2017 at 8:15 AM, Nathan Sidwell <nathan@acm.org> wrote:
> Bug 78495 is a wrong code bug caused by an invisiref parm.  When building
> the inherited ctor, we use forward_parm to create a forwarding reference.
> That code generation considers the type of the parm to be 'T'.  We thus end
> up with a tree that looks like *&PARM
>
> The subsequent call gets 'CALL_FROM_THUNK' set.  We need that set so code
> generation doesn't copy pass-by-invisiref objects that are specified by the
> C abi (but not specified so by the C++ abi) (POD structs, commonly).
>
> cp_genericize is used to mutate invisiref parm_decls from T to &T and bash
> their uses from PARM to *PARM (with suitable checking of &PARM turning into
> plain PARM).  However it explicitly skips genericizing the args of a
> CALL_FROM_THUNK tree. Thus we end up taking the address of the reference
> parm itself.
>
> That appears to be incorrect now we have forward_parm.  This patch deletes
> that skipping.

Hmm, I would guess that if we're going to do this we need to also use
forward_parm in maybe_thunk_body.

Jason
Nathan Sidwell Jan. 20, 2017, 4:12 p.m. UTC | #2
On 01/20/2017 08:45 AM, Jason Merrill wrote:

> Hmm, I would guess that if we're going to do this we need to also use
> forward_parm in maybe_thunk_body.

Apparently not needed.  maybe_thunk_body is called after we've 
genericized the parent ctor.  That ctor's parms have been bashed to 
reference and have DECL_BY_REFERENCE set appropriately.  The clone's 
parms have also been bashed, but do not have DECL_BY_REFERENCE set.

For POD struct (which remain as struct T) the backend machinery already 
DTRT.

nathan
Jason Merrill Jan. 20, 2017, 4:34 p.m. UTC | #3
On Fri, Jan 20, 2017 at 11:12 AM, Nathan Sidwell <nathan@acm.org> wrote:
> On 01/20/2017 08:45 AM, Jason Merrill wrote:
>
>> Hmm, I would guess that if we're going to do this we need to also use
>> forward_parm in maybe_thunk_body.
>
>
> Apparently not needed.  maybe_thunk_body is called after we've genericized
> the parent ctor.  That ctor's parms have been bashed to reference and have
> DECL_BY_REFERENCE set appropriately.  The clone's parms have also been
> bashed, but do not have DECL_BY_REFERENCE set.
>
> For POD struct (which remain as struct T) the backend machinery already
> DTRT.

OK, then.

Jason
diff mbox

Patch

2017-01-20  Nathan Sidwell  <nathan@acm.org>

	PR c++/78495 - wrong code inherited ctor and invisi-ref parm
	* cp-gimplify.c (cp_generize_r): Don't skip thunks.

	PR c++/79495
	* g++.dg/cpp1z/inh-ctor38.C: New.

Index: cp/cp-gimplify.c
===================================================================
--- cp/cp-gimplify.c	(revision 244596)
+++ cp/cp-gimplify.c	(working copy)
@@ -1103,15 +1103,7 @@  cp_genericize_r (tree *stmt_p, int *walk
       && omp_var_to_track (stmt))
     omp_cxx_notice_variable (wtd->omp_ctx, stmt);
 
-  /* Don't dereference parms in a thunk, pass the references through. */
-  if ((TREE_CODE (stmt) == CALL_EXPR && CALL_FROM_THUNK_P (stmt))
-      || (TREE_CODE (stmt) == AGGR_INIT_EXPR && AGGR_INIT_FROM_THUNK_P (stmt)))
-    {
-      *walk_subtrees = 0;
-      return NULL;
-    }
-
-  /* Otherwise, do dereference invisible reference parms.  */
+  /* Dereference invisible reference parms.  */
   if (wtd->handle_invisiref_parm_p && is_invisiref_parm (stmt))
     {
       *stmt_p = convert_from_reference (stmt);
Index: testsuite/g++.dg/cpp1z/inh-ctor38.C
===================================================================
--- testsuite/g++.dg/cpp1z/inh-ctor38.C	(revision 0)
+++ testsuite/g++.dg/cpp1z/inh-ctor38.C	(working copy)
@@ -0,0 +1,31 @@ 
+// { dg-do run { target c++11 } }
+// PR78495 failed to propagate pass-by-value struct to base ctor.
+
+struct Ptr {
+  void *ptr = 0;
+
+  Ptr() {}
+  Ptr(Ptr const&) = delete;
+  Ptr(Ptr&& other) : ptr (other.ptr) {}
+};
+
+struct Base {
+  Ptr val;
+  Base(Ptr val_) : val(static_cast<Ptr&&>(val_)) {}
+};
+
+struct Derived: Base {
+  using Base::Base;
+};
+
+void *Foo () {
+  Ptr ptr;
+
+  Derived d(static_cast<Ptr&&>(ptr));
+
+  return d.val.ptr;
+}
+
+int main () {
+  return Foo () != 0;
+}