Skip to content

[ELF] Rename IsRela to HasAddend #96592

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.

Sign up for GitHub

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 2 commits into from
Jun 26, 2024
Merged

[ELF] Rename IsRela to HasAddend #96592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc7cadd
[𝘀𝗽𝗿] initial version
MaskRay Jun 25, 2024
223fd48
xxx
MaskRay Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions lld/ELF/InputSection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ void InputSection::copyRelocations(uint8_t *buf,
auto *p = reinterpret_cast<typename ELFT::Rela *>(buf);
buf += sizeof(RelTy);

if (RelTy::IsRela)
if (RelTy::HasAddend)
p->r_addend = rel.addend;

// Output section VA is zero for -r, so r_offset is an offset within the
Expand Down Expand Up @@ -452,7 +452,7 @@ void InputSection::copyRelocations(uint8_t *buf,

int64_t addend = rel.addend;
const uint8_t *bufLoc = sec->content().begin() + rel.offset;
if (!RelTy::IsRela)
if (!RelTy::HasAddend)
addend = target.getImplicitAddend(bufLoc, type);

if (config->emachine == EM_MIPS &&
Expand All @@ -471,7 +471,7 @@ void InputSection::copyRelocations(uint8_t *buf,
addend += sec->getFile<ELFT>()->mipsGp0;
}

if (RelTy::IsRela)
if (RelTy::HasAddend)
p->r_addend = sym.getVA(addend) - section->getOutputSection()->addr;
// For SHF_ALLOC sections relocated by REL, append a relocation to
// sec->relocations so that relocateAlloc transitively called by
Expand Down Expand Up @@ -934,7 +934,7 @@ void InputSection::relocateNonAlloc(uint8_t *buf, ArrayRef<RelTy> rels) {
const uint64_t offset = rel.r_offset;
uint8_t *bufLoc = buf + offset;
int64_t addend = getAddend<ELFT>(rel);
if (!RelTy::IsRela)
if (!RelTy::HasAddend)
addend += target.getImplicitAddend(bufLoc, type);

Symbol &sym = f->getRelocTargetSym(rel);
Expand Down Expand Up @@ -1007,10 +1007,11 @@ void InputSection::relocateNonAlloc(uint8_t *buf, ArrayRef<RelTy> rels) {
}
}

// For a relocatable link, content relocated by RELA remains unchanged and
// we can stop here, while content relocated by REL referencing STT_SECTION
// needs updating implicit addends.
if (config->relocatable && (RelTy::IsRela || sym.type != STT_SECTION))
// For a relocatable link, content relocated by relocation types with an
// explicit addend, such as RELA, remain unchanged and we can stop here.
// While content relocated by relocation types with an implicit addend, such
// as REL, needs the implicit addend updated.
if (config->relocatable && (RelTy::HasAddend || sym.type != STT_SECTION))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comment above might be worth updating assuming CREL will behave in the same way. Perhaps something like:

For a relocatable link, content relocated by relocation types with an explicit addend, such as RELA, remain unchanged and we can stop here. While content relocated by relocation types with an implicit addend, such as REL, needs the implicit addend updated.

continue;

// R_ABS/R_DTPREL and some other relocations can be used from non-SHF_ALLOC
Expand Down
5 changes: 3 additions & 2 deletions lld/ELF/Relocations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,8 @@ int64_t RelocationScanner::computeMipsAddend(const RelTy &rel, RelExpr expr,

// The ABI says that the paired relocation is used only for REL.
// See p. 4-17 at ftp://www.linux-mips.org/pub/linux/mips/doc/ABI/mipsabi.pdf
if (RelTy::IsRela)
// This generalises to relocation types with implicit addends.
if (RelTy::HasAddend)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming MIPS will support CREL, but the ABI won't be updated to explicitly mention it, could be worth a small addition at the end.

The ABI says the paired relocation  is used only for REL.
  // See p. 4-17 at ftp://www.linux-mips.org/pub/linux/mips/doc/ABI/mipsabi.pdf this generalises to relocation types with implicit addends.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx. Added. (I think MIPS will die before anyone implements CREL for it...)

return 0;

RelType type = rel.getType(config->isMips64EL);
Expand Down Expand Up @@ -1448,7 +1449,7 @@ template <class ELFT, class RelTy> void RelocationScanner::scanOne(RelTy *&i) {
return;

RelExpr expr = target->getRelExpr(type, sym, sec->content().data() + offset);
int64_t addend = RelTy::IsRela
int64_t addend = RelTy::HasAddend
? getAddend<ELFT>(rel)
: target->getImplicitAddend(
sec->content().data() + rel.r_offset, type);
Expand Down
8 changes: 4 additions & 4 deletions llvm/include/llvm/Object/ELFTypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ struct Elf_Dyn_Impl : Elf_Dyn_Base<ELFT> {
template <endianness Endianness>
struct Elf_Rel_Impl<ELFType<Endianness, false>, false> {
LLVM_ELF_IMPORT_TYPES(Endianness, false)
static const bool IsRela = false;
static const bool HasAddend = false;
Elf_Addr r_offset; // Location (file byte offset, or program virtual addr)
Elf_Word r_info; // Symbol table index and type of relocation to apply

Expand Down Expand Up @@ -420,14 +420,14 @@ template <endianness Endianness>
struct Elf_Rel_Impl<ELFType<Endianness, false>, true>
: public Elf_Rel_Impl<ELFType<Endianness, false>, false> {
LLVM_ELF_IMPORT_TYPES(Endianness, false)
static const bool IsRela = true;
static const bool HasAddend = true;
Elf_Sword r_addend; // Compute value for relocatable field by adding this
};

template <endianness Endianness>
struct Elf_Rel_Impl<ELFType<Endianness, true>, false> {
LLVM_ELF_IMPORT_TYPES(Endianness, true)
static const bool IsRela = false;
static const bool HasAddend = false;
Elf_Addr r_offset; // Location (file byte offset, or program virtual addr)
Elf_Xword r_info; // Symbol table index and type of relocation to apply

Expand Down Expand Up @@ -473,7 +473,7 @@ template <endianness Endianness>
struct Elf_Rel_Impl<ELFType<Endianness, true>, true>
: public Elf_Rel_Impl<ELFType<Endianness, true>, false> {
LLVM_ELF_IMPORT_TYPES(Endianness, true)
static const bool IsRela = true;
static const bool HasAddend = true;
Elf_Sxword r_addend; // Compute value for relocatable field by adding this.
};

Expand Down
Loading