Skip to content

[ELF] Consistently use gotEntrySize for GOT entries #142064

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

Merged
merged 1 commit into from
Jun 4, 2025

Conversation

jrtc27
Copy link
Collaborator

@jrtc27 jrtc27 commented May 29, 2025

d624134 ("[lld][X86] Restore gotEntrySize.") (re-)introduced
gotEntrySize and used it for various GOT calculations, but was not
exhaustive (nor consistent; Symbol::getGotOffset was modified but not
GotSection::finalizeContents, so we undercompute the size, on top of
computing the wrong offsets for TLS), and since then even more uses have
been added that use wordsize instead of gotEntrySize (presumably due to
looking at the existing incorrect ones).

This doesn't really matter upstream, as the only architecture where the
two differ is X32, and from looking at the code it's not properly
supported (e.g. TLS relaxation assumes LP64 sequences), but downstream
in CHERI LLVM it does matter, as CHERI's pointers are more than just an
integer address (a machine word).

Note this ignores the special MipsGotSection; on MIPS, wordsize and
gotEntrySize are the same, CHERI-MIPS is no longer something we support
downstream and even when we did we didn't reuse that implementation.

d624134 ("[lld][X86] Restore gotEntrySize.") (re-)introduced
gotEntrySize and used it for various GOT calculations, but was not
exhaustive (nor consistent; Symbol::getGotOffset was modified but not
GotSection::finalizeContents, so we undercompute the size, on top of
computing the wrong offsets for TLS), and since then even more uses have
been added that use wordsize instead of gotEntrySize (presumably due to
looking at the existing incorrect ones).

This doesn't really matter upstream, as the only architecture where the
two differ is X32, and from looking at the code it's not properly
supported (e.g. TLS relaxation assumes LP64 sequences), but downstream
in CHERI LLVM it does matter, as CHERI's pointers are more than just an
integer address (a machine word).

Note this ignores the special MipsGotSection; on MIPS, wordsize and
gotEntrySize are the same, CHERI-MIPS is no longer something we support
downstream and even when we did we didn't reuse that implementation.
@llvmbot
Copy link
Member

llvmbot commented May 29, 2025

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: Jessica Clarke (jrtc27)

Changes

d624134 ("[lld][X86] Restore gotEntrySize.") (re-)introduced
gotEntrySize and used it for various GOT calculations, but was not
exhaustive (nor consistent; Symbol::getGotOffset was modified but not
GotSection::finalizeContents, so we undercompute the size, on top of
computing the wrong offsets for TLS), and since then even more uses have
been added that use wordsize instead of gotEntrySize (presumably due to
looking at the existing incorrect ones).

This doesn't really matter upstream, as the only architecture where the
two differ is X32, and from looking at the code it's not properly
supported (e.g. TLS relaxation assumes LP64 sequences), but downstream
in CHERI LLVM it does matter, as CHERI's pointers are more than just an
integer address (a machine word).

Note this ignores the special MipsGotSection; on MIPS, wordsize and
gotEntrySize are the same, CHERI-MIPS is no longer something we support
downstream and even when we did we didn't reuse that implementation.


Full diff: https://github.com/llvm/llvm-project/pull/142064.diff

