[NFCI][ELF] Introduce explicit Computed state for DynamicReloc #150799
Conversation
Created using spr 1.3.5 [skip ci]
Created using spr 1.3.5
|
@llvm/pr-subscribers-lld-elf Author: Jessica Clarke (jrtc27) ChangesCurrently we set the kind to AddendOnly in computeRaw() in order to By introducing a new (internal) Computed kind we can be explicit and add Full diff: https://github.com/llvm/llvm-project/pull/150799.diff 2 Files Affected:
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index efec41a737b62..16f1ab36a88b1 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -1647,6 +1647,8 @@ uint64_t DynamicReloc::getOffset() const {
int64_t DynamicReloc::computeAddend(Ctx &ctx) const {
switch (kind) {
+ case Computed:
+ llvm_unreachable("addend already computed");
case AddendOnly:
assert(sym == nullptr);
return addend;
@@ -1748,7 +1750,7 @@ void DynamicReloc::computeRaw(Ctx &ctx, SymbolTableBaseSection *symt) {
r_offset = getOffset();
r_sym = getSymIndex(symt);
addend = computeAddend(ctx);
- kind = AddendOnly; // Catch errors
+ kind = Computed; // Catch errors
}
void RelocationBaseSection::computeRels() {
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 5f01513630597..8e069e3dd9565 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -419,6 +419,9 @@ class StringTableSection final : public SyntheticSection {
class DynamicReloc {
public:
enum Kind {
+ /// The resulting dynamic relocation has already had its addend computed.
+ /// Calling computeAddend() is an error. Only for internal use.
+ Computed,
/// The resulting dynamic relocation does not reference a symbol (#sym must
/// be nullptr) and uses #addend as the result of computeAddend(ctx).
AddendOnly,
@@ -461,6 +464,7 @@ class DynamicReloc {
uint64_t getOffset() const;
uint32_t getSymIndex(SymbolTableBaseSection *symTab) const;
bool needsDynSymIndex() const {
+ assert(kind != Computed && "cannot check kind after computeRaw");
return kind == AgainstSymbol || kind == AgainstSymbolWithTargetVA;
}
|
|
@llvm/pr-subscribers-lld Author: Jessica Clarke (jrtc27) ChangesCurrently we set the kind to AddendOnly in computeRaw() in order to By introducing a new (internal) Computed kind we can be explicit and add Full diff: https://github.com/llvm/llvm-project/pull/150799.diff 2 Files Affected:
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index efec41a737b62..16f1ab36a88b1 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -1647,6 +1647,8 @@ uint64_t DynamicReloc::getOffset() const {
int64_t DynamicReloc::computeAddend(Ctx &ctx) const {
switch (kind) {
+ case Computed:
+ llvm_unreachable("addend already computed");
case AddendOnly:
assert(sym == nullptr);
return addend;
@@ -1748,7 +1750,7 @@ void DynamicReloc::computeRaw(Ctx &ctx, SymbolTableBaseSection *symt) {
r_offset = getOffset();
r_sym = getSymIndex(symt);
addend = computeAddend(ctx);
- kind = AddendOnly; // Catch errors
+ kind = Computed; // Catch errors
}
void RelocationBaseSection::computeRels() {
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 5f01513630597..8e069e3dd9565 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -419,6 +419,9 @@ class StringTableSection final : public SyntheticSection {
class DynamicReloc {
public:
enum Kind {
+ /// The resulting dynamic relocation has already had its addend computed.
+ /// Calling computeAddend() is an error. Only for internal use.
+ Computed,
/// The resulting dynamic relocation does not reference a symbol (#sym must
/// be nullptr) and uses #addend as the result of computeAddend(ctx).
AddendOnly,
@@ -461,6 +464,7 @@ class DynamicReloc {
uint64_t getOffset() const;
uint32_t getSymIndex(SymbolTableBaseSection *symTab) const;
bool needsDynSymIndex() const {
+ assert(kind != Computed && "cannot check kind after computeRaw");
return kind == AgainstSymbol || kind == AgainstSymbolWithTargetVA;
}
|
|
NB: This is a transient state, #150812 removes this member. This makes the history a bit odd, but I think it makes it clearer what's going on to include this in the history. |
Created using spr 1.3.5
jrtc27
changed the base branch from
users/jrtc27/spr/main.nfcielf-introduce-explicit-computed-state-for-dynamicreloc
to
main
jrtc27
requested review from
cyndyishida,
aaupov,
maksfb,
rafaelauler,
ayermolo,
yota9,
paschalis-mpeis,
JDevlieghere,
lanza,
bcardosolopes,
xlauko,
andykaylor,
Endilll,
nikic and
a team
as code owners
jrtc27
deleted the
users/jrtc27/spr/nfcielf-introduce-explicit-computed-state-for-dynamicreloc
branch
…eloc Currently we set the kind to AddendOnly in computeRaw() in order to catch cases where we're not treating the DynamicReloc as computed. Specifically, computeAddend() will then assert that sym is nullptr, so can catch any subsequent calls for relocations that have sym set. However, if the DynamicReloc was already AddendOnly (or MipsMultiGotPage), we will silently allow this, which does work correctly, but is not the intended use. We also cannot catch cases where needsDynSymIndex() is called after this point, which would give a misleading value if the kind were previously against a symbol. By introducing a new (internal) Computed kind we can be explicit and add more rigorous assertions, rather than abusing AddendOnly. Reviewers: arichardson, MaskRay Reviewed By: arichardson, MaskRay Pull Request: llvm/llvm-project#150799
Currently we set the kind to AddendOnly in computeRaw() in order to
catch cases where we're not treating the DynamicReloc as computed.
Specifically, computeAddend() will then assert that sym is nullptr, so
can catch any subsequent calls for relocations that have sym set.
However, if the DynamicReloc was already AddendOnly (or
MipsMultiGotPage), we will silently allow this, which does work
correctly, but is not the intended use. We also cannot catch cases where
needsDynSymIndex() is called after this point, which would give a
misleading value if the kind were previously against a symbol.
By introducing a new (internal) Computed kind we can be explicit and add
more rigorous assertions, rather than abusing AddendOnly.