Skip to content

Commit 079b657

Browse files
committed
Fix evaluation of expressions containing forward references
This commit fixes evaluation of expressions containing forward references. At a high-level, eld evaluates assignments in sequence and consequently the expressions containing forward references needs to be reevaluated when the forward reference value is computed for the correct computation results. For example: u = v1; // Assignment 1 v1 = v2; // Assignment 2 v2 = 0xa; // Assignment 3 eld evaluates the assignments in order, that is, the evaluation happens as: [Assignment 1, Assignment 2, Assignment 3]. If we follow this evaluation order, the symbols `u` and `v1` will have incorrect values because the value of `v2` is unknown when the assignments of `u` and `v1` are evaluated. This commit fixes evaluation of expressions containing forward references by making the below two changes: 1) Assignments which cannot be completely evaluated during the sequential evaluation of expressions are marked as pending assignments. After the layout is done, but before the relaxation, these pending assignments are recursively reevaluated until there is no improvement in pending assignments or a MaxIteration limit is reached. 2) During a layout iteration, assignments may get evaluated multiple times as layout needs to reset itself based on a few conditions (new segment needs to be created, and so on...). It is important to reset the assignments and the symbol values if a layout gets reset. Let's see why it is important with the help of an example: SECTIONS { FOO : { u = v; // Assignment 1 v = 0xa; // Assignment 2 *(.text.foo) } BAR : { v = 0xb; // Assignment 3 } v = 0xc; // Assignment 4 } The sequential assignment evaluation order is: [A1, A2, A3, A4]. When A1 is evaluated, `v` is not defined, hence we mark A1 as pending assignment. However, if the layout gets reset after evaluating A2, then A1 will be evaluated again, but this time, `v` is defined (from assignment 2 evaluation) and thus A1 can be completely evaluated. This is wrong because A1 should get the value from the assignment 4 instead of assignment 2! We fix this issue by resetting the assignments and the symbol values whenever a layout is reset. The same issue happens when the layout needs to be recomputed after a relaxation pass. And the same solution of resetting the assignments and the symbol values works for this case as well. This commit also adds a new trace category 'pending-assignments' for tracing pending assignment evaluation. Closes #169 Signed-off-by: Parth Arora <[email protected]>
1 parent fc7860b commit 079b657

24 files changed

+654
-169
lines changed

include/eld/Diagnostics/DiagVerbose.inc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,3 +163,9 @@ DIAG(verbose_linker_script_output_file, DiagnosticEngine::Verbose,
163163
"Using output file name '%0' specified in the linker script '%1'")
164164
DIAG(verbose_rule_matching_cache_func_hash, DiagnosticEngine::Verbose,
165165
"Rule-matching cache-functionailty hash: %0")
166+
DIAG(verbose_performing_layout_iteration, DiagnosticEngine::Verbose,
167+
"Performing layout iteration %0")
168+
DIAG(verbose_eval_pending_assignments, DiagnosticEngine::Verbose,
169+
"Evaluating pending assignments")
170+
DIAG(verbose_reset_tracked_assignment, DiagnosticEngine::Verbose,
171+
"Resetting tracked assignments")

include/eld/Diagnostics/DiagnosticPrinter.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class DiagnosticPrinter {
4747
TraceMergeStrings = 0x8000,
4848
TraceLinkerScript = 0x10000,
4949
TraceSymDef = 0x100000,
50+
TracePendingAssignments = 0x200000
5051
};
5152

