Do not forget to register ODR types to odr type hash

Message ID 20180418113138.GE26063@kam.mff.cuni.cz
State New
Headers show
Series
  • Do not forget to register ODR types to odr type hash
Related show

Commit Message

Jan Hubicka April 18, 2018, 11:31 a.m.
Hi,
this patch fixes misplaced conditional on TYPE_CANONICAL that makes odr_type_hash incomplete
and thus we may miss some ODR warnings and/or diverge in devirtualization decisions.

Bootstrapped/regtested x86_64-linux, also tested on libreoffice and firefox.

Honza

	PR lto/85391
	* lto.c (lto_read_decls): Do not test TYPE_CANONICAL before registering odr
	types.
	* g++.dg/lto/pr83121_0.C: Update template.
	* g++.dg/lto/pr83121_1.C: Update template.
	* g++.dg/lto/pr84805_0.C: Update template.
	* g++.dg/lto/pr84805_1.C: Update template.
	* g++.dg/lto/pr84805_2.C: Update template.

Patch

Index: lto/lto.c
===================================================================
--- lto/lto.c	(revision 259434)
+++ lto/lto.c	(working copy)
@@ -1772,12 +1772,13 @@ 
 		  seen_type = true;
 		  num_prevailing_types++;
 		  lto_fixup_prevailing_type (t);
-		}
-	      /* Compute the canonical type of all types.
-		 ???  Should be able to assert that !TYPE_CANONICAL.  */
-	      if (TYPE_P (t) && !TYPE_CANONICAL (t))
-		{
-		  gimple_register_canonical_type (t);
+
+	          /* Compute the canonical type of all types.
+		     Because SCC components ar estreame in random (hash) order
+		     we may have enountered the type before while registering
+		     type canonical of a derived type in the same SCC.  */
+		  if (!TYPE_CANONICAL (t))
+		    gimple_register_canonical_type (t);
 		  if (odr_type_p (t))
 		    register_odr_type (t);
 		}
