Skip to content

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Sep 25, 2025

Backport cff5a43

Requested by: @atrosinenko

…m#157433)

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 machine passes to take them into account.

This patch fixes miscompilation possibility observed when compiling with
GlobalISel enabled or at -O0 optimization level.

(cherry picked from commit cff5a43)
@llvmbot
Copy link
Member Author

llvmbot commented Sep 25, 2025

@aemerson What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Sep 25, 2025

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-aarch64

Author: None (llvmbot)

Changes

Backport cff5a43

Requested by: @atrosinenko


Full diff: https://github.com/llvm/llvm-project/pull/160668.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp (+3-3)
  • (added) llvm/test/CodeGen/AArch64/GlobalISel/ptrauth-elf-got.mir (+23)
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index 1381a9b70df87..450915518cc2f 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -2977,10 +2977,10 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
     }
 
     if (OpFlags & AArch64II::MO_GOT) {
-      I.setDesc(TII.get(MF.getInfo<AArch64FunctionInfo>()->hasELFSignedGOT()
-                            ? AArch64::LOADgotAUTH
-                            : AArch64::LOADgot));
+      bool IsGOTSigned = MF.getInfo<AArch64FunctionInfo>()->hasELFSignedGOT();
+      I.setDesc(TII.get(IsGOTSigned ? AArch64::LOADgotAUTH : AArch64::LOADgot));
       I.getOperand(1).setTargetFlags(OpFlags);
+      I.addImplicitDefUseOperands(MF);
     } 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)
+...

@dyung
Copy link
Collaborator

dyung commented Oct 1, 2025

Hi @aemerson, can you review this patch and approve it if you feel it should be merged into the release branch?

@dyung dyung moved this from Needs Triage to Needs Review in LLVM Release Status Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Review
Development

Successfully merging this pull request may close these issues.

3 participants