5253
enum Verbose : uint16_t { None = 0x0, Default = 0x1 };
@@ -68,6 +69,8 @@ class DiagnosticPrinter {
6869

6970
bool traceAssignments() { return Trace & TraceAssignments; }
7071

72+
bool tracePendingAssignments() { return Trace & TracePendingAssignments; }
73+
7174
bool traceFiles() { return Trace & TraceFiles; }
7275

7376
bool traceSymbols() { return Trace & TraceSymbols; }

include/eld/Driver/GnuLinkerOptions.td

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,8 @@ defm trace
658658
"\t\t\t --trace=trampolines : trace trampolines\n"
659659
"\t\t\t --trace=wrap-symbols : trace symbol wrap options\n"
660660
"\t\t\t --trace=symdef : trace symbol resolution from symdef files\n"
661-
"\t\t\t --trace=dynamic-linking : trace dynamic linking">,
661+
"\t\t\t --trace=dynamic-linking : trace dynamic linking\n"
662+
"\t\t\t --trace=pending-assignments : trace pending symbol assignments evaluation">,
662663
MetaVarName<"<trace-type>">,
663664
Group<grp_diagopts>;
664665
defm trace_symbol : smDashTwoWithOpt<"y", "trace-symbol", "trace_symbol",

include/eld/Script/Assignment.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ class Assignment : public ScriptCommand {
8383
eld::Expected<void> activate(Module &CurModule) override;
8484

8585
/// assign - evaluate the rhs and assign the result to lhs.
86-
bool assign(Module &CurModule, const ELFSection *Section);
86+
bool assign(Module &CurModule, const ELFSection *Section,
87+
bool EvaluatePendingOnly);
8788

8889
LDSymbol *symbol() const { return ThisSymbol; }
8990

@@ -128,6 +129,9 @@ class Assignment : public ScriptCommand {
128129

129130
bool isUsed() const { return IsUsed; }
130131

132+
/// Reset the assignment value and the expression node.
133+
void reset();
134+
131135
private:
132136
bool checkLinkerScript(Module &CurModule);
133137

include/eld/Script/Expression.h

Lines changed: 115 additions & 47 deletions
Large diffs are not rendered by default.

include/eld/Target/GNULDBackend.h

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,28 @@ class GNULDBackend {
826826

827827
const ResolveInfo *findAbsolutePLT(ResolveInfo *I) const;
828828

829+
/// Returns true if the symbol is partially evaluated. That
830+
/// is, the symbol assignment expression was partially evaluated.
831+
bool isPartiallyEvaluated(const ResolveInfo *RI) const {
832+
return PartiallyEvaluatedSymbols.count(RI);
833+
}
834+
835+
void addPartiallyEvaluatedSymbol(const ResolveInfo *RI) {
836+
PartiallyEvaluatedSymbols.insert(RI);
837+
}
838+
839+
void removePartiallyEvaluatedSymbol(const ResolveInfo *RI) {
840+
PartiallyEvaluatedSymbols.erase(RI);
841+
}
842+
843+
void resetPartiallyEvalAssignsAndSymbols() {
844+
PartiallyEvaluatedAssignments.clear();
845+
PartiallyEvaluatedSymbols.clear();
846+
}
847+
848+
/// Recursively evaluates the partially evaluated assignments.
849+
void evaluatePendingAssignments();
850+
829851
protected:
830852
virtual int numReservedSegments() const { return m_NumReservedSegments; }
831853

@@ -1000,6 +1022,34 @@ class GNULDBackend {
10001022
// Setup TLS alignment and check for any layout issues
10011023
bool setupTLS();
10021024

1025+
/// Evaluates the assignment and tracks the partially evaluated assignments.
1026+
/// Partially evaluated assignments can be requested to be (recursively)
1027+
/// reevaluated using evaluatePendingAssignments.
1028+
void evaluateAssignmentAndTrackPartiallyEvalAssignments(Assignment &A,
1029+
const ELFSection *S);
1030+
1031+
/// Reset the tracked assignments and the TrackedAssignments vector.
1032+
/// Resetting an assignment resets the assignment node and all the
1033+
/// expression nodes contained in the assignment.
1034+
void resetTrackedAssignments();
1035+
1036+
/// Stores the snapshot of linker script symbols.
1037+
void takeLSSymbolsSnapshot();
1038+
1039+
/// Restores the value of linker script symbols using the linker script symbol
1040+
/// snapshot stored using takeLSSymbolsSnapshot.
1041+
void restoreLSSymbolsUsingSnapshot();
1042+
1043+
void takePartiallyEvalAssignsAndSymbolsSnapshot() {
1044+
PartiallyEvaluatedAssignmentsSnapshot = PartiallyEvaluatedAssignments;
1045+
PartiallyEvaluatedSymbolsSnapshot = PartiallyEvaluatedSymbols;
1046+
}
1047+
1048+
void restorePartiallyEvalAssignsAndSymbolsUsingSnapshot() {
1049+
PartiallyEvaluatedAssignments = PartiallyEvaluatedAssignmentsSnapshot;
1050+
PartiallyEvaluatedSymbols = PartiallyEvaluatedSymbolsSnapshot;
1051+
}
1052+
10031053
protected:
10041054
Module &m_Module;
10051055

@@ -1130,6 +1180,30 @@ class GNULDBackend {
11301180
bool m_NeedEhdr = false;
11311181

11321182
bool m_NeedPhdr = false;
1183+
1184+
std::unordered_set<const ResolveInfo *> PartiallyEvaluatedSymbols;
1185+
1186+
/// Stores the assignments which needs to be reevaluated later.
1187+
std::vector<std::pair<Assignment *, const ELFSection *>>
1188+
PartiallyEvaluatedAssignments;
1189+
1190+
std::unordered_set<const ResolveInfo *> PartiallyEvaluatedSymbolsSnapshot;
1191+
1192+
/// Stores the assignments which needs to be reevaluated later.
1193+
std::vector<std::pair<Assignment *, const ELFSection *>>
1194+
PartiallyEvaluatedAssignmentsSnapshot;
1195+
1196+
/// TrackedAssignments stores the assignments which gets evaluated if
1197+
/// TrackAssignments is true. It is used to reset the evaluated assignments.
1198+
bool TrackAssignments = false;
1199+
std::vector<Assignment *> TrackedAssignments;
1200+
1201+
struct LSSymbolInfo {
1202+
ResolveInfo *RI = nullptr;
1203+
uint64_t Value = 0;
1204+
bool HasScriptValue = false;
1205+
};
1206+
std::vector<LSSymbolInfo> LSSymbolsSnapshot;
11331207
};
11341208

11351209
} // namespace eld

lib/Config/GeneralOptions.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,8 @@ eld::Expected<void> GeneralOptions::setTrace(const char *PTraceType) {
230230
DiagEngine->getPrinter()->TraceDynamicLinking)
231231
.Case("linker-script", DiagEngine->getPrinter()->TraceLinkerScript)
232232
.Case("symdef", DiagEngine->getPrinter()->TraceSymDef)
233+
.Case("pending-assignments",
234+
DiagEngine->getPrinter()->TracePendingAssignments)
233235
.Default(std::nullopt);
234236
}
235237
// Warn if trace category is unknown.

lib/LinkerWrapper/LinkerWrapper.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,9 @@ eld::Expected<void> LinkerWrapper::finishAssignOutputSections() {
217217
eld::Expected<void> LinkerWrapper::reassignVirtualAddresses() {
218218
if (getState() != State::CreatingSegments)
219219
RETURN_INVALID_LINK_STATE_ERR("CreatingSegments");
220-
m_Module.getBackend().createScriptProgramHdrs();
220+
auto &Backend = m_Module.getBackend();
221+
Backend.createScriptProgramHdrs();
222+
Backend.evaluatePendingAssignments();
221223
return {};
222224
}
223225

lib/Object/ObjectLinker.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,10 @@ bool ObjectLinker::createOutputSection(ObjectBuilder &Builder,
892892

893893
// force input alignment from ldscript if any
894894
if (Output->prolog().hasSubAlign()) {
895-
Output->prolog().subAlign().eval();
895+
// FIXME: eval() is an implementation function, it should not be
896+
// used outside the component. Instead, evaluateAndReturnError and
897+
// evaluateAndRaiseError should be used.
898+
Output->prolog().subAlign().eval(/*EvaluatePendingOnly=*/false);
896899
Output->prolog().subAlign().commit();
897900
InAlign = Output->prolog().subAlign().result();
898901
HasSubAlign = true;

lib/Script/Assignment.cpp

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "eld/Readers/Section.h"
2020
#include "eld/Script/Expression.h"
2121
#include "eld/SymbolResolver/LDSymbol.h"
22+
#include "eld/Target/GNULDBackend.h"
2223
#include "llvm/Support/Casting.h"
2324
#include <cassert>
2425

@@ -205,7 +206,8 @@ void Assignment::getSymbols(std::vector<ResolveInfo *> &Symbols) const {
205206
ExpressionToEvaluate->getSymbols(Symbols);
206207
}
207208

208-
bool Assignment::assign(Module &CurModule, const ELFSection *Section) {
209+
bool Assignment::assign(Module &CurModule, const ELFSection *Section,
210+
bool EvaluatePendingOnly) {
209211

210212
if (Section && !Section->isAlloc() && isDot()) {
211213
CurModule.getConfig().raise(Diag::error_dot_lhs_in_non_alloc)
@@ -215,7 +217,11 @@ bool Assignment::assign(Module &CurModule, const ELFSection *Section) {
215217
}
216218

217219
// evaluate, commit, then get the result of the expression
218-
auto Result = ExpressionToEvaluate->evaluateAndRaiseError();
220+
std::optional<uint64_t> Result;
221+
if (EvaluatePendingOnly)
222+
Result = ExpressionToEvaluate->evaluatePendingAndRaiseError();
223+
else
224+
Result = ExpressionToEvaluate->evaluateAndRaiseError();
219225
if (!Result)
220226
return false;
221227
ExpressionValue = *Result;
@@ -224,13 +230,23 @@ bool Assignment::assign(Module &CurModule, const ELFSection *Section) {
224230
return false;
225231

226232
LDSymbol *Sym = CurModule.getNamePool().findSymbol(Name);
233+
// ASSERT(Sym, "Sym must exist!");
234+
// FIXME: Why is it okay for the Sym to be nullptr?
227235
if (Sym != nullptr) {
228236
ThisSymbol = Sym;
229237
ThisSymbol->setValue(ExpressionValue);
230238
ThisSymbol->setScriptValueDefined();
239+
GNULDBackend &Backend = CurModule.getBackend();
240+
const ResolveInfo *RI = ThisSymbol->resolveInfo();
241+
if (ExpressionToEvaluate->hasPendingEvaluation())
242+
Backend.addPartiallyEvaluatedSymbol(RI);
243+
else
244+
Backend.removePartiallyEvaluatedSymbol(RI);
231245
}
232246

233-
if (CurModule.getPrinter()->traceAssignments())
247+
auto DP = CurModule.getPrinter();
248+
if (DP->traceAssignments() ||
249+
(EvaluatePendingOnly && DP->tracePendingAssignments()))
234250
trace(llvm::outs());
235251
return true;
236252
}
@@ -276,3 +292,10 @@ std::unordered_set<std::string> Assignment::getSymbolNames() const {
276292
ExpressionToEvaluate->getSymbolNames(SymbolNames);
277293
return SymbolNames;
278294
}
295+
296+
void Assignment::reset() {
297+
ExpressionValue = 0;
298+
ThisSymbol = nullptr;
299+
if (ExpressionToEvaluate)
300+
ExpressionToEvaluate->resetRecursively();
301+
}

0 commit comments

Comments
 (0)