PR c++/61982 - dead stores to destroyed objects.
authorJason Merrill <jason@redhat.com>
Mon, 30 Apr 2018 15:21:01 +0000 (11:21 -0400)
committerJason Merrill <jason@gcc.gnu.org>
Mon, 30 Apr 2018 15:21:01 +0000 (11:21 -0400)
gcc/cp/
* 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.
gcc/
* gimplify.c (gimplify_modify_expr): Simplify complex lvalue on LHS
of clobber.

From-SVN: r259772

gcc/ChangeLog
gcc/cp/ChangeLog
gcc/cp/call.c
gcc/cp/cp-tree.h
gcc/cp/init.c
gcc/gimplify.c
gcc/testsuite/g++.dg/tree-ssa/lifetime-dse1.C [new file with mode: 0644]

index eccb89a..0a776ae 100644 (file)
@@ -1,5 +1,11 @@
 2018-04-30  Jason Merrill  <jason@redhat.com>
 
+       PR c++/61982 - dead stores to destroyed objects.
+       * gimplify.c (gimplify_modify_expr): Simplify complex lvalue on LHS
+       of clobber.
+
+2018-04-30  Jason Merrill  <jason@redhat.com>
+
        * tree.c (build_clobber): New.
        * tree.h: Declare it.
        * gimplify.c (gimplify_bind_expr, gimplify_target_expr): Use it.
index df6d611..1ab9158 100644 (file)
@@ -1,5 +1,11 @@
 2018-04-30  Jason Merrill  <jason@redhat.com>
 
+       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.
+
        * init.c (build_dtor_call): Use build_special_member_call.
        (build_delete): Remove redundant uses of save_addr.
 
index fb6d71d..d3ee152 100644 (file)
@@ -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)))
index 048cb0f..3cd7421 100644 (file)
@@ -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> **,
index 9b64ec3..221a831 100644 (file)
@@ -4664,126 +4664,113 @@ 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;
-       }
+  tree head = NULL_TREE;
+  tree do_delete = NULL_TREE;
+  tree ifexp;
 
-      if (auto_delete != sfk_deleting_destructor)
-       return void_node;
+  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));
+    }
 
-      return build_op_delete_call (DELETE_EXPR, addr,
-                                  cxx_sizeof_nowarn (type),
-                                  use_global_delete,
-                                  /*placement=*/NULL_TREE,
-                                  /*alloc_fn=*/NULL_TREE,
-                                  complain);
+  /* For `::delete x', we must not use the deleting destructor
+     since then we would not be sure to get the global `operator
+     delete'.  */
+  if (use_global_delete && auto_delete == sfk_deleting_destructor)
+    {
+      /* We will use ADDR multiple times so we must save it.  */
+      addr = save_expr (addr);
+      head = get_target_expr (build_headof (addr));
+      /* Delete the object.  */
+      do_delete = build_op_delete_call (DELETE_EXPR,
+                                       head,
+                                       cxx_sizeof_nowarn (type),
+                                       /*global_p=*/true,
+                                       /*placement=*/NULL_TREE,
+                                       /*alloc_fn=*/NULL_TREE,
+                                       complain);
+      /* Otherwise, treat this like a complete object destructor
+        call.  */
+      auto_delete = sfk_complete_destructor;
     }
-  else
+  /* If the destructor is non-virtual, there is no deleting
+     variant.  Instead, we must explicitly call the appropriate
+     `operator delete' here.  */
+  else if (!virtual_p
+          && auto_delete == sfk_deleting_destructor)
     {
-      tree head = NULL_TREE;
-      tree do_delete = NULL_TREE;
-      tree ifexp;
+      /* We will use ADDR multiple times so we must save it.  */
+      addr = save_expr (addr);
+      /* Build the call.  */
+      do_delete = build_op_delete_call (DELETE_EXPR,
+                                       addr,
+                                       cxx_sizeof_nowarn (type),
+                                       /*global_p=*/false,
+                                       /*placement=*/NULL_TREE,
+                                       /*alloc_fn=*/NULL_TREE,
+                                       complain);
+      /* Call the complete object destructor.  */
+      auto_delete = sfk_complete_destructor;
+    }
+  else if (auto_delete == sfk_deleting_destructor
+          && TYPE_GETS_REG_DELETE (type))
+    {
+      /* Make sure we have access to the member op delete, even though
+        we'll actually be calling it from the destructor.  */
+      build_op_delete_call (DELETE_EXPR, addr, cxx_sizeof_nowarn (type),
+                           /*global_p=*/false,
+                           /*placement=*/NULL_TREE,
+                           /*alloc_fn=*/NULL_TREE,
+                           complain);
+    }
 
-      if (CLASSTYPE_LAZY_DESTRUCTOR (type))
-       lazily_declare_fn (sfk_destructor, type);
+  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;
 
