Skip to content

ELF: Add support for R_AARCH64_PATCHINST relocation type. #133534

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 20 commits into from
Aug 11, 2025
Merged

ELF: Add support for R_AARCH64_PATCHINST relocation type. #133534

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9928f6e
[𝘀𝗽𝗿] changes to main this commit is based on
pcc Mar 28, 2025
fdcf6f4
[𝘀𝗽𝗿] initial version
pcc Mar 28, 2025
5db4682
[𝘀𝗽𝗿] changes introduced through rebase
pcc May 13, 2025
de0f3a2
Rebase
pcc May 13, 2025
ad4e0ff
[𝘀𝗽𝗿] changes introduced through rebase
pcc May 24, 2025
0d90bca
Rebase
pcc May 24, 2025
a45639b
[𝘀𝗽𝗿] changes introduced through rebase
pcc Jul 9, 2025
cccd109
Rebase
pcc Jul 9, 2025
099bf34
[𝘀𝗽𝗿] changes introduced through rebase
pcc Jul 10, 2025
efb98de
Rename to R_AARCH64_PATCHINST and add test
pcc Jul 10, 2025
41e42c0
[𝘀𝗽𝗿] changes introduced through rebase
pcc Jul 11, 2025
ab863bf
Address review comments
pcc Jul 11, 2025
fcf4206
[𝘀𝗽𝗿] changes introduced through rebase
kazutakahirata Jul 19, 2025
240e141
Rebase
pcc Jul 19, 2025
f403ad9
[𝘀𝗽𝗿] changes introduced through rebase
AlexVlx Jul 30, 2025
7546eb8
Rebase
pcc Jul 30, 2025
0a100d1
[𝘀𝗽𝗿] changes introduced through rebase
philnik777 Aug 1, 2025
988db55
Rebase
pcc Aug 1, 2025
4e2946f
[𝘀𝗽𝗿] changes introduced through rebase
pcc Aug 11, 2025
068ad5c
Split out MC part
pcc Aug 11, 2025
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
12 changes: 12 additions & 0 deletions lld/ELF/Arch/AArch64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ RelExpr AArch64::getRelExpr(RelType type, const Symbol &s,
case R_AARCH64_MOVW_UABS_G2_NC:
case R_AARCH64_MOVW_UABS_G3:
return R_ABS;
case R_AARCH64_PATCHINST:
if (!isAbsolute(s))
Err(ctx) << getErrorLoc(ctx, loc)
<< "R_AARCH64_PATCHINST relocation against non-absolute symbol "
<< &s;
return R_ABS;
case R_AARCH64_AUTH_ABS64:
return RE_AARCH64_AUTH;
case R_AARCH64_TLSDESC_ADR_PAGE21:
Expand Down Expand Up @@ -506,6 +512,12 @@ void AArch64::relocate(uint8_t *loc, const Relocation &rel,
checkIntUInt(ctx, loc, val, 32, rel);
write32(ctx, loc, val);
break;
case R_AARCH64_PATCHINST:
if (!rel.sym->isUndefined()) {
checkUInt(ctx, loc, val, 32, rel);
write32le(loc, val);
}
break;
case R_AARCH64_PLT32:
case R_AARCH64_GOTPCREL32:
checkInt(ctx, loc, val, 32, rel);
Expand Down
2 changes: 1 addition & 1 deletion lld/ELF/Relocations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ static RelType getMipsPairType(RelType type, bool isLocal) {

// True if non-preemptable symbol always has the same value regardless of where
// the DSO is loaded.
static bool isAbsolute(const Symbol &sym) {
bool elf::isAbsolute(const Symbol &sym) {
if (sym.isUndefined())
return true;
if (const auto *dr = dyn_cast<Defined>(&sym))
Expand Down
2 changes: 2 additions & 0 deletions lld/ELF/Relocations.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ void addGotEntry(Ctx &ctx, Symbol &sym);
void hexagonTLSSymbolUpdate(Ctx &ctx);
bool hexagonNeedsTLSSymbol(ArrayRef<OutputSection *> outputSections);

bool isAbsolute(const Symbol &sym);

class ThunkSection;
class Thunk;
class InputSectionDescription;
Expand Down
87 changes: 87 additions & 0 deletions lld/test/ELF/aarch64-patchinst.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# RUN: rm -rf %t && split-file %s %t
Copy link
Member

@MaskRay MaskRay Aug 9, 2025

Choose a reason for hiding this comment

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

consider adding && cd %t so that we can remove %t/ below, which clutter up the commands...

Copy link
Contributor Author

@pcc pcc Aug 11, 2025

Choose a reason for hiding this comment

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

I prefer this form so that the commands are copy-pastable into the shell.

# RUN: llvm-mc -filetype=obj -triple=aarch64 %t/use.s -o %t/use-le.o
# RUN: llvm-mc -filetype=obj -triple=aarch64 %t/def.s -o %t/def-le.o
# RUN: llvm-mc -filetype=obj -triple=aarch64 %t/rel.s -o %t/rel-le.o

## Deactivation symbol used without being defined: instruction emitted as usual.
# RUN: ld.lld -o %t/undef-le %t/use-le.o --emit-relocs
# RUN: llvm-objdump -r %t/undef-le | FileCheck --check-prefix=RELOC %s
# RUN: llvm-objdump -d %t/undef-le | FileCheck --check-prefix=UNDEF %s
# RUN: ld.lld -pie -o %t/undef-le %t/use-le.o --emit-relocs
# RUN: llvm-objdump -r %t/undef-le | FileCheck --check-prefix=RELOC %s
# RUN: llvm-objdump -d %t/undef-le | FileCheck --check-prefix=UNDEF %s

## Deactivation symbol defined: instructions overwritten with NOPs.
# RUN: ld.lld -o %t/def-le %t/use-le.o %t/def-le.o --emit-relocs
# RUN: llvm-objdump -r %t/def-le | FileCheck --check-prefix=RELOC %s
# RUN: llvm-objdump -d %t/def-le | FileCheck --check-prefix=DEF %s
# RUN: ld.lld -pie -o %t/def-le %t/use-le.o %t/def-le.o --emit-relocs
# RUN: llvm-objdump -r %t/def-le | FileCheck --check-prefix=RELOC %s
# RUN: llvm-objdump -d %t/def-le | FileCheck --check-prefix=DEF %s

## Relocation pointing to a non-SHN_UNDEF non-SHN_ABS symbol is an error.
# RUN: not ld.lld -o %t/rel-le %t/use-le.o %t/rel-le.o 2>&1 | FileCheck --check-prefix=ERROR %s
# RUN: not ld.lld -pie -o %t/rel-le %t/use-le.o %t/rel-le.o 2>&1 | FileCheck --check-prefix=ERROR %s

## Behavior unchanged by endianness: relocation always written as little endian.
# RUN: llvm-mc -filetype=obj -triple=aarch64_be %t/use.s -o %t/use-be.o
# RUN: llvm-mc -filetype=obj -triple=aarch64_be %t/def.s -o %t/def-be.o
# RUN: llvm-mc -filetype=obj -triple=aarch64_be %t/rel.s -o %t/rel-be.o
# RUN: ld.lld -o %t/undef-be %t/use-be.o --emit-relocs
# RUN: llvm-objdump -r %t/undef-be | FileCheck --check-prefix=RELOC %s
# RUN: llvm-objdump -d %t/undef-be | FileCheck --check-prefix=UNDEF %s
# RUN: ld.lld -pie -o %t/undef-be %t/use-be.o --emit-relocs
# RUN: llvm-objdump -r %t/undef-be | FileCheck --check-prefix=RELOC %s
# RUN: llvm-objdump -d %t/undef-be | FileCheck --check-prefix=UNDEF %s
# RUN: ld.lld -o %t/def-be %t/use-be.o %t/def-be.o --emit-relocs
# RUN: llvm-objdump -r %t/def-be | FileCheck --check-prefix=RELOC %s
# RUN: llvm-objdump -d %t/def-be | FileCheck --check-prefix=DEF %s
# RUN: ld.lld -pie -o %t/def-be %t/use-be.o %t/def-be.o --emit-relocs
# RUN: llvm-objdump -r %t/def-be | FileCheck --check-prefix=RELOC %s
# RUN: llvm-objdump -d %t/def-be | FileCheck --check-prefix=DEF %s
# RUN: not ld.lld -o %t/rel-be %t/use-be.o %t/rel-be.o 2>&1 | FileCheck --check-prefix=ERROR %s
# RUN: not ld.lld -pie -o %t/rel-be %t/use-be.o %t/rel-be.o 2>&1 | FileCheck --check-prefix=ERROR %s

# RELOC: R_AARCH64_JUMP26
# RELOC-NEXT: R_AARCH64_PATCHINST ds
# RELOC-NEXT: R_AARCH64_PATCHINST ds
# RELOC-NEXT: R_AARCH64_PATCHINST ds0+0xd503201f

#--- use.s
.weak ds
.weak ds0
# This instruction has a single relocation: the DS relocation.
# UNDEF: add x0, x1, x2
# DEF: nop
# ERROR: R_AARCH64_PATCHINST relocation against non-absolute symbol ds
.reloc ., R_AARCH64_PATCHINST, ds
add x0, x1, x2
# This instruction has two relocations: the DS relocation and the JUMP26 to f1.
# Make sure that the DS relocation takes precedence.
.reloc ., R_AARCH64_PATCHINST, ds
Copy link
Collaborator

@smithp35 smithp35 Jul 11, 2025

Choose a reason for hiding this comment

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

Could be worth a test with emit-relocs to show both relocations coming out.

Thinking of Bolt, which relies on emit-relocs, I expect that it would just ignore the R_AARCH_PATCHINST relocations on their own as it wouldn't know how to recreate the original value [1]. It would have to discard any relocation at the same location. We're hoping to create a binary analysis ABI supplement soon to document conventions that binary analysis tools are using. First step ARM-software/abi-aa#333

[1] In theory if we did want to let Bolt reverse a patch, the emit-relocs output could give the reverse patch.

Copy link
Contributor Author

@pcc pcc Jul 11, 2025

Choose a reason for hiding this comment

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

Done

# UNDEF: b {{.*}} <f1>
# DEF: nop
# ERROR: R_AARCH64_PATCHINST relocation against non-absolute symbol ds
b f1
# Alternative representation: instruction opcode stored in addend.
# UNDEF: add x3, x4, x5
# DEF: nop
# ERROR: R_AARCH64_PATCHINST relocation against non-absolute symbol ds0
.reloc ., R_AARCH64_PATCHINST, ds0 + 0xd503201f
add x3, x4, x5

.section .text.f1,"ax",@progbits
f1:
ret

#--- def.s
.globl ds
ds = 0xd503201f
.globl ds0
ds0 = 0

#--- rel.s
.globl ds
ds:
.globl ds0
ds0:
1 change: 1 addition & 0 deletions llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ ELF_RELOC(R_AARCH64_LD64_GOT_LO12_NC, 0x138)
ELF_RELOC(R_AARCH64_LD64_GOTPAGE_LO15, 0x139)
ELF_RELOC(R_AARCH64_PLT32, 0x13a)
ELF_RELOC(R_AARCH64_GOTPCREL32, 0x13b)
ELF_RELOC(R_AARCH64_PATCHINST, 0x13c)
Copy link
Member

@MaskRay MaskRay Aug 9, 2025

Choose a reason for hiding this comment

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

Also add a test to tools/llvm-readobj/ELF/reloc-types-aarch64.test

Copy link
Contributor Author

@pcc pcc Aug 11, 2025

Choose a reason for hiding this comment

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

Done

// General dynamic TLS relocations
ELF_RELOC(R_AARCH64_TLSGD_ADR_PREL21, 0x200)
ELF_RELOC(R_AARCH64_TLSGD_ADR_PAGE21, 0x201)
Expand Down
12 changes: 12 additions & 0 deletions llvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class AArch64ELFObjectWriter : public MCELFObjectTargetWriter {
bool IsPCRel) const override;
bool needsRelocateWithSymbol(const MCValue &, unsigned Type) const override;
bool isNonILP32reloc(const MCFixup &Fixup, AArch64::Specifier RefKind) const;
void sortRelocs(std::vector<ELFRelocationEntry> &Relocs) override;

bool IsILP32;
};
Expand Down Expand Up @@ -497,6 +498,17 @@ bool AArch64ELFObjectWriter::needsRelocateWithSymbol(const MCValue &Val,
Val.getSpecifier());
}

void AArch64ELFObjectWriter::sortRelocs(
std::vector<ELFRelocationEntry> &Relocs) {
// PATCHINST relocations should be applied last because they may overwrite the
// whole instruction and so should take precedence over other relocations that
// modify operands of the original instruction.
std::stable_partition(Relocs.begin(), Relocs.end(),
[](const ELFRelocationEntry &R) {
return R.Type != ELF::R_AARCH64_PATCHINST;
});
}

std::unique_ptr<MCObjectTargetWriter>
llvm::createAArch64ELFObjectWriter(uint8_t OSABI, bool IsILP32) {
return std::make_unique<AArch64ELFObjectWriter>(OSABI, IsILP32);
Expand Down
7 changes: 7 additions & 0 deletions llvm/test/MC/AArch64/patchinst.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// RUN: llvm-mc -triple aarch64-elf -filetype=obj %s -o - | llvm-objdump -r - | FileCheck %s

// Test that PATCHINST appears after JUMP26.
// CHECK: R_AARCH64_JUMP26
// CHECK-NEXT: R_AARCH64_PATCHINST
.reloc ., R_AARCH64_PATCHINST, ds
b f1
Copy link
Member

@MaskRay MaskRay Aug 9, 2025

Choose a reason for hiding this comment

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

Improve the test to test that with more than one fragments and more than one PATCHINST, the relocations are still ordered.

.reloc ., R_AARCH64_PATCHINST, ds
b f1
.balign 8
.reloc ., R_AARCH64_PATCHINST, ds
b f2

Copy link
Contributor Author

@pcc pcc Aug 11, 2025

Choose a reason for hiding this comment

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

Done in 55e71c0

Loading