This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Add isAuthenticating predicate to MCInstDesc
ClosedPublic

Authored by vsk on Nov 15 2019, 11:26 AM.

Details

Summary

Add a predicate to MCInstDesc that allows tools to determine whether an
instruction authenticates a pointer. This can be used by diagnostic
tools to hint at pointer authentication failures.

Diff Detail

Event Timeline

vsk created this revision.Nov 15 2019, 11:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2019, 11:26 AM

IIUC, I think "isAuthenticating" would be a more accurate description/name than "isAuthenticated".
In that the instruction on which this property is set does authenticate a pointer, rather than the instruction itself having been authenticated (whatever that may mean) before.
Does that seem fair?

vsk updated this revision to Diff 229877.Nov 18 2019, 10:23 AM
vsk retitled this revision from [AArch64] Add isAuthenticated predicate to MCInstDesc to [AArch64] Add isAuthenticating predicate to MCInstDesc.
vsk edited the summary of this revision. (Show Details)

Rename isAuthenticated -> isAuthenticating, for clarity.

vsk added a comment.Dec 2 2019, 9:59 AM

Gentle ping.

rjmccall resigned from this revision.Dec 2 2019, 10:11 AM

This looks like it covers the right set of instructions, but I don't really know enough about backends to review, sorry.

Hi! Does this cover pre v8.3 pointer authenticating instructions (AUTIASP/AUTIBSP) too?

vsk updated this revision to Diff 236443.Jan 6 2020, 1:04 PM

Cover pre-8.3 auti*sp instructions.

ab accepted this revision.Jan 9 2020, 3:23 PM

Seems reasonable. This is an interesting use of MCID flags, in that llvm itself doesn't need it, but I'm all for better disassembler support!

IIUC, I think "isAuthenticating" would be a more accurate description/name than "isAuthenticated".

FWIW, I actually find "isAuthenticated" more idiomatic, but I'm not enough of a native speaker to justify it ;)

Hi! Does this cover pre v8.3 pointer authenticating instructions (AUTIASP/AUTIBSP) too?

I don't think it makes sense to include AUT* on v8.3a, but it does on v8.6a, and shouldn't hurt elsewhere, so I'm fine with having them.

llvm/include/llvm/Target/Target.td
533

nit: one too many spaces between ';' and '//'

llvm/lib/Target/AArch64/AArch64InstrInfo.td
822 ↗(On Diff #236443)

I suggest wrapping the AUT defs with let isAuthenticating = 1, this doesn't need a class.

This revision is now accepted and ready to land.Jan 9 2020, 3:23 PM
This revision was automatically updated to reflect the committed changes.
vsk marked 2 inline comments as done.