[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

[Momchil Velikov via Phabricator via llvm-commits]

chill added a comment.

I'm afraid the patch does not work yet. For example, when the following program

  void *f() {
  	void g();
  	g();
  	return __builtin_return_address(0);
  }

is compiled with

  ./bin/clang -target aarch64-eabi -march=armv8.3-a  -mbranch-protection=pac-ret -S -O2 h.c

The issue is that the definition of the instructions `XPAC{D,I}` is incorrect: it does not mention at all the operand to those insns.
Another problem is that the patch does not work with `-O0`. When compiling without optimisations, AArch64 backend used GlobalISel.

I have patches for these two issues. I'll post the one for XPAC{D,O} tomorrow and perhaps in a couple of days the GlobalISel one and we're good to go.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6119
+    SDValue Reg =
+        DAG.getCopyToReg(DAG.getEntryNode(), DL, AArch64::X0, ReturnAddress);
+    SDNode *St = DAG.getMachineNode(AArch64::XPACI, DL, VT, Reg);
----------------
We shouldn't be hardcoding the `X0` register here.  We already have the encoded return address in `ReturnAddress`
can simply do:

   SDNode *St = DAG.getMachineNode(AArch64::XPACI, DL, VT, ReturnAddress);


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6124
+    // XPACLRI operates on LR therefore we must move the operand accordingly.
+    SDValue Reg =
+        DAG.getCopyToReg(DAG.getEntryNode(), DL, AArch64::LR, ReturnAddress);
----------------
Rename `Reg` to `Chain`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75044/new/

https://reviews.llvm.org/D75044