[clang] Add support for __ptrauth being applied to integer types
#137580
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-clang Author: Oliver Hunt (ojhunt) Changes__ptrauth_restricted_intptr provides a mechanism to apply pointer authentication to pointer sized integer types. Patch is 49.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/137580.diff 26 Files Affected:
diff --git a/clang/docs/PointerAuthentication.rst b/clang/docs/PointerAuthentication.rst
index 41818d43ac687..2278971d757c9 100644
--- a/clang/docs/PointerAuthentication.rst
+++ b/clang/docs/PointerAuthentication.rst
@@ -326,6 +326,20 @@ a discriminator determined as follows:
is ``ptrauth_blend_discriminator(&x, discriminator)``; see
`ptrauth_blend_discriminator`_.
+__ptrauth_restricted_intptr qualifier
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+This is a variant of the ``__ptrauth`` qualifier, that applies to pointer sized
+integers. See the documentation for ``__ptrauth qualifier``.
+
+This feature exists to support older APIs that use [u]intptrs to hold opaque
+pointer types.
+
+Care must be taken to avoid using the signature bit components of the signed
+integers or subsequent authentication of the signed value may fail.
+
+Note: When applied to a global initialiser a signed uintptr can only be
+initialised with the value 0 or a global address.
+
``<ptrauth.h>``
~~~~~~~~~~~~~~~
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index eb2e8f2b8a6c0..fe9badf7ba97a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -608,7 +608,7 @@ Arm and AArch64 Support
ARM targets, however this will now disable NEON instructions being generated. The ``simd`` option is
also now printed when the ``--print-supported-extensions`` option is used.
-- Support for __ptrauth type qualifier has been added.
+- Support for __ptrauth and __ptrauth_restricted_intptr type qualifiers has been added.
- For AArch64, added support for generating executable-only code sections by using the
``-mexecute-only`` or ``-mpure-code`` compiler flags. (#GH125688)
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 3e1fb05ad537c..6516c976e66c5 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -168,8 +168,13 @@ class PointerAuthQualifier {
AuthenticatesNullValuesBits = 1,
AuthenticatesNullValuesMask = ((1 << AuthenticatesNullValuesBits) - 1)
<< AuthenticatesNullValuesShift,
- KeyShift = AuthenticatesNullValuesShift + AuthenticatesNullValuesBits,
- KeyBits = 10,
+ RestrictedIntegralShift =
+ AuthenticatesNullValuesShift + AuthenticatesNullValuesBits,
+ RestrictedIntegralBits = 1,
+ RestrictedIntegralMask = ((1 << RestrictedIntegralBits) - 1)
+ << RestrictedIntegralShift,
+ KeyShift = RestrictedIntegralShift + RestrictedIntegralBits,
+ KeyBits = 9,
KeyMask = ((1 << KeyBits) - 1) << KeyShift,
DiscriminatorShift = KeyShift + KeyBits,
DiscriminatorBits = 16,
@@ -178,32 +183,33 @@ class PointerAuthQualifier {
// bits: |0 |1 |2..3 |4 |
// |Enabled|Address|AuthenticationMode|ISA pointer|
- // bits: |5 |6..15| 16...31 |
- // |AuthenticatesNull|Key |Discriminator|
+ // bits: |5 |6 |7..15| 16...31 |
+ // |AuthenticatesNull|RestrictedIntegral|Key |Discriminator|
uint32_t Data = 0;
// The following static assertions check that each of the 32 bits is present
// exactly in one of the constants.
static_assert((EnabledBits + AddressDiscriminatedBits +
AuthenticationModeBits + IsaPointerBits +
- AuthenticatesNullValuesBits + KeyBits + DiscriminatorBits) ==
- 32,
+ AuthenticatesNullValuesBits + RestrictedIntegralBits +
+ KeyBits + DiscriminatorBits) == 32,
"PointerAuthQualifier should be exactly 32 bits");
static_assert((EnabledMask + AddressDiscriminatedMask +
AuthenticationModeMask + IsaPointerMask +
- AuthenticatesNullValuesMask + KeyMask + DiscriminatorMask) ==
- 0xFFFFFFFF,
+ AuthenticatesNullValuesMask + RestrictedIntegralMask +
+ KeyMask + DiscriminatorMask) == 0xFFFFFFFF,
"All masks should cover the entire bits");
static_assert((EnabledMask ^ AddressDiscriminatedMask ^
AuthenticationModeMask ^ IsaPointerMask ^
- AuthenticatesNullValuesMask ^ KeyMask ^ DiscriminatorMask) ==
- 0xFFFFFFFF,
+ AuthenticatesNullValuesMask ^ RestrictedIntegralMask ^
+ KeyMask ^ DiscriminatorMask) == 0xFFFFFFFF,
"All masks should cover the entire bits");
PointerAuthQualifier(unsigned Key, bool IsAddressDiscriminated,
unsigned ExtraDiscriminator,
PointerAuthenticationMode AuthenticationMode,
- bool IsIsaPointer, bool AuthenticatesNullValues)
+ bool IsIsaPointer, bool AuthenticatesNullValues,
+ bool IsRestrictedIntegral)
: Data(EnabledMask |
(IsAddressDiscriminated
? llvm::to_underlying(AddressDiscriminatedMask)
@@ -212,8 +218,8 @@ class PointerAuthQualifier {
(llvm::to_underlying(AuthenticationMode)
<< AuthenticationModeShift) |
(ExtraDiscriminator << DiscriminatorShift) |
- (IsIsaPointer << IsaPointerShift) |
- (AuthenticatesNullValues << AuthenticatesNullValuesShift)) {
+ (AuthenticatesNullValues << AuthenticatesNullValuesShift) |
+ (IsRestrictedIntegral << RestrictedIntegralShift)) {
assert(Key <= KeyNoneInternal);
assert(ExtraDiscriminator <= MaxDiscriminator);
assert((Data == 0) ==
@@ -237,13 +243,13 @@ class PointerAuthQualifier {
static PointerAuthQualifier
Create(unsigned Key, bool IsAddressDiscriminated, unsigned ExtraDiscriminator,
PointerAuthenticationMode AuthenticationMode, bool IsIsaPointer,
- bool AuthenticatesNullValues) {
+ bool AuthenticatesNullValues, bool IsRestrictedIntegral) {
if (Key == PointerAuthKeyNone)
Key = KeyNoneInternal;
assert(Key <= KeyNoneInternal && "out-of-range key value");
return PointerAuthQualifier(Key, IsAddressDiscriminated, ExtraDiscriminator,
AuthenticationMode, IsIsaPointer,
- AuthenticatesNullValues);
+ AuthenticatesNullValues, IsRestrictedIntegral);
}
bool isPresent() const {
@@ -290,6 +296,10 @@ class PointerAuthQualifier {
return hasKeyNone() ? PointerAuthQualifier() : *this;
}
+ bool isRestrictedIntegral() const {
+ return (Data & RestrictedIntegralMask) >> RestrictedIntegralShift;
+ }
+
friend bool operator==(PointerAuthQualifier Lhs, PointerAuthQualifier Rhs) {
return Lhs.Data == Rhs.Data;
}
@@ -2563,7 +2573,9 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
bool isFunctionProtoType() const { return getAs<FunctionProtoType>(); }
bool isPointerType() const;
bool isPointerOrReferenceType() const;
- bool isSignableType() const;
+ bool isSignableType(const ASTContext &Ctx) const;
+ bool isSignablePointerType() const;
+ bool isSignableIntegerType(const ASTContext &Ctx) const;
bool isAnyPointerType() const; // Any C pointer or ObjC object pointer
bool isCountAttributedType() const;
bool isBlockPointerType() const;
@@ -8216,7 +8228,13 @@ inline bool Type::isAnyPointerType() const {
return isPointerType() || isObjCObjectPointerType();
}
-inline bool Type::isSignableType() const { return isPointerType(); }
+inline bool Type::isSignableType(const ASTContext &Ctx) const {
+ return isSignablePointerType() || isSignableIntegerType(Ctx);
+}
+
+inline bool Type::isSignablePointerType() const {
+ return isPointerType() || isObjCClassType() || isObjCQualifiedClassType();
+}
inline bool Type::isBlockPointerType() const {
return isa<BlockPointerType>(CanonicalType);
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index dcdcff8c46fe2..50bfe76a86619 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3578,7 +3578,8 @@ def ObjCRequiresPropertyDefs : InheritableAttr {
}
def PointerAuth : TypeAttr {
- let Spellings = [CustomKeyword<"__ptrauth">];
+ let Spellings = [CustomKeyword<"__ptrauth">,
+ CustomKeyword<"__ptrauth_restricted_intptr">];
let Args = [IntArgument<"Key">,
BoolArgument<"AddressDiscriminated", 1>,
IntArgument<"ExtraDiscriminator", 1>];
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 4c96142e28134..79e26ff9293f0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1021,20 +1021,27 @@ def err_ptrauth_indirect_goto_addrlabel_arithmetic : Error<
"supported with ptrauth indirect gotos">;
// __ptrauth qualifier
-def err_ptrauth_qualifier_invalid : Error<
- "%select{return type|parameter type|property}1 may not be qualified with '__ptrauth'; type is %0">;
-def err_ptrauth_qualifier_cast : Error<
- "cannot cast to '__ptrauth'-qualified type %0">;
-def err_ptrauth_qualifier_nonpointer : Error<
- "'__ptrauth' qualifier only applies to pointer types; %0 is invalid">;
+def err_ptrauth_qualifier_invalid
+ : Error<
+ "%select{return type|parameter type|property}0 may not be qualified "
+ "with '%select{__ptrauth_restricted_intptr|__ptrauth}1'; type is %2">;
+def err_ptrauth_qualifier_cast : Error<"cannot cast to "
+ "'%select{__ptrauth_restricted_intptr|__"
+ "ptrauth}0'-qualified type %1">;
+def err_ptrauth_qualifier_invalid_target
+ : Error<"'%select{__ptrauth_restricted_intptr|__ptrauth}0' qualifier only "
+ "applies to %select{pointer sized integer|pointer}0 types; %1 is "
+ "invalid">;
def err_ptrauth_qualifier_redundant : Error<
"type %0 is already %1-qualified">;
-def err_ptrauth_arg_not_ice : Error<
- "argument to '__ptrauth' must be an integer constant expression">;
-def err_ptrauth_address_discrimination_invalid : Error<
- "invalid address discrimination flag '%0'; '__ptrauth' requires '0' or '1'">;
-def err_ptrauth_extra_discriminator_invalid : Error<
- "invalid extra discriminator flag '%0'; '__ptrauth' requires a value between '0' and '%1'">;
+def err_ptrauth_arg_not_ice
+ : Error<"argument to '%0' must be an integer constant expression">;
+def err_ptrauth_address_discrimination_invalid
+ : Error<
+ "invalid address discrimination flag '%0'; '%1' requires '0' or '1'">;
+def err_ptrauth_extra_discriminator_invalid
+ : Error<"invalid extra discriminator flag '%0'; '%1' requires a value "
+ "between '0' and '%2'">;
/// main()
// static main() is not an error in C, just in C++.
diff --git a/clang/include/clang/Basic/Features.def b/clang/include/clang/Basic/Features.def
index 14bff8a68846d..c859167e47e57 100644
--- a/clang/include/clang/Basic/Features.def
+++ b/clang/include/clang/Basic/Features.def
@@ -108,6 +108,7 @@ FEATURE(dataflow_sanitizer, LangOpts.Sanitize.has(SanitizerKind::DataFlow))
FEATURE(scudo, LangOpts.Sanitize.hasOneOf(SanitizerKind::Scudo))
FEATURE(ptrauth_intrinsics, LangOpts.PointerAuthIntrinsics)
EXTENSION(ptrauth_qualifier, LangOpts.PointerAuthIntrinsics)
+EXTENSION(ptrauth_restricted_intptr_qualifier, LangOpts.PointerAuthIntrinsics)
FEATURE(ptrauth_calls, LangOpts.PointerAuthCalls)
FEATURE(ptrauth_returns, LangOpts.PointerAuthReturns)
FEATURE(ptrauth_vtable_pointer_address_discrimination, LangOpts.PointerAuthVTPtrAddressDiscrimination)
diff --git a/clang/include/clang/Basic/TokenKinds.def b/clang/include/clang/Basic/TokenKinds.def
index 868e851342eb8..85c0f58181c98 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -349,6 +349,7 @@ KEYWORD(__func__ , KEYALL)
KEYWORD(__objc_yes , KEYALL)
KEYWORD(__objc_no , KEYALL)
KEYWORD(__ptrauth , KEYALL)
+KEYWORD(__ptrauth_restricted_intptr , KEYALL)
// C2y
UNARY_EXPR_OR_TYPE_TRAIT(_Countof, CountOf, KEYNOCXX)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0c77c5b5ca30a..e438194230cf7 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3599,7 +3599,8 @@ class Sema final : public SemaBase {
PADAK_ExtraDiscPtrAuth,
};
- bool checkPointerAuthDiscriminatorArg(Expr *Arg, PointerAuthDiscArgKind Kind,
+ bool checkPointerAuthDiscriminatorArg(const AttributeCommonInfo &AttrInfo,
+ Expr *Arg, PointerAuthDiscArgKind Kind,
unsigned &IntVal);
/// Diagnose function specifiers on a declaration of an identifier that
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index c95e733f30494..73051f46444f8 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -2924,6 +2924,9 @@ bool ASTContext::hasUniqueObjectRepresentations(
// All integrals and enums are unique.
if (Ty->isIntegralOrEnumerationType()) {
+ // Address discriminated __ptrauth_restricted_intptr types are not unique
+ if (Ty.hasAddressDiscriminatedPointerAuth())
+ return false;
// Except _BitInt types that have padding bits.
if (const auto *BIT = Ty->getAs<BitIntType>())
return getTypeSize(BIT) == BIT->getNumBits();
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 111a642173418..4bf4a02d47fbd 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -5056,6 +5056,12 @@ AttributedType::stripOuterNullability(QualType &T) {
return std::nullopt;
}
+bool Type::isSignableIntegerType(const ASTContext &Ctx) const {
+ if (!isIntegralType(Ctx) || isEnumeralType())
+ return false;
+ return Ctx.getTypeSize(this) == Ctx.getTypeSize(Ctx.VoidPtrTy);
+}
+
bool Type::isBlockCompatibleObjCPointerType(ASTContext &ctx) const {
const auto *objcPtr = getAs<ObjCObjectPointerType>();
if (!objcPtr)
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index cba1a2d98d660..fe4283abc7017 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -2534,7 +2534,10 @@ void PointerAuthQualifier::print(raw_ostream &OS,
if (!isPresent())
return;
- OS << "__ptrauth(";
+ if (isRestrictedIntegral())
+ OS << "__ptrauth_restricted_intptr(";
+ else
+ OS << "__ptrauth(";
OS << getKey();
OS << "," << unsigned(isAddressDiscriminated()) << ","
<< getExtraDiscriminator() << ")";
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index b21ebeee4bed1..d196e39ea374c 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -2444,6 +2444,9 @@ ConstantEmitter::tryEmitPrivate(const APValue &Value, QualType DestType,
EnablePtrAuthFunctionTypeDiscrimination)
.tryEmit();
case APValue::Int:
+ if (PointerAuthQualifier PointerAuth = DestType.getPointerAuth();
+ PointerAuth && (PointerAuth.authenticatesNullValues() || Value.getInt() != 0))
+ return nullptr;
return llvm::ConstantInt::get(CGM.getLLVMContext(), Value.getInt());
case APValue::FixedPoint:
return llvm::ConstantInt::get(CGM.getLLVMContext(),
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 8dbbcdaef25d8..ff84e3c3f87e9 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2287,7 +2287,7 @@ static bool isLValueKnownNonNull(CodeGenFunction &CGF, const Expr *E) {
}
bool CodeGenFunction::isPointerKnownNonNull(const Expr *E) {
- assert(E->getType()->isSignableType());
+ assert(E->getType()->isSignableType(getContext()));
E = E->IgnoreParens();
diff --git a/clang/lib/CodeGen/CGPointerAuth.cpp b/clang/lib/CodeGen/CGPointerAuth.cpp
index 0a183a8524c17..01f43630edce3 100644
--- a/clang/lib/CodeGen/CGPointerAuth.cpp
+++ b/clang/lib/CodeGen/CGPointerAuth.cpp
@@ -175,7 +175,7 @@ CGPointerAuthInfo CodeGenModule::getPointerAuthInfoForPointeeType(QualType T) {
/// pointer type.
static CGPointerAuthInfo getPointerAuthInfoForType(CodeGenModule &CGM,
QualType PointerType) {
- assert(PointerType->isSignableType());
+ assert(PointerType->isSignableType(CGM.getContext()));
// Block pointers are currently not signed.
if (PointerType->isBlockPointerType())
@@ -209,7 +209,7 @@ emitLoadOfOrigPointerRValue(CodeGenFunction &CGF, const LValue &LV,
/// needlessly resigning the pointer.
std::pair<llvm::Value *, CGPointerAuthInfo>
CodeGenFunction::EmitOrigPointerRValue(const Expr *E) {
- assert(E->getType()->isSignableType());
+ assert(E->getType()->isSignableType(getContext()));
E = E->IgnoreParens();
if (const auto *Load = dyn_cast<ImplicitCastExpr>(E)) {
@@ -291,7 +291,10 @@ static bool equalAuthPolicies(const CGPointerAuthInfo &Left,
if (Left.isSigned() != Right.isSigned())
return false;
return Left.getKey() == Right.getKey() &&
- Left.getAuthenticationMode() == Right.getAuthenticationMode();
+ Left.getAuthenticationMode() == Right.getAuthenticationMode() &&
+ Left.isIsaPointer() == Right.isIsaPointer() &&
+ Left.authenticatesNullValues() == Right.authenticatesNullValues() &&
+ Left.getDiscriminator() == Right.getDiscriminator();
}
// Return the discriminator or return zero if the discriminator is null.
@@ -590,8 +593,9 @@ CodeGenModule::computeVTPointerAuthentication(const CXXRecordDecl *ThisClass) {
}
return PointerAuthQualifier::Create(Key, AddressDiscriminated, Discriminator,
PointerAuthenticationMode::SignAndAuth,
- /* IsIsaPointer */ false,
- /* AuthenticatesNullValues */ false);
+ /*IsIsaPointer=*/false,
+ /*AuthenticatesNullValues=*/false,
+ /*IsRestrictedIntegral=*/false);
}
std::optional<PointerAuthQualifier>
@@ -642,10 +646,10 @@ llvm::Value *CodeGenFunction::authPointerToPointerCast(llvm::Value *ResultPtr,
QualType SourceType,
QualType DestType) {
CGPointerAuthInfo CurAuthInfo, NewAuthInfo;
- if (SourceType->isSignableType())
+ if (SourceType->isSignableType(getContext()))
CurAuthInfo = getPointerAuthInfoForType(CGM, SourceType);
- if (DestType->isSignableType())
+ if (DestType->isSignableType(getContext()))
NewAuthInfo = getPointerAuthInfoForType(CGM, DestType);
if (!CurAuthInfo && !NewAuthInfo)
@@ -667,10 +671,10 @@ Address CodeGenFunction::authPointerToPointerCast(Address Ptr,
QualType SourceType,
QualType DestType) {
CGPointerAuthInfo CurAuthInfo, NewAuthInfo;
- if (SourceType->isSignableType())
+ if (SourceType->isSignableType(getContext()))
CurAuthInfo = getPointerAuthInfoForType(CGM, SourceType);
- if (DestType->isSignableType())
+ if (DestType->isSignableType(getContext()))
NewAuthInfo = getPointerAuthInfoForType(CGM, DestType);
if (!CurAuthInfo && !NewAuthInfo)
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 69d40baaf4298..4db4c8670cc31 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3401,11 +3401,12 @@ void Parser::DistributeCLateParsedAttrs(Decl *Dcl,
}
/// type-qualifier:
-/// ('__ptrauth') '(' constant-expression
+/// ('__ptrauth' | '__ptrauth_restricted_intptr') '(' constant-expression
/// (',' constant-expression)[opt]
/// (',' constant-expression)[opt] ')'
void Parser::ParsePtrauthQualifier(ParsedAttributes &Attrs) {
- assert(Tok.is(tok::kw___ptrauth));
+ assert(Tok.is(tok::kw___ptrauth) ||
+ Tok.is(tok::kw___ptrauth_restricted_intptr));
IdentifierInfo *KwName = Tok.getIdentifierInfo();
SourceLo...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ojhunt
force-pushed
the
eng/oliver/ptrauth-restricted-intptr
branch
from
7af378b to
0129e28
Compare
|
Perhaps silly initial question: why do we need a whole different qualifier for this? Why can you not write |
Not a silly question, back when first implemented we spent time thinking about this. The concern was basically |
Thank you for the explanation! This is a tough situation in some ways. We already went over all the reasons adding a new qualifier is a challenge when adding I realize you've got downstream users making use of this additional qualifier. Can you mention how prevalent the use is? Do you have evidence that the second qualifier catches issues for users, or was this mostly a theoretical problem that you wanted to head off before users ran into it? I'd like to avoid another RFC, but this is a sufficiently large extension that it may warrant it. My inclination is that this problem can be solved via good diagnostics with the other qualifier and so we don't really need an additional qualifier. |
it's used a bunch in libcxx, libcxxabi, libunwind, compiler-rt and a few other places. We can obviously use a macro to wrap this, but we need to have a way to distinguish compilers that permit __ptrauth on non-pointers, and those that require __ptrauth_restricted_intptr. There is a feature flag check available for __ptrauth_restricted_intptr so you could make an argument for pretending clang has always supported __ptrauth on integer types and have code assume that works, though that obviously causes problems where people might introduce __ptrauth qualified integers that will fail to build with older apple clangs due to the different qualifiers. Again this can be mitigated by ifdefs, e.g. #if !__has_feature(__ptrauth_restricted_intptr)
#define __ptrauth_restricted_intptr(...) __ptrauth(__VA_ARGS__)
#endifWhich in principle could go in the general
The latter, the raw __ptrauth qualifier never applied to non-pointer types so there was never the opportunity for it cause it problem. It does mean there's work required to ensure the correct spelling later on (during debugging, etc) which obviously goes away if they have the same spelling, but that going away would result in (assuming an approximation of the above) the displayed type not matching the type as written in the source, and that never happens anywhere else :D |
ojhunt
force-pushed
the
eng/oliver/ptrauth-restricted-intptr
branch
from
0129e28 to
9c29520
Compare
|
@AaronBallman have you made a decision? (if the keyword has to go away I need to replace all the tests and work out how to integrate with other libraries downstream in a way that works in both modes) |
ojhunt
force-pushed
the
eng/oliver/ptrauth-restricted-intptr
branch
from
9c29520 to
484b8cc
Compare
|
Thank you for your patience! In general, I think adding a new qualifier to Clang needs to come with significant justification for the implementation and maintenance costs. In this specific case, those costs are somewhat amortized because we already have the So on balance, I think we should only upstream the single qualifier and use a header file + feature tests for compatibility with older compilers. Is that something you can get behind? (You're still the expert in this space, so if I'm trivializing concerns, please speak up!) |
It was a very 50/50 decision to have a different spelling at the time, so I'm fine with dropping this, I just need to do some work to determine how to do the feature checks nicely.
The implementation actually becomes simpler - it removes a bunch of work for debugging, and the type checking itself is just "__ptrauth only applies to pointers and __ptrauth_restricted_intptr only applies to integers".
@AaronBallman we might need a feature check -- maybe (please help with what it should be called, I'm terrible at naming) In the mean time I'm going to update this PR to remove the new spelling and related shenanigans, and then update tests. |
ojhunt
changed the title
__ptrauth_restricted_intptr qualifier__ptrauth being applied to integer types
This adds support for the __ptrauth qualifier to be applied to pointer sized integer types.
ojhunt
force-pushed
the
eng/oliver/ptrauth-restricted-intptr
branch
from
484b8cc to
07b6275
Compare
Excellent, thank you!
Would it make sense to add the feature macro to your downstream instead? e.g., downstream supports |
We have that, I just need to determine if there's any code that treats the lack of that qualifier as meaning no ptrauth+intptr - the current PR does not technically allow this to be distinguished, which may be ok, it just requires me doing some work first - given the lack of a unique qualifier the current PR intentionally doesn't have a feature/extension flag. I think it would be reasonable to consider it done at this point, and if we do run into backwards compatibility problems we can discuss it again at that point, does that seem reasonable? As it is, with the removal of the distinct qualifier this PR becomes much simpler - just changes the exact type query, and updates integer codegen in the places it matters. |
That seems reasonable to me. |
AaronBallman
left a comment
AaronBallman
left a comment
There was a problem hiding this comment.
LGTM aside from a bit of reworking around the diagnostics.
3 participants
__ptrauth_restricted_intptr provides a mechanism to apply pointer authentication to pointer sized integer types.