-      /* For `::delete x', we must not use the deleting destructor
-        since then we would not be sure to get the global `operator
-        delete'.  */
-      if (use_global_delete && auto_delete == sfk_deleting_destructor)
-       {
-         /* We will use ADDR multiple times so we must save it.  */
-         addr = save_expr (addr);
-         head = get_target_expr (build_headof (addr));
-         /* Delete the object.  */
-         do_delete = build_op_delete_call (DELETE_EXPR,
-                                           head,
-                                           cxx_sizeof_nowarn (type),
-                                           /*global_p=*/true,
-                                           /*placement=*/NULL_TREE,
-                                           /*alloc_fn=*/NULL_TREE,
-                                           complain);
-         /* Otherwise, treat this like a complete object destructor
-            call.  */
-         auto_delete = sfk_complete_destructor;
-       }
-      /* 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))
-              && auto_delete == sfk_deleting_destructor)
-       {
-         /* We will use ADDR multiple times so we must save it.  */
-         addr = save_expr (addr);
-         /* Build the call.  */
-         do_delete = build_op_delete_call (DELETE_EXPR,
-                                           addr,
-                                           cxx_sizeof_nowarn (type),
-                                           /*global_p=*/false,
-                                           /*placement=*/NULL_TREE,
-                                           /*alloc_fn=*/NULL_TREE,
-                                           complain);
-         /* Call the complete object destructor.  */
-         auto_delete = sfk_complete_destructor;
-       }
-      else if (auto_delete == sfk_deleting_destructor
-              && TYPE_GETS_REG_DELETE (type))
-       {
-         /* Make sure we have access to the member op delete, even though
-            we'll actually be calling it from the destructor.  */
-         build_op_delete_call (DELETE_EXPR, addr, cxx_sizeof_nowarn (type),
-                               /*global_p=*/false,
-                               /*placement=*/NULL_TREE,
-                               /*alloc_fn=*/NULL_TREE,
-                               complain);
-       }
+  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.
 
-      expr = build_dtor_call (cp_build_fold_indirect_ref (addr),
-                             auto_delete, flags, complain);
-      if (expr == error_mark_node)
-       return error_mark_node;
-      if (do_delete)
-       /* The delete operator must be called, regardless of whether
-          the destructor throws.
-
-          [expr.delete]/7 The deallocation function is called
-          regardless of whether the destructor for the object or some
-          element of the array throws an exception.  */
-       expr = build2 (TRY_FINALLY_EXPR, void_type_node, expr, do_delete);
-
-      /* We need to calculate this before the dtor changes the vptr.  */
-      if (head)
-       expr = build2 (COMPOUND_EXPR, void_type_node, head, expr);
-
-      if (flags & LOOKUP_DESTRUCTOR)
-       /* Explicit destructor call; don't check for null pointer.  */
-       ifexp = integer_one_node;
-      else
-       {
-         /* Handle deleting a null pointer.  */
-         warning_sentinel s (warn_address);
-         ifexp = cp_build_binary_op (input_location, NE_EXPR, addr,
-                                     nullptr_node, complain);
-         if (ifexp == error_mark_node)
-           return error_mark_node;
-         /* This is a compiler generated comparison, don't emit
-            e.g. -Wnonnull-compare warning for it.  */
-         else if (TREE_CODE (ifexp) == NE_EXPR)
-           TREE_NO_WARNING (ifexp) = 1;
-       }
+       [expr.delete]/7 The deallocation function is called
+       regardless of whether the destructor for the object or some
+       element of the array throws an exception.  */
+    expr = build2 (TRY_FINALLY_EXPR, void_type_node, expr, do_delete);
 
-      if (ifexp != integer_one_node)
-       expr = build3 (COND_EXPR, void_type_node, ifexp, expr, void_node);
+  /* We need to calculate this before the dtor changes the vptr.  */
+  if (head)
+    expr = build2 (COMPOUND_EXPR, void_type_node, head, expr);
 
-      return expr;
+  if (flags & LOOKUP_DESTRUCTOR)
+    /* Explicit destructor call; don't check for null pointer.  */
+    ifexp = integer_one_node;
+  else
+    {
+      /* Handle deleting a null pointer.  */
+      warning_sentinel s (warn_address);
+      ifexp = cp_build_binary_op (input_location, NE_EXPR, addr,
+                                 nullptr_node, complain);
+      if (ifexp == error_mark_node)
+       return error_mark_node;
+      /* This is a compiler generated comparison, don't emit
+        e.g. -Wnonnull-compare warning for it.  */
+      else if (TREE_CODE (ifexp) == NE_EXPR)
+       TREE_NO_WARNING (ifexp) = 1;
     }
+
+  if (ifexp != integer_one_node)
+    expr = build3 (COND_EXPR, void_type_node, ifexp, expr, void_node);
+
+  return expr;
 }
 
 /* At the beginning of a destructor, push cleanups that will call the
index b5b80ab..d27aae2 100644 (file)
@@ -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 (file)
index 0000000..90c90f2
--- /dev/null
@@ -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;
+}