It's possible for two types, which are isomorphic, to be added to the
authorBill Wendling <isanbard@gmail.com>
Fri, 23 Mar 2012 23:17:38 +0000 (23:17 +0000)
committerBill Wendling <isanbard@gmail.com>
Fri, 23 Mar 2012 23:17:38 +0000 (23:17 +0000)
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>

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@153351 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Linker/LinkModules.cpp

index dca523c7ab18e3bfe4f5ac9cdb4bb49eb291641c..765fcc88235b23822a00cc9a24a0c8e01f019fa5 100644 (file)
@@ -596,10 +596,14 @@ void ModuleLinker::computeTypeMapping() {
   // 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 @@ void ModuleLinker::computeTypeMapping() {
     
     // 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 @@ bool Linker::LinkModules(Module *Dest, Module *Src, unsigned Mode,
     if (ErrorMsg) *ErrorMsg = TheLinker.ErrorMsg;
     return true;
   }
-  
+
   return false;
 }