diff mbox

[C++] Fix ICE during VEC_INIT_EXPR gimplification (PR c++/77739)

Message ID 20161122204526.GG3541@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 22, 2016, 8:45 p.m. UTC 
Hi!

As mentioned in the PR, we ICE because part of the body is genericized
twice and each time it wraps is_invisiref_parm RESULT_DECL (in this case,
could be also PARM_DEC) into REFERENCE_REF_P INDIRECT_REF.
The first time it is desirable, but when done again during VEC_INIT_EXPR
gimplification which calls cp_genericize_tree again, it is undesirable.

The following patch fixes it by only wrapping the invisiref parms/result
during the first cp_genericize_tree when the whole function is genericized.
I'd expect that any references to invisiref parms/result should be only
present in the VEC_INIT_EXPR arguments (which should be genericized already)
and that build_vec_init shouldn't create new ones out of the air.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

As mentioned in the PR, another option I see is special case
REFERENCE_REF_P INDIRECT_REFs and MEM_REFs into which they are gimplified
in cp_genericize_r by not changing is_invisiref_parm decls if they are
already wrapped in those.

2016-11-22  Jakub Jelinek  <jakub@redhat.com>

	PR c++/77739
	* cp-gimplify.c (cp_gimplify_tree) <case VEC_INIT_EXPR>: Pass
	false as handle_invisiref_parm_p to cp_genericize_tree.
	(struct cp_genericize_data): Add handle_invisiref_parm_p field.
	(cp_genericize_r): Don't wrap is_invisiref_parm into references
	if !wtd->handle_invisiref_parm_p.
	(cp_genericize_tree): Add handle_invisiref_parm_p argument,
	set wtd.handle_invisiref_parm_p to it.
	(cp_genericize): Pass true as handle_invisiref_parm_p to
	cp_genericize_tree.  Formatting fix.

	* g++.dg/cpp1y/pr77739.C: New test.


	Jakub

Comments

Jason Merrill Nov. 23, 2016, 2:11 p.m. UTC | #1 
On Tue, Nov 22, 2016 at 3:45 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> As mentioned in the PR, another option I see is special case
> REFERENCE_REF_P INDIRECT_REFs and MEM_REFs into which they are gimplified
> in cp_genericize_r by not changing is_invisiref_parm decls if they are
> already wrapped in those.

That sounds more robust.

Jason
Jakub Jelinek Nov. 23, 2016, 2:31 p.m. UTC | #2 
On Wed, Nov 23, 2016 at 09:11:02AM -0500, Jason Merrill wrote:
> On Tue, Nov 22, 2016 at 3:45 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > As mentioned in the PR, another option I see is special case
> > REFERENCE_REF_P INDIRECT_REFs and MEM_REFs into which they are gimplified
> > in cp_genericize_r by not changing is_invisiref_parm decls if they are
> > already wrapped in those.
> 
> That sounds more robust.

Actually, now that I think about it, it sounds less robust.
E.g. if the invisiref parm is initially used as operand of ADDR_EXPR,
then during the first cp_genericize_tree it will be turned into
ADDR_EXPR <INDIRECT_REF <decl>>, but gimplification will turn that just
into decl, and if we genericize that again, we turn it into INDIRECT_REF <decl>
because we won't see INDIRECT_REF/MEM_REF wrapping it.
Is there any easy way to construct a testcase where VEC_INIT_EXPR
initializer will be an arbitrary expression where such &parm could appear
if parm is passed by invisible reference?
Also, gimplification already performs tons of other folding (through fold_stmt),
making it less likely to recognize those.

	Jakub
Jason Merrill Nov. 23, 2016, 3:48 p.m. UTC | #3 
On Wed, Nov 23, 2016 at 9:31 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Nov 23, 2016 at 09:11:02AM -0500, Jason Merrill wrote:
>> On Tue, Nov 22, 2016 at 3:45 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > As mentioned in the PR, another option I see is special case
>> > REFERENCE_REF_P INDIRECT_REFs and MEM_REFs into which they are gimplified
>> > in cp_genericize_r by not changing is_invisiref_parm decls if they are
>> > already wrapped in those.
>>
>> That sounds more robust.
>
> Actually, now that I think about it, it sounds less robust.
> E.g. if the invisiref parm is initially used as operand of ADDR_EXPR,
> then during the first cp_genericize_tree it will be turned into
> ADDR_EXPR <INDIRECT_REF <decl>>, but gimplification will turn that just
> into decl, and if we genericize that again, we turn it into INDIRECT_REF <decl>
> because we won't see INDIRECT_REF/MEM_REF wrapping it.

