[ELF][Mips] Fix addend for preemptible static TLS #150729
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.5
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-backend-mips Author: Jessica Clarke (jrtc27) ChangesIf the symbol is preemptible the addend should be 0, not our This means we can remove AgainstSymbolWithTargetVA, which is a rather Full diff: https://github.com/llvm/llvm-project/pull/150729.diff 5 Files Affected:
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index efec41a737b62..0bb00c6d2bcff 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -1065,9 +1065,8 @@ void MipsGotSection::build() {
// for the TP-relative offset as we don't know how much other data will
// be allocated before us in the static TLS block.
if (s->isPreemptible || ctx.arg.shared)
- ctx.mainPart->relaDyn->addReloc(
- {ctx.target->tlsGotRel, this, offset,
- DynamicReloc::AgainstSymbolWithTargetVA, *s, 0, R_ABS});
+ ctx.mainPart->relaDyn->addAddendOnlyRelocIfNonPreemptible(
+ ctx.target->tlsGotRel, *this, offset, *s, ctx.target->symbolicRel);
}
for (std::pair<Symbol *, size_t> &p : got.dynTlsSymbols) {
Symbol *s = p.first;
@@ -1160,6 +1159,7 @@ void MipsGotSection::writeTo(uint8_t *buf) {
// if we had to do this.
writeUint(ctx, buf + ctx.arg.wordsize,
(uint64_t)1 << (ctx.arg.wordsize * 8 - 1));
+ ctx.target->relocateAlloc(*this, buf);
for (const FileGot &g : gots) {
auto write = [&](size_t i, const Symbol *s, int64_t a) {
uint64_t va = a;
@@ -1189,9 +1189,10 @@ void MipsGotSection::writeTo(uint8_t *buf) {
write(p.second, p.first, 0);
for (const std::pair<Symbol *, size_t> &p : g.relocs)
write(p.second, p.first, 0);
- for (const std::pair<Symbol *, size_t> &p : g.tls)
- write(p.second, p.first,
- p.first->isPreemptible || ctx.arg.shared ? 0 : -0x7000);
+ for (const std::pair<Symbol *, size_t> &p : g.tls) {
+ if (!p.first->isPreemptible && !ctx.arg.shared)
+ write(p.second, p.first, -0x7000);
+ }
for (const std::pair<Symbol *, size_t> &p : g.dynTlsSymbols) {
if (p.first == nullptr && !ctx.arg.shared)
write(p.second, nullptr, 1);
@@ -1653,8 +1654,7 @@ int64_t DynamicReloc::computeAddend(Ctx &ctx) const {
case AgainstSymbol:
assert(sym != nullptr);
return addend;
- case AddendOnlyWithTargetVA:
- case AgainstSymbolWithTargetVA: {
+ case AddendOnlyWithTargetVA: {
uint64_t ca = inputSec->getRelocTargetVA(
ctx, Relocation{expr, type, 0, addend, sym}, getOffset());
return ctx.arg.is64 ? ca : SignExtend64<32>(ca);
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 5f01513630597..7612915b5b1dc 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -429,11 +429,6 @@ class DynamicReloc {
/// The resulting dynamic relocation references symbol #sym from the dynamic
/// symbol table and uses #addend as the value of computeAddend(ctx).
AgainstSymbol,
- /// The resulting dynamic relocation references symbol #sym from the dynamic
- /// symbol table and uses InputSection::getRelocTargetVA() + #addend for the
- /// final addend. It can be used for relocations that write the symbol VA as
- // the addend (e.g. R_MIPS_TLS_TPREL64) but still reference the symbol.
- AgainstSymbolWithTargetVA,
/// This is used by the MIPS multi-GOT implementation. It relocates
/// addresses of 64kb pages that lie inside the output section.
MipsMultiGotPage,
@@ -460,9 +455,7 @@ class DynamicReloc {
uint64_t getOffset() const;
uint32_t getSymIndex(SymbolTableBaseSection *symTab) const;
- bool needsDynSymIndex() const {
- return kind == AgainstSymbol || kind == AgainstSymbolWithTargetVA;
- }
+ bool needsDynSymIndex() const { return kind == AgainstSymbol; }
/// Computes the addend of the dynamic relocation. Note that this is not the
/// same as the #addend member variable as it may also include the symbol
diff --git a/lld/test/ELF/mips-mgot.s b/lld/test/ELF/mips-mgot.s
index 6978b5d9623b4..67bd5e6619f12 100644
--- a/lld/test/ELF/mips-mgot.s
+++ b/lld/test/ELF/mips-mgot.s
@@ -23,7 +23,7 @@
# CHECK: Contents of section .got:
# CHECK-NEXT: 70000 00000000 80000000 [[FOO0]] [[FOO2]]
-# CHECK-NEXT: 70010 00000000 00000004 00010000 00020000
+# CHECK-NEXT: 70010 00000000 00000000 00010000 00020000
# CHECK-NEXT: 70020 00030000 00040000 00050000 00060000
# CHECK-NEXT: 70030 00000000 00000000 00000000 00000000
# CHECK-NEXT: 70040 00000000 00000000 00000000
diff --git a/lld/test/ELF/mips-tls-64.s b/lld/test/ELF/mips-tls-64.s
index 3976b50274be4..8a00b93c77e2f 100644
--- a/lld/test/ELF/mips-tls-64.s
+++ b/lld/test/ELF/mips-tls-64.s
@@ -75,7 +75,7 @@
# DIS-SO: Contents of section .got:
# DIS-SO-NEXT: 30000 00000000 00000000 80000000 00000000
# DIS-SO-NEXT: 30010 00000000 00000000 00000000 00000000
-# DIS-SO-NEXT: 30020 00000000 00000004 00000000 00000000
+# DIS-SO-NEXT: 30020 00000000 00000000 00000000 00000000
# DIS-SO-NEXT: 30030 00000000 00000000 00000000 00000000
# DIS-SO-NEXT: 30040 00000000 00000000 00000000 00000000
# DIS-SO-NEXT: 30050 00000000 00000000
diff --git a/lld/test/ELF/mips-tls.s b/lld/test/ELF/mips-tls.s
index b98f3bb53d18d..804b29cf724d9 100644
--- a/lld/test/ELF/mips-tls.s
+++ b/lld/test/ELF/mips-tls.s
@@ -71,7 +71,7 @@
# DIS-SO: Contents of section .got:
# DIS-SO-NEXT: 30000 00000000 80000000 00000000 00000000
-# DIS-SO-NEXT: 30010 00000004 00000000 00000000 00000000
+# DIS-SO-NEXT: 30010 00000000 00000000 00000000 00000000
# DIS-SO-NEXT: 30020 00000000 00000000 00000000
# SO: Relocations [
|
|
@llvm/pr-subscribers-lld-elf Author: Jessica Clarke (jrtc27) ChangesIf the symbol is preemptible the addend should be 0, not our This means we can remove AgainstSymbolWithTargetVA, which is a rather Full diff: https://github.com/llvm/llvm-project/pull/150729.diff 5 Files Affected:
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index efec41a737b62..0bb00c6d2bcff 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -1065,9 +1065,8 @@ void MipsGotSection::build() {
// for the TP-relative offset as we don't know how much other data will
// be allocated before us in the static TLS block.
if (s->isPreemptible || ctx.arg.shared)
- ctx.mainPart->relaDyn->addReloc(
- {ctx.target->tlsGotRel, this, offset,
- DynamicReloc::AgainstSymbolWithTargetVA, *s, 0, R_ABS});
+ ctx.mainPart->relaDyn->addAddendOnlyRelocIfNonPreemptible(
+ ctx.target->tlsGotRel, *this, offset, *s, ctx.target->symbolicRel);
}
for (std::pair<Symbol *, size_t> &p : got.dynTlsSymbols) {
Symbol *s = p.first;
@@ -1160,6 +1159,7 @@ void MipsGotSection::writeTo(uint8_t *buf) {
// if we had to do this.
writeUint(ctx, buf + ctx.arg.wordsize,
(uint64_t)1 << (ctx.arg.wordsize * 8 - 1));
+ ctx.target->relocateAlloc(*this, buf);
for (const FileGot &g : gots) {
auto write = [&](size_t i, const Symbol *s, int64_t a) {
uint64_t va = a;
@@ -1189,9 +1189,10 @@ void MipsGotSection::writeTo(uint8_t *buf) {
write(p.second, p.first, 0);
for (const std::pair<Symbol *, size_t> &p : g.relocs)
write(p.second, p.first, 0);
- for (const std::pair<Symbol *, size_t> &p : g.tls)
- write(p.second, p.first,
- p.first->isPreemptible || ctx.arg.shared ? 0 : -0x7000);
+ for (const std::pair<Symbol *, size_t> &p : g.tls) {
+ if (!p.first->isPreemptible && !ctx.arg.shared)
+ write(p.second, p.first, -0x7000);
+ }
for (const std::pair<Symbol *, size_t> &p : g.dynTlsSymbols) {
if (p.first == nullptr && !ctx.arg.shared)
write(p.second, nullptr, 1);
@@ -1653,8 +1654,7 @@ int64_t DynamicReloc::computeAddend(Ctx &ctx) const {
case AgainstSymbol:
assert(sym != nullptr);
return addend;
- case AddendOnlyWithTargetVA:
- case AgainstSymbolWithTargetVA: {
+ case AddendOnlyWithTargetVA: {
uint64_t ca = inputSec->getRelocTargetVA(
ctx, Relocation{expr, type, 0, addend, sym}, getOffset());
return ctx.arg.is64 ? ca : SignExtend64<32>(ca);
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 5f01513630597..7612915b5b1dc 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -429,11 +429,6 @@ class DynamicReloc {
/// The resulting dynamic relocation references symbol #sym from the dynamic
/// symbol table and uses #addend as the value of computeAddend(ctx).
AgainstSymbol,
- /// The resulting dynamic relocation references symbol #sym from the dynamic
- /// symbol table and uses InputSection::getRelocTargetVA() + #addend for the
- /// final addend. It can be used for relocations that write the symbol VA as
- // the addend (e.g. R_MIPS_TLS_TPREL64) but still reference the symbol.
- AgainstSymbolWithTargetVA,
/// This is used by the MIPS multi-GOT implementation. It relocates
/// addresses of 64kb pages that lie inside the output section.
MipsMultiGotPage,
@@ -460,9 +455,7 @@ class DynamicReloc {
uint64_t getOffset() const;
uint32_t getSymIndex(SymbolTableBaseSection *symTab) const;
- bool needsDynSymIndex() const {
- return kind == AgainstSymbol || kind == AgainstSymbolWithTargetVA;
- }
+ bool needsDynSymIndex() const { return kind == AgainstSymbol; }
/// Computes the addend of the dynamic relocation. Note that this is not the
/// same as the #addend member variable as it may also include the symbol
diff --git a/lld/test/ELF/mips-mgot.s b/lld/test/ELF/mips-mgot.s
index 6978b5d9623b4..67bd5e6619f12 100644
--- a/lld/test/ELF/mips-mgot.s
+++ b/lld/test/ELF/mips-mgot.s
@@ -23,7 +23,7 @@
# CHECK: Contents of section .got:
# CHECK-NEXT: 70000 00000000 80000000 [[FOO0]] [[FOO2]]
-# CHECK-NEXT: 70010 00000000 00000004 00010000 00020000
+# CHECK-NEXT: 70010 00000000 00000000 00010000 00020000
# CHECK-NEXT: 70020 00030000 00040000 00050000 00060000
# CHECK-NEXT: 70030 00000000 00000000 00000000 00000000
# CHECK-NEXT: 70040 00000000 00000000 00000000
diff --git a/lld/test/ELF/mips-tls-64.s b/lld/test/ELF/mips-tls-64.s
index 3976b50274be4..8a00b93c77e2f 100644
--- a/lld/test/ELF/mips-tls-64.s
+++ b/lld/test/ELF/mips-tls-64.s
@@ -75,7 +75,7 @@
# DIS-SO: Contents of section .got:
# DIS-SO-NEXT: 30000 00000000 00000000 80000000 00000000
# DIS-SO-NEXT: 30010 00000000 00000000 00000000 00000000
-# DIS-SO-NEXT: 30020 00000000 00000004 00000000 00000000
+# DIS-SO-NEXT: 30020 00000000 00000000 00000000 00000000
# DIS-SO-NEXT: 30030 00000000 00000000 00000000 00000000
# DIS-SO-NEXT: 30040 00000000 00000000 00000000 00000000
# DIS-SO-NEXT: 30050 00000000 00000000
diff --git a/lld/test/ELF/mips-tls.s b/lld/test/ELF/mips-tls.s
index b98f3bb53d18d..804b29cf724d9 100644
--- a/lld/test/ELF/mips-tls.s
+++ b/lld/test/ELF/mips-tls.s
@@ -71,7 +71,7 @@
# DIS-SO: Contents of section .got:
# DIS-SO-NEXT: 30000 00000000 80000000 00000000 00000000
-# DIS-SO-NEXT: 30010 00000004 00000000 00000000 00000000
+# DIS-SO-NEXT: 30010 00000000 00000000 00000000 00000000
# DIS-SO-NEXT: 30020 00000000 00000000 00000000
# SO: Relocations [
|
Created using spr 1.3.5
jrtc27
changed the base branch from
main
to
users/jrtc27/spr/main.elfmips-fix-addend-for-preemptible-static-tls
Created using spr 1.3.5
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the symbol is preemptible the addend should be 0, not our
definition's VA. Note that by using addAddendOnlyRelocIfNonPreemptible
the generic Elf_Rel code will ensure the VA is written out as the addend
if the symbol is non-preemptible, and so writeTo only needs to write out
the VA in the case that we don't call it (so long as we make sure to
call relocateAlloc to actually apply any such relocations).