This is an archive of the discontinued LLVM Phabricator instance.

Implement target(branch-protection) attribute for AArch64
ClosedPublic

Authored by chill on Oct 9 2019, 8:57 AM.

Details

Summary

This patch implements __attribute__((target("branch-protection=..."))) in a manner, compatible with the analogous GCC feature:

https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gcc/AArch64-Function-Attributes.html#AArch64-Function-Attributes

Diff Detail

Event Timeline

chill created this revision.Oct 9 2019, 8:57 AM
chill updated this revision to Diff 224074.Oct 9 2019, 9:04 AM

Removed some accidental change.

You should also update AttrDocs.td for the new feature.

clang/include/clang/Basic/DiagnosticSemaKinds.td
2598

Invalid -> invalid

should perhaps have '%0' to show what part is invalid?

clang/lib/CodeGen/TargetInfo.cpp
5062–5063

Don't do hasAttr<> followed by getAttr<>, you should do:

if (const auto *TA = FD->getAttr<TargetAttr>())
5071

auto *

chill updated this revision to Diff 224993.Oct 15 2019, 3:47 AM
chill marked 3 inline comments as done.

I think this also needs a check to make sure the selected architecture supports PAC/BTI. Gcc gives an unknown target attribute or pragma error if this is used for a non-AArch64 target.

chill planned changes to this revision.Oct 29 2019, 6:36 AM
chill updated this revision to Diff 227112.Oct 30 2019, 8:35 AM

Moved parsing out of Attr.td to TargetInfo, following the example (good or bad) of validateOutputConstraint or validateGlobalregisterVariable ;)
Now issue a warning if attribute is not supported by target, like for other unsupported target options.

ostannard added inline comments.Nov 4 2019, 2:04 AM
clang/lib/Basic/Targets/AArch64.cpp
108

This completely duplicates the ParseAArch64BranchProtection function in the driver, could we share one implementation between the command line option and attribute?

chill updated this revision to Diff 228099.Nov 6 2019, 10:24 AM

Moved parsing of branch protection spec to TargetParser.

Herald added a project: Restricted Project. · View Herald TranscriptNov 6 2019, 10:24 AM
chill marked an inline comment as done.Nov 6 2019, 10:25 AM
ostannard accepted this revision.Nov 14 2019, 5:30 AM

LGTM, thanks

This revision is now accepted and ready to land.Nov 14 2019, 5:30 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2019, 7:42 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript