This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFA (tree-cfg): PATCH for 68983 (ICE in copy constructor)


This testcase was broken by my earlier fix for 67557, which added an assert to store_field to catch inappropriate use of bitwise copying of TREE_ADDRESSABLE types. In this testcase, the copying is actually fine, since no temporary object is created. So one solution would be to recognize that situation better.

But it occurs to me that since the real problem I was trying to catch is creation of temporaries of TREE_ADDRESSABLE type in the back end, we should guard that instead. So this patch moves the assert into assign_temp.

That change revealed a couple of places that needed to be adjusted: convert_to_void in the front end and fixup_noreturn_call in the back end were removing the information about what object is being initialized by a function that returns by invisible reference. If we are insisting that objects of TREE_ADDRESSABLE type need to be created in the front end, these places need to not strip that information.

Tested x86_64-pc-linux-gnu.  OK for trunk?
commit 1e437dea62b2c4abb2122c276e58b59e3769eb97
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jan 7 09:46:59 2016 -0500

    	PR c++/68983
    	PR c++/67557
    gcc/
    	* function.c (assign_temp): Guard against TREE_ADDRESSABLE types here.
    	* expr.c (store_field): Not here.
    	* tree-cfgcleanup.c (fixup_noreturn_call): Don't clear LHS of a
    	call with TREE_ADDRESSABLE type.
    	* tree-cfg.c (verify_gimple_call): Adjust.
    gcc/cp/
    	* cvt.c (convert_to_void): Don't strip a TARGET_EXPR of
    	TREE_ADDRESSABLE type.

diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index d79f13b..f381f9b 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -1229,11 +1229,12 @@ convert_to_void (tree expr, impl_conv_void implicit, tsubst_flags_t complain)
 
     case TARGET_EXPR:
       /* Don't bother with the temporary object returned from a function if
-	 we don't use it and don't need to destroy it.  We'll still
+	 we don't use it, don't need to destroy it, and won't abort in
+	 assign_temp.  We'll still
 	 allocate space for it in expand_call or declare_return_variable,
 	 but we don't need to track it through all the tree phases.  */
       if (TARGET_EXPR_IMPLICIT_P (expr)
-	  && TYPE_HAS_TRIVIAL_DESTRUCTOR (TREE_TYPE (expr)))
+	  && !TREE_ADDRESSABLE (TREE_TYPE (expr)))
 	{
 	  tree init = TARGET_EXPR_INITIAL (expr);
 	  if (TREE_CODE (init) == AGGR_INIT_EXPR
diff --git a/gcc/expr.c b/gcc/expr.c
index 8973893..0c5dff8 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -6655,9 +6655,6 @@ store_field (rtx target, HOST_WIDE_INT bitsize, HOST_WIDE_INT bitpos,
       rtx temp;
       gimple *nop_def;
 
-      /* Using bitwise copy is not safe for TREE_ADDRESSABLE types.  */
-      gcc_assert (!TREE_ADDRESSABLE (TREE_TYPE (exp)));
-
       /* If EXP is a NOP_EXPR of precision less than its mode, then that
 	 implies a mask operation.  If the precision is the same size as
 	 the field we're storing into, that mask is redundant.  This is
diff --git a/gcc/function.c b/gcc/function.c
index 603ea80..bffd617 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -956,6 +956,10 @@ assign_temp (tree type_or_decl, int memory_required,
   unsignedp = TYPE_UNSIGNED (type);
 #endif
 
+  /* Allocating temporaries of TREE_ADDRESSABLE type must be done
+     in the front end.  */
+  gcc_assert (!TREE_ADDRESSABLE (type));
+
   if (mode == BLKmode || memory_required)
     {
       HOST_WIDE_INT size = int_size_in_bytes (type);
diff --git a/gcc/testsuite/g++.dg/init/base1.C b/gcc/testsuite/g++.dg/init/base1.C
new file mode 100644
index 0000000..bce2f17
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/base1.C
@@ -0,0 +1,14 @@
+// PR c++/68983
+
+class SvxOptionsGrid {
+  int nFldDrawX;
+  bool bEqualGrid;
+public:
+  ~SvxOptionsGrid();
+};
+class A : SvxOptionsGrid {
+public:
+  A(SvxOptionsGrid p1) : SvxOptionsGrid(p1) {}
+};
+SvxOptionsGrid a;
+void fn1() { A b(a); }
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index e75774e..dbfa506 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -3371,7 +3371,8 @@ verify_gimple_call (gcall *stmt)
   if (lhs
       && gimple_call_ctrl_altering_p (stmt)
       && gimple_call_noreturn_p (stmt)
-      && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST)
+      && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
+      && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))
     {
       error ("LHS in noreturn call");
       return true;
diff --git a/gcc/tree-cfgcleanup.c b/gcc/tree-cfgcleanup.c
index bd7c3c9..46d0fa3 100644
--- a/gcc/tree-cfgcleanup.c
+++ b/gcc/tree-cfgcleanup.c
@@ -601,9 +601,11 @@ fixup_noreturn_call (gimple *stmt)
 
   /* If there is an LHS, remove it, but only if its type has fixed size.
      The LHS will need to be recreated during RTL expansion and creating
-     temporaries of variable-sized types is not supported.  */
+     temporaries of variable-sized types is not supported.  Also don't
+     do this with TREE_ADDRESSABLE types, as assign_temp will abort.  */
   tree lhs = gimple_call_lhs (stmt);
-  if (lhs && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST)
+  if (lhs && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST
+      && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))
     {
       gimple_call_set_lhs (stmt, NULL_TREE);
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]