1 Files Affected:

  • (modified) lld/ELF/SyntheticSections.cpp (+10-9)
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 6f513b89d8f34..c21fce0ce15ec 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -668,7 +668,8 @@ void GotSection::addEntry(const Symbol &sym) {
 }
 
 void GotSection::addAuthEntry(const Symbol &sym) {
-  authEntries.push_back({(numEntries - 1) * ctx.arg.wordsize, sym.isFunc()});
+  authEntries.push_back(
+      {(numEntries - 1) * ctx.target->gotEntrySize, sym.isFunc()});
 }
 
 bool GotSection::addTlsDescEntry(const Symbol &sym) {
@@ -679,8 +680,8 @@ bool GotSection::addTlsDescEntry(const Symbol &sym) {
 }
 
 void GotSection::addTlsDescAuthEntry() {
-  authEntries.push_back({(numEntries - 2) * ctx.arg.wordsize, true});
-  authEntries.push_back({(numEntries - 1) * ctx.arg.wordsize, false});
+  authEntries.push_back({(numEntries - 2) * ctx.target->gotEntrySize, true});
+  authEntries.push_back({(numEntries - 1) * ctx.target->gotEntrySize, false});
 }
 
 bool GotSection::addDynTlsEntry(const Symbol &sym) {
@@ -696,13 +697,13 @@ bool GotSection::addDynTlsEntry(const Symbol &sym) {
 bool GotSection::addTlsIndex() {
   if (tlsIndexOff != uint32_t(-1))
     return false;
-  tlsIndexOff = numEntries * ctx.arg.wordsize;
+  tlsIndexOff = numEntries * ctx.target->gotEntrySize;
   numEntries += 2;
   return true;
 }
 
 uint32_t GotSection::getTlsDescOffset(const Symbol &sym) const {
-  return sym.getTlsDescIdx(ctx) * ctx.arg.wordsize;
+  return sym.getTlsDescIdx(ctx) * ctx.target->gotEntrySize;
 }
 
 uint64_t GotSection::getTlsDescAddr(const Symbol &sym) const {
@@ -710,11 +711,11 @@ uint64_t GotSection::getTlsDescAddr(const Symbol &sym) const {
 }
 
 uint64_t GotSection::getGlobalDynAddr(const Symbol &b) const {
-  return this->getVA() + b.getTlsGdIdx(ctx) * ctx.arg.wordsize;
+  return this->getVA() + b.getTlsGdIdx(ctx) * ctx.target->gotEntrySize;
 }
 
 uint64_t GotSection::getGlobalDynOffset(const Symbol &b) const {
-  return b.getTlsGdIdx(ctx) * ctx.arg.wordsize;
+  return b.getTlsGdIdx(ctx) * ctx.target->gotEntrySize;
 }
 
 void GotSection::finalizeContents() {
@@ -723,7 +724,7 @@ void GotSection::finalizeContents() {
       !ctx.sym.globalOffsetTable)
     size = 0;
   else
-    size = numEntries * ctx.arg.wordsize;
+    size = numEntries * ctx.target->gotEntrySize;
 }
 
 bool GotSection::isNeeded() const {
@@ -1199,7 +1200,7 @@ void MipsGotSection::writeTo(uint8_t *buf) {
 // consistent across both 64-bit PowerPC ABIs as well as the 32-bit PowerPC ABI.
 GotPltSection::GotPltSection(Ctx &ctx)
     : SyntheticSection(ctx, ".got.plt", SHT_PROGBITS, SHF_ALLOC | SHF_WRITE,
-                       ctx.arg.wordsize) {
+                       ctx.target->gotEntrySize) {
   if (ctx.arg.emachine == EM_PPC) {
     name = ".plt";
   } else if (ctx.arg.emachine == EM_PPC64) {

@jrtc27 jrtc27 merged commit 6be4670 into llvm:main Jun 4, 2025
14 checks passed
@jrtc27 jrtc27 deleted the gotentrysize branch June 4, 2025 00:30
rorth pushed a commit to rorth/llvm-project that referenced this pull request Jun 11, 2025
d624134 ("[lld][X86] Restore gotEntrySize.") (re-)introduced
gotEntrySize and used it for various GOT calculations, but was not
exhaustive (nor consistent; Symbol::getGotOffset was modified but not
GotSection::finalizeContents, so we undercompute the size, on top of
computing the wrong offsets for TLS), and since then even more uses have
been added that use wordsize instead of gotEntrySize (presumably due to
looking at the existing incorrect ones).

This doesn't really matter upstream, as the only architecture where the
two differ is X32, and from looking at the code it's not properly
supported (e.g. TLS relaxation assumes LP64 sequences), but downstream
in CHERI LLVM it does matter, as CHERI's pointers are more than just an
integer address (a machine word).

Note this ignores the special MipsGotSection; on MIPS, wordsize and
gotEntrySize are the same, CHERI-MIPS is no longer something we support
downstream and even when we did we didn't reuse that implementation.
DhruvSrivastavaX pushed a commit to DhruvSrivastavaX/lldb-for-aix that referenced this pull request Jun 12, 2025
d624134 ("[lld][X86] Restore gotEntrySize.") (re-)introduced
gotEntrySize and used it for various GOT calculations, but was not
exhaustive (nor consistent; Symbol::getGotOffset was modified but not
GotSection::finalizeContents, so we undercompute the size, on top of
computing the wrong offsets for TLS), and since then even more uses have
been added that use wordsize instead of gotEntrySize (presumably due to
looking at the existing incorrect ones).

This doesn't really matter upstream, as the only architecture where the
two differ is X32, and from looking at the code it's not properly
supported (e.g. TLS relaxation assumes LP64 sequences), but downstream
in CHERI LLVM it does matter, as CHERI's pointers are more than just an
integer address (a machine word).

Note this ignores the special MipsGotSection; on MIPS, wordsize and
gotEntrySize are the same, CHERI-MIPS is no longer something we support
downstream and even when we did we didn't reuse that implementation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants