From 1c14f2864d7e7b87609c9d4c9b8295d0140ec978 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Thu, 3 Dec 2015 18:20:05 +0000 Subject: [PATCH] [ThinLTO] Appending linkage fixes Summary: Fix import from module with appending var, which cannot be imported. The first fix is to remove an overly-aggressive error check. The second fix is to deal with restructuring introduced to the module linker yesterday in r254418 (actually, this fix was included already in r254559, just added some additional cleanup). Test by Mehdi Amini. Reviewers: joker.eph, rafael Subscribers: joker.eph, llvm-commits Differential Revision: http://reviews.llvm.org/D15156 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@254624 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Linker/LinkModules.cpp | 14 ++++++++----- .../Inputs/funcimport_appending_global.ll | 6 ++++++ test/Linker/funcimport_appending_global.ll | 20 +++++++++++++++++++ 3 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 test/Linker/Inputs/funcimport_appending_global.ll create mode 100644 test/Linker/funcimport_appending_global.ll diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp index 4bc0ad039ac..55ab1824740 100644 --- a/lib/Linker/LinkModules.cpp +++ b/lib/Linker/LinkModules.cpp @@ -726,8 +726,10 @@ GlobalValue::LinkageTypes ModuleLinker::getLinkage(const GlobalValue *SGV) { // It would be incorrect to import an appending linkage variable, // since it would cause global constructors/destructors to be // executed multiple times. This should have already been handled - // by linkGlobalValueProto. - llvm_unreachable("Cannot import appending linkage variable"); + // by linkIfNeeded, and we will assert in shouldLinkFromSource + // if we try to import, so we simply return AppendingLinkage here + // as this helper is called more widely in getLinkedToGlobal. + return GlobalValue::AppendingLinkage; case GlobalValue::InternalLinkage: case GlobalValue::PrivateLinkage: @@ -1015,8 +1017,7 @@ bool ModuleLinker::shouldLinkFromSource(bool &LinkFromSrc, // We always have to add Src if it has appending linkage. if (Src.hasAppendingLinkage()) { - // Caller should have already determined that we can't link from source - // when importing (see comments in linkGlobalValueProto). + // Should have prevented importing for appending linkage in linkIfNeeded. assert(!isPerformingImport()); LinkFromSrc = true; return false; @@ -1387,9 +1388,12 @@ Constant *ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) { // Handle the ultra special appending linkage case first. assert(!DGV || SGV->hasAppendingLinkage() == DGV->hasAppendingLinkage()); - if (SGV->hasAppendingLinkage()) + if (SGV->hasAppendingLinkage()) { + // Should have prevented importing for appending linkage in linkIfNeeded. + assert(!isPerformingImport()); return linkAppendingVarProto(cast_or_null(DGV), cast(SGV)); + } bool LinkFromSrc = true; Comdat *C = nullptr; diff --git a/test/Linker/Inputs/funcimport_appending_global.ll b/test/Linker/Inputs/funcimport_appending_global.ll new file mode 100644 index 00000000000..413b890b02a --- /dev/null +++ b/test/Linker/Inputs/funcimport_appending_global.ll @@ -0,0 +1,6 @@ +@v = weak global i8 1 +@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* @v}] + +define void @foo() { + ret void +} diff --git a/test/Linker/funcimport_appending_global.ll b/test/Linker/funcimport_appending_global.ll new file mode 100644 index 00000000000..190d31ee8c7 --- /dev/null +++ b/test/Linker/funcimport_appending_global.ll @@ -0,0 +1,20 @@ +; RUN: llvm-as -function-summary %s -o %t.bc +; RUN: llvm-as -function-summary %p/Inputs/funcimport_appending_global.ll -o %t2.bc +; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc + +; Do the import now +; RUN: llvm-link %t.bc -functionindex=%t3.thinlto.bc -import=foo:%t2.bc -S | FileCheck %s + +; Ensure that global constructor (appending linkage) is not imported +; CHECK-NOT: @llvm.global_ctors = {{.*}}@foo + +declare void @f() +@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @f, i8* null}] + +define i32 @main() { +entry: + call void @foo() + ret i32 0 +} + +declare void @foo() -- 2.34.1