Tim Northover via llvm-dev
2020-Dec-08 12:30 UTC
[llvm-dev] RFC: ConstantPtrAuth for signed pointers on AArch64
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?
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.
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".
Why is the address discriminator not just a bool?
-------------------------------------------------
The relocations in MachO only allow you to use the address being relocated as
the address-discriminator, so at first sight it seems excessive to have an
entire pointer field that has only one legitimate value anyway when an i1 would
suffice.
However, transformations like constant propagation can separate these
ptrauth(...) constants from their storage location, at which point CodeGen would
have no idea what address to use. So the field does need to be the actual
address.
Chris Lattner via llvm-dev
2020-Dec-08 18:20 UTC
[llvm-dev] RFC: ConstantPtrAuth for signed pointers on AArch64
On Dec 8, 2020, at 4:30 AM, Tim Northover via llvm-dev <llvm-dev at lists.llvm.org> wrote:> > 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
Chris Tetreault via llvm-dev
2020-Dec-08 19:51 UTC
[llvm-dev] RFC: ConstantPtrAuth for signed pointers on AArch64
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
-----Original Message-----
From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Tim
Northover via llvm-dev
Sent: Tuesday, December 8, 2020 4:31 AM
To: LLVM Developers Mailing List <llvm-dev at lists.llvm.org>
Subject: [EXT] [llvm-dev] RFC: ConstantPtrAuth for signed pointers on AArch64
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?
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.
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".
Why is the address discriminator not just a bool?
-------------------------------------------------
The relocations in MachO only allow you to use the address being relocated as
the address-discriminator, so at first sight it seems excessive to have an
entire pointer field that has only one legitimate value anyway when an i1 would
suffice.
However, transformations like constant propagation can separate these
ptrauth(...) constants from their storage location, at which point CodeGen would
have no idea what address to use. So the field does need to be the actual
address.
_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Peter Collingbourne via llvm-dev
2020-Dec-08 20:36 UTC
[llvm-dev] RFC: ConstantPtrAuth for signed pointers on AArch64
On Tue, Dec 8, 2020 at 4:31 AM Tim Northover via llvm-dev < llvm-dev at lists.llvm.org> wrote:> 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> > Why is the address discriminator not just a bool? > ------------------------------------------------- > > The relocations in MachO only allow you to use the address being relocated > as > the address-discriminator, so at first sight it seems excessive to have an > entire pointer field that has only one legitimate value anyway when an i1 > would > suffice. > > However, transformations like constant propagation can separate these > ptrauth(...) constants from their storage location, at which point CodeGen > would > have no idea what address to use. So the field does need to be the actual > address. > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-- -- Peter -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201208/77b46988/attachment.html>