[PATCH] D84502: [AArch64][GlobalISel] Implement __builtin_return_address for PAC-RET
Daniel Kiss via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 31 12:30:29 PDT 2020
danielkiss added inline comments.
================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:4720
EntryBuilder.setInstr(*EntryBlock.begin());
- EntryBuilder.buildCopy({DstReg}, {Register(AArch64::LR)});
+ if (MF.getFunction().hasFnAttribute("sign-return-address")) {
+ if (STI.hasV8_3aOps()) {
----------------
chill wrote:
> ab wrote:
> > chill wrote:
> > > ab wrote:
> > > > I don't know what model you have for the attribute and the command line options, and I guess you're probably aware, but this seems unsafe for depths > 0. I'm not sure there's a better way to deal with this, short of just forcing the strips in any aarch64 code, which is not an option. Maybe emitting strips when +pa is available? I imagine that's not at all reliable either.
> > > > ... I guess you're probably aware, but this seems unsafe for depths > 0.
> > > In what sense is it unsafe? The clear instructions do not require a valid PAC
> > > to be present, so even if a caller is not using PAC-RET we still get the return
> > > address.
> > I'm thinking of the opposite scenario, where a caller in an unrelated library does have a signed return address, but this function wasn't compiled with that support: it wouldn't know to strip.
> I see. Looks like checking for `+pa`, while not perfect, will work in more cases.
`XPACLRI` can always be emitted because it is in the NOP space. Besides it is not nice to emit always one extra presumably unnecessary instruction but I don't see why it won't work.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84502/new/
https://reviews.llvm.org/D84502
More information about the llvm-commits
mailing list