[llvm-commits] [llvm] r153351 - /llvm/trunk/lib/Linker/LinkModules.cpp

[Bill Wendling]

Author: void
Date: Fri Mar 23 18:17:38 2012
New Revision: 153351

URL: http://llvm.org/viewvc/llvm-project?rev=153351&view=rev
Log:
It's possible for two types, which are isomorphic, to be added to the
destination module, but one of them isn't used in the destination module. If
another module comes along and the uses the unused type, there could be type
conflicts when the modules are finally linked together. (This happened when
building LLVM.)

The test that was reduced is:

Module A:

%Z = type { %A }
%A = type { %B.1, [7 x x86_fp80] }
%B.1 = type { %C }
%C = type { i8* }

declare void @func_x(%C*, i64, i64)
declare void @func_z(%Z* nocapture)

Module B:

%B = type { %C.1 }
%C.1 = type { i8* }
%A.2 = type { %B.3, [5 x x86_fp80] }
%B.3 = type { %C.1 }

define void @func_z() {
  %x = alloca %A.2, align 16
  %y = getelementptr inbounds %A.2* %x, i64 0, i32 0, i32 0
  call void @func_x(%C.1* %y, i64 37, i64 927) nounwind
  ret void
}

declare void @func_x(%C.1*, i64, i64)
declare void @func_y(%B* nocapture)

(Unfortunately, this test doesn't fail under llvm-link, only during an LTO
linking.) The '%C' and '%C.1' clash. The destination module gets the '%C'
declaration. When merging Module B, it looks at the '%C.1' subtype of the '%B'
structure. It adds that in, because that's cool. And when '%B.3' is processed,
it uses the '%C.1'. But the '%B' has used '%C' and we prefer to use '%C'. So the
'@func_x' type is changed to 'void (%C*, i64, i64)', but the type of '%x' in
'@func_z' remains '%A.2'. The GEP resolves to a '%C.1', which conflicts with the
'@func_x' signature.

We can resolve this situation by making sure that the type is used in the
destination before saying that it should be used in the module being merged in.

With this fix, LLVM and Clang both compile under LTO.
<rdar://problem/10913281>

Modified:
    llvm/trunk/lib/Linker/LinkModules.cpp

Modified: llvm/trunk/lib/Linker/LinkModules.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=153351&r1=153350&r2=153351&view=diff
==============================================================================
--- llvm/trunk/lib/Linker/LinkModules.cpp (original)
+++ llvm/trunk/lib/Linker/LinkModules.cpp Fri Mar 23 18:17:38 2012
@@ -596,10 +596,14 @@
   // it had the same type, it would have been renamed to "%foo.42 = { i32 }".
   std::vector<StructType*> SrcStructTypes;
   SrcM->findUsedStructTypes(SrcStructTypes);
-  
   SmallPtrSet<StructType*, 32> SrcStructTypesSet(SrcStructTypes.begin(),
                                                  SrcStructTypes.end());
-  
+
+  std::vector<StructType*> DstStructTypes;
+  DstM->findUsedStructTypes(DstStructTypes);
+  SmallPtrSet<StructType*, 32> DstStructTypesSet(DstStructTypes.begin(),
+                                                 DstStructTypes.end());
+
   for (unsigned i = 0, e = SrcStructTypes.size(); i != e; ++i) {
     StructType *ST = SrcStructTypes[i];
     if (!ST->hasName()) continue;
@@ -612,9 +616,24 @@
     
     // Check to see if the destination module has a struct with the prefix name.
     if (StructType *DST = DstM->getTypeByName(ST->getName().substr(0, DotPos)))
-      // Don't use it if this actually came from the source module.  They're in
-      // the same LLVMContext after all.
-      if (!SrcStructTypesSet.count(DST))
+      // Don't use it if this actually came from the source module. They're in
+      // the same LLVMContext after all. Also don't use it unless the type is
+      // actually used in the destination module. This can happen in situations
+      // like this:
+      //
+      //      Module A                         Module B
+      //      --------                         --------
+      //   %Z = type { %A }                %B = type { %C.1 }
+      //   %A = type { %B.1, [7 x i8] }    %C.1 = type { i8* }
+      //   %B.1 = type { %C }              %A.2 = type { %B.3, [5 x i8] }
+      //   %C = type { i8* }               %B.3 = type { %C.1 }
+      //
+      // When we link Module B with Module A, the '%B' in Module B is
+      // used. However, that would then use '%C.1'. But when we process '%C.1',
+      // we prefer to take the '%C' version. So we are then left with both
+      // '%C.1' and '%C' being used for the same types. This leads to some
+      // variables using one type and some using the other.
+      if (!SrcStructTypesSet.count(DST) && DstStructTypesSet.count(DST))
         TypeMap.addTypeMapping(DST, ST);
   }
 
@@ -1313,6 +1332,6 @@
     if (ErrorMsg) *ErrorMsg = TheLinker.ErrorMsg;
     return true;
   }
-  
+
   return false;
 }