RFA (clobbers, gimplification): PATCH for c++/61982, dead stores to destroyed objects

[Jason Merrill <jason at redhat dot com>]

61982 notes that an explicit destructor call or delete expression ends
the lifetime of an object, but we weren't clobbering affected objects
if their destructors are trivial.  This patch fixes that.

The first commit just adds a helper function, build_clobber.

The second commit changes explicit destruction to clobber the affected
object.  As a result, I needed to change the gimplifier to handle the
more general forms of lvalue we might be clobbering, by introducing a
temporary.  I'm not sure why clobbers are so picky about the form of
lvalue they can use, but this makes it work.

Tested x86_64-pc-linux-gnu.  OK for trunk (9)?

commit f9da73d68a552474e75a1baa561f62dc7564f220
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Apr 25 16:52:02 2018 -0400

    build-clobber

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 03bc041780f..07f3a61fed6 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -14874,8 +14874,7 @@ build_clobber_this ()
   if (!vbases)
     ctype = CLASSTYPE_AS_BASE (ctype);
 
-  tree clobber = build_constructor (ctype, NULL);
-  TREE_THIS_VOLATILE (clobber) = true;
+  tree clobber = build_clobber (ctype);
 
   tree thisref = current_class_ref;
   if (ctype != current_class_type)
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index c32869b4c59..b5b80ab7d98 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1379,9 +1379,8 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
 	      && !is_gimple_reg (t)
 	      && flag_stack_reuse != SR_NONE)
 	    {
-	      tree clobber = build_constructor (TREE_TYPE (t), NULL);
+	      tree clobber = build_clobber (TREE_TYPE (t));
 	      gimple *clobber_stmt;
-	      TREE_THIS_VOLATILE (clobber) = 1;
 	      clobber_stmt = gimple_build_assign (t, clobber);
 	      gimple_set_location (clobber_stmt, end_locus);
 	      gimplify_seq_add_stmt (&cleanup, clobber_stmt);
@@ -6603,9 +6602,7 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 	{
 	  if (flag_stack_reuse == SR_ALL)
 	    {
-	      tree clobber = build_constructor (TREE_TYPE (temp),
-						NULL);
-	      TREE_THIS_VOLATILE (clobber) = true;
+	      tree clobber = build_clobber (TREE_TYPE (temp));
 	      clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
 	      gimple_push_cleanup (temp, clobber, false, pre_p, true);
 	    }
diff --git a/gcc/tree.c b/gcc/tree.c
index e93f24dd4d3..b661d3d0dcd 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -2059,6 +2059,16 @@ build_constructor_va (tree type, int nelts, ...)
   return build_constructor (type, v);
 }
 
+/* Return a node of type TYPE for which TREE_CLOBBER_P is true.  */
+
+tree
+build_clobber (tree type)
+{
+  tree clobber = build_constructor (type, NULL);
+  TREE_THIS_VOLATILE (clobber) = true;
+  return clobber;
+}
+
 /* Return a new FIXED_CST node whose type is TYPE and value is F.  */
 
 tree
diff --git a/gcc/tree.h b/gcc/tree.h
index 1e14d9f5866..74a0d1881a6 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4173,6 +4173,7 @@ extern tree build_constructor (tree, vec<constructor_elt, va_gc> *);
 extern tree build_constructor_single (tree, tree, tree);
 extern tree build_constructor_from_list (tree, tree);
 extern tree build_constructor_va (tree, int, ...);
+extern tree build_clobber (tree);
 extern tree build_real_from_int_cst (tree, const_tree);
 extern tree build_complex (tree, tree, tree);
 extern tree build_complex_inf (tree, bool);

commit c34ac8c8066afb18c13ca44b47521a1a2d824e96
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Apr 27 13:14:30 2018 -0400

            PR c++/61982 - dead stores to destroyed objects.
    
            * call.c (build_trivial_dtor_call): New, assigns a clobber.
            (build_over_call, build_special_member_call): Use it.
            * cp-tree.h: Declare it.
            * init.c (build_delete): Remove trivial path.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index fb6d71d260d..d3ee152808a 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7629,6 +7629,33 @@ conv_binds_ref_to_prvalue (conversion *c)
   return false;
 }
 
