From 245a456e0a24825cf1f9d0ca05c62f1af6d49ed2 Mon Sep 17 00:00:00 2001 From: Anatoly Trosinenko Date: Sat, 6 Sep 2025 14:08:12 +0300 Subject: [PATCH 1/3] [AArch64][PAC][GISel] Add missing clobbering info to LOADgotAUTH When LOADgotAUTH is selected by GlobalISel, the existing MachineInstr is updated in-place instead of constructing a fresh instance by calling MIB.buildInstr(). This way, implicit-def operands have to be added manually for register allocator to take them into account. This patch fixes miscompilation possibility observed when compiling with GlobalISel enabled or at -O0 optimization level. --- .../GISel/AArch64InstructionSelector.cpp | 11 ++++++--- .../AArch64/GlobalISel/ptrauth-elf-got.mir | 23 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 llvm/test/CodeGen/AArch64/GlobalISel/ptrauth-elf-got.mir diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp index 5748556d07285..b1843d92e2a93 100644 --- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp +++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp @@ -2914,10 +2914,15 @@ bool AArch64InstructionSelector::select(MachineInstr &I) { } if (OpFlags & AArch64II::MO_GOT) { - I.setDesc(TII.get(MF.getInfo()->hasELFSignedGOT() - ? AArch64::LOADgotAUTH - : AArch64::LOADgot)); + bool GOTIsSigned = MF.getInfo()->hasELFSignedGOT(); + I.setDesc(TII.get(GOTIsSigned ? AArch64::LOADgotAUTH : AArch64::LOADgot)); I.getOperand(1).setTargetFlags(OpFlags); + if (GOTIsSigned) { + MachineInstrBuilder MIB(MF, I); + MIB.addDef(AArch64::X16, RegState::Implicit); + MIB.addDef(AArch64::X17, RegState::Implicit); + MIB.addDef(AArch64::NZCV, RegState::Implicit); + } } else if (TM.getCodeModel() == CodeModel::Large && !TM.isPositionIndependent()) { // Materialize the global using movz/movk instructions. diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/ptrauth-elf-got.mir b/llvm/test/CodeGen/AArch64/GlobalISel/ptrauth-elf-got.mir new file mode 100644 index 0000000000000..faf2cb8221ec7 --- /dev/null +++ b/llvm/test/CodeGen/AArch64/GlobalISel/ptrauth-elf-got.mir @@ -0,0 +1,23 @@ +# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4 +# RUN: llc -O0 -mtriple=aarch64-linux-gnu -relocation-model=pic -run-pass=instruction-select -global-isel-abort=1 -verify-machineinstrs %s -o - | FileCheck %s + +--- | + @var_got = external global i8 + define ptr @loadgotauth_implicit_defs() { ret ptr null } + + !llvm.module.flags = !{!0} + !0 = !{i32 8, !"ptrauth-elf-got", i32 1} +... + +--- +name: loadgotauth_implicit_defs +legalized: true +regBankSelected: true +body: | + bb.0: + ; CHECK-LABEL: name: loadgotauth_implicit_defs + ; CHECK: [[LOADgotAUTH:%[0-9]+]]:gpr64common = LOADgotAUTH target-flags(aarch64-got) @var_got, implicit-def $x16, implicit-def $x17, implicit-def $nzcv + ; CHECK-NEXT: $x0 = COPY [[LOADgotAUTH]] + %0:gpr(p0) = G_GLOBAL_VALUE @var_got + $x0 = COPY %0(p0) +... From cafdddbddc73839e8064ca137e7598c6fc0ccd31 Mon Sep 17 00:00:00 2001 From: Anatoly Trosinenko Date: Mon, 8 Sep 2025 22:15:29 +0300 Subject: [PATCH 2/3] Address review comments --- .../AArch64/GISel/AArch64InstructionSelector.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp index b1843d92e2a93..3e5ce8461ff04 100644 --- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp +++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp @@ -2914,15 +2914,11 @@ bool AArch64InstructionSelector::select(MachineInstr &I) { } if (OpFlags & AArch64II::MO_GOT) { - bool GOTIsSigned = MF.getInfo()->hasELFSignedGOT(); - I.setDesc(TII.get(GOTIsSigned ? AArch64::LOADgotAUTH : AArch64::LOADgot)); + bool IsGOTSigned = MF.getInfo()->hasELFSignedGOT(); + I.setDesc(TII.get(IsGOTSigned ? AArch64::LOADgotAUTH : AArch64::LOADgot)); I.getOperand(1).setTargetFlags(OpFlags); - if (GOTIsSigned) { - MachineInstrBuilder MIB(MF, I); - MIB.addDef(AArch64::X16, RegState::Implicit); - MIB.addDef(AArch64::X17, RegState::Implicit); - MIB.addDef(AArch64::NZCV, RegState::Implicit); - } + if (IsGOTSigned) + I.addImplicitDefUseOperands(MF); } else if (TM.getCodeModel() == CodeModel::Large && !TM.isPositionIndependent()) { // Materialize the global using movz/movk instructions. From 4ef494702e0a0f7a9993e43701d4573e65fa69a0 Mon Sep 17 00:00:00 2001 From: Anatoly Trosinenko Date: Tue, 9 Sep 2025 13:22:29 +0300 Subject: [PATCH 3/3] Call addImplicitDefUseOperands unconditionally --- llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp index 3e5ce8461ff04..96cc3f3cac91c 100644 --- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp +++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp @@ -2917,8 +2917,7 @@ bool AArch64InstructionSelector::select(MachineInstr &I) { bool IsGOTSigned = MF.getInfo()->hasELFSignedGOT(); I.setDesc(TII.get(IsGOTSigned ? AArch64::LOADgotAUTH : AArch64::LOADgot)); I.getOperand(1).setTargetFlags(OpFlags); - if (IsGOTSigned) - I.addImplicitDefUseOperands(MF); + I.addImplicitDefUseOperands(MF); } else if (TM.getCodeModel() == CodeModel::Large && !TM.isPositionIndependent()) { // Materialize the global using movz/movk instructions.