True.  Then your patch is OK.

> Is there any easy way to construct a testcase where VEC_INIT_EXPR
> initializer will be an arbitrary expression where such &parm could appear
> if parm is passed by invisible reference?

I can't think of anything offhand.

Jason
diff mbox

Patch

--- gcc/cp/cp-gimplify.c.jj	2016-11-15 16:18:49.000000000 +0100
+++ gcc/cp/cp-gimplify.c	2016-11-22 19:12:07.606813783 +0100
@@ -38,7 +38,7 @@  along with GCC; see the file COPYING3.
 
 static tree cp_genericize_r (tree *, int *, void *);
 static tree cp_fold_r (tree *, int *, void *);
-static void cp_genericize_tree (tree*);
+static void cp_genericize_tree (tree*, bool);
 static tree cp_fold (tree);
 
 /* Local declarations.  */
@@ -623,7 +623,7 @@  cp_gimplify_expr (tree *expr_p, gimple_s
 				  tf_warning_or_error);
 	hash_set<tree> pset;
 	cp_walk_tree (expr_p, cp_fold_r, &pset, NULL);
-	cp_genericize_tree (expr_p);
+	cp_genericize_tree (expr_p, false);
 	ret = GS_OK;
 	input_location = loc;
       }
@@ -995,6 +995,7 @@  struct cp_genericize_data
   struct cp_genericize_omp_taskreg *omp_ctx;
   tree try_block;
   bool no_sanitize_p;
+  bool handle_invisiref_parm_p;
 };
 
 /* Perform any pre-gimplification folding of C++ front end trees to
@@ -1111,7 +1112,7 @@  cp_genericize_r (tree *stmt_p, int *walk
     }
 
   /* Otherwise, do dereference invisible reference parms.  */
-  if (is_invisiref_parm (stmt))
+  if (wtd->handle_invisiref_parm_p && is_invisiref_parm (stmt))
     {
       *stmt_p = convert_from_reference (stmt);
       *walk_subtrees = 0;
@@ -1511,7 +1512,7 @@  cp_genericize_r (tree *stmt_p, int *walk
 /* Lower C++ front end trees to GENERIC in T_P.  */
 
 static void
-cp_genericize_tree (tree* t_p)
+cp_genericize_tree (tree* t_p, bool handle_invisiref_parm_p)
 {
   struct cp_genericize_data wtd;
 
@@ -1520,6 +1521,7 @@  cp_genericize_tree (tree* t_p)
   wtd.omp_ctx = NULL;
   wtd.try_block = NULL_TREE;
   wtd.no_sanitize_p = false;
+  wtd.handle_invisiref_parm_p = handle_invisiref_parm_p;
   cp_walk_tree (t_p, cp_genericize_r, &wtd, NULL);
   delete wtd.p_set;
   wtd.bind_expr_stack.release ();
@@ -1639,12 +1641,12 @@  cp_genericize (tree fndecl)
   /* Expand all the array notations here.  */
   if (flag_cilkplus 
       && contains_array_notation_expr (DECL_SAVED_TREE (fndecl)))
-    DECL_SAVED_TREE (fndecl) = 
-      expand_array_notation_exprs (DECL_SAVED_TREE (fndecl));
+    DECL_SAVED_TREE (fndecl)
+      = expand_array_notation_exprs (DECL_SAVED_TREE (fndecl));
 
   /* We do want to see every occurrence of the parms, so we can't just use
      walk_tree's hash functionality.  */
-  cp_genericize_tree (&DECL_SAVED_TREE (fndecl));
+  cp_genericize_tree (&DECL_SAVED_TREE (fndecl), true);
 
   if (flag_sanitize & SANITIZE_RETURN
       && do_ubsan_in_current_function ())
--- gcc/testsuite/g++.dg/cpp1y/pr77739.C.jj	2016-11-22 19:15:02.182659407 +0100
+++ gcc/testsuite/g++.dg/cpp1y/pr77739.C	2016-11-22 19:13:37.000000000 +0100
@@ -0,0 +1,15 @@ 
+// PR c++/77739
+// { dg-do compile { target c++14 } }
+
+struct A {
+  A();
+  A(const A &);
+};
+struct B {
+  B();
+  template <typename... Args> auto g(Args &&... p1) {
+    return [=] { f(p1...); };
+  }
+  void f(A, const char *);
+};
+B::B() { g(A(), ""); }