Page MenuHomePhabricator

[clang][ARM] PACBTI-M frontend support
ClosedPublic

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

Details

Summary

Handle branch protection option on the commandline as well as a function
attribute. One patch for both mechanisms, as they use the same underlying
parsing mechanism.

These are recorded in a set of LLVM IR module-level attributes like we do for
AArch64 PAC/BTI (see https://reviews.llvm.org/D85649):

  • command-line options are "translated" to module-level LLVM IR attributes (metadata).
  • functions have PAC/BTI specific attributes iff the attribute((target("branch-protection=...))) was used in the function declaration.
  • command-line option -mbranch-protection to armclang targeting Arm,

following this grammar:

branch-protection ::= "-mbranch-protection=" <protection>
protection ::= "none" | "standard" | "bti" [ "+" <pac-ret-clause> ]

| <pac-ret-clause> [ "+" "bti"]

pac-ret-clause ::= "pac-ret" [ "+" <pac-ret-option> ]
pac-ret-option ::= "leaf" ["+" "b-key"] | "b-key" ["+" "leaf"]

b-key is simply a placeholder to make it consistent with AArch64's
version. In Arm, however, it triggers a warning informing that b-key is
unsupported and a-key will be selected instead.

  • Handle _attribute_((target(("branch-protection=..."))) for AArch32 with the

same grammer as the commandline options.

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:

  • Momchil Velikov
  • Victor Campos
  • Ties Stuij

Diff Detail

Event Timeline

stuij created this revision.Oct 25 2021, 3:10 AM
stuij requested review of this revision.Oct 25 2021, 3:10 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 25 2021, 3:10 AM

In the commit message: s/armclang/clang/

clang/lib/Driver/ToolChains/Clang.cpp
1832–1833

This block is duplicating the behaviour of the CollectARMPACBTIOptions call below.

aaron.ballman added inline comments.Oct 28 2021, 6:56 AM
clang/include/clang/Basic/DiagnosticSemaKinds.td
2983
clang/lib/Basic/Targets/AArch64.cpp
143–145

This change surprises me. Why should AArch64TargetInfo prefer calling into ARM instead?

vhscampos added inline comments.Oct 28 2021, 8:24 AM
clang/lib/Basic/Targets/AArch64.cpp
143–145

Since that particular function ended up identical in both ARM and AArch64, we removed the AArch64 specific function and kept only one under ARM. You can spot the removal further down the patch.

The ARM namespace under ARMTargetParser.h already had code used in AArch64TargetParser, so we did not introduce new cross dependencies.

chill added a subscriber: chill.Oct 28 2021, 9:15 AM
chill added inline comments.
clang/lib/Basic/Targets/AArch64.cpp
143–145

It's the unfortunate overload of "ARM" used to denote the backend and the organisation.

aaron.ballman added inline comments.Oct 29 2021, 5:46 AM
clang/lib/Basic/Targets/AArch64.cpp
143–145

Ah, that's good to know, thank you for the explanation. (And yeah, that is an unfortunate overload of the term.)

danielkiss added inline comments.
clang/lib/Basic/Targets/AArch64.cpp
143–145

Wondering will this link when the LLVM_TARGETS_TO_BUILD does not contains ARM but AArch64?

stuij updated this revision to Diff 384724.Nov 4 2021, 5:54 AM

removed stray block duplicating the behaviour of CollectARMPACBTIOptions

chill added inline comments.Nov 4 2021, 6:01 AM
clang/lib/Basic/Targets/AArch64.cpp
143–145

Should link, the function is in lib/Support/TragetParser.cpp

vhscampos added inline comments.Thu, Nov 25, 8:38 AM
clang/include/clang/Basic/DiagnosticSemaKinds.td
2983

Still need to remove extraneous whitespace

clang/lib/CodeGen/TargetInfo.cpp
6377

I reckon selecting the string using a switch statement on BPI.SignReturnAddr is more type safe than doing it like this. The current selection is prone to out of bounds accesses to the array in case the enum changes. Please consider so.

chill added inline comments.Thu, Nov 25, 8:57 AM
clang/lib/CodeGen/TargetInfo.cpp
6377

Guard it with an assert.

stuij updated this revision to Diff 390021.Fri, Nov 26, 5:04 AM

addressed review comments

stuij marked 4 inline comments as done.Fri, Nov 26, 5:05 AM
vhscampos accepted this revision.Mon, Nov 29, 1:52 AM
This revision is now accepted and ready to land.Mon, Nov 29, 1:52 AM
stuij updated this revision to Diff 390316.Mon, Nov 29, 5:42 AM

minor clang-format conformance changes

stuij updated this revision to Diff 390958.Wed, Dec 1, 2:19 AM

rebased and resolved merge conflict

This revision was landed with ongoing or failed builds.Wed, Dec 1, 2:37 AM
This revision was automatically updated to reflect the committed changes.
RKSimon added a subscriber: RKSimon.Wed, Dec 1, 4:09 AM

@stuij This is causing buildbot failures, please can you take a look? https://lab.llvm.org/buildbot/#/builders/139

stuij added a comment.Wed, Dec 1, 4:11 AM

Yes, pushed a temporary fix.

I just found that build failure in our downstream, and did a little investigation. Running the clang-invocations directly resulted in :

[ekeane1@scsel-clx-24 llvm]$ /localdisk2/ekeane1/workspaces/xmain-web/builds/xmainefi2linux_debug/llvm/bin/clang -target arm-arm-none-eabi -mbranch-protection=pac-ret+b-key -c /localdisk2/ekeane1/workspaces/xmain-web/llvm/clang/test/Frontend/arm-invalid-branch-protection.c -o /dev/null 2>&1
clang-14: warning: invalid branch protection option 'b-key' in '-mbranch-protection=pac-ret+b-key' [-Wbranch-protection]
error: unable to create target: 'No available targets are compatible with triple "armv4t-arm-none-eabi"'
1 error generated.
[ekeane1@scsel-clx-24 llvm]$ /localdisk2/ekeane1/workspaces/xmain-web/builds/xmainefi2linux_debug/llvm/bin/clang -target arm-arm-none-eabi -mbranch-protection=pac-ret+b-key+leaf -c /localdisk2/ekeane1/workspaces/xmain-web/llvm/clang/test/Frontend/arm-invalid-branch-protection.c -o /dev/null 2>&1
clang-14: warning: invalid branch protection option 'b-key' in '-mbranch-protection=pac-ret+b-key+leaf' [-Wbranch-protection]
error: unable to create target: 'No available targets are compatible with triple "armv4t-arm-none-eabi"'
1 error generated.
[ekeane1@scsel-clx-24 llvm]$ /localdisk2/ekeane1/workspaces/xmain-web/builds/xmainefi2linux_debug/llvm/bin/clang -target arm-arm-none-eabi -mbranch-protection=bti+pac-ret+b-key -c /localdisk2/ekeane1/workspaces/xmain-web/llvm/clang/test/Frontend/arm-invalid-branch-protection.c -o /dev/null 2>&1
clang-14: warning: invalid branch protection option 'b-key' in '-mbranch-protection=bti+pac-ret+b-key' [-Wbranch-protection]
error: unable to create target: 'No available targets are compatible with triple "armv4t-arm-none-eabi"'
1 error generated.
[ekeane1@scsel-clx-24 llvm]$ /localdisk2/ekeane1/workspaces/xmain-web/builds/xmainefi2linux_debug/llvm/bin/clang -target arm-arm-none-eabi -mbranch-protection=bti+pac-ret+b-key+leaf -c /localdisk2/ekeane1/workspaces/xmain-web/llvm/clang/test/Frontend/arm-invalid-branch-protection.c -o /dev/null 2>&1
clang-14: warning: invalid branch protection option 'b-key' in '-mbranch-protection=bti+pac-ret+b-key+leaf' [-Wbranch-protection]
error: unable to create target: 'No available targets are compatible with triple "armv4t-arm-none-eabi"'

I suspect the problem with the bots is that the test needs to have the 'requires' clause added to the top, like you did with the CodeGen tests.