[PATCH] D69097: [AArch64][MachineOutliner] Return address signing for outlined functions

Tue Oct 22 02:49:05 PDT 2019

tellenbach marked an inline comment as done.
tellenbach added a comment.

In D69097#1716855 <https://reviews.llvm.org/D69097#1716855>, @paquette wrote:

> I think this needs some more tests. In particular, I'd like to see some test cases that show what happens when
>
> - We tail call
> - We save LR to the stack
> - We don't save LR to the stack (but emit a `bl`)
> - We get an outlined thunk

I agree, thanks.

> Also another thing to consider here is whether or not we should ever outline, say, PAC/AUT instructions. It might be good to mark those as illegal to outline. That will also need a test. I think that writing a MIR test should make this fairly easy to check for.

I think this should never happen anyways: After/before any PAC/AUT instruction a cfi instruction is inserted which is a position (`isPosition()` returns `true`). Positions don't get outlined on AArch64 (`AArch64InstrInfo.cpp:5565`) and the one instruction for the actual signing that remains, is too small to get outlined.

However, making this explicit is probably the better way to do it.

> edit: I had a couple other concerns there, but they were actually already addressed in the patch. :) I think most of my concerns here are to have explicit test cases using MIR.

Thanks for your comments!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69097