Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LLD][COFF] Support anti-dependency symbols #112542

Merged
merged 1 commit into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lld/COFF/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ StringRef LinkerDriver::mangleMaybe(Symbol *s) {
// If we find a similar mangled symbol, make this an alias to it and return
// its name.
log(unmangled->getName() + " aliased to " + mangled->getName());
unmangled->weakAlias = ctx.symtab.addUndefined(mangled->getName());
unmangled->setWeakAlias(ctx.symtab.addUndefined(mangled->getName()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the weakAlias member hasn't been made private here, I presume, and we're passing the default of false to the antiDep parameter, I guess these changes strictly are unnecessary? It's of course nice for consistency to add them, but if working on the codebase going forward, there's no guarantee that new calls will use this form, as they may just as well just assign to weakAlias?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make it private, but it will need a few more changes. As getWeakAlias follows the chain, it's not a replacement for accessing weakAlias directly. I will create a follow-up, thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if that's really necessary - I was just checking I understood the situation correctly :-)

return mangled->getName();
}

Expand Down Expand Up @@ -2520,7 +2520,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
continue;
if (auto *u = dyn_cast<Undefined>(sym))
if (!u->weakAlias)
u->weakAlias = ctx.symtab.addUndefined(to);
u->setWeakAlias(ctx.symtab.addUndefined(to));
}

