RFC: ConstantPtrAuth for signed pointers on AArch64

Project Infrastructure LLVM Dev List Archives
1

To put signed pointers into global initializers (e.g. vtables) and other
desirable features there needs to be a Constant that represents "sign this
pointer in such and such way". I propose adding a new child of Constant,
ConstantPtrAuth to represent this:

    @var = global i32* ptrauth(i32* @something, ; Pointer to be signed
                               i32 0, ; Key to use for the signature
                               i32* @var, ; Address
discriminator, null if none
                               i16 1234) ; Discriminator

Anyone who has looked at the Xcode compiler output for arm64e will recognise
this as a pretty close analogue of the pseudo-Global that it emits
currently.

That was always intended to be a placeholder while the code was internal so that
we didn't introduce bitcode incompatibility with OSS. Now that arm64e is being
upstreamed, it's a good time to fix what was always really a ConstantExpr
pretending to be a global.

The fields specified were chosen to broadly match the features and relocations
available in arm64e MachO files: a 16-bit discriminator, possibly address
discriminated too.

I've posted the initial patch on Phabricator at
⚙ D92834 [IR,AArch64] Add a new "ptrauth(...)" Constant to represent signed pointers. I've anticipated some
questions and done my best to respond below. What else occurs to people?

Shouldn't this be a ConstantExpr?

2

To put signed pointers into global initializers (e.g. vtables) and other
desirable features there needs to be a Constant that represents "sign this
pointer in such and such way".

Cool.

Shouldn't this be a ConstantExpr?
---------------------------------

There is one key interface for ConstantExpr we cannot currently support:
getAsInstruction. When an address discriminator is present, the ptrauth constant
represents an @llvm.ptrauth.blend followed by an @llvm.ptrauth.sign, two
separate instructions.

Because of that I've so far implemented it as its own separate entity in the
Constant hierarchy (kind of like blockaddress).

It might well be better to give up on intrinsic orthogonality and add one for
this case though. It would simplify the changes to lib/IR etc.

I think that making this a ConstantExpr is the right way to go.

It looks like getAsInstruction is only called in two passes (GlobalOpt and ConstantHoisting) and the later one is only called on ConstantExpr casts. I’d recommend reworking ConstantHoisting to not call this (instead just ask the cast opcode and make its own cast), and sink getAsInstruction into GlobalOpt as a static function.

-Chris

3

Responding specifically to the question of the key being an i2: If we're not going to use integer types other than i1, and the powers of 2 that are evenly divisible by 8, then why even bother having these types? I feel that if i2 is the logically correct size, then we should just use it. If work needs to be done to legalize it, then so be it. We shouldn't have to think about what the system will do with 0xDEADBEEF in `ptrauth(i32* @something, i32 0xDEADBEEF, i32* @var, i16 1234)`, it should just be out of range and the types should preclude incorrect usage where possible.

Thanks,
   Christopher Tetreault

4

To put signed pointers into global initializers (e.g. vtables) and other
desirable features there needs to be a Constant that represents “sign this
pointer in such and such way”. I propose adding a new child of Constant,
ConstantPtrAuth to represent this:

@var = global i32* ptrauth(i32* @something, ; Pointer to be signed
i32 0, ; Key to use for the signature
i32* @var, ; Address
discriminator, null if none
i16 1234) ; Discriminator

Anyone who has looked at the Xcode compiler output for arm64e will recognise
this as a pretty close analogue of the pseudo-Global that it emits
currently.

That was always intended to be a placeholder while the code was internal so that
we didn’t introduce bitcode incompatibility with OSS. Now that arm64e is being
upstreamed, it’s a good time to fix what was always really a ConstantExpr
pretending to be a global.

The fields specified were chosen to broadly match the features and relocations
available in arm64e MachO files: a 16-bit discriminator, possibly address
discriminated too.

I’ve posted the initial patch on Phabricator at
https://reviews.llvm.org/D92834. I’ve anticipated some
questions and done my best to respond below. What else occurs to people?

Could we make the discriminator a 64-bit field? For non-address-discriminated pointers it is possible to materialize the discriminator in a single instruction with almost 19 bits of entropy by making use of bit 30 (i.e. the selection between MOVN and MOVZ) and the HW bits. And I suppose that multi-instruction materialization of the discriminator is also possible (although it may not be practical to fit all of the bits into common object formats).

Shouldn’t this be a ConstantExpr?

There is one key interface for ConstantExpr we cannot currently support:
getAsInstruction. When an address discriminator is present, the ptrauth constant
represents an @llvm.ptrauth.blend followed by an @llvm.ptrauth.sign, two
separate instructions.

Because of that I’ve so far implemented it as its own separate entity in the
Constant hierarchy (kind of like blockaddress).

It might well be better to give up on intrinsic orthogonality and add one for
this case though. It would simplify the changes to lib/IR etc.

I mildly prefer keeping it as a Constant and not a ConstantExpr if we don’t realistically expect this to be converted between constant and non-constant operands. That’s similar to what we did for DSOLocalEquivalent for example. But I think I’d be fine either way.

Shouldn’t the key be i2 or something?

Possibly. That has legality implications in CodeGen though and is really not a
commonly used part of LLVM (there’s currently no llvm_i2_ty in
Intrinsics.td). i32 generally signals “don’t care”.

It seems like it should be wider than i2 at least. There is also the GA key which I suppose it’s conceivable that someone may want to use. And making it an i32 would easily allow future expansion, e.g. if more keys are added in the future, without requiring a bitcode upgrade path.

Peter

5

Could we make the discriminator a 64-bit field? For non-address-discriminated pointers it is possible to materialize the discriminator in a single instruction with almost 19 bits of entropy by making use of bit 30 (i.e. the selection between MOVN and MOVZ) and the HW bits. And I suppose that multi-instruction materialization of the discriminator is also possible (although it may not be practical to fit all of the bits into common object formats).

That hadn't occurred to me (the current implementation only allows low
16-bits), but it seems like a pretty good reason to expand the field.
I'll make the changes.

Not directly related to this patch but I think I'd favour an error
rather than multi-instruction materialization for "bad"
discriminators, at least to begin with.

I mildly prefer keeping it as a Constant and not a ConstantExpr if we don't realistically expect this to be converted between constant and non-constant operands. That's similar to what we did for DSOLocalEquivalent for example. But I think I'd be fine either way.

Not that many places do, but in principle this definitely can be
converted in a way I don't think dso_local_equivalent can if I'm
reading the LangRef right. It's got a well-defined expansion in terms
of ptrauth intrinsics and syntactically it's indistinguishable from a
constant expression (unlike both dso_local_equivalent and blockaddress
which includes a normally non-constant basic-block component).

I was pretty disappointed that it didn't fit originally and quite like
Chris's suggestion to sink getAsInstruction as an alternative.

It seems like it should be wider than i2 at least. There is also the GA key which I suppose it's conceivable that someone may want to use. And making it an i32 would easily allow future expansion, e.g. if more keys are added in the future, without requiring a bitcode upgrade path.

Ah yes, I'd forgotten about GA (we don't really use it right now as
far as I know).

Tim.