This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Implement setjmp BTI placement for PACBTI-M
ClosedPublic

Authored by stuij on Oct 25 2021, 3:30 AM.

Details

Summary

This patch intends to guard indirect branches performed by longjmp
by inserting BTI instructions after calls to setjmp.

Calls with 'returns-twice' are lowered to a new pseudo-instruction
named t2CALL_BTI that is later expanded to a bundle of {tBL,t2BTI}.

This patch is part of a series that adds support for the PACBTI-M extension of
the Armv8.1-M architecture, as detailed here:

https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension

The PACBTI-M specification can be found in the Armv8-M Architecture Reference
Manual:

https://developer.arm.com/documentation/ddi0553/latest

The following people contributed to this patch:

  • Alexandros Lamprineas
  • Ties Stuij

Diff Detail

Event Timeline

stuij created this revision.Oct 25 2021, 3:30 AM
stuij requested review of this revision.Oct 25 2021, 3:30 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 25 2021, 3:30 AM
chill added a subscriber: chill.Oct 27 2021, 8:23 AM

I would suggest to add a command line option to control placement of BTI after setjmp/etc.
The default should be as it is now, but IMHO we should provide means for people who *know* how their
runtime is implemented[1] to avoid placing the extra BTI instructions.

[1] e.g. longjmp uses PAC, in which case destination address is already authenticated, or
longjmp uses a non-BTI setting instruction to jump, in which case the BTI does nothing
other than take space and provide opportunities to jump from elsewhere.

We only perform this transformation when armlib is not in use.
That information is passed on by the module flags.

This was the downstream strategy. Like Momchill pointed out, it'd be preferable to provide a way for the user to control the codegen for 'returns-twice' calls.

stuij updated this revision to Diff 388891.Nov 22 2021, 5:39 AM

addressed review comments, namely adding a commandline argument, -mno-bti-at-return-twice, to not place a bti instruction after a setjmp call.

Herald added a project: Restricted Project. · View Herald TranscriptNov 22 2021, 5:39 AM
stuij edited the summary of this revision. (Show Details)Nov 24 2021, 6:53 AM
labrinea accepted this revision.Nov 25 2021, 7:11 AM
This revision is now accepted and ready to land.Nov 25 2021, 7:11 AM
stuij updated this revision to Diff 390633.Nov 30 2021, 2:11 AM

slight rewording

This revision was landed with ongoing or failed builds.Dec 6 2021, 3:07 AM
This revision was automatically updated to reflect the committed changes.
DavidSpickett added inline comments.
llvm/lib/Target/ARM/ARMInstrThumb2.td
5727

Should this require IsMClass instead/also? Though I wasn't able to get anything weird to happen when using an A profile triple so maybe I'm missing a check elsewhere that means you'd never get to this point with A profile Arm.

For example this A profile triple:

$ ./bin/clang --target=thumbv8-arm-none-eabi /tmp/test.c -o /tmp/test.o -o - -S -mbranch-protection=bti -mthumb

Doesn't put anything after a call to setjmp, nop or otherwise, but I can't place where that decision is made.

chill added inline comments.Jan 10 2022, 9:04 AM
DavidSpickett added inline comments.Jan 10 2022, 9:10 AM
llvm/lib/Target/ARM/ARMInstrThumb2.td
5727

Never mind, I figured it out as per usual just after leaving the comment.

static bool GetBranchTargetEnforcement(MachineFunction &MF) {
  const auto &Subtarget = MF.getSubtarget<ARMSubtarget>();
  if (!Subtarget.isMClass() || !Subtarget.hasV7Ops())
    return false;

This returns false for the A profile which means that GuardWithBTI is false so we don't add a BTI. Maybe one could craft some IR example that got around that but doesn't seem a likely issue.

DavidSpickett added inline comments.Jan 10 2022, 9:12 AM
llvm/lib/Target/ARM/ARMInstrThumb2.td
5727

The decision is made in ARMMachineFunctionInfo

Left my comment just as you did. Thanks! I see how it works now.