// If any inputs are bitcode files, the LTO code generator may create
Expand Down
39 changes: 24 additions & 15 deletions lld/COFF/InputFiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,25 @@ std::string lld::toString(const coff::InputFile *file) {
/// If Source is Undefined and has no weak alias set, makes it a weak
/// alias to Target.
static void checkAndSetWeakAlias(COFFLinkerContext &ctx, InputFile *f,
Symbol *source, Symbol *target) {
Symbol *source, Symbol *target,
bool isAntiDep) {
if (auto *u = dyn_cast<Undefined>(source)) {
if (u->weakAlias && u->weakAlias != target) {
// Weak aliases as produced by GCC are named in the form
// .weak.<weaksymbol>.<othersymbol>, where <othersymbol> is the name
// of another symbol emitted near the weak symbol.
// Just use the definition from the first object file that defined
// this weak symbol.
if (ctx.config.allowDuplicateWeak)
// Ignore duplicated anti-dependency symbols.
if (isAntiDep)
return;
ctx.symtab.reportDuplicate(source, f);
if (!u->isAntiDep) {
// Weak aliases as produced by GCC are named in the form
// .weak.<weaksymbol>.<othersymbol>, where <othersymbol> is the name
// of another symbol emitted near the weak symbol.
// Just use the definition from the first object file that defined
// this weak symbol.
if (ctx.config.allowDuplicateWeak)
return;
ctx.symtab.reportDuplicate(source, f);
}
}
u->weakAlias = target;
u->setWeakAlias(target, isAntiDep);
}
}

Expand Down Expand Up @@ -436,7 +442,8 @@ void ObjFile::initializeSymbols() {
uint32_t numSymbols = coffObj->getNumberOfSymbols();
symbols.resize(numSymbols);

SmallVector<std::pair<Symbol *, uint32_t>, 8> weakAliases;
SmallVector<std::pair<Symbol *, const coff_aux_weak_external *>, 8>
weakAliases;
std::vector<uint32_t> pendingIndexes;
pendingIndexes.reserve(numSymbols);

Expand All @@ -451,8 +458,8 @@ void ObjFile::initializeSymbols() {
symbols[i] = createUndefined(coffSym);
} else if (coffSym.isWeakExternal()) {
symbols[i] = createUndefined(coffSym);
uint32_t tagIndex = coffSym.getAux<coff_aux_weak_external>()->TagIndex;
weakAliases.emplace_back(symbols[i], tagIndex);
weakAliases.emplace_back(symbols[i],
coffSym.getAux<coff_aux_weak_external>());
} else if (std::optional<Symbol *> optSym =
createDefined(coffSym, comdatDefs, prevailingComdat)) {
symbols[i] = *optSym;
Expand Down Expand Up @@ -491,8 +498,10 @@ void ObjFile::initializeSymbols() {

for (auto &kv : weakAliases) {
Symbol *sym = kv.first;
uint32_t idx = kv.second;
checkAndSetWeakAlias(ctx, this, sym, symbols[idx]);
const coff_aux_weak_external *aux = kv.second;
checkAndSetWeakAlias(ctx, this, sym, symbols[aux->TagIndex],
aux->Characteristics ==
IMAGE_WEAK_EXTERN_ANTI_DEPENDENCY);
}

// Free the memory used by sparseChunks now that symbol loading is finished.
Expand Down Expand Up @@ -1202,7 +1211,7 @@ void BitcodeFile::parse() {
sym = ctx.symtab.addUndefined(symName, this, true);
std::string fallback = std::string(objSym.getCOFFWeakExternalFallback());
Symbol *alias = ctx.symtab.addUndefined(saver.save(fallback));
checkAndSetWeakAlias(ctx, this, sym, alias);
checkAndSetWeakAlias(ctx, this, sym, alias, false);
} else if (comdatIndex != -1) {
if (symName == obj->getComdatTable()[comdatIndex].first) {
sym = comdat[comdatIndex].first;
Expand Down
2 changes: 1 addition & 1 deletion lld/COFF/SymbolTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ void SymbolTable::loadMinGWSymbols() {
warn("Resolving " + origName + " by linking to " + newName);
else
log("Resolving " + origName + " by linking to " + newName);
undef->weakAlias = l;
undef->setWeakAlias(l);
continue;
}
}
Expand Down
5 changes: 5 additions & 0 deletions lld/COFF/Symbols.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ Symbol *Undefined::getWeakAlias() {
// A weak alias may be a weak alias to another symbol, so check recursively.
DenseSet<Symbol *> weakChain;
for (Symbol *a = weakAlias; a; a = cast<Undefined>(a)->weakAlias) {
// Anti-dependency symbols can't be chained.
if (a->isAntiDep)
break;
if (!isa<Undefined>(a))
return a;
if (!weakChain.insert(a).second)
Expand All @@ -135,6 +138,7 @@ bool Undefined::resolveWeakAlias() {
// Symbols. For that reason we need to check which type of symbol we
// are dealing with and copy the correct number of bytes.
StringRef name = getName();
bool wasAntiDep = isAntiDep;
if (isa<DefinedRegular>(d))
memcpy(this, d, sizeof(DefinedRegular));
else if (isa<DefinedAbsolute>(d))
Expand All @@ -144,6 +148,7 @@ bool Undefined::resolveWeakAlias() {

nameData = name.data();
nameSize = name.size();
isAntiDep = wasAntiDep;
return true;
}

Expand Down
10 changes: 9 additions & 1 deletion lld/COFF/Symbols.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class Symbol {
: symbolKind(k), isExternal(true), isCOMDAT(false),
writtenToSymtab(false), isUsedInRegularObj(false),
pendingArchiveLoad(false), isGCRoot(false), isRuntimePseudoReloc(false),
deferUndefined(false), canInline(true), isWeak(false),
deferUndefined(false), canInline(true), isWeak(false), isAntiDep(false),
nameSize(n.size()), nameData(n.empty() ? nullptr : n.data()) {
assert((!n.empty() || k <= LastDefinedCOFFKind) &&
"If the name is empty, the Symbol must be a DefinedCOFF.");
Expand Down Expand Up @@ -145,6 +145,9 @@ class Symbol {
// managing weak symbol overrides.
unsigned isWeak : 1;

// True if the symbol is an anti-dependency.
unsigned isAntiDep : 1;

protected:
// Symbol name length. Assume symbol lengths fit in a 32-bit integer.
uint32_t nameSize;
Expand Down Expand Up @@ -345,6 +348,11 @@ class Undefined : public Symbol {
return dyn_cast_or_null<Defined>(getWeakAlias());
}

void setWeakAlias(Symbol *sym, bool antiDep = false) {
weakAlias = sym;
isAntiDep = antiDep;
}

// If this symbol is external weak, replace this object with aliased symbol.
bool resolveWeakAlias();
};
Expand Down
68 changes: 68 additions & 0 deletions lld/test/COFF/weak-antidep-chain.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
REQUIRES: x86
RUN: split-file %s %t.dir && cd %t.dir

RUN: llvm-mc -filetype=obj -triple=x86_64-windows chain-bad.s -o chain-bad.obj
RUN: llvm-mc -filetype=obj -triple=x86_64-windows chain-bad2.s -o chain-bad2.obj
RUN: llvm-mc -filetype=obj -triple=x86_64-windows globals-bad.s -o globals-bad.obj
RUN: llvm-mc -filetype=obj -triple=x86_64-windows chain-good.s -o chain-good.obj
RUN: llvm-mc -filetype=obj -triple=x86_64-windows chain-good2.s -o chain-good2.obj
RUN: llvm-mc -filetype=obj -triple=x86_64-windows globals-good.s -o globals-good.obj

Temporary anti-dependency chains are allowed as long as they are broken by non-alias symbols.

RUN: lld-link -machine:amd64 -dll -noentry -out:test.dll chain-good.obj globals-good.obj
RUN: lld-link -machine:amd64 -dll -noentry -out:test.dll chain-good2.obj globals-good.obj

Chaining of anti-dependency symbols is not allowed.

RUN: not lld-link -machine:amd64 -dll -noentry -out:test.dll chain-bad.obj globals-bad.obj 2>&1 \
RUN: | FileCheck -check-prefix=ANTIDEP %s
RUN: not lld-link -machine:amd64 -dll -noentry -out:test.dll chain-bad2.obj globals-bad.obj 2>&1 \
RUN: | FileCheck -check-prefix=ANTIDEP %s

ANTIDEP: lld-link: error: undefined symbol: sym
ANTIDEP-NEXT: >>> referenced by chain-bad

#--- chain-bad.s
.weak_anti_dep sym
.set sym,sym2
.weak_anti_dep sym2
.set sym2,sym3

#--- chain-bad2.s
.weak_anti_dep sym2
.set sym2,sym3
.weak sym
.set sym,sym2

#--- globals-bad.s
.section .test,"r"
.global sym3
.set sym3,3

#--- chain-good.s
.weak_anti_dep sym
.set sym,sym2
.weak_anti_dep sym2
.set sym2,sym3
.weak_anti_dep sym3
.set sym3,sym4
.weak_anti_dep sym4

#--- chain-good2.s
.weak_anti_dep sym
.set sym,sym2
.weak_anti_dep sym2
.set sym2,sym3
.weak_anti_dep sym3
.set sym3,weak_sym
.weak weak_sym
.set weak_sym,sym4
.weak_anti_dep sym4

#--- globals-good.s
.section .test,"r"
.global sym2
.set sym2,2
.global sym4
.set sym4,4
54 changes: 54 additions & 0 deletions lld/test/COFF/weak-antidep.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
REQUIRES: x86
RUN: split-file %s %t.dir && cd %t.dir

RUN: llvm-mc -filetype=obj -triple=x86_64-windows antidep.s -o antidep.obj
RUN: llvm-mc -filetype=obj -triple=x86_64-windows antidep2.s -o antidep2.obj
RUN: llvm-mc -filetype=obj -triple=x86_64-windows weak.s -o weak.obj
RUN: llvm-mc -filetype=obj -triple=x86_64-windows test.s -o test.obj

Check that a regular weak alias overrides an anti-dependency symbol.

RUN: lld-link -dll -noentry -out:out1.dll antidep.obj weak.obj test.obj
RUN: llvm-readobj --hex-dump=.test out1.dll | FileCheck --check-prefix=CHECK2 %s

RUN: lld-link -dll -noentry -out:out2.dll weak.obj antidep.obj test.obj
RUN: llvm-readobj --hex-dump=.test out2.dll | FileCheck --check-prefix=CHECK2 %s

RUN: lld-link -dll -noentry -out:out3.dll antidep.obj weak.obj test.obj -lld-allow-duplicate-weak
RUN: llvm-readobj --hex-dump=.test out3.dll | FileCheck --check-prefix=CHECK2 %s

RUN: lld-link -dll -noentry -out:out4.dll weak.obj antidep.obj test.obj -lld-allow-duplicate-weak
RUN: llvm-readobj --hex-dump=.test out4.dll | FileCheck --check-prefix=CHECK2 %s

When an anti-dependency symbol is duplicated, the first definition takes precedence over subsequent ones.

RUN: lld-link -dll -noentry -out:out5.dll antidep.obj antidep2.obj test.obj
RUN: llvm-readobj --hex-dump=.test out5.dll | FileCheck --check-prefix=CHECK1 %s

RUN: lld-link -dll -noentry -out:out6.dll antidep2.obj antidep.obj test.obj
RUN: llvm-readobj --hex-dump=.test out6.dll | FileCheck --check-prefix=CHECK2 %s

CHECK1: 01000000
CHECK2: 02000000

#--- antidep.s
.weak_anti_dep sym
.set sym,target1

#--- antidep2.s
.weak_anti_dep sym
.set sym,target2

#--- weak.s
.weak sym
.set sym,target2

#--- test.s
.section .target,"dr"
.globl target1
.set target1,1
.globl target2
.set target2,2

.section .test,"dr"
.long sym
Loading