[llvm] r254538 - Fix linking when we copy over only a decl.

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 2 13:07:04 PST 2015


Fixed in r254543. Sorry about that.

The root problem is, I think, that we use IR level comdats with local
keys to mark that a COFF function is gcable.

Regular comdats (ELF and global key on COFF) are quite different.

Fixing that is on my TODO list, but it will take some time for me to get there.

Cheers,
Rafael


On 2 December 2015 at 15:44, Mike Aizatsky <aizatsky at google.com> wrote:
> For some reason this appear to break CFI test on windows bot. Is this a
> problem in the change or somewhere else?
>
> http://lab.llvm.org:8011/builders/sanitizer-windows/builds/13706
>
> Command 7: "C:/b/slave/sanitizer-windows/build/./bin/clang.exe"
> "-fuse-ld=lld" "-flto" "-fsanitize=cfi" "-o"
> "C:\b\slave\sanitizer-windows\build\projects\compiler-rt\test\cfi\Output\anon-namespace.cpp.tmp2"
> "C:\b\slave\sanitizer-windows\build\projects\compiler-rt\test\cfi\Output\anon-namespace.cpp.tmp1.o"
> "C:\b\slave\sanitizer-windows\build\projects\compiler-rt\test\cfi\Output\anon-namespace.cpp.tmp2.o"
>
> Command 7 Result: 1
>
> Command 7 Output:
>
>
>
>
>
> Command 7 Stderr:
>
> Alias must point to a definition
>
>
> i8** @"\01??_7B@?A@@6B at .2"
>
>
> LLVM ERROR: Broken module found, compilation aborted!
>
>
> clang.exe: error: linker command failed with exit code 1 (use -v to see
> invocation)
>
>
>
>
>
>
>
> On Wed, Dec 2, 2015 at 11:33 AM Rafael Espindola via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: rafael
>> Date: Wed Dec  2 13:30:52 2015
>> New Revision: 254538
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=254538&view=rev
>> Log:
>> Fix linking when we copy over only a decl.
>>
>> We were failing to copy the fact that the GV is weak and in the case of
>> an alias, producing invalid IR.
>>
>> Added:
>>     llvm/trunk/test/Linker/Inputs/comdat14.ll
>>     llvm/trunk/test/Linker/comdat14.ll
>> 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=254538&r1=254537&r2=254538&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)
>> +++ llvm/trunk/lib/Linker/LinkModules.cpp Wed Dec  2 13:30:52 2015
>> @@ -453,7 +453,7 @@ private:
>>    /// Handles cloning of a global values from the source module into
>>    /// the destination module, including setting the attributes and
>> visibility.
>>    GlobalValue *copyGlobalValueProto(TypeMapTy &TypeMap, const GlobalValue
>> *SGV,
>> -                                    const GlobalValue *DGV = nullptr);
>> +                                    const GlobalValue *DGV, bool
>> ForDefinition);
>>
>>    /// Check if we should promote the given local value to global scope.
>>    bool doPromoteLocalToGlobal(const GlobalValue *SGV);
>> @@ -594,14 +594,8 @@ static void forceRenaming(GlobalValue *G
>>  void ModuleLinker::copyGVAttributes(GlobalValue *NewGV,
>>                                      const GlobalValue *SrcGV) {
>>    auto *GA = dyn_cast<GlobalAlias>(SrcGV);
>> -  // Check for the special case of converting an alias (definition) to a
>> -  // non-alias (declaration). This can happen when we are importing and
>> -  // encounter a weak_any alias (weak_any defs may not be imported, see
>> -  // comments in ModuleLinker::getLinkage) or an alias whose base object
>> is
>> -  // being imported as a declaration. In that case copy the attributes
>> from the
>> -  // base object.
>>    if (GA && !dyn_cast<GlobalAlias>(NewGV)) {
>> -    assert(isPerformingImport() && !doImportAsDefinition(GA));
>> +    // FIXME: this is likelly bogus:
>>      NewGV->copyAttributesFrom(GA->getBaseObject());
>>    } else
>>      NewGV->copyAttributesFrom(SrcGV);
>> @@ -779,11 +773,12 @@ ModuleLinker::copyGlobalVariableProto(Ty
>>    // No linking to be performed or linking from the source: simply create
>> an
>>    // identical version of the symbol over in the dest module... the
>>    // initializer will be filled in later by LinkGlobalInits.
>> -  GlobalVariable *NewDGV = new GlobalVariable(
>> -      DstM, TypeMap.get(SGVar->getType()->getElementType()),
>> -      SGVar->isConstant(), getLinkage(SGVar), /*init*/ nullptr,
>> getName(SGVar),
>> -      /*insertbefore*/ nullptr, SGVar->getThreadLocalMode(),
>> -      SGVar->getType()->getAddressSpace());
>> +  GlobalVariable *NewDGV =
>> +      new GlobalVariable(DstM,
>> TypeMap.get(SGVar->getType()->getElementType()),
>> +                         SGVar->isConstant(),
>> GlobalValue::ExternalLinkage,
>> +                         /*init*/ nullptr, getName(SGVar),
>> +                         /*insertbefore*/ nullptr,
>> SGVar->getThreadLocalMode(),
>> +                         SGVar->getType()->getAddressSpace());
>>
>>    return NewDGV;
>>  }
>> @@ -794,8 +789,8 @@ Function *ModuleLinker::copyFunctionProt
>>                                            const Function *SF) {
>>    // If there is no linkage to be performed or we are linking from the
>> source,
>>    // bring SF over.
>> -  return Function::Create(TypeMap.get(SF->getFunctionType()),
>> getLinkage(SF),
>> -                          getName(SF), &DstM);
>> +  return Function::Create(TypeMap.get(SF->getFunctionType()),
>> +                          GlobalValue::ExternalLinkage, getName(SF),
>> &DstM);
>>  }
>>
>>  /// Set up prototypes for any aliases that come over from the source
>> module.
>> @@ -829,7 +824,7 @@ GlobalValue *ModuleLinker::copyGlobalAli
>>    // bring over SGA.
>>    auto *Ty = TypeMap.get(SGA->getValueType());
>>    return GlobalAlias::create(Ty,
>> SGA->getType()->getPointerAddressSpace(),
>> -                             getLinkage(SGA), getName(SGA), &DstM);
>> +                             GlobalValue::ExternalLinkage, getName(SGA),
>> &DstM);
>>  }
>>
>>  static GlobalValue::VisibilityTypes
>> @@ -857,14 +852,31 @@ void ModuleLinker::setVisibility(GlobalV
>>
>>  GlobalValue *ModuleLinker::copyGlobalValueProto(TypeMapTy &TypeMap,
>>                                                  const GlobalValue *SGV,
>> -                                                const GlobalValue *DGV) {
>> +                                                const GlobalValue *DGV,
>> +                                                bool ForDefinition) {
>>    GlobalValue *NewGV;
>> -  if (auto *SGVar = dyn_cast<GlobalVariable>(SGV))
>> +  if (auto *SGVar = dyn_cast<GlobalVariable>(SGV)) {
>>      NewGV = copyGlobalVariableProto(TypeMap, SGVar);
>> -  else if (auto *SF = dyn_cast<Function>(SGV))
>> +  } else if (auto *SF = dyn_cast<Function>(SGV)) {
>>      NewGV = copyFunctionProto(TypeMap, SF);
>> -  else
>> -    NewGV = copyGlobalAliasProto(TypeMap, cast<GlobalAlias>(SGV));
>> +  } else {
>> +    if (ForDefinition)
>> +      NewGV = copyGlobalAliasProto(TypeMap, cast<GlobalAlias>(SGV));
>> +    else
>> +      NewGV = new GlobalVariable(
>> +          DstM, TypeMap.get(SGV->getType()->getElementType()),
>> +          /*isConstant*/ false, GlobalValue::ExternalLinkage,
>> +          /*init*/ nullptr, getName(SGV),
>> +          /*insertbefore*/ nullptr, SGV->getThreadLocalMode(),
>> +          SGV->getType()->getAddressSpace());
>> +  }
>> +
>> +  if (ForDefinition)
>> +    NewGV->setLinkage(getLinkage(SGV));
>> +  else if (SGV->hasAvailableExternallyLinkage() || SGV->hasWeakLinkage()
>> ||
>> +           SGV->hasLinkOnceLinkage())
>> +    NewGV->setLinkage(GlobalValue::ExternalWeakLinkage);
>> +
>>    copyGVAttributes(NewGV, SGV);
>>    setVisibility(NewGV, SGV, DGV);
>>    return NewGV;
>> @@ -1446,7 +1458,7 @@ bool ModuleLinker::linkGlobalValueProto(
>>      // When linking from source we setVisibility from
>> copyGlobalValueProto.
>>      setVisibility(NewGV, SGV, DGV);
>>    } else {
>> -    NewGV = copyGlobalValueProto(TypeMap, SGV, DGV);
>> +    NewGV = copyGlobalValueProto(TypeMap, SGV, DGV, LinkFromSrc);
>>
>>      if (isPerformingImport() && !doImportAsDefinition(SGV))
>>        DoNotLinkFromSource.insert(SGV);
>>
>> Added: llvm/trunk/test/Linker/Inputs/comdat14.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/comdat14.ll?rev=254538&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/Linker/Inputs/comdat14.ll (added)
>> +++ llvm/trunk/test/Linker/Inputs/comdat14.ll Wed Dec  2 13:30:52 2015
>> @@ -0,0 +1,12 @@
>> +$c = comdat any
>> +
>> + at v2 = weak global i32 0, comdat ($c)
>> +define i32* @f2() {
>> +  ret i32* @v2
>> +}
>> +
>> + at v3 = weak alias i32, i32* @v2
>> +define i32* @f3() {
>> +  ret i32* @v3
>> +}
>> +
>>
>> Added: llvm/trunk/test/Linker/comdat14.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/comdat14.ll?rev=254538&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/Linker/comdat14.ll (added)
>> +++ llvm/trunk/test/Linker/comdat14.ll Wed Dec  2 13:30:52 2015
>> @@ -0,0 +1,9 @@
>> +; RUN: llvm-link -S %s %p/Inputs/comdat14.ll -o - | FileCheck %s
>> +
>> +$c = comdat any
>> +
>> + at v = global i32 0, comdat ($c)
>> +
>> +; CHECK: @v = global i32 0, comdat($c)
>> +; CHECK: @v2 = extern_weak global i32
>> +; CHECK: @v3 = extern_weak global i32
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list