diff mbox

C++ PATCH to 4.9 for c++/61659 (devirt referencing undefined comdats)

Message ID 54105FAE.1090203@redhat.com
State New
Headers show

Commit Message

Jason Merrill Sept. 10, 2014, 2:26 p.m. UTC
My change to always set DECL_COMDAT on vague-linkage decls has caused 
various problems on the trunk, so I'm not comfortable applying it to 
4.9.  So this patch does a much more limited version of the same thing: 
only set it on vague-linkage functions that might not be defined, and 
don't set it until after we are done with any front-end semantics that 
might be implied.

Tested x86_64-pc-linux-gnu, applying to 4.9.
diff mbox

Patch

commit e26f8f5e9cb8048ccf7183c6d8a27fa38b5b7a62
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Jul 29 17:28:44 2014 -0400

    	PR lto/53808
    	PR c++/61659
    	* decl2.c (note_comdat_fn): New.
    	(set_comdat): New.
    	(cp_write_global_declarations): Call set_comdat.
    	* method.c (implicitly_declare_fn): Call note_comdat_fn.
    	* pt.c (tsubst_decl) [FUNCTION_DECL]: Likewise.
    	* decl2.c (mark_needed): Mark clones.
    	(import_export_decl): Not here.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 0b52dff..99cc7ec 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5352,6 +5352,7 @@  extern tree get_tls_wrapper_fn			(tree);
 extern void mark_needed				(tree);
 extern bool decl_needed_p			(tree);
 extern void note_vague_linkage_fn		(tree);
+extern void note_comdat_fn			(tree);
 extern tree build_artificial_parm		(tree, tree);
 extern bool possibly_inlined_p			(tree);
 extern int parm_index                           (tree);
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 6c52e53..a2626d4 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -99,6 +99,10 @@  static GTY(()) vec<tree, va_gc> *pending_statics;
    may need to emit outline anyway.  */
 static GTY(()) vec<tree, va_gc> *deferred_fns;
 
+/* A list of functions which we might want to set DECL_COMDAT on at EOF.  */
+
+static GTY(()) vec<tree, va_gc> *maybe_comdat_fns;
+
 /* A list of decls that use types with no linkage, which we need to make
    sure are defined.  */
 static GTY(()) vec<tree, va_gc> *no_linkage_decls;
@@ -1896,6 +1900,12 @@  mark_needed (tree decl)
 	 definition.  */
       struct cgraph_node *node = cgraph_get_create_node (decl);
       node->forced_by_abi = true;
+
+      /* #pragma interface and -frepo code can call mark_needed for
+          maybe-in-charge 'tors; mark the clones as well.  */
+      tree clone;
+      FOR_EACH_CLONE (clone, decl)
+	mark_needed (clone);
     }
   else if (TREE_CODE (decl) == VAR_DECL)
     {
@@ -2678,17 +2688,7 @@  import_export_decl (tree decl)
     {
       /* The repository indicates that this entity should be defined
 	 here.  Make sure the back end honors that request.  */
-      if (VAR_P (decl))
-	mark_needed (decl);
-      else if (DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (decl)
-	       || DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (decl))
-	{
-	  tree clone;
-	  FOR_EACH_CLONE (clone, decl)
-	    mark_needed (clone);
-	}
-      else
-	mark_needed (decl);
+      mark_needed (decl);
       /* Output the definition as an ordinary strong definition.  */
       DECL_EXTERNAL (decl) = 0;
       DECL_INTERFACE_KNOWN (decl) = 1;
@@ -4231,6 +4231,34 @@  dump_tu (void)
     }
 }
 
+/* Much like the above, but not necessarily defined.  4.9 hack for setting
+   DECL_COMDAT on DECL_EXTERNAL functions, along with set_comdat.  */
+
+void
+note_comdat_fn (tree decl)
+{
+  vec_safe_push (maybe_comdat_fns, decl);
+}
+
+/* DECL is a function with vague linkage that was not
+   instantiated/synthesized in this translation unit.  Set DECL_COMDAT for
+   the benefit of can_refer_decl_in_current_unit_p.  */
+
+static void
+set_comdat (tree decl)
+{
+  DECL_COMDAT (decl) = true;
+
+  tree clone;
+  FOR_EACH_CLONE (clone, decl)
+    set_comdat (clone);
+
+  if (DECL_VIRTUAL_P (decl))
+    for (tree thunk = DECL_THUNKS (decl); thunk;
+	 thunk = DECL_CHAIN (thunk))
+      DECL_COMDAT (thunk) = true;
+}
+
 /* This routine is called at the end of compilation.
    Its job is to create all the code needed to initialize and
    destroy the global aggregates.  We do the destruction
@@ -4608,6 +4636,10 @@  cp_write_global_declarations (void)
       vtv_build_vtable_verify_fndecl ();
     }
 
+  FOR_EACH_VEC_SAFE_ELT (maybe_comdat_fns, i, decl)
+    if (!DECL_COMDAT (decl) && vague_linkage_p (decl))
+      set_comdat (decl);
+
   finalize_compilation_unit ();
 
   if (flag_vtable_verify)
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 11bff7f..b074d74 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1773,6 +1773,7 @@  implicitly_declare_fn (special_function_kind kind, tree type,
   DECL_EXTERNAL (fn) = true;
   DECL_NOT_REALLY_EXTERN (fn) = 1;
   DECL_DECLARED_INLINE_P (fn) = 1;
+  note_comdat_fn (fn);
   gcc_assert (!TREE_USED (fn));
 
   /* Restore PROCESSING_TEMPLATE_DECL.  */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 3296fda..91ff32a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -10677,6 +10677,9 @@  tsubst_decl (tree t, tree args, tsubst_flags_t complain)
 		 the type earlier (template/friend54.C).  */
 	      RETURN (new_r);
 
