[clang] a26bd95 - [LinkerWrapper] Fix static library symbol resolution
Joseph Huber via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 1 04:52:00 PDT 2023
Author: Joseph Huber
Date: 2023-06-01T06:51:51-05:00
New Revision: a26bd95325f120d9737a0b03c80eabddb56f46db
URL: https://github.com/llvm/llvm-project/commit/a26bd95325f120d9737a0b03c80eabddb56f46db
DIFF: https://github.com/llvm/llvm-project/commit/a26bd95325f120d9737a0b03c80eabddb56f46db.diff
LOG: [LinkerWrapper] Fix static library symbol resolution
The linker wrapper performs its own very basic symbol resolution for the
purpose of supporting standard static library semantics. We do this here
because the Nvidia `nvlink` wrapper does not support static linking and
we have some offloading specific extensions.
Currently, we always place symbols in the "table" even if they aren't
extracted. This caused the logic to fail when many files were used that
referenced the same undefined variable. This patch changes the pass to
only add the symbols to the global "table" if the file is actually
extracted.
Reviewed By: tra
Differential Revision: https://reviews.llvm.org/D151839
Added:
Modified:
clang/test/Driver/linker-wrapper-libs.c
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
Removed:
################################################################################
diff --git a/clang/test/Driver/linker-wrapper-libs.c b/clang/test/Driver/linker-wrapper-libs.c
index acb7c38165d04..2073092bdbcf9 100644
--- a/clang/test/Driver/linker-wrapper-libs.c
+++ b/clang/test/Driver/linker-wrapper-libs.c
@@ -12,6 +12,9 @@ int __attribute__((visibility("protected"))) global;
int __attribute__((visibility("hidden"))) weak;
#elif defined(HIDDEN)
int __attribute__((visibility("hidden"))) hidden;
+#elif defined(UNDEFINED)
+extern int sym;
+int baz() { return sym; }
#else
extern int sym;
@@ -26,7 +29,11 @@ int bar() { return weak; }
//
// RUN: %clang -cc1 %s -triple nvptx64-nvidia-cuda -emit-llvm-bc -DRESOLVES -o %t.nvptx.resolves.bc
// RUN: %clang -cc1 %s -triple amdgcn-amd-amdhsa -emit-llvm-bc -DRESOLVES -o %t.amdgpu.resolves.bc
+// RUN: %clang -cc1 %s -triple nvptx64-nvidia-cuda -emit-llvm-bc -DUNDEFINED -o %t.nvptx.undefined.bc
+// RUN: %clang -cc1 %s -triple amdgcn-amd-amdhsa -emit-llvm-bc -DUNDEFINED -o %t.amdgpu.undefined.bc
// RUN: clang-offload-packager -o %t-lib.out \
+// RUN: --image=file=%t.nvptx.undefined.bc,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70 \
+// RUN: --image=file=%t.amdgpu.undefined.bc,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx1030 \
// RUN: --image=file=%t.nvptx.resolves.bc,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70 \
// RUN: --image=file=%t.amdgpu.resolves.bc,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx1030
// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o -fembed-offload-object=%t-lib.out
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index 2d96e0a344e11..0af0f2e371b18 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -1163,20 +1163,21 @@ enum Symbol : uint32_t {
/// Scan the symbols from a BitcodeFile \p Buffer and record if we need to
/// extract any symbols from it.
Expected<bool> getSymbolsFromBitcode(MemoryBufferRef Buffer, OffloadKind Kind,
- StringSaver &Saver,
+ bool IsArchive, StringSaver &Saver,
DenseMap<StringRef, Symbol> &Syms) {
Expected<IRSymtabFile> IRSymtabOrErr = readIRSymtab(Buffer);
if (!IRSymtabOrErr)
return IRSymtabOrErr.takeError();
- bool ShouldExtract = false;
+ bool ShouldExtract = !IsArchive;
+ DenseMap<StringRef, Symbol> TmpSyms;
for (unsigned I = 0; I != IRSymtabOrErr->Mods.size(); ++I) {
for (const auto &Sym : IRSymtabOrErr->TheReader.module_symbols(I)) {
if (Sym.isFormatSpecific() || !Sym.isGlobal())
continue;
bool NewSymbol = Syms.count(Sym.getName()) == 0;
- auto &OldSym = Syms[Saver.save(Sym.getName())];
+ auto OldSym = NewSymbol ? Sym_None : Syms[Sym.getName()];
// We will extract if it defines a currenlty undefined non-weak symbol.
bool ResolvesStrongReference =
@@ -1192,23 +1193,31 @@ Expected<bool> getSymbolsFromBitcode(MemoryBufferRef Buffer, OffloadKind Kind,
// Update this symbol in the "table" with the new information.
if (OldSym & Sym_Undefined && !Sym.isUndefined())
- OldSym = static_cast<Symbol>(OldSym & ~Sym_Undefined);
+ TmpSyms[Saver.save(Sym.getName())] =
+ static_cast<Symbol>(OldSym & ~Sym_Undefined);
if (Sym.isUndefined() && NewSymbol)
- OldSym = static_cast<Symbol>(OldSym | Sym_Undefined);
+ TmpSyms[Saver.save(Sym.getName())] =
+ static_cast<Symbol>(OldSym | Sym_Undefined);
if (Sym.isWeak())
- OldSym = static_cast<Symbol>(OldSym | Sym_Weak);
+ TmpSyms[Saver.save(Sym.getName())] =
+ static_cast<Symbol>(OldSym | Sym_Weak);
}
}
+ // If the file gets extracted we update the table with the new symbols.
+ if (ShouldExtract)
+ Syms.insert(std::begin(TmpSyms), std::end(TmpSyms));
+
return ShouldExtract;
}
/// Scan the symbols from an ObjectFile \p Obj and record if we need to extract
/// any symbols from it.
Expected<bool> getSymbolsFromObject(const ObjectFile &Obj, OffloadKind Kind,
- StringSaver &Saver,
+ bool IsArchive, StringSaver &Saver,
DenseMap<StringRef, Symbol> &Syms) {
- bool ShouldExtract = false;
+ bool ShouldExtract = !IsArchive;
+ DenseMap<StringRef, Symbol> TmpSyms;
for (SymbolRef Sym : Obj.symbols()) {
auto FlagsOrErr = Sym.getFlags();
if (!FlagsOrErr)
@@ -1223,7 +1232,7 @@ Expected<bool> getSymbolsFromObject(const ObjectFile &Obj, OffloadKind Kind,
return NameOrErr.takeError();
bool NewSymbol = Syms.count(*NameOrErr) == 0;
- auto &OldSym = Syms[Saver.save(*NameOrErr)];
+ auto OldSym = NewSymbol ? Sym_None : Syms[*NameOrErr];
// We will extract if it defines a currenlty undefined non-weak symbol.
bool ResolvesStrongReference = (OldSym & Sym_Undefined) &&
@@ -1240,12 +1249,19 @@ Expected<bool> getSymbolsFromObject(const ObjectFile &Obj, OffloadKind Kind,
// Update this symbol in the "table" with the new information.
if (OldSym & Sym_Undefined && !(*FlagsOrErr & SymbolRef::SF_Undefined))
- OldSym = static_cast<Symbol>(OldSym & ~Sym_Undefined);
+ TmpSyms[Saver.save(*NameOrErr)] =
+ static_cast<Symbol>(OldSym & ~Sym_Undefined);
if (*FlagsOrErr & SymbolRef::SF_Undefined && NewSymbol)
- OldSym = static_cast<Symbol>(OldSym | Sym_Undefined);
+ TmpSyms[Saver.save(*NameOrErr)] =
+ static_cast<Symbol>(OldSym | Sym_Undefined);
if (*FlagsOrErr & SymbolRef::SF_Weak)
- OldSym = static_cast<Symbol>(OldSym | Sym_Weak);
+ TmpSyms[Saver.save(*NameOrErr)] = static_cast<Symbol>(OldSym | Sym_Weak);
}
+
+ // If the file gets extracted we update the table with the new symbols.
+ if (ShouldExtract)
+ Syms.insert(std::begin(TmpSyms), std::end(TmpSyms));
+
return ShouldExtract;
}
@@ -1255,18 +1271,19 @@ Expected<bool> getSymbolsFromObject(const ObjectFile &Obj, OffloadKind Kind,
/// 1) It defines an undefined symbol in a regular object filie.
/// 2) It defines a global symbol without hidden visibility that has not
/// yet been defined.
-Expected<bool> getSymbols(StringRef Image, OffloadKind Kind, StringSaver &Saver,
+Expected<bool> getSymbols(StringRef Image, OffloadKind Kind, bool IsArchive,
+ StringSaver &Saver,
DenseMap<StringRef, Symbol> &Syms) {
MemoryBufferRef Buffer = MemoryBufferRef(Image, "");
switch (identify_magic(Image)) {
case file_magic::bitcode:
- return getSymbolsFromBitcode(Buffer, Kind, Saver, Syms);
+ return getSymbolsFromBitcode(Buffer, Kind, IsArchive, Saver, Syms);
case file_magic::elf_relocatable: {
Expected<std::unique_ptr<ObjectFile>> ObjFile =
ObjectFile::createObjectFile(Buffer);
if (!ObjFile)
return ObjFile.takeError();
- return getSymbolsFromObject(**ObjFile, Kind, Saver, Syms);
+ return getSymbolsFromObject(**ObjFile, Kind, IsArchive, Saver, Syms);
}
default:
return false;
@@ -1341,13 +1358,14 @@ Expected<SmallVector<OffloadFile>> getDeviceInput(const ArgList &Args) {
if (IsArchive && !WholeArchive && !Syms.count(Binary))
continue;
- Expected<bool> ExtractOrErr = getSymbols(
- Binary.getBinary()->getImage(),
- Binary.getBinary()->getOffloadKind(), Saver, Syms[Binary]);
+ Expected<bool> ExtractOrErr =
+ getSymbols(Binary.getBinary()->getImage(),
+ Binary.getBinary()->getOffloadKind(), IsArchive, Saver,
+ Syms[Binary]);
if (!ExtractOrErr)
return ExtractOrErr.takeError();
- Extracted = IsArchive && !WholeArchive && *ExtractOrErr;
+ Extracted = !WholeArchive && *ExtractOrErr;
if (!IsArchive || WholeArchive || Extracted)
InputFiles.emplace_back(std::move(Binary));
More information about the cfe-commits
mailing list