https://github.com/aganea updated https://github.com/llvm/llvm-project/pull/229477
>From 4cd3f9adca65e04f24213c2f8f49eae8faba68ac Mon Sep 17 00:00:00 2001 From: Alexandre Ganea <[email protected]> Date: Tue, 6 Oct 2026 12:34:33 -0400 Subject: [PATCH 1/4] [Clang][AArch64] Prevent hotpatching from intercepting loop backedges AArch64 instructions are atomically patchable, but this alone does not satisfy the /hotpatch requirement that no internal branch target the first instruction of a function. When the entry block emits no code, a successor block can share the function's entry address, allowing a loop backedge to reach the instruction overwritten by a hotpatch. Propagate the patchable-function attribute to AArch64 and implement PATCHABLE_OP lowering. At emission time, insert a NOP when the entry block would otherwise emit no instructions, and conservatively pad entries beginning with inline assembly. Skip zero-byte instructions and Windows unwind directives, and emit matching unwind information when padding a prologue. Handle zero-length patchable-function-entry attributes without duplicating existing entry padding. Preserve AArch64 function alignment and existing x86 behavior. Fixes #228596. --- clang/lib/CodeGen/CodeGenFunction.cpp | 11 +- clang/test/CodeGen/patchable-function-entry.c | 1 + clang/test/DebugInfo/CXX/hotpatch-aarch64.cpp | 3 +- llvm/lib/CodeGen/PatchableFunction.cpp | 21 +++- llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 34 +++++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 4 + .../CodeGen/AArch64/patchable-prologue.ll | 116 ++++++++++++++++++ .../CodeGen/AArch64/patchable-prologue.mir | 78 ++++++++++++ 8 files changed, 256 insertions(+), 12 deletions(-) create mode 100644 llvm/test/CodeGen/AArch64/patchable-prologue.ll create mode 100644 llvm/test/CodeGen/AArch64/patchable-prologue.mir diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 62dade6cf6f49c..d7d41e6ce13617 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -963,12 +963,13 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy, if (!Section.empty()) Fn->addFnAttr("patchable-function-entry-section", Section); } - // Instruct that functions for COFF/CodeView targets should start with a - // patchable instruction, but only on x86/x64. Don't forward this to ARM/ARM64 - // backends as they don't need it -- instructions on these architectures are - // always atomically patchable at runtime. + // Ensure that the entry block contains a patchable instruction. AArch64 + // instructions are already atomically patchable, but an empty entry block + // would let branches to the next block target the function's first + // instruction. if (CGM.getCodeGenOpts().HotPatch && - getContext().getTargetInfo().getTriple().isX86() && + (getContext().getTargetInfo().getTriple().isX86() || + getContext().getTargetInfo().getTriple().isAArch64()) && getContext().getTargetInfo().getTriple().getEnvironment() != llvm::Triple::CODE16) Fn->addFnAttr("patchable-function", "prologue-short-redirect"); diff --git a/clang/test/CodeGen/patchable-function-entry.c b/clang/test/CodeGen/patchable-function-entry.c index 2acd748758490f..22af60d849a29f 100644 --- a/clang/test/CodeGen/patchable-function-entry.c +++ b/clang/test/CodeGen/patchable-function-entry.c @@ -1,6 +1,7 @@ // RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s // RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -fpatchable-function-entry=1 -o - | FileCheck --check-prefixes=CHECK,OPT %s // RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -fms-hotpatch -o - | FileCheck --check-prefixes=HOTPATCH %s +// RUN: %clang_cc1 -triple aarch64-pc-windows-msvc -emit-llvm %s -fms-hotpatch -o - | FileCheck --check-prefixes=HOTPATCH %s // CHECK: define{{.*}} void @f0() #0 __attribute__((patchable_function_entry(0))) void f0(void) {} diff --git a/clang/test/DebugInfo/CXX/hotpatch-aarch64.cpp b/clang/test/DebugInfo/CXX/hotpatch-aarch64.cpp index ff2dfc19961c08..6f125da8fe073a 100644 --- a/clang/test/DebugInfo/CXX/hotpatch-aarch64.cpp +++ b/clang/test/DebugInfo/CXX/hotpatch-aarch64.cpp @@ -1,7 +1,8 @@ // REQUIRES: aarch64-registered-target /// /// Check that using /hotpatch doesn't generate an error. -/// Binaries are always hotpatchable on ARM/ARM64. +/// ARM64 instructions are always atomically patchable, but /hotpatch still +/// ensures that the first instruction of a function is not a branch target. /// // RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /hotpatch /Z7 -- %s 2>&1 /// diff --git a/llvm/lib/CodeGen/PatchableFunction.cpp b/llvm/lib/CodeGen/PatchableFunction.cpp index 31ad4d851b19eb..cdb9daedc58ca8 100644 --- a/llvm/lib/CodeGen/PatchableFunction.cpp +++ b/llvm/lib/CodeGen/PatchableFunction.cpp @@ -20,6 +20,7 @@ #include "llvm/CodeGen/TargetSubtargetInfo.h" #include "llvm/InitializePasses.h" #include "llvm/Pass.h" +#include "llvm/Target/TargetMachine.h" using namespace llvm; @@ -58,14 +59,21 @@ PatchableFunctionPass::run(MachineFunction &MF, bool PatchableFunction::run(MachineFunction &MF) { MachineBasicBlock &FirstMBB = *MF.begin(); + bool IsAArch64 = MF.getTarget().getTargetTriple().isAArch64(); + bool HasPatchableEntry = + MF.getFunction().hasFnAttribute("patchable-function-entry"); - if (MF.getFunction().hasFnAttribute("patchable-function-entry")) { + if (HasPatchableEntry) { const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo(); // The initial .loc covers PATCHABLE_FUNCTION_ENTER. BuildMI(FirstMBB, FirstMBB.begin(), DebugLoc(), TII->get(TargetOpcode::PATCHABLE_FUNCTION_ENTER)); - return true; - } else if (MF.getFunction().hasFnAttribute("patchable-function")) { + } + + // On AArch64, PATCHABLE_OP only ensures the entry block isn't empty. It is + // needed even with patchable-function-entry, which may be "0" (N,N). + if (MF.getFunction().hasFnAttribute("patchable-function") && + (!HasPatchableEntry || IsAArch64)) { #ifndef NDEBUG Attribute PatchAttr = MF.getFunction().getFnAttribute("patchable-function"); StringRef PatchType = PatchAttr.getValueAsString(); @@ -74,11 +82,12 @@ bool PatchableFunction::run(MachineFunction &MF) { auto *TII = MF.getSubtarget().getInstrInfo(); BuildMI(FirstMBB, FirstMBB.begin(), DebugLoc(), TII->get(TargetOpcode::PATCHABLE_OP)) - .addImm(2); - MF.ensureAlignment(Align(16)); + .addImm(IsAArch64 ? 4 : 2); + if (!IsAArch64) + MF.ensureAlignment(Align(16)); return true; } - return false; + return HasPatchableEntry; } char PatchableFunctionLegacy::ID = 0; diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp index b26362491eab9a..b3bd9fb49eea34 100644 --- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp +++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp @@ -155,6 +155,7 @@ class AArch64AsmPrinter : public AsmPrinter { const MachineInstr &MI); void LowerFAULTING_OP(const MachineInstr &MI); + void LowerPATCHABLE_OP(const MachineInstr &MI); void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI); void LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr &MI); void LowerPATCHABLE_TAIL_CALL(const MachineInstr &MI); @@ -506,6 +507,35 @@ void AArch64AsmPrinter::emitFunctionHeaderComment() { OutStreamer->getCommentOS() << ' ' << OutlinerString; } +void AArch64AsmPrinter::LowerPATCHABLE_OP(const MachineInstr &MI) { + assert(MI.getOperand(0).getImm() == 4 && + "Expected a single patchable AArch64 instruction"); + + const MachineBasicBlock &MBB = *MI.getParent(); + const auto *TII = STI->getInstrInfo(); + auto NextMI = + std::find_if(std::next(MI.getIterator()), MBB.end().getInstrIterator(), + [&](const MachineInstr &Next) { + return Next.isInlineAsm() || + (!AArch64InstrInfo::isSEHInstruction(Next) && + TII->getInstSizeInBytes(Next) != 0); + }); + + // Every AArch64 instruction is atomically patchable. If this block emits no + // instructions, however, the next block would share the function's entry + // address. Pad it so a backedge cannot reach the hotpatch instruction. As on + // x86, conservatively pad inline assembly, whose first instruction may have + // a label targeted by branches within the assembly. + if (NextMI == MBB.end() || NextMI->isInlineAsm()) { + emitNops(1); + if (MF->hasWinCFI()) { + auto *TS = static_cast<AArch64TargetStreamer *>( + OutStreamer->getTargetStreamer()); + TS->emitARM64WinCFINop(); + } + } +} + void AArch64AsmPrinter::LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI) { const Function &F = MF->getFunction(); @@ -3913,6 +3943,10 @@ void AArch64AsmPrinter::emitInstruction(const MachineInstr *MI) { case TargetOpcode::FAULTING_OP: return LowerFAULTING_OP(*MI); + case TargetOpcode::PATCHABLE_OP: + LowerPATCHABLE_OP(*MI); + return; + case TargetOpcode::PATCHABLE_FUNCTION_ENTER: LowerPATCHABLE_FUNCTION_ENTER(*MI); return; diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 485ba3ffd82d37..78d824221fd0ac 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -276,6 +276,10 @@ unsigned AArch64InstrInfo::getInstSizeInBytes(const MachineInstr &MI) const { if (NumBytes == 0) NumBytes = 4; break; + case TargetOpcode::PATCHABLE_OP: + // Hotpatch padding emits at most one instruction. + NumBytes = 4; + break; case TargetOpcode::PATCHABLE_FUNCTION_ENTER: // If `patchable-function-entry` is set, PATCHABLE_FUNCTION_ENTER // instructions are expanded to the specified number of NOPs. Otherwise, diff --git a/llvm/test/CodeGen/AArch64/patchable-prologue.ll b/llvm/test/CodeGen/AArch64/patchable-prologue.ll new file mode 100644 index 00000000000000..ea262d407af5a8 --- /dev/null +++ b/llvm/test/CodeGen/AArch64/patchable-prologue.ll @@ -0,0 +1,116 @@ +; RUN: llc -mtriple=aarch64-pc-windows-msvc -O1 -verify-machineinstrs < %s | FileCheck %s +; RUN: llc -mtriple=aarch64-pc-windows-msvc -O3 -verify-machineinstrs < %s | FileCheck %s +; RUN: llc -mtriple=aarch64-pc-windows-msvc -verify-machineinstrs -filetype=obj -o %t < %s +; RUN: llvm-objdump -d %t | FileCheck %s --check-prefix=OBJ + +; OBJ-LABEL: <empty_entry>: +; OBJ-NEXT: {{[0-9a-f]+}}: {{[0-9a-f]+}} nop +; OBJ-NEXT: {{[0-9a-f]+}}: {{[0-9a-f]+}} ldr w8, [x0] +; OBJ: b.lo 0x4 <empty_entry+0x4> + +; The entry block becomes empty after register allocation. The loop backedge +; must skip the instruction that will be overwritten by a hotpatch. +define void @empty_entry(ptr %a, ptr %b) #0 { +; CHECK-LABEL: empty_entry: +; CHECK: // %bb.0: +; CHECK-NEXT: nop +; CHECK-NEXT: [[LOOP:\.LBB[0-9]+_[0-9]+]]: +; CHECK: ldr w8, [x0] +; CHECK: b.lo [[LOOP]] +entry: + br label %loop +loop: + %p = phi ptr [ %a, %entry ], [ %next, %loop ] + %value = load i32, ptr %p + %inc = add i32 %value, 1 + store i32 %inc, ptr %p + %next = getelementptr i32, ptr %p, i64 1 + %again = icmp ult ptr %next, %b + br i1 %again, label %loop, label %exit +exit: + ret void +} + +; An entry block that already emits an instruction needs no padding. +define void @nonempty_entry(ptr %a, ptr %b) #0 { +; CHECK-LABEL: nonempty_entry: +; CHECK-NOT: nop +; CHECK: ldr w8, [x0] +; CHECK-NOT: nop +; CHECK: [[LOOP:\.LBB[0-9]+_[0-9]+]]: +; CHECK: b.lo [[LOOP]] +entry: + %initial = load i32, ptr %a + br label %loop +loop: + %p = phi ptr [ %a, %entry ], [ %next, %loop ] + store i32 %initial, ptr %p + %next = getelementptr i32, ptr %p, i64 1 + %again = icmp ult ptr %next, %b + br i1 %again, label %loop, label %exit +exit: + ret void +} + +define void @return_only() #0 { +; CHECK-LABEL: return_only: +; CHECK: // %bb.0: +; CHECK-NEXT: ret + ret void +} + +; Inline assembly may emit no instructions, or define a label at its first +; instruction. Conservatively pad an entry that begins with inline assembly. +define void @inline_asm() #0 { +; CHECK-LABEL: inline_asm: +; CHECK: // %bb.0: +; CHECK-NEXT: nop +; CHECK-NEXT: //APP +; CHECK: //NO_APP +; CHECK-NEXT: ret + call void asm sideeffect "", ""() + ret void +} + +; A zero-length patchable-function-entry must not suppress hotpatch padding. +define void @zero_patchable_entry() #1 { +; CHECK-LABEL: zero_patchable_entry: +; CHECK: // %bb.0: +; CHECK-NEXT: nop +; CHECK-NEXT: [[LOOP:\.LBB[0-9]+_[0-9]+]]: +; CHECK: b [[LOOP]] +entry: + br label %loop +loop: + br label %loop +} + +; Existing patchable entry padding already separates the function entry from +; the loop header, so it must not acquire a second NOP. +define void @padded_entry() #2 { +; CHECK-LABEL: padded_entry: +; CHECK: // %bb.0: +; CHECK-NEXT: nop +; CHECK-NEXT: [[LOOP:\.LBB[0-9]+_[0-9]+]]: +; CHECK: b [[LOOP]] +entry: + br label %loop +loop: + br label %loop +} + +; Functions without the hotpatch attribute retain their original layout. +define void @unpatched_entry() { +; CHECK-LABEL: unpatched_entry: +; CHECK: // %bb.0: +; CHECK-NEXT: [[LOOP:\.LBB[0-9]+_[0-9]+]]: +; CHECK: b [[LOOP]] +entry: + br label %loop +loop: + br label %loop +} + +attributes #0 = { "patchable-function"="prologue-short-redirect" } +attributes #1 = { "patchable-function"="prologue-short-redirect" "patchable-function-entry"="0" } +attributes #2 = { "patchable-function"="prologue-short-redirect" "patchable-function-entry"="1" } diff --git a/llvm/test/CodeGen/AArch64/patchable-prologue.mir b/llvm/test/CodeGen/AArch64/patchable-prologue.mir new file mode 100644 index 00000000000000..5daed23ffb795b --- /dev/null +++ b/llvm/test/CodeGen/AArch64/patchable-prologue.mir @@ -0,0 +1,78 @@ +# RUN: llc -mtriple=aarch64-pc-windows-msvc -start-after=patchable-function -verify-machineinstrs %s -o - | FileCheck %s +# RUN: llc -mtriple=aarch64-pc-windows-msvc -start-after=patchable-function -verify-machineinstrs -filetype=obj %s -o %t +# RUN: llvm-readobj -u %t | FileCheck %s --check-prefix=UNWIND + +## Instructions that emit no bytes must not hide an empty entry block. +# CHECK-LABEL: meta_entry: +# CHECK: // %bb.0: +# CHECK-NEXT: nop +# CHECK-NEXT: // kill: +# CHECK-NEXT: // implicit-def: +# CHECK-NEXT: [[LOOP:\.LBB[0-9]+_[0-9]+]]: +# CHECK: b [[LOOP]] + +## Windows unwind directives also emit no bytes. A NOP inserted before the end +## of the prologue needs a corresponding unwind directive. +# CHECK-LABEL: seh_entry: +# CHECK: // %bb.0: +# CHECK-NEXT: nop +# CHECK-NEXT: .seh_nop +# CHECK-NEXT: .seh_endprologue +# CHECK-NEXT: [[LOOP:\.LBB[0-9]+_[0-9]+]]: +# CHECK: b [[LOOP]] + +## The prologue unwind codes must describe the NOP. +# UNWIND: Function: seh_entry +# UNWIND: Prologue [ +# UNWIND-NEXT: 0xe3 ; nop +# UNWIND-NEXT: 0xe4 ; end +# UNWIND-NEXT: ] + +--- | + define void @meta_entry() { + entry: + br label %loop + loop: + br label %loop + } + + define void @seh_entry() uwtable { + entry: + br label %loop + loop: + br label %loop + } +... +--- +name: meta_entry +alignment: 4 +tracksRegLiveness: true +body: | + bb.0.entry: + successors: %bb.1 + liveins: $x0 + + PATCHABLE_OP 4 + KILL $x0 + $x1 = IMPLICIT_DEF + + bb.1.loop: + successors: %bb.1 + B %bb.1 +... +--- +name: seh_entry +alignment: 4 +tracksRegLiveness: true +hasWinCFI: true +body: | + bb.0.entry: + successors: %bb.1 + + PATCHABLE_OP 4 + frame-setup SEH_PrologEnd + + bb.1.loop: + successors: %bb.1 + B %bb.1 +... >From 26d2c29b7aefaa417267757d5d0bc486a284c24b Mon Sep 17 00:00:00 2001 From: Alexandre Ganea <[email protected]> Date: Tue, 6 Oct 2026 13:01:25 -0400 Subject: [PATCH 2/4] Fix hotpatch metadata in debug information --- clang/test/DebugInfo/CXX/hotpatch-aarch64.cpp | 48 +++++++++++++------ clang/test/DebugInfo/CXX/hotpatch-arm.cpp | 2 +- llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp | 3 +- llvm/test/MC/AArch64/coff-debug.ll | 5 +- 4 files changed, 38 insertions(+), 20 deletions(-) diff --git a/clang/test/DebugInfo/CXX/hotpatch-aarch64.cpp b/clang/test/DebugInfo/CXX/hotpatch-aarch64.cpp index 6f125da8fe073a..b145ec22058909 100644 --- a/clang/test/DebugInfo/CXX/hotpatch-aarch64.cpp +++ b/clang/test/DebugInfo/CXX/hotpatch-aarch64.cpp @@ -1,21 +1,41 @@ // REQUIRES: aarch64-registered-target -/// -/// Check that using /hotpatch doesn't generate an error. -/// ARM64 instructions are always atomically patchable, but /hotpatch still -/// ensures that the first instruction of a function is not a branch target. -/// -// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /hotpatch /Z7 -- %s 2>&1 -/// -/// Ensure that we set the hotpatchable flag in the debug information. -/// -// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /Z7 -o %t.obj -- %s +// +// ARM64 instructions are atomically patchable, but /hotpatch is needed to +// ensure that the first instruction of a function is not a branch target. +// The CodeView hotpatchable flag must match this protection, with or without +// debug information. +// +// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /clang:-O1 /hotpatch /Z7 -o %t.obj -- %s +// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH +// RUN: llvm-objdump -d --no-show-raw-insn %t.obj | FileCheck %s --check-prefix=HOTPATCH-CODE + +// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /clang:-O1 /hotpatch -o %t.obj -- %s // RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH +// RUN: llvm-objdump -d --no-show-raw-insn %t.obj | FileCheck %s --check-prefix=HOTPATCH-CODE + +// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /clang:-O1 /Z7 -o %t.obj -- %s +// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=NO-HOTPATCH +// RUN: llvm-objdump -d --no-show-raw-insn %t.obj | FileCheck %s --check-prefix=NO-HOTPATCH-CODE + +// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /clang:-O1 -o %t.obj -- %s +// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=NO-HOTPATCH +// RUN: llvm-objdump -d --no-show-raw-insn %t.obj | FileCheck %s --check-prefix=NO-HOTPATCH-CODE +// // HOTPATCH: S_COMPILE3 [size = [[#]]] // HOTPATCH: flags = hot patchable +// NO-HOTPATCH: S_COMPILE3 [size = [[#]]] +// NO-HOTPATCH: flags = none // -// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c -o %t.obj -- %s -// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH +// HOTPATCH-CODE-LABEL: <loop>: +// HOTPATCH-CODE-NEXT: 0: nop +// HOTPATCH-CODE-NEXT: 4: ldr w8, [x0] +// HOTPATCH-CODE: b.lo 0x4 <loop+0x4> +// NO-HOTPATCH-CODE-LABEL: <loop>: +// NO-HOTPATCH-CODE-NEXT: 0: ldr w8, [x0] +// NO-HOTPATCH-CODE: b.lo 0x0 <loop> -int main() { - return 0; +extern "C" void loop(int *a, int *b) { + do { + ++*a++; + } while (a < b); } diff --git a/clang/test/DebugInfo/CXX/hotpatch-arm.cpp b/clang/test/DebugInfo/CXX/hotpatch-arm.cpp index e31c762b08872f..3b0411233d41d5 100644 --- a/clang/test/DebugInfo/CXX/hotpatch-arm.cpp +++ b/clang/test/DebugInfo/CXX/hotpatch-arm.cpp @@ -1,7 +1,7 @@ // REQUIRES: arm-registered-target /// /// Check that using /hotpatch doesn't generate an error. -/// Binaries are always hotpatchable on ARM/ARM64. +/// Binaries are always marked hotpatchable on ARM. /// // RUN: %clang_cl --target=arm-pc-windows-msvc /c /hotpatch /Z7 -- %s 2>&1 /// diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp index 84ea5e349f01d1..e93fd8ba225f46 100644 --- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp @@ -894,8 +894,7 @@ void CodeViewDebug::emitCompilerInformation() { } using ArchType = llvm::Triple::ArchType; ArchType Arch = MMI->getModule()->getTargetTriple().getArch(); - if (CompilerInfoAsm->TM.Options.Hotpatch || Arch == ArchType::thumb || - Arch == ArchType::aarch64) { + if (CompilerInfoAsm->TM.Options.Hotpatch || Arch == ArchType::thumb) { Flags |= static_cast<uint32_t>(CompileSym3Flags::HotPatch); } diff --git a/llvm/test/MC/AArch64/coff-debug.ll b/llvm/test/MC/AArch64/coff-debug.ll index ea1cc3286b0429..ffecdd5ba081e4 100644 --- a/llvm/test/MC/AArch64/coff-debug.ll +++ b/llvm/test/MC/AArch64/coff-debug.ll @@ -77,9 +77,8 @@ attributes #0 = { noinline nounwind optnone "correctly-rounded-divide-sqrt-fp-ma ; CHECK: Compile3Sym { ; CHECK: Kind: S_COMPILE3 (0x113C) ; CHECK: Language: C (0x0) -; CHECK: Flags [ (0x4000 -; CHECK: HotPatch (0x4000) -; CHECK: ] +; CHECK: Flags [ (0x0) +; CHECK-NEXT: ] ; CHECK: } ; CHECK: ] ; CHECK: Subsection [ >From d6bb78f7bf096c3b0247a04806e7dffc92ce4cfe Mon Sep 17 00:00:00 2001 From: Alexandre Ganea <[email protected]> Date: Tue, 6 Oct 2026 16:31:54 -0400 Subject: [PATCH 3/4] [AArch64] Place hotpatch padding after empty Windows prologues Insert hotpatch NOPs directly in the PatchableFunction pass, after zero-byte instructions and before inline assembly or the next block. This keeps padding outside empty Windows unwind prologues, avoiding unnecessary unwind codes while preserving loop backedge protection. Report zero size for AArch64 SEH directives and remove the AArch64 PATCHABLE_OP lowering. --- llvm/lib/CodeGen/PatchableFunction.cpp | 50 ++++++++--- llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp | 34 ------- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 8 +- .../CodeGen/AArch64/patchable-prologue.mir | 90 ++++++++++++++++--- 4 files changed, 119 insertions(+), 63 deletions(-) diff --git a/llvm/lib/CodeGen/PatchableFunction.cpp b/llvm/lib/CodeGen/PatchableFunction.cpp index cdb9daedc58ca8..1f9af982654107 100644 --- a/llvm/lib/CodeGen/PatchableFunction.cpp +++ b/llvm/lib/CodeGen/PatchableFunction.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "llvm/CodeGen/PatchableFunction.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/CodeGen/MachineFunction.h" #include "llvm/CodeGen/MachineFunctionPass.h" #include "llvm/CodeGen/MachineInstrBuilder.h" @@ -57,37 +58,62 @@ PatchableFunctionPass::run(MachineFunction &MF, return getMachineFunctionPassPreservedAnalyses(); } +// AArch64 instructions are atomically patchable, but an entry block that emits +// no code lets a branch to the next block (e.g. a loop backedge) target the +// function's first instruction. Pad it with a NOP, placed after any zero-byte +// instructions (e.g. an empty Windows prologue). Like x86, also pad when the +// entry starts with inline assembly, which may define a branch target. +static bool padEmptyEntryBlock(MachineFunction &MF) { + MachineBasicBlock &MBB = MF.front(); + const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo(); + auto FirstEmitting = llvm::find_if(MBB, [&](const MachineInstr &MI) { + return MI.isInlineAsm() || TII->getInstSizeInBytes(MI) != 0; + }); + if (FirstEmitting != MBB.end() && !FirstEmitting->isInlineAsm()) + return false; + TII->insertNoop(MBB, FirstEmitting); + return true; +} + bool PatchableFunction::run(MachineFunction &MF) { MachineBasicBlock &FirstMBB = *MF.begin(); bool IsAArch64 = MF.getTarget().getTargetTriple().isAArch64(); bool HasPatchableEntry = MF.getFunction().hasFnAttribute("patchable-function-entry"); + bool Changed = false; if (HasPatchableEntry) { const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo(); // The initial .loc covers PATCHABLE_FUNCTION_ENTER. BuildMI(FirstMBB, FirstMBB.begin(), DebugLoc(), TII->get(TargetOpcode::PATCHABLE_FUNCTION_ENTER)); + Changed = true; } - // On AArch64, PATCHABLE_OP only ensures the entry block isn't empty. It is - // needed even with patchable-function-entry, which may be "0" (N,N). - if (MF.getFunction().hasFnAttribute("patchable-function") && - (!HasPatchableEntry || IsAArch64)) { + if (!MF.getFunction().hasFnAttribute("patchable-function")) + return Changed; + #ifndef NDEBUG - Attribute PatchAttr = MF.getFunction().getFnAttribute("patchable-function"); - StringRef PatchType = PatchAttr.getValueAsString(); - assert(PatchType == "prologue-short-redirect" && "Only possibility today!"); + Attribute PatchAttr = MF.getFunction().getFnAttribute("patchable-function"); + StringRef PatchType = PatchAttr.getValueAsString(); + assert(PatchType == "prologue-short-redirect" && "Only possibility today!"); #endif + + // On AArch64 this is needed even with patchable-function-entry, which may be + // "0" (N,N), and the padding must be computed after PATCHABLE_FUNCTION_ENTER + // has been inserted so that its size is accounted for. + if (IsAArch64) + return padEmptyEntryBlock(MF) || Changed; + + if (!HasPatchableEntry) { auto *TII = MF.getSubtarget().getInstrInfo(); BuildMI(FirstMBB, FirstMBB.begin(), DebugLoc(), TII->get(TargetOpcode::PATCHABLE_OP)) - .addImm(IsAArch64 ? 4 : 2); - if (!IsAArch64) - MF.ensureAlignment(Align(16)); - return true; + .addImm(2); + MF.ensureAlignment(Align(16)); + Changed = true; } - return HasPatchableEntry; + return Changed; } char PatchableFunctionLegacy::ID = 0; diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp index b3bd9fb49eea34..b26362491eab9a 100644 --- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp +++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp @@ -155,7 +155,6 @@ class AArch64AsmPrinter : public AsmPrinter { const MachineInstr &MI); void LowerFAULTING_OP(const MachineInstr &MI); - void LowerPATCHABLE_OP(const MachineInstr &MI); void LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI); void LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr &MI); void LowerPATCHABLE_TAIL_CALL(const MachineInstr &MI); @@ -507,35 +506,6 @@ void AArch64AsmPrinter::emitFunctionHeaderComment() { OutStreamer->getCommentOS() << ' ' << OutlinerString; } -void AArch64AsmPrinter::LowerPATCHABLE_OP(const MachineInstr &MI) { - assert(MI.getOperand(0).getImm() == 4 && - "Expected a single patchable AArch64 instruction"); - - const MachineBasicBlock &MBB = *MI.getParent(); - const auto *TII = STI->getInstrInfo(); - auto NextMI = - std::find_if(std::next(MI.getIterator()), MBB.end().getInstrIterator(), - [&](const MachineInstr &Next) { - return Next.isInlineAsm() || - (!AArch64InstrInfo::isSEHInstruction(Next) && - TII->getInstSizeInBytes(Next) != 0); - }); - - // Every AArch64 instruction is atomically patchable. If this block emits no - // instructions, however, the next block would share the function's entry - // address. Pad it so a backedge cannot reach the hotpatch instruction. As on - // x86, conservatively pad inline assembly, whose first instruction may have - // a label targeted by branches within the assembly. - if (NextMI == MBB.end() || NextMI->isInlineAsm()) { - emitNops(1); - if (MF->hasWinCFI()) { - auto *TS = static_cast<AArch64TargetStreamer *>( - OutStreamer->getTargetStreamer()); - TS->emitARM64WinCFINop(); - } - } -} - void AArch64AsmPrinter::LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI) { const Function &F = MF->getFunction(); @@ -3943,10 +3913,6 @@ void AArch64AsmPrinter::emitInstruction(const MachineInstr *MI) { case TargetOpcode::FAULTING_OP: return LowerFAULTING_OP(*MI); - case TargetOpcode::PATCHABLE_OP: - LowerPATCHABLE_OP(*MI); - return; - case TargetOpcode::PATCHABLE_FUNCTION_ENTER: LowerPATCHABLE_FUNCTION_ENTER(*MI); return; diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index 78d824221fd0ac..d9a1d03fad1d23 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -246,6 +246,10 @@ unsigned AArch64InstrInfo::getInstSizeInBytes(const MachineInstr &MI) const { return NumBytes; } + // Windows unwind directives describe the code but emit no bytes. + if (isSEHInstruction(MI)) + return 0; + // Size should be preferably set in // llvm/lib/Target/AArch64/AArch64InstrInfo.td (default case). // Specific cases handle instructions of variable sizes @@ -276,10 +280,6 @@ unsigned AArch64InstrInfo::getInstSizeInBytes(const MachineInstr &MI) const { if (NumBytes == 0) NumBytes = 4; break; - case TargetOpcode::PATCHABLE_OP: - // Hotpatch padding emits at most one instruction. - NumBytes = 4; - break; case TargetOpcode::PATCHABLE_FUNCTION_ENTER: // If `patchable-function-entry` is set, PATCHABLE_FUNCTION_ENTER // instructions are expanded to the specified number of NOPs. Otherwise, diff --git a/llvm/test/CodeGen/AArch64/patchable-prologue.mir b/llvm/test/CodeGen/AArch64/patchable-prologue.mir index 5daed23ffb795b..d18e5777e4f2ad 100644 --- a/llvm/test/CodeGen/AArch64/patchable-prologue.mir +++ b/llvm/test/CodeGen/AArch64/patchable-prologue.mir @@ -1,47 +1,90 @@ -# RUN: llc -mtriple=aarch64-pc-windows-msvc -start-after=patchable-function -verify-machineinstrs %s -o - | FileCheck %s -# RUN: llc -mtriple=aarch64-pc-windows-msvc -start-after=patchable-function -verify-machineinstrs -filetype=obj %s -o %t +# RUN: llc -mtriple=aarch64-pc-windows-msvc -start-before=patchable-function -verify-machineinstrs %s -o - | FileCheck %s +# RUN: llc -mtriple=aarch64-pc-windows-msvc -start-before=patchable-function -verify-machineinstrs -filetype=obj %s -o %t # RUN: llvm-readobj -u %t | FileCheck %s --check-prefix=UNWIND +# RUN: llvm-objdump -d --no-show-raw-insn %t | FileCheck %s --check-prefix=OBJ ## Instructions that emit no bytes must not hide an empty entry block. # CHECK-LABEL: meta_entry: # CHECK: // %bb.0: -# CHECK-NEXT: nop # CHECK-NEXT: // kill: # CHECK-NEXT: // implicit-def: +# CHECK-NEXT: nop # CHECK-NEXT: [[LOOP:\.LBB[0-9]+_[0-9]+]]: # CHECK: b [[LOOP]] -## Windows unwind directives also emit no bytes. A NOP inserted before the end -## of the prologue needs a corresponding unwind directive. +## Windows unwind directives also emit no bytes. The NOP is placed after the end +## of an empty prologue, so it needs no corresponding unwind directive. # CHECK-LABEL: seh_entry: # CHECK: // %bb.0: -# CHECK-NEXT: nop -# CHECK-NEXT: .seh_nop +# CHECK-NEXT: // kill: # CHECK-NEXT: .seh_endprologue +# CHECK-NEXT: nop # CHECK-NEXT: [[LOOP:\.LBB[0-9]+_[0-9]+]]: # CHECK: b [[LOOP]] -## The prologue unwind codes must describe the NOP. +## The NOP is still the first instruction, and the backedge must skip it. +# OBJ-LABEL: <seh_entry>: +# OBJ-NEXT: {{[0-9a-f]+}}: nop +# OBJ-NEXT: {{[0-9a-f]+}}: b {{.*}} <seh_entry+0x4> + +## No NOP unwind code is needed for padding outside the prologue. # UNWIND: Function: seh_entry # UNWIND: Prologue [ -# UNWIND-NEXT: 0xe3 ; nop # UNWIND-NEXT: 0xe4 ; end # UNWIND-NEXT: ] +## Padding must still precede inline assembly, which may begin with a branch +## target. +# CHECK-LABEL: seh_inline_asm_entry: +# CHECK: // %bb.0: +# CHECK-NEXT: .seh_endprologue +# CHECK-NEXT: nop +# CHECK-NEXT: //APP +# CHECK: //NO_APP +# CHECK-NEXT: ret + +# OBJ-LABEL: <seh_inline_asm_entry>: +# OBJ-NEXT: {{[0-9a-f]+}}: nop +# OBJ-NEXT: {{[0-9a-f]+}}: b {{.*}} <seh_inline_asm_entry+0x4> +# OBJ-NEXT: {{[0-9a-f]+}}: ret + +# UNWIND: Function: seh_inline_asm_entry +# UNWIND: Prologue [ +# UNWIND-NEXT: 0xe4 ; end +# UNWIND-NEXT: ] + +## An entry that already emits an instruction must not acquire padding. +# CHECK-LABEL: seh_nonempty_entry: +# CHECK: // %bb.0: +# CHECK-NEXT: .seh_endprologue +# CHECK-NEXT: ret + +# OBJ-LABEL: <seh_nonempty_entry>: +# OBJ-NEXT: {{[0-9a-f]+}}: ret + --- | - define void @meta_entry() { + define void @meta_entry() "patchable-function"="prologue-short-redirect" { entry: br label %loop loop: br label %loop } - define void @seh_entry() uwtable { + define void @seh_entry() uwtable "patchable-function"="prologue-short-redirect" { entry: br label %loop loop: br label %loop } + + define void @seh_inline_asm_entry() uwtable "patchable-function"="prologue-short-redirect" { + call void asm sideeffect "1: b 1b", ""() + ret void + } + + define void @seh_nonempty_entry() uwtable "patchable-function"="prologue-short-redirect" { + ret void + } ... --- name: meta_entry @@ -52,7 +95,6 @@ body: | successors: %bb.1 liveins: $x0 - PATCHABLE_OP 4 KILL $x0 $x1 = IMPLICIT_DEF @@ -68,11 +110,33 @@ hasWinCFI: true body: | bb.0.entry: successors: %bb.1 + liveins: $x0 - PATCHABLE_OP 4 + KILL $x0 frame-setup SEH_PrologEnd bb.1.loop: successors: %bb.1 B %bb.1 ... +--- +name: seh_inline_asm_entry +alignment: 4 +tracksRegLiveness: true +hasWinCFI: true +body: | + bb.0: + frame-setup SEH_PrologEnd + INLINEASM &"1: b 1b", 1 + RET undef $lr +... +--- +name: seh_nonempty_entry +alignment: 4 +tracksRegLiveness: true +hasWinCFI: true +body: | + bb.0: + frame-setup SEH_PrologEnd + RET undef $lr +... >From 4325813c6c59cc1024ce8c21ff6deaa4c7c83367 Mon Sep 17 00:00:00 2001 From: Alexandre Ganea <[email protected]> Date: Tue, 6 Oct 2026 16:56:17 -0400 Subject: [PATCH 4/4] Better pattern match the addresses --- llvm/test/CodeGen/AArch64/patchable-prologue.mir | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/llvm/test/CodeGen/AArch64/patchable-prologue.mir b/llvm/test/CodeGen/AArch64/patchable-prologue.mir index d18e5777e4f2ad..d88167cdad3bbb 100644 --- a/llvm/test/CodeGen/AArch64/patchable-prologue.mir +++ b/llvm/test/CodeGen/AArch64/patchable-prologue.mir @@ -24,8 +24,8 @@ ## The NOP is still the first instruction, and the backedge must skip it. # OBJ-LABEL: <seh_entry>: -# OBJ-NEXT: {{[0-9a-f]+}}: nop -# OBJ-NEXT: {{[0-9a-f]+}}: b {{.*}} <seh_entry+0x4> +# OBJ-NEXT: [[#%x,ENTRY:]]: nop +# OBJ-NEXT: [[#ENTRY + 4]]: b 0x[[#ENTRY + 4]] <seh_entry+0x4> ## No NOP unwind code is needed for padding outside the prologue. # UNWIND: Function: seh_entry @@ -44,9 +44,9 @@ # CHECK-NEXT: ret # OBJ-LABEL: <seh_inline_asm_entry>: -# OBJ-NEXT: {{[0-9a-f]+}}: nop -# OBJ-NEXT: {{[0-9a-f]+}}: b {{.*}} <seh_inline_asm_entry+0x4> -# OBJ-NEXT: {{[0-9a-f]+}}: ret +# OBJ-NEXT: [[#%x,ASM_ENTRY:]]: nop +# OBJ-NEXT: [[#ASM_ENTRY + 4]]: b 0x[[#ASM_ENTRY + 4]] <seh_inline_asm_entry+0x4> +# OBJ-NEXT: [[#ASM_ENTRY + 8]]: ret # UNWIND: Function: seh_inline_asm_entry # UNWIND: Prologue [ @@ -60,7 +60,7 @@ # CHECK-NEXT: ret # OBJ-LABEL: <seh_nonempty_entry>: -# OBJ-NEXT: {{[0-9a-f]+}}: ret +# OBJ-NEXT: [[#%x,]]: ret --- | define void @meta_entry() "patchable-function"="prologue-short-redirect" { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