+	    if (!DECL_FRIEND_P (r))
+	      note_comdat_fn (r);
+
 	    /* We're not supposed to instantiate default arguments
 	       until they are called, for a template.  But, for a
 	       declaration like:
diff --git a/gcc/testsuite/g++.dg/abi/no-weak1.C b/gcc/testsuite/g++.dg/abi/no-weak1.C
new file mode 100644
index 0000000..d539015
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/no-weak1.C
@@ -0,0 +1,13 @@ 
+// { dg-options "-fno-weak" }
+// { dg-final { scan-assembler "local\[ \t\]*_ZZL1fvE1i" { target x86_64-*-*gnu } } }
+
+static inline void f()
+{
+  static int i;
+  ++i;
+};
+
+int main()
+{
+  f();
+}
diff --git a/gcc/testsuite/g++.dg/abi/spec1.C b/gcc/testsuite/g++.dg/abi/spec1.C
new file mode 100644
index 0000000..153c0cf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/spec1.C
@@ -0,0 +1,4 @@ 
+// { dg-final { scan-assembler-not "weak" } }
+
+template <class T> struct A { static int i; };
+template<> int A<int>::i = 42;
diff --git a/gcc/testsuite/g++.dg/opt/devirt4.C b/gcc/testsuite/g++.dg/opt/devirt4.C
index 5a24eec..72f56af 100644
--- a/gcc/testsuite/g++.dg/opt/devirt4.C
+++ b/gcc/testsuite/g++.dg/opt/devirt4.C
@@ -1,8 +1,7 @@ 
 // PR lto/53808
-// Devirtualization + inlining should produce a non-virtual
-// call to ~foo.
-// { dg-options "-O -fdevirtualize" }
-// { dg-final { scan-assembler "_ZN3fooD2Ev" } }
+// Devirtualization should not produce an external ref to ~bar.
+// { dg-options "-O2" }
+// { dg-final { scan-assembler-not "_ZN3barD0Ev" } }
 
 struct foo {
  virtual ~foo();
diff --git a/gcc/testsuite/g++.dg/opt/devirt5.C b/gcc/testsuite/g++.dg/opt/devirt5.C
new file mode 100644
index 0000000..f839cbe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/devirt5.C
@@ -0,0 +1,19 @@ 
+// PR c++/61659
+// { dg-options "-O3" }
+// { dg-final { scan-assembler-not "_ZN6parserIiE9getOptionEv" } }
+
+struct generic_parser_base {
+  virtual void getOption();
+  void getExtraOptionNames() { getOption(); }
+};
+template <class DataType> struct parser : public generic_parser_base {
+  virtual void getOption() {}
+};
+struct PassNameParser : public parser<int> {
+  PassNameParser();
+};
+struct list {
+  PassNameParser Parser;
+  virtual void getExtraOptionNames() { return Parser.getExtraOptionNames(); }
+};
+list PassList;
diff --git a/gcc/testsuite/g++.dg/template/friend56.C b/gcc/testsuite/g++.dg/template/friend56.C
new file mode 100644
index 0000000..7dd5d48
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/friend56.C
@@ -0,0 +1,13 @@ 
+// Make sure we don't mistakenly mark f as DECL_COMDAT.
+// { dg-final { scan-assembler "_Z1fv" } }
+
+void f();
+
+template <class T> struct A
+{
+  friend void f();
+};
+
+A<int> a;
+
+void f() { }
diff --git a/gcc/testsuite/g++.dg/template/spec38.C b/gcc/testsuite/g++.dg/template/spec38.C
new file mode 100644
index 0000000..6f06f14
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/spec38.C
@@ -0,0 +1,6 @@ 
+// PR ipa/61659
+
+// { dg-final { scan-assembler "_Z1fIiEvPT_" } }
+
+template <typename T> inline void f (T *);
+template <> void f (int *) { }