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 10 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: 11 additions & 1 deletion lld/ELF/Arch/AArch64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ AArch64::AArch64(Ctx &ctx) : TargetInfo(ctx) {
copyRel = R_AARCH64_COPY;
relativeRel = R_AARCH64_RELATIVE;
iRelativeRel = R_AARCH64_IRELATIVE;
iRelSymbolicRel = R_AARCH64_FUNCINIT64;
gotRel = R_AARCH64_GLOB_DAT;
pltRel = R_AARCH64_JUMP_SLOT;
symbolicRel = R_AARCH64_ABS64;
Expand All @@ -136,7 +137,9 @@ RelExpr AArch64::getRelExpr(RelType type, const Symbol &s,
switch (type) {
case R_AARCH64_ABS16:
case R_AARCH64_ABS32:
case R_AARCH64_PATCHINST:
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.

If the relocation is used as described in the RFC, with the target symbol absolute all looks well. If a non ABS symbol is used in a position independent context I think you'll get a recompile with PIC error message. If not position independent then I think the relocation will pass through and give a likely unpredictable value to patch the instruction.

Could it be worth putting some checking (likely in Relocation.cpp) to make sure the target symbol is SHN_ABS?

A related question is whether non-zero addends should be supported? For most instructions the addend doesn't make logical sense. There could be some instructions where the immediate field is in the right place to make it work, but I think these cases may not be worth supporting.

An alternative formulation for RELA is to completely ignore the symbol value and just use the bottom 32-bits of the addend field. That would give the object producer that defines the relocations more control over what is patched in. Not got a strong opinion on whether that is better though.

Copy link
Contributor Author

@pcc pcc Jul 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Could it be worth putting some checking (likely in Relocation.cpp) to make sure the target symbol is SHN_ABS?

That makes sense. getRelExpr in Arch/AArch64.cpp seems like a slightly better place since it's arch-specific semantics, so I added it there.

A related question is whether non-zero addends should be supported? For most instructions the addend doesn't make logical sense. There could be some instructions where the immediate field is in the right place to make it work, but I think these cases may not be worth supporting.

An alternative formulation for RELA is to completely ignore the symbol value and just use the bottom 32-bits of the addend field. That would give the object producer that defines the relocations more control over what is patched in. Not got a strong opinion on whether that is better though.

This could be useful for something like an hypothetical PAC variant that takes three operands like most other instructions:

pacda x0, x1, x2

meaning sign x1 using x2 and store the result in x0. Then we could encode the instruction

mov x0, x1

in the relocation addend and have the object defining the deactivation symbol define it to absolute 0. And then of course you can have the replacement instruction use whichever registers the original instruction did (or replace it entirely with NOP if the source and destination registers are the same). This doesn't seem useful for PAC given how the existing instructions work but maybe there's a non-PAC use case for this.

It doesn't seem harmful to have the flexibility of letting the instruction be defined by the symbol, the relocation or both. On the psABI issue we discussed restrictions for things like erratum fixes but for that purpose it doesn't seem to matter how the instruction word is computed. We can make it the responsibility of the object file producer(s) to ensure that the instruction formed by value+addend follows the rules.

case R_AARCH64_ABS64:
case R_AARCH64_FUNCINIT64:
case R_AARCH64_ADD_ABS_LO12_NC:
case R_AARCH64_LDST128_ABS_LO12_NC:
case R_AARCH64_LDST16_ABS_LO12_NC:
Expand Down Expand Up @@ -261,7 +264,8 @@ bool AArch64::usesOnlyLowPageBits(RelType type) const {
}

RelType AArch64::getDynRel(RelType type) const {
if (type == R_AARCH64_ABS64 || type == R_AARCH64_AUTH_ABS64)
if (type == R_AARCH64_ABS64 || type == R_AARCH64_AUTH_ABS64 ||
type == R_AARCH64_FUNCINIT64)
return type;
return R_AARCH64_NONE;
}
Expand Down Expand Up @@ -506,6 +510,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
22 changes: 20 additions & 2 deletions lld/ELF/Relocations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -990,8 +990,8 @@ bool RelocationScanner::isStaticLinkTimeConstant(RelExpr e, RelType type,
// only the low bits are used.
if (e == R_GOT || e == R_PLT)
return ctx.target->usesOnlyLowPageBits(type) || !ctx.arg.isPic;
// R_AARCH64_AUTH_ABS64 requires a dynamic relocation.
if (e == RE_AARCH64_AUTH)
// R_AARCH64_AUTH_ABS64 and iRelSymbolicRel require a dynamic relocation.
if (e == RE_AARCH64_AUTH || type == ctx.target->iRelSymbolicRel)
return false;

// The behavior of an undefined weak reference is implementation defined.
Expand Down Expand Up @@ -1165,6 +1165,24 @@ void RelocationScanner::processAux(RelExpr expr, RelType type, uint64_t offset,
}
return;
}
if (LLVM_UNLIKELY(type == ctx.target->iRelSymbolicRel)) {
if (sym.isPreemptible) {
auto diag = Err(ctx);
diag << "relocation " << type
<< " cannot be used against preemptible symbol '" << &sym << "'";
printLocation(diag, *sec, sym, offset);
} else if (isIfunc) {
auto diag = Err(ctx);
diag << "relocation " << type
<< " cannot be used against ifunc symbol '" << &sym << "'";
printLocation(diag, *sec, sym, offset);
} else {
part.relaDyn->addReloc({ctx.target->iRelativeRel, sec, offset,
DynamicReloc::AddendOnlyWithTargetVA, sym,
addend, R_ABS});
return;
}
}
part.relaDyn->addSymbolReloc(rel, *sec, offset, sym, addend, type);

// MIPS ABI turns using of GOT and dynamic relocations inside out.
Expand Down
1 change: 1 addition & 0 deletions lld/ELF/Target.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class TargetInfo {
RelType relativeRel = 0;
RelType iRelativeRel = 0;
RelType symbolicRel = 0;
RelType iRelSymbolicRel = 0;
RelType tlsDescRel = 0;
RelType tlsGotRel = 0;
RelType tlsModuleIndexRel = 0;
Expand Down
18 changes: 18 additions & 0 deletions lld/test/ELF/aarch64-funcinit64-invalid.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# REQUIRES: aarch64

# RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t.o
# RUN: not ld.lld %t.o -o %t 2>&1 | FileCheck --check-prefix=ERR %s

.rodata
# ERR: relocation R_AARCH64_FUNCINIT64 cannot be used against local symbol
.8byte func@FUNCINIT

.data
# ERR: relocation R_AARCH64_FUNCINIT64 cannot be used against ifunc symbol 'ifunc'
.8byte ifunc@FUNCINIT

.text
func:
.type ifunc, @gnu_indirect_function
ifunc:
ret
19 changes: 19 additions & 0 deletions lld/test/ELF/aarch64-funcinit64.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# REQUIRES: aarch64

# RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t.o
# RUN: ld.lld %t.o -o %t
# RUN: llvm-readelf -s -r %t | FileCheck %s
# RUN: ld.lld %t.o -o %t -pie
# RUN: llvm-readelf -s -r %t | FileCheck %s
# RUN: not ld.lld %t.o -o %t -shared 2>&1 | FileCheck --check-prefix=ERR %s

.data
# CHECK: R_AARCH64_IRELATIVE [[FOO:[0-9a-f]*]]
# ERR: relocation R_AARCH64_FUNCINIT64 cannot be used against preemptible symbol 'foo'
.8byte foo@FUNCINIT

.text
# CHECK: {{0*}}[[FOO]] {{.*}} foo
.globl foo
foo:
ret
41 changes: 41 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,41 @@
# 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

## Deactivation symbol used without being defined: instruction emitted as usual.
# RUN: ld.lld -o %t/undef-le %t/use-le.o
# 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
# RUN: llvm-objdump -d %t/def-le | FileCheck --check-prefix=DEF %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: ld.lld -o %t/undef-be %t/use-be.o
# 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
# RUN: llvm-objdump -d %t/def-be | FileCheck --check-prefix=DEF %s

#--- use.s
.weak ds
# This instruction has a single relocation: the DS relocation.
# UNDEF: add x0, x1, x2
# DEF: nop
.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
b f1

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

#--- def.s
.globl ds
ds = 0xd503201f
2 changes: 2 additions & 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 Expand Up @@ -140,6 +141,7 @@ ELF_RELOC(R_AARCH64_TLS_DTPREL64, 0x405)
ELF_RELOC(R_AARCH64_TLS_TPREL64, 0x406)
ELF_RELOC(R_AARCH64_TLSDESC, 0x407)
ELF_RELOC(R_AARCH64_IRELATIVE, 0x408)
ELF_RELOC(R_AARCH64_FUNCINIT64, 0x409)
// PAuthABI static and dynamic relocations: defined in pauthabielf64,
// https://github.com/ARM-software/abi-aa
ELF_RELOC(R_AARCH64_AUTH_ABS64, 0x244)
Expand Down
Loading
Loading