Skip to content

Commit f55be80

Browse files
MaskRayZijunZhaoCCK
authored andcommitted
[ELF] Respect orders of symbol assignments and DEFINED (llvm#65866)
Fix llvm#64600: the currently implementation is minimal (see https://reviews.llvm.org/D83758), and an assignment like `__TEXT_REGION_ORIGIN__ = DEFINED(__TEXT_REGION_ORIGIN__) ? __TEXT_REGION_ORIGIN__ : 0;` (used by avr-ld[1]) leads to a value of zero (default value in `declareSymbol`), which is unexpected. Assign orders to symbol assignments and references so that for a script-defined symbol, the `DEFINED` results match users' expectation. I am unclear about GNU ld's exact behavior, but this hopefully matches its behavior in the majority of cases. [1]: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=ld/scripttempl/avr.sc
1 parent 88c6506 commit f55be80

File tree

7 files changed

+74
-28
lines changed

7 files changed

+74
-28
lines changed

lld/ELF/Config.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,12 @@ struct Ctx {
476476
// True if we need to reserve two .got entries for local-dynamic TLS model.
477477
std::atomic<bool> needsTlsLd{false};
478478

479+
// Each symbol assignment and DEFINED(sym) reference is assigned an increasing
480+
// order. Each DEFINED(sym) evaluation checks whether the reference happens
481+
// before a possible `sym = expr;`.
482+
unsigned scriptSymOrderCounter = 1;
483+
llvm::DenseMap<const Symbol *, unsigned> scriptSymOrder;
484+
479485
void reset();
480486

481487
llvm::raw_fd_ostream openAuxiliaryFile(llvm::StringRef, std::error_code &);

lld/ELF/Driver.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ void Ctx::reset() {
106106
backwardReferences.clear();
107107
hasSympart.store(false, std::memory_order_relaxed);
108108
needsTlsLd.store(false, std::memory_order_relaxed);
109+
scriptSymOrderCounter = 1;
110+
scriptSymOrder.clear();
109111
}
110112

111113
llvm::raw_fd_ostream Ctx::openAuxiliaryFile(llvm::StringRef filename,

lld/ELF/LinkerScript.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,13 @@ static void declareSymbol(SymbolAssignment *cmd) {
247247
Defined newSym(nullptr, cmd->name, STB_GLOBAL, visibility, STT_NOTYPE, 0, 0,
248248
nullptr);
249249

250-
// We can't calculate final value right now.
250+
// If the symbol is already defined, its order is 0 (with absence indicating
251+
// 0); otherwise it's assigned the order of the SymbolAssignment.
251252
Symbol *sym = symtab.insert(cmd->name);
253+
if (!sym->isDefined())
254+
ctx.scriptSymOrder.insert({sym, cmd->symOrder});
255+
256+
// We can't calculate final value right now.
252257
sym->mergeProperties(newSym);
253258
newSym.overwrite(*sym);
254259

lld/ELF/LinkerScript.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,9 @@ struct SectionCommand {
8686

8787
// This represents ". = <expr>" or "<symbol> = <expr>".
8888
struct SymbolAssignment : SectionCommand {
89-
SymbolAssignment(StringRef name, Expr e, std::string loc)
89+
SymbolAssignment(StringRef name, Expr e, unsigned symOrder, std::string loc)
9090
: SectionCommand(AssignmentKind), name(name), expression(e),
91-
location(loc) {}
91+
symOrder(symOrder), location(loc) {}
9292

9393
static bool classof(const SectionCommand *c) {
9494
return c->kind == AssignmentKind;
@@ -105,6 +105,8 @@ struct SymbolAssignment : SectionCommand {
105105
bool provide = false;
106106
bool hidden = false;
107107

108+
unsigned symOrder;
109+
108110
// Holds file name and line number for error reporting.
109111
std::string location;
110112

lld/ELF/ScriptParser.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ void ScriptParser::readDefsym(StringRef name) {
291291
Expr e = readExpr();
292292
if (!atEOF())
293293
setError("EOF expected, but got " + next());
294-
SymbolAssignment *cmd = make<SymbolAssignment>(name, e, getCurrentLocation());
294+
auto *cmd = make<SymbolAssignment>(name, e, 0, getCurrentLocation());
295295
script->sectionCommands.push_back(cmd);
296296
}
297297

@@ -568,7 +568,7 @@ SmallVector<SectionCommand *, 0> ScriptParser::readOverlay() {
568568
max = std::max(max, cast<OutputDesc>(cmd)->osec.size);
569569
return addrExpr().getValue() + max;
570570
};
571-
v.push_back(make<SymbolAssignment>(".", moveDot, getCurrentLocation()));
571+
v.push_back(make<SymbolAssignment>(".", moveDot, 0, getCurrentLocation()));
572572
return v;
573573
}
574574

@@ -1047,7 +1047,7 @@ SymbolAssignment *ScriptParser::readProvideHidden(bool provide, bool hidden) {
10471047
SymbolAssignment *ScriptParser::readAssignment(StringRef tok) {
10481048
// Assert expression returns Dot, so this is equal to ".=."
10491049
if (tok == "ASSERT")
1050-
return make<SymbolAssignment>(".", readAssert(), getCurrentLocation());
1050+
return make<SymbolAssignment>(".", readAssert(), 0, getCurrentLocation());
10511051

10521052
size_t oldPos = pos;
10531053
SymbolAssignment *cmd = nullptr;
@@ -1117,7 +1117,8 @@ SymbolAssignment *ScriptParser::readSymbolAssignment(StringRef name) {
11171117
}
11181118
};
11191119
}
1120-
return make<SymbolAssignment>(name, e, getCurrentLocation());
1120+
return make<SymbolAssignment>(name, e, ctx.scriptSymOrderCounter++,
1121+
getCurrentLocation());
11211122
}
11221123

11231124
// This is an operator-precedence parser to parse a linker
@@ -1465,9 +1466,13 @@ Expr ScriptParser::readPrimary() {
14651466
}
14661467
if (tok == "DEFINED") {
14671468
StringRef name = unquote(readParenLiteral());
1469+
// Return 1 if s is defined. If the definition is only found in a linker
1470+
// script, it must happen before this DEFINED.
1471+
auto order = ctx.scriptSymOrderCounter++;
14681472
return [=] {
1469-
Symbol *b = symtab.find(name);
1470-
return (b && b->isDefined()) ? 1 : 0;
1473+
Symbol *s = symtab.find(name);
1474+
return s && s->isDefined() && ctx.scriptSymOrder.lookup(s) < order ? 1
1475+
: 0;
14711476
};
14721477
}
14731478
if (tok == "LENGTH") {

lld/test/ELF/linkerscript/define.test

Lines changed: 0 additions & 19 deletions
This file was deleted.
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# REQUIRES: x86
2+
# RUN: llvm-mc -filetype=obj -triple=x86_64 %p/Inputs/define.s -o %t.o
3+
# RUN: ld.lld -o %t --defsym vv=1 --script %s %t.o
4+
# RUN: llvm-readelf -S -s %t | FileCheck %s
5+
6+
# CHECK: [Nr] Name Type Address Off Size ES Flg Lk Inf Al
7+
# CHECK-NEXT: [ 0] NULL 0000000000000000 000000 000000 00 0 0 0
8+
# CHECK-NEXT: [ 1] .foo PROGBITS 0000000000011000 001000 000008 00 A 0 0 1
9+
# CHECK-NEXT: [ 2] .bar PROGBITS 0000000000013000 003000 000008 00 A 0 0 1
10+
# CHECK-NEXT: [ 3] .test PROGBITS 0000000000015000 005000 000008 00 A 0 0 1
11+
# CHECK-NEXT: [ 4] .text PROGBITS 0000000000015008 005008 000000 00 AX 0 0 4
12+
13+
# CHECK: Value Size Type Bind Vis Ndx Name
14+
# CHECK-DAG: 0000000000000001 0 NOTYPE GLOBAL DEFAULT ABS vv
15+
# CHECK-DAG: 0000000000000009 0 NOTYPE GLOBAL DEFAULT ABS ww
16+
# CHECK-DAG: 0000000000000001 0 NOTYPE GLOBAL DEFAULT ABS x1
17+
# CHECK-DAG: 0000000000000002 0 NOTYPE GLOBAL DEFAULT ABS x2
18+
# CHECK-DAG: 0000000000000001 0 NOTYPE GLOBAL DEFAULT ABS y1
19+
# CHECK-DAG: 0000000000000002 0 NOTYPE GLOBAL DEFAULT ABS y2
20+
21+
EXTERN(extern_defined)
22+
SECTIONS {
23+
. = DEFINED(defined) ? 0x11000 : .;
24+
.foo : { *(.foo*) }
25+
. = DEFINED(notdefined) ? 0x12000 : 0x13000;
26+
.bar : { *(.bar*) }
27+
. = DEFINED(extern_defined) ? 0x14000 : 0x15000;
28+
29+
## Take the value from --defsym.
30+
vv = DEFINED(vv) ? vv : 9;
31+
## 9 as ww is undefined.
32+
ww = DEFINED(ww) ? ww : 9;
33+
34+
## 1 as xx is not yet defined.
35+
x1 = DEFINED(xx) ? 2 : 1;
36+
.test : {
37+
xx = .;
38+
*(.test*)
39+
}
40+
x2 = DEFINED(xx) ? 2 : 1;
41+
42+
y1 = DEFINED(yy) ? 2 : 1;
43+
yy = .;
44+
y2 = DEFINED(yy) ? 2 : 1;
45+
}

0 commit comments

Comments
 (0)