Index: testsuite/g++.dg/lto/pr83121_0.C
===================================================================
--- testsuite/g++.dg/lto/pr83121_0.C	(revision 259434)
+++ testsuite/g++.dg/lto/pr83121_0.C	(working copy)
@@ -4,8 +4,8 @@ 
    from being optimized away.  */
 
 struct Environment { // { dg-lto-warning "8: type 'struct Environment' violates the C\\+\\+ One Definition Rule" }
-  struct AsyncHooks {
-    int providers_[2]; // { dg-lto-message "a field of same name but different type is defined in another translation unit" }
+  struct AsyncHooks { // { dg-lto-warning "10: type 'struct AsyncHooks' violates the C\\+\\+ One Definition Rule" }
+    int providers_[2]; // { dg-lto-message "the first difference of corresponding definitions is field 'providers_'" }
   };
   AsyncHooks async_hooks_;
 };
Index: testsuite/g++.dg/lto/pr83121_1.C
===================================================================
--- testsuite/g++.dg/lto/pr83121_1.C	(revision 259434)
+++ testsuite/g++.dg/lto/pr83121_1.C	(working copy)
@@ -1,8 +1,8 @@ 
 struct Environment {
-  struct AsyncHooks { // { dg-lto-warning "10: type 'struct AsyncHooks' violates the C\\+\\+ One Definition Rule" }
-    int providers_[1]; // { dg-lto-message "the first difference of corresponding definitions is field 'providers_'" }
+  struct AsyncHooks {
+    int providers_[1];
   };
-  AsyncHooks async_hooks_;
+  AsyncHooks async_hooks_; // { dg-lto-message "a field of same name but different type is defined in another translation unit" }
 };
 void fn1() { Environment a; }
 int main ()
Index: testsuite/g++.dg/lto/pr84805_0.C
===================================================================
--- testsuite/g++.dg/lto/pr84805_0.C	(revision 259434)
+++ testsuite/g++.dg/lto/pr84805_0.C	(working copy)
@@ -1,5 +1,5 @@ 
 // { dg-lto-do link }
-// { dg-lto-options {{-O2 -fPIC -shared -flto}} }
+// { dg-lto-options {{-O0 -fPIC -shared -flto}} }
 
 template < typename _Tp, _Tp __v > struct integral_constant {
   static constexpr _Tp value = __v;
@@ -9,7 +9,7 @@ 
 struct is_void : __is_void_helper {};
 template < typename > struct is_array : false_type {};
 namespace __gnu_cxx {
-enum _Lock_policy { _S_single, _S_mutex, _S_atomic };
+enum _Lock_policy { _S_single, _S_mutex, _S_atomic }; // { dg-lto-warning "6: type '_Lock_policy' violates the C\\+\\+ One Definition Rule" }
 const _Lock_policy __default_lock_policy = _S_atomic;
 } namespace std {
 using __gnu_cxx::_Lock_policy;
@@ -21,7 +21,7 @@ 
            bool = is_void::value >
 class __shared_ptr_access {};
 template < typename _Tp, _Lock_policy _Lp >
-class __shared_ptr : __shared_ptr_access< _Tp, _Lp > {
+class __shared_ptr : __shared_ptr_access< _Tp, _Lp > { // { dg-lto-warning "7: type 'struct __shared_ptr' violates the C\\+\\+ One Definition Rule" }
   using element_type = _Tp;
   element_type *_M_ptr;
   __shared_count< _Lp > _M_refcount;
@@ -88,7 +88,7 @@ 
 class ExtSheetBuffer;
 class ExcelToSc;
 class XclImpColRowSettings;
-struct RootData {
+struct RootData { // { dg-lto-warning "8: type 'struct RootData' violates the C\\+\\+ One Definition Rule" }
   BiffTyp eDateiTyp;
   ExtSheetBuffer *pExtSheetBuff;
   SharedFormulaBuffer *pShrfmlaBuff;
Index: testsuite/g++.dg/lto/pr84805_1.C
===================================================================
--- testsuite/g++.dg/lto/pr84805_1.C	(revision 259434)
+++ testsuite/g++.dg/lto/pr84805_1.C	(working copy)
@@ -3,7 +3,7 @@ 
   virtual ~XclRoot();
 };
 class XclImpRoot : XclRoot {};
-struct RootData { // { dg-lto-warning "8: type 'struct RootData' violates the C\\+\\+ One Definition Rule" }
+struct RootData {
   XclImpRoot pIR;
 };
 class ExcRoot {
Index: testsuite/g++.dg/lto/pr84805_2.C
===================================================================
--- testsuite/g++.dg/lto/pr84805_2.C	(revision 259434)
+++ testsuite/g++.dg/lto/pr84805_2.C	(working copy)
@@ -15,7 +15,7 @@ 
 template < typename a, _Lock_policy, bool = g< a >::d, bool = t::d >
 class __shared_ptr_access {};
 template < typename a, _Lock_policy l >
-class __shared_ptr : __shared_ptr_access< a, l > { // { dg-lto-warning "7: type 'struct __shared_ptr' violates the C\\+\\+ One Definition Rule" }
+class __shared_ptr : __shared_ptr_access< a, l > {
   using m = a;
   m *_M_ptr;
   __shared_count< l > _M_refcount;
Index: testsuite/g++.dg/torture/pr43760.C
===================================================================
--- testsuite/g++.dg/torture/pr43760.C	(nonexistent)
+++ testsuite/g++.dg/torture/pr43760.C	(working copy)
@@ -0,0 +1,40 @@ 
+typedef __builtin_va_list a;
+
+class b
+{
+public:
+  virtual void c (int, const char *, a &);
+  char d;
+  void m_fn2 ()
+  {
+    a a;
+    c (2, &d, a);
+  }
+};
+
+class e:b
+{
+  virtual void f ()
+  {
+  }
+  void c (int, const char *, a &);
+};
+
+class g
+{
+protected:
+  b h;
+};
+
+class i:g
+{
+  int j ();
+};
+
+int
+i::j ()
+{
+  h.m_fn2 ();
+  return 0;
+}
+