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]

[WPA PATCH] Comdat group splitting


honza,
this is the fix for the partitioned WPA bug I was tracking down.

We have base and complete dtors sharing a comdat group (one's an alias for the other). The linker tells us the complete dtor is PREVAILING_DEF, as it's referenced from some other library. The base dtor is UNKNOWN.

We therefore internalize the base dtor, making it PREVAILING_DEF_IRONLY and split the comdat group. But the comdat group splitting also internalizes the complete dtor /and/ it does so inconsistently.

The bug manifested at runtime w/o link error as the complete dtor resolved to zero in the final link from wpa partitions that didn't contain its definition. (For extra fun, that was via a call to __cxa_at_exit registering a null function pointer, and getting the subsequent seg fault at program exit.) When we created WPA partitions node->externally_visible was still set, so we thought the now-internalized complete dtor was still externally visible -- but varasm looks at the DECL itself and emits an internal one. Plus the references to it were weak (& now hidden), so resolved to zero, rather than link error. And the external library either had its own definition which then prevailed for it. All rather 'ew'.

Anyway, this patch does 3 things
1) Moves the next->unique_name adjustment to before make_decl_local for members of the comdat group -- that matches the behaviour of the decl of interest itself.

2) For LDPR_PREVAILING_DEF members we don't make_decl_local, but instead clear DECL_COMDAT and DECL_WEAK. Thus forcing this decl to be the prevailing decl in the final link

3) For decls we localize, we also clear node->externally_visible and node->force_by_abi. That matches the behavior for the decl of interest too and will clue the wpa partitioning logic into knowing it needs to hidden-externalize the decl.

ok?

nathan

--
Nathan Sidwell
2017-01-18  Nathan Sidwell  <nathan@acm.org>

	* ipa-visibility.c (localize_node): Set comdat's unique name
	before adjusting resolution. Make PREVAILING_DEF members strongly
	public. Set visibility to false for localized decls.

Index: ipa-visibility.c
===================================================================
--- ipa-visibility.c	(revision 244546)
+++ ipa-visibility.c	(working copy)
@@ -542,16 +542,32 @@ localize_node (bool whole_program, symta
       for (symtab_node *next = node->same_comdat_group;
 	   next != node; next = next->same_comdat_group)
 	{
-	  next->set_comdat_group (NULL);
-	  if (!next->alias)
-	    next->set_section (NULL);
-	  if (!next->transparent_alias)
-	    next->make_decl_local ();
 	  next->unique_name
 	    |= ((next->resolution == LDPR_PREVAILING_DEF_IRONLY
 		 || next->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP)
 		&& TREE_PUBLIC (next->decl)
 		&& !flag_incremental_link);
+
+	  next->set_comdat_group (NULL);
+	  if (!next->alias)
+	    next->set_section (NULL);
+	  if (next->transparent_alias)
+	    /* Do nothing.  */;
+	  else if (next->resolution == LDPR_PREVAILING_DEF)
+	    {
+	      /* Make this a strong defn, so the external
+		 users don't mistakenly choose some other
+		 instance.  */
+	      DECL_COMDAT (next->decl) = false;
+	      DECL_WEAK (next->decl) = false;
+	    }
+	  else
+	    {
+	      next->externally_visible = false;
+	      next->forced_by_abi = false;
+	      next->resolution = LDPR_PREVAILING_DEF_IRONLY;
+	      next->make_decl_local ();
+	    }
 	}
 
       /* Now everything's localized, the grouping has no meaning, and

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