@llvm/pr-subscribers-backend-aarch64

Author: Oskar Wirga (oskarwirga)

diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index 2b8db27599d3c..47281791ff7a0 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -2680,9 +2680,13 @@ AArch64AsmPrinter::lowerConstantPtrAuth(const ConstantPtrAuth &CPA) {
   MCContext &Ctx = OutContext;
 
   // Figure out the base symbol and the addend, if any.
+  // LookThroughIntToPtr handles Swift patterns like:
+  //   inttoptr (i64 add (i64 ptrtoint (ptr @global to i64), i64 2) to ptr)
   APInt Offset(64, 0);
   const Value *BaseGV = CPA.getPointer()->stripAndAccumulateConstantOffsets(
-      getDataLayout(), Offset, /*AllowNonInbounds=*/true);
+      getDataLayout(), Offset, /*AllowNonInbounds=*/true,
+      /*AllowInvariantGroup=*/false, /*ExternalAnalysis=*/nullptr,
+      /*LookThroughIntToPtr=*/true);
 
   auto *BaseGVB = dyn_cast<GlobalValue>(BaseGV);
 
diff --git a/llvm/test/CodeGen/AArch64/ptrauth-reloc.ll b/llvm/test/CodeGen/AArch64/ptrauth-reloc.ll
index 14f5571fc2deb..befb726bcdab8 100644
--- a/llvm/test/CodeGen/AArch64/ptrauth-reloc.ll
+++ b/llvm/test/CodeGen/AArch64/ptrauth-reloc.ll
@@ -113,6 +113,20 @@
 
 @g.weird_ref.da.0 = constant i64 ptrtoint (ptr inttoptr (i64 ptrtoint (ptr ptrauth (ptr getelementptr (i8, ptr @g, i64 16), i32 2) to i64) to ptr) to i64)
 
+; Swift generates inttoptr(add(ptrtoint(@global), offset)) inside ptrauth.
+
+; CHECK-ELF-LABEL:     .globl g.inttoptr_add.da.0
+; CHECK-ELF-NEXT:      .p2align 3
+; CHECK-ELF-NEXT:    g.inttoptr_add.da.0:
+; CHECK-ELF-NEXT:      .xword (g+2)@AUTH(da,0)
+
+; CHECK-MACHO-LABEL:   .globl _g.inttoptr_add.da.0
+; CHECK-MACHO-NEXT:    .p2align 3
+; CHECK-MACHO-NEXT:  _g.inttoptr_add.da.0:
+; CHECK-MACHO-NEXT:    .quad (_g+2)@AUTH(da,0)
+
+@g.inttoptr_add.da.0 = constant ptr ptrauth (ptr inttoptr (i64 add (i64 ptrtoint (ptr @g to i64), i64 2) to ptr), i32 2)
+
 ; CHECK-ELF-LABEL:     .globl g_weak.ref.ia.42
 ; CHECK-ELF-NEXT:      .p2align 3
 ; CHECK-ELF-NEXT:    g_weak.ref.ia.42:

🐧 Linux x64 Test Results

• 193175 tests passed
• 5009 tests skipped

✅ The build succeeded and all tests passed.

🪟 Windows x64 Test Results

• 133132 tests passed
• 3069 tests skipped

✅ The build succeeded and all tests passed.

leading to the program crashing.

How does this work in Swift then? Is there downstream change? Can you point to it?

How does this work in Swift then? Is there downstream change? Can you point to it?

Swift's fork does not contain this fix. I encountered this when lowerConstantPtrAuth received a ConstantPtrAuth with a null base and with an inttoptr(add(ptrtoint)) pattern. The test case in ptrauth-reloc.ll is derived from the crashing IR.

I've updated the PR description's wording to be more precise.

@ahmedbougacha I don't think this should be merged until you've looked at this.

@oskarwirga Just being curious: could you please share a minimal swift reproducer (with compiler invocation command which you use) which results in such a pattern which needs special handling?

@rjmccall would you mind looking at the mentioned swift codegen issue? Is that some idiosyncrasy we don't normally generate or is there something important we've missed in upstreaming?

@rjmccall would you mind looking at the mentioned swift codegen issue? Is that some idiosyncrasy we don't normally generate or is there something important we've missed in upstreaming?

ignore me - I thought this was one of the other PRs (one of the codegen ones), this one is asm printing and I assume we just don't have existing tests

@oskarwirga Just being curious: could you please share a minimal swift reproducer (with compiler invocation command which you use) which results in such a pattern which needs special handling?

After some more investigation, this was indeed a non-standard application of compiler flags/custom toolchain. I encountered this inttoptr(add(ptrtoint)) pattern when compiling some swift with -ptrauth-emit-wrapper-globals=false which emits ConstantPtrAuth instead of relying on the llvm.ptrauth section. That said, you can reproduce the miscompilation just with LLC:

@g = external global i32
@null_signed = constant ptr ptrauth (ptr null, i32 2)
@offset_signed = constant ptr ptrauth (ptr inttoptr (i64 add (i64 ptrtoint (ptr @g to i64), i64 2) to ptr), i32 2)

