AArch64: PAC: Do not attempt ICP on authenticating calls.

[pcc]

Summary

Not only would the check that we would produce be incorrect since it
would not account for the fact that the function pointer would be signed,
we would also end up producing invalid IR as the call on the direct call
branch would have a ptrauth operand bundle. Therefore, do not ICP these
calls for now.

[tejohnson]

The change itself looks fine to me, although needs a comment. But this would be better to test in llvm/test/Transforms/PGOProfile (see the icp_* and indirect_call_promotion* tests in that directory). The SampleProfile tests seem more geared towards specifically testing the SamplePGO matching related issues, and this is a more generic ICP fix. Thus the tests in PGOProfile don't tend to use a separate input file with the profile, rather the profile is already embedded in the IR with VP metadata.

llvm/lib/Transforms/Utils/CallPromotionUtils.cpp

388

Please add a comment here about why.

[ab]

To make sure I follow: would it make sense to have the legality check compare the ptrauth bits, and if they match, remove the bundle and promote? We'd have to pass the pointer (not the Function) around. And I don't see how we'd get a signed pointer in the SampleProfile case, so we wouldn't promote there anyway (without a much more interesting target check at least?) But it seems like the other users would generate a direct ptrauth-bundle call to ptrauth-signed target; InstCombine should handle that.

But in the meantime this does seem reasonable, though I'll defer to Teresa on tests.

llvm/lib/Transforms/Utils/CallPromotionUtils.cpp

389

llvm/lib/Transforms/Utils/CallPromotionUtils.cpp

 }
 
 bool llvm::isLegalToPromote(const CallBase &CB, Function *Callee,
                             const char **FailureReason) {
   assert(!CB.getCalledFunction() && "Only indirect call sites can be promoted");
 
   auto &DL = Callee->getParent()->getDataLayout();
 
+  if (CB.countOperandBundlesOfType(LLVMContext::OB_ptrauth)) {
+    if (*FailureReason)
+      *FailureReason = "Authenticating calls are unsupported";
+    return false;
+  }
+
   // Check the return type. The callee's return value type must be bitcast
   // compatible with the call site's type.
   Type *CallRetTy = CB.getType();
   Type *FuncRetTy = Callee->getReturnType();
   if (CallRetTy != FuncRetTy)
     if (!CastInst::isBitOrNoopPointerCastable(FuncRetTy, CallRetTy, DL)) {
       if (FailureReason)
         *FailureReason = "Return type mismatch";

llvm/test/Transforms/SampleProfile/Inputs/indirect-call.prof

1: foo_inline1:3000
   11: 3000
  2: return_arg:3000
   1: 3000
 branch_prof_valid:4000:0
  1: 1000
  2: foo_inline3:3000
   1: 0
   2: 3000
+branch_prof_ptrauth:4000:0
+ 1: 1000
+ 2: foo_inline3:3000
+  1: 0
+  2: 3000

llvm/test/Transforms/SampleProfile/indirect-call.ll

define void @branch_prof_valid(ptr %t0) #0 !dbg !33 {
   store ptr %t0, ptr %t1
   %t2 = load ptr, ptr %t1
   ; CHECK-NOT: call {{.*}}
   ; CHECK: br i1 {{.*}}, label %if.true.direct_targ, label %if.false.orig_indirect, {{.*}}, !prof ![[BR3:[0-9]+]]
   call void %t2(), !dbg !34
   ret void
 }
 
+; CHECK-LABEL: @branch_prof_ptrauth
+; Make sure that we don't attempt ICP when the call includes an authentication
+; instruction.
+define void @branch_prof_ptrauth(ptr %t0) #0 !dbg !36 {
+  %t1 = alloca ptr
+  store ptr %t0, ptr %t1
+  %t2 = load ptr, ptr %t1
+  ; CHECK-NOT: br i1
+  call void %t2() [ "ptrauth"(i32 0, i64 0) ], !dbg !37
+  ret void
+}
+
 @x = global i32 0, align 4
 @y = global ptr null, align 8
 
 define ptr @foo_inline1(ptr %x) #0 !dbg !14 {
   ret ptr %x
 }
 
 define ptr @foo_inline_strip.suffix(ptr %x) #0 !dbg !15 {

 !28 = distinct !DISubprogram(name: "foo_direct_i32", scope: !1, file: !1, line: 11, unit: !0)
 !29 = distinct !DISubprogram(name: "return_arg", scope: !1, file: !1, line: 11, unit: !0)
 !30 = distinct !DISubprogram(name: "return_arg_caller", scope: !1, file: !1, line: 11, unit: !0)
 !31 = !DILocation(line: 12, scope: !30)
 !32 = !DILocation(line: 13, scope: !30)
 !33 = distinct !DISubprogram(name: "branch_prof_valid", scope: !1, file: !1, line: 25, unit: !0)
 !34 = !DILocation(line: 27, scope: !33)
 !35 = distinct !DISubprogram(name: "foo_inline3", scope: !1, file: !1, line: 29, unit: !0)
+!36 = distinct !DISubprogram(name: "branch_prof_ptrauth", scope: !1, file: !1, line: 30, unit: !0)
+!37 = !DILocation(line: 32, scope: !36)