This is an archive of the discontinued LLVM Phabricator instance.

[Triple][MachO] Define "arm64e", an AArch64 subarch for Pointer Auth.
ClosedPublic

Authored by ab on Sep 3 2020, 9:33 AM.

Details

Summary

This is the first of many Pointer Authentication-related patches we're happy to finally upstream =]
For a high-level overview, see our llvm-dev RFC: http://lists.llvm.org/pipermail/llvm-dev/2019-October/136091.html, as well as the devmtg talk we did at the same time last year.
For concrete code that builds on this, see the staging PR in apple/llvm-project: https://github.com/apple/llvm-project/pull/14. Though we've made changes downstream since then, the general concepts and added constructs are mostly identical.

Per commit message:

This also teaches MachO writers/readers about the MachO cpu subtype,
beyond the minimal subtype reader support present at the moment.

This also defines a preprocessor macro to allow users to distinguish
__arm64__ from __arm64e__.

arm64e defaults to an "apple-a12" CPU, which supports v8.3a, allowing
pointer-authentication codegen.
It also currently defaults to ios14+ and macos11+.

If it helps, we can obviously split this patch further, but I feel like this all belongs together, as the core boilerplate needed for new (darwin) targets.

Note that this adds a Triple::isArm64e(), which is a bit of a departure from prior subarches, in part because we check it more often. The obvious alternative would be to compare Triple::getArchName() with "arm64e"; I'm fine with either. However, I would like to preserve the "arm64e" naming, as we refer to that extensively in Darwin-land.

Diff Detail

Event Timeline

ab created this revision.Sep 3 2020, 9:33 AM
Herald added a reviewer: MaskRay. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
ab requested review of this revision.Sep 3 2020, 9:33 AM
pcc added a subscriber: pcc.Sep 3 2020, 11:00 AM

Hi, thanks for getting started on upstreaming this!

But I was wondering: shouldn't this be the *last* patch? I was imagining that you would first upstream support for the -fptrauth-* flags, and then at the end you would add an arm64e subarch that turns them on by default. Otherwise the upstream compiler will temporarily produce ABI-incompatible objects when targeting arm64e.

ab added a comment.Sep 3 2020, 11:17 AM
In D87095#2255010, @pcc wrote:

But I was wondering: shouldn't this be the *last* patch? I was imagining that you would first upstream support for the -fptrauth-* flags, and then at the end you would add an arm64e subarch that turns them on by default. Otherwise the upstream compiler will temporarily produce ABI-incompatible objects when targeting arm64e.

Good point, you're right. ABI compatibility has obviously been a major issue as we keep iterating on this, so we added the concept of ptrauth ABI versions to the arm64e mach-o cpusubtype. With this patch, upstream clang produces "unversioned" binaries, which are rejected on macOS11/iOS14. Once we're done upstreaming, I'll make the final patch to match the ABI version. In the meantime, that means upstream can't produce anything that runs on macOS, which should avoid this problem (and lets us use arm64e checks in the remaining patches, though I can obviously extract that to have the patches done the other way)

jhenderson added inline comments.Sep 4 2020, 1:00 AM
llvm/lib/Object/MachOObjectFile.cpp
2749–2750

Nit: it's probably worth fixing these two Linter issues, since this function is small anyway.

zzheng added a subscriber: zzheng.Sep 16 2020, 9:51 AM
ab updated this revision to Diff 292557.Sep 17 2020, 10:29 AM

Reformat ValidArchs.

ab marked an inline comment as done.Sep 17 2020, 10:29 AM

I had a look through this and everything seemed to be in place. I think the ABI versioning is probably sufficient to prevent surprises and land this now, to avoid churn in the rest of the code. If more is really needed, we could add a temporary warning ("incomplete ABI") to the Clang driver.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 3 2020, 7:54 AM
This revision was automatically updated to reflect the committed changes.