+/* Call the trivial destructor for INSTANCE, which can be either an lvalue of
+   class type or a pointer to class type.  */
+
+tree
+build_trivial_dtor_call (tree instance)
+{
+  gcc_assert (!is_dummy_object (instance));
+
+  if (!flag_lifetime_dse)
+    {
+    no_clobber:
+      return fold_convert (void_type_node, instance);
+    }
+
+  if (POINTER_TYPE_P (TREE_TYPE (instance)))
+    {
+      if (VOID_TYPE_P (TREE_TYPE (TREE_TYPE (instance))))
+	goto no_clobber;
+      instance = cp_build_fold_indirect_ref (instance);
+    }
+
+  /* A trivial destructor should still clobber the object.  */
+  tree clobber = build_clobber (TREE_TYPE (instance));
+  return build2 (MODIFY_EXPR, void_type_node,
+		 instance, clobber);
+}
+
 /* Subroutine of the various build_*_call functions.  Overload resolution
    has chosen a winning candidate CAND; build up a CALL_EXPR accordingly.
    ARGS is a TREE_LIST of the unconverted arguments to the call.  FLAGS is a
@@ -8240,7 +8267,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
   else if (trivial_fn_p (fn))
     {
       if (DECL_DESTRUCTOR_P (fn))
-	return fold_convert (void_type_node, argarray[0]);
+	return build_trivial_dtor_call (argarray[0]);
       else if (default_ctor_p (fn))
 	{
 	  if (is_dummy_object (argarray[0]))
@@ -8863,6 +8890,18 @@ build_special_member_call (tree instance, tree name, vec<tree, va_gc> **args,
   tree ret;
 
   gcc_assert (IDENTIFIER_CDTOR_P (name) || name == assign_op_identifier);
+
+  if (error_operand_p (instance))
+    return error_mark_node;
+
+  if (IDENTIFIER_DTOR_P (name))
+    {
+      gcc_assert (args == NULL || vec_safe_is_empty (*args));
+      if (!type_build_dtor_call (TREE_TYPE (instance)))
+	/* Shortcut to avoid lazy destructor declaration.  */
+	return build_trivial_dtor_call (instance);
+    }
+
   if (TYPE_P (binfo))
     {
       /* Resolve the name.  */
@@ -8881,9 +8920,6 @@ build_special_member_call (tree instance, tree name, vec<tree, va_gc> **args,
     instance = build_dummy_object (class_type);
   else
     {
-      if (IDENTIFIER_DTOR_P (name))
-	gcc_assert (args == NULL || vec_safe_is_empty (*args));
-
       /* Convert to the base class, if necessary.  */
       if (!same_type_ignoring_top_level_qualifiers_p
 	  (TREE_TYPE (instance), BINFO_TYPE (binfo)))
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 048cb0f0051..3cd7421ba92 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6037,6 +6037,7 @@ extern bool null_member_pointer_value_p		(tree);
 extern bool sufficient_parms_p			(const_tree);
 extern tree type_decays_to			(tree);
 extern tree extract_call_expr			(tree);
+extern tree build_trivial_dtor_call		(tree);
 extern tree build_user_type_conversion		(tree, tree, int,
 						 tsubst_flags_t);
 extern tree build_new_function_call		(tree, vec<tree, va_gc> **,
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index db3c9644e18..b74fca0e2c8 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -4662,35 +4662,17 @@ build_delete (tree otype, tree addr, special_function_kind auto_delete,
       addr = convert_force (build_pointer_type (type), addr, 0, complain);
     }
 
-  if (TYPE_HAS_TRIVIAL_DESTRUCTOR (type))
-    {
-      /* Make sure the destructor is callable.  */
-      if (type_build_dtor_call (type))
-	{
-	  expr = build_dtor_call (cp_build_fold_indirect_ref (addr),
-				  sfk_complete_destructor, flags, complain);
-	  if (expr == error_mark_node)
-	    return error_mark_node;
-	}
-
-      if (auto_delete != sfk_deleting_destructor)
-	return void_node;
-
-      return build_op_delete_call (DELETE_EXPR, addr,
-				   cxx_sizeof_nowarn (type),
-				   use_global_delete,
-				   /*placement=*/NULL_TREE,
-				   /*alloc_fn=*/NULL_TREE,
-				   complain);
-    }
-  else
-    {
   tree head = NULL_TREE;
   tree do_delete = NULL_TREE;
   tree ifexp;
 
+  bool virtual_p = false;
+  if (type_build_dtor_call (type))
+    {
       if (CLASSTYPE_LAZY_DESTRUCTOR (type))
 	lazily_declare_fn (sfk_destructor, type);
+      virtual_p = DECL_VIRTUAL_P (CLASSTYPE_DESTRUCTOR (type));
+    }
 
   /* For `::delete x', we must not use the deleting destructor
      since then we would not be sure to get the global `operator
@@ -4715,7 +4697,7 @@ build_delete (tree otype, tree addr, special_function_kind auto_delete,
   /* If the destructor is non-virtual, there is no deleting
      variant.  Instead, we must explicitly call the appropriate
      `operator delete' here.  */
-      else if (!DECL_VIRTUAL_P (CLASSTYPE_DESTRUCTOR (type))
+  else if (!virtual_p
 	   && auto_delete == sfk_deleting_destructor)
     {
       /* We will use ADDR multiple times so we must save it.  */
@@ -4743,11 +4725,17 @@ build_delete (tree otype, tree addr, special_function_kind auto_delete,
 			    complain);
     }
 
+  if (type_build_dtor_call (type))
     expr = build_dtor_call (cp_build_fold_indirect_ref (addr),
 			    auto_delete, flags, complain);
+  else
+    expr = build_trivial_dtor_call (addr);
   if (expr == error_mark_node)
     return error_mark_node;
-      if (do_delete)
+
+  if (do_delete && !TREE_SIDE_EFFECTS (expr))
+    expr = do_delete;
+  else if (do_delete)
     /* The delete operator must be called, regardless of whether
        the destructor throws.
 
@@ -4782,7 +4770,6 @@ build_delete (tree otype, tree addr, special_function_kind auto_delete,
 
   return expr;
 }
-}
 
 /* At the beginning of a destructor, push cleanups that will call the
    destructors for our base classes and members.
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index b5b80ab7d98..d27aae2eea8 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -5558,8 +5558,13 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
       ret = gimplify_expr (to_p, pre_p, post_p, is_gimple_lvalue, fb_lvalue);
       if (ret == GS_ERROR)
 	return ret;
-      gcc_assert (!want_value
-		  && (VAR_P (*to_p) || TREE_CODE (*to_p) == MEM_REF));
+      gcc_assert (!want_value);
+      if (!VAR_P (*to_p) && TREE_CODE (*to_p) != MEM_REF)
+	{
+	  tree addr = get_initialized_tmp_var (build_fold_addr_expr (*to_p),
+					       pre_p, post_p);
+	  *to_p = build_simple_mem_ref_loc (EXPR_LOCATION (*to_p), addr);
+	}
       gimplify_seq_add_stmt (pre_p, gimple_build_assign (*to_p, *from_p));
       *expr_p = NULL;
       return GS_ALL_DONE;
diff --git a/gcc/testsuite/g++.dg/tree-ssa/lifetime-dse1.C b/gcc/testsuite/g++.dg/tree-ssa/lifetime-dse1.C
new file mode 100644
index 00000000000..90c90f2650c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/lifetime-dse1.C
@@ -0,0 +1,18 @@
+// PR c++/61982
+// { dg-additional-options "-O2 -fdump-tree-optimized" }
+// { dg-final { scan-tree-dump-not "= 0" "optimized" } }
+
+struct X { 
+  int i; 
+  void clear() { i = 0; }
+}; 
+
+void f(X* x) { 
+  x->clear(); 
+  x->~X(); 
+} 
+
+void g(X* x) {
+  x->clear();
+  delete x;
+}