RFA: New ipa-devirt PATCH for c++/58678 (devirt vs. KDE)

[Jason Merrill <jason at redhat dot com>]

This patch fixes the latest 58678 testcase by avoiding speculative 
devirtualization to the destructor of an abstract class, which can never 
succeed: you can't create an object of an abstract class, so the pointer 
must point to an object of a derived class, and the derived class 
necessarily has its own destructor.  Other virtual member functions of 
an abstract class are OK for devirtualization: the destructor is the 
only virtual function that is always overridden in every class.

We could also detect an abstract class by searching through the vtable 
for __cxa_pure_virtual, but I figured it was easy enough for the front 
end to set a flag on the BINFO.

Tested x86_64-pc-linux-gnu.  OK for trunk?

commit b64f52066f3f4cdc9d5a30e2d48aaf6dd5efd3d4
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Mar 5 11:35:07 2014 -0500

    	PR c++/58678
    gcc/
    	* tree.h (BINFO_ABSTRACT_P): New.
    	* ipa-devirt.c (abstract_class_dtor_p): New.
    	(likely_target_p): Check it.
    gcc/cp/
    	* search.c (get_pure_virtuals): Set BINFO_ABSTRACT_P.
    	* tree.c (copy_binfo): Copy it.

diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index c3eed90..7a3ea4b 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -2115,6 +2115,8 @@ get_pure_virtuals (tree type)
      which it is a primary base will contain vtable entries for the
      pure virtuals in the base class.  */
   dfs_walk_once (TYPE_BINFO (type), NULL, dfs_get_pure_virtuals, type);
+  if (CLASSTYPE_PURE_VIRTUALS (type))
+    BINFO_ABSTRACT_P (TYPE_BINFO (type)) = true;
 }
 
 /* Debug info for C++ classes can get very large; try to avoid
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 5567253..3836e16 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -1554,6 +1554,7 @@ copy_binfo (tree binfo, tree type, tree t, tree *igo_prev, int virt)
 
       BINFO_OFFSET (new_binfo) = BINFO_OFFSET (binfo);
       BINFO_VIRTUALS (new_binfo) = BINFO_VIRTUALS (binfo);
+      BINFO_ABSTRACT_P (new_binfo) = BINFO_ABSTRACT_P (binfo);
 
       /* We do not need to copy the accesses, as they are read only.  */
       BINFO_BASE_ACCESSES (new_binfo) = BINFO_BASE_ACCESSES (binfo);
diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 2f84f17..3f4a1d5 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1674,6 +1674,19 @@ update_type_inheritance_graph (void)
   timevar_pop (TV_IPA_INHERITANCE);
 }
 
+/* A destructor for an abstract class is not likely because it can never be
+   called through the vtable; any actual object will have a derived type,
+   which will have its own destructor.  */
+
+static bool
+abstract_class_dtor_p (tree fn)
+{
+  if (!DECL_CXX_DESTRUCTOR_P (fn))
+    return false;
+  tree type = DECL_CONTEXT (fn);
+  tree binfo = TYPE_BINFO (type);
+  return BINFO_ABSTRACT_P (binfo);
+}
 
 /* Return true if N looks like likely target of a polymorphic call.
    Rule out cxa_pure_virtual, noreturns, function declared cold and
@@ -1694,6 +1707,8 @@ likely_target_p (struct cgraph_node *n)
     return false;
   if (n->frequency < NODE_FREQUENCY_NORMAL)
     return false;
+  if (abstract_class_dtor_p (n->decl))
+    return false;
   return true;
 }
 
diff --git a/gcc/testsuite/g++.dg/ipa/devirt-30.C b/gcc/testsuite/g++.dg/ipa/devirt-30.C
new file mode 100644
index 0000000..c4ac694
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/devirt-30.C
@@ -0,0 +1,25 @@
+// PR c++/58678
+// { dg-options "-O3 -fdump-ipa-devirt" }
+
+// We shouldn't speculatively devirtualize to ~B because B is an abstract
+// class; any actual object passed to f will be of some derived class which
+// has its own destructor.
+
+struct A
+{
+  virtual void f() = 0;
+  virtual ~A();
+};
+
+struct B : A
+{
+  virtual ~B() {}
+};
+
+void f(B* b)
+{
+  delete b;
+}
+
+// { dg-final { scan-ipa-dump-not "Speculatively devirtualizing" "devirt" } }
+// { dg-final { cleanup-ipa-dump "devirt" } }
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index e548a0d..708a8ab 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -786,6 +786,9 @@ struct GTY(()) tree_base {
        PREDICT_EXPR_OUTCOME in
 	   PREDICT_EXPR
 
+       BINFO_ABSTRACT_P in
+           TREE_BINFO
+
    static_flag:
 
        TREE_STATIC in
diff --git a/gcc/tree.h b/gcc/tree.h
index 0dc8d0d..01e23ec 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1804,6 +1804,9 @@ extern void protected_set_expr_location (tree, location_t);
 /* Nonzero means that the derivation chain is via a `virtual' declaration.  */
 #define BINFO_VIRTUAL_P(NODE) (TREE_BINFO_CHECK (NODE)->base.static_flag)
 
+/* Nonzero means that the base is an abstract class.  */
+#define BINFO_ABSTRACT_P(NODE) (TREE_BINFO_CHECK (NODE)->base.addressable_flag)
+
 /* Flags for language dependent use.  */
 #define BINFO_MARKED(NODE) TREE_LANG_FLAG_0 (TREE_BINFO_CHECK (NODE))
 #define BINFO_FLAG_1(NODE) TREE_LANG_FLAG_1 (TREE_BINFO_CHECK (NODE))