llc -mtriple arm64e-apple-darwin -o - repro.ll

@offset_signed emits (0)@AUTH(da,0) instead of the correct (_g+2)@AUTH(da,0).

I've pushed a commit to remove the Swift blame (sorry Swift!) and updated the description to suit.

If we don't need it, I'd rather not set LookThroughIntToPtr to true, I think? (The other change should ensure we don't miscompile.)

If we don't need it, I'd rather not set LookThroughIntToPtr to true, I think? (The other change should ensure we don't miscompile.)

I see both sides to this. I lean towards supporting this pattern mainly because the parameter already exists and its trivial to support it but since we don't have default cases leading to this pattern we'd be widening what lowerConstantPtrAuth needs to support.

inttoptr(add(ptrtoint(@global), offset)) is valid IR but it isn't generated by default so I don't know if this path needs to support it.

According to the documentation of stripAndAccumulateConstantOffsets

  /// If \p LookThroughIntToPtr is true then this method also looks through
  /// IntToPtr and PtrToInt constant expressions. The returned pointer may not
  /// have the same provenance as this value.

I'm not too experienced with all that pointer provenance stuff - on one hand, we are at the very end of the pipeline at this point, and there is no LLVM IR consumers past it. On the other hand, having pointers computed with plain add instead of proper pointer arithmetic may possibly indicate some issue, I guess. For a simple C source

int storage[10];

int * __ptrauth(2, 0, 0) result = storage + 2;

the following LLVM IR is produced by ./bin/clang -target aarch64-linux-pauthtest -S -o- example.c (note that getelementptr is generated):

@storage = dso_local global [10 x i32] zeroinitializer, align 4
@result = dso_local global ptr ptrauth (ptr getelementptr (i8, ptr @storage, i64 8), i32 2), align 8

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32"
target triple = "aarch64-unknown-linux-pauthtest"

@storage = dso_local global [10 x i32] zeroinitializer, align 4
@result = dso_local global ptr ptrauth (ptr getelementptr (i8, ptr @storage, i64 8), i32 2), align 8

!llvm.module.flags = !{!0, !1, !2, !3, !4, !5, !6, !7}
!llvm.ident = !{!8}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 8, !"ptrauth-sign-personality", i32 1}
!2 = !{i32 1, !"aarch64-elf-pauthabi-platform", i32 268435458}
!3 = !{i32 1, !"aarch64-elf-pauthabi-version", i32 1791}
!4 = !{i32 8, !"PIC Level", i32 2}
!5 = !{i32 7, !"PIE Level", i32 2}
!6 = !{i32 7, !"uwtable", i32 2}
!7 = !{i32 7, !"frame-pointer", i32 4}
!8 = !{!"clang version 23.0.0git ([email protected]:llvm/llvm-project.git 19c862dd277b386bffb18247d53c801ae441390b)"}

The assembly output is as follows (both with this PR and on the mainline):

result:
        .xword  (storage+8)@AUTH(da,0)
        .size   result, 8

        .aeabi_subsection       aeabi_pauthabi, required, uleb128
        .aeabi_attribute        1, 268435458    // Tag_PAuth_Platform
        .aeabi_attribute        2, 1791 // Tag_PAuth_Schema
        .section        .note.gnu.property,"a",@note
        .p2align        3, 0x0
        .word   4
        .word   24
        .word   5
        .asciz  "GNU"
        .word   3221225473
        .word   16
        .xword  268435458
        .xword  1791
.Lsec_end0:
        .text
        .file   "test.c"
        .type   storage,@object                 // @storage
        .bss
        .globl  storage
        .p2align        2, 0x0
storage:
        .zero   40
        .size   storage, 40

        .type   result,@object                  // @result
        .data
        .globl  result
        .p2align        3, 0x0
result:
        .xword  (storage+8)@AUTH(da,0)
        .size   result, 8

        .ident  "clang version 23.0.0git ([email protected]:llvm/llvm-project.git 19c862dd277b386bffb18247d53c801ae441390b)"
        .section        ".note.GNU-stack","",@progbits
        .addrsig
        .addrsig_sym storage

Replacing the definition of @result with

@result = dso_local global ptr ptrauth (ptr getelementptr (i8, ptr null, i64 8), i32 2), align 8

changes .xword (storage+8)@AUTH(da,0) to .xword (8)@AUTH(da,0) (both with this PR and in the mainline). Furthermore, changing the definition of @result to

@result = dso_local global ptr ptrauth (ptr null, i32 2), align 8

turns the assembly output into .xword (0)@AUTH(da,0) in both versions.

The patch looks mostly good to me, except that I'm not sure we should actually enable LookThroughIntToPtr "just in case" (i.e. if it is not actually emitted by Swift, for example). But explicitly checking that we finally got null base instead of just assuming "anything unknown is null" is definitely important.

Sounds good to me, thanks for diving into it :) I've updated the PR + desc to drop LookThroughIntToPtr but kept the explicit null-handling.