Skip to content

Commit c965ad0

Browse files
ahatanakahatanak
committed
Address review comments
1 parent b6adace commit c965ad0

File tree

14 files changed

+83
-81
lines changed

14 files changed

+83
-81
lines changed

‎clang/docs/ReleaseNotes.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,8 @@ X86 Support
193193
Arm and AArch64 Support
194194
^^^^^^^^^^^^^^^^^^^^^^^
195195

196+
- Support for __ptrauth type qualifier has been added.
197+
196198
Android Support
197199
^^^^^^^^^^^^^^^
198200

‎clang/include/clang/AST/Type.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ class Qualifiers {
810810
"PointerAuthQualifier must be 32 bits");
811811

812812
static constexpr uint64_t PtrAuthShift = 32;
813-
static constexpr uint64_t PtrAuthMask = uint64_t(0xffffffff) << PtrAuthShift;
813+
static constexpr uint64_t PtrAuthMask = UINT64_C(0xffffffff) << PtrAuthShift;
814814

815815
static constexpr uint64_t UMask = 0x8;
816816
static constexpr uint64_t UShift = 3;

‎clang/include/clang/Basic/DiagnosticSemaKinds.td‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -982,22 +982,21 @@ def err_ptrauth_indirect_goto_addrlabel_arithmetic : Error<
982982

983983
// __ptrauth qualifier
984984
def err_ptrauth_qualifier_invalid : Error<
985-
"%select{return types|parameter types|properties}1 may not be qualified with __ptrauth; type is %0">;
985+
"%select{return type|parameter type|property}1 may not be qualified with '__ptrauth'; type is %0">;
986986
def err_ptrauth_qualifier_cast : Error<
987-
"cast types may not be qualified with __ptrauth; type is %0">;
987+
"cannot cast to '__ptrauth'-qualified type %0">;
988988
def err_ptrauth_qualifier_nonpointer : Error<
989-
"__ptrauth qualifier may only be applied to pointer types; type here is %0">;
989+
"'__ptrauth' qualifier only applies to pointer types; %0 is invalid">;
990990
def err_ptrauth_qualifier_redundant : Error<
991991
"type %0 is already %1-qualified">;
992992
def err_ptrauth_qualifier_bad_arg_count : Error<
993-
"%0 qualifier must take between 1 and 3 arguments">;
993+
"'__ptrauth' qualifier must take between 1 and 3 arguments">;
994994
def err_ptrauth_arg_not_ice : Error<
995-
"argument to __ptrauth must be an integer constant expression">;
995+
"argument to '__ptrauth' must be an integer constant expression">;
996996
def err_ptrauth_address_discrimination_invalid : Error<
997-
"address discrimination flag for __ptrauth must be 0 or 1; value is %0">;
997+
"invalid address discrimination flag '%0'; '__ptrauth' requires '0' or '1'">;
998998
def err_ptrauth_extra_discriminator_invalid : Error<
999-
"extra discriminator for __ptrauth must be between "
1000-
"0 and %1; value is %0">;
999+
"invalid extra discriminator flag '%0'; '__ptrauth' requires a value between '0' and '%1'">;
10011000

10021001
/// main()
10031002
// static main() is not an error in C, just in C++.
@@ -3963,7 +3962,7 @@ def note_cannot_use_trivial_abi_reason : Note<
39633962
"it is polymorphic|"
39643963
"it has a base of a non-trivial class type|it has a virtual base|"
39653964
"it has a __weak field|it has a field of a non-trivial class type|"
3966-
"it has an address-discriminated __ptrauth field}1">;
3965+
"it has an address-discriminated '__ptrauth' field}1">;
39673966

39683967
// Availability attribute
39693968
def warn_availability_unknown_platform : Warning<
@@ -5039,8 +5038,8 @@ def note_ovl_candidate_bad_ownership : Note<
50395038
"__autoreleasing}5 ownership">;
50405039
def note_ovl_candidate_bad_ptrauth : Note<
50415040
"candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
5042-
"%ordinal8 argument (%3) has %select{no ptrauth|%5}4 qualifier,"
5043-
" but parameter has %select{no ptrauth|%7}6 qualifier">;
5041+
"%ordinal8 argument (%3) has %select{no '__ptrauth'|%5}4 qualifier,"
5042+
" but parameter has %select{no '__ptrauth'|%7}6 qualifier">;
50445043
def note_ovl_candidate_bad_cvr_this : Note<
50455044
"candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
50465045
"'this' argument has type %3, but method is not marked "
@@ -6136,7 +6135,7 @@ def note_deleted_special_member_class_subobject : Note<
61366135
"%select{default|corresponding|default|default|default}4 constructor}0|"
61376136
"destructor}5"
61386137
"%select{||s||}4"
6139-
"|is an ObjC pointer|has an address-discriminated ptrauth qualifier}6">;
6138+
"|is an ObjC pointer|has an address-discriminated '__ptrauth' qualifier}6">;
61406139
def note_default_constructed_field
61416140
: Note<"default constructed field %0 declared here">;
61426141
def note_deleted_default_ctor_uninit_field : Note<
@@ -8991,7 +8990,7 @@ def err_typecheck_incompatible_ptrauth : Error<
89918990
"|%diff{sending $ to parameter of type $|"
89928991
"sending to parameter of different type}0,1"
89938992
"|%diff{casting $ to type $|casting between types}0,1}2"
8994-
" changes pointer-authentication of pointee type">;
8993+
" changes pointer authentication of pointee type">;
89958994
def err_typecheck_comparison_of_distinct_blocks : Error<
89968995
"comparison of distinct block types%diff{ ($ and $)|}0,1">;
89978996

@@ -9399,7 +9398,7 @@ def ext_typecheck_cond_pointer_integer_mismatch : ExtWarn<
93999398
"%diff{ ($ and $)|}0,1">,
94009399
InGroup<DiagGroup<"conditional-type-mismatch">>;
94019400
def err_typecheck_cond_incompatible_ptrauth : Error<
9402-
"__ptrauth qualification mismatch%diff{ ($ and $)|}0,1">;
9401+
"'__ptrauth' qualification mismatch%diff{ ($ and $)|}0,1">;
94039402
def err_typecheck_choose_expr_requires_constant : Error<
94049403
"'__builtin_choose_expr' requires a constant expression">;
94059404
def warn_unused_expr : Warning<"expression result unused">,

‎clang/lib/AST/TypePrinter.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2542,9 +2542,9 @@ bool Qualifiers::isEmptyWhenPrinted(const PrintingPolicy &Policy) const {
25422542
if (!(lifetime == Qualifiers::OCL_Strong && Policy.SuppressStrongLifetime))
25432543
return false;
25442544

2545-
if (PointerAuthQualifier PointerAuth = getPointerAuth())
2546-
if (!PointerAuth.isEmptyWhenPrinted(Policy))
2547-
return false;
2545+
if (PointerAuthQualifier PointerAuth = getPointerAuth();
2546+
PointerAuth && !PointerAuth.isEmptyWhenPrinted(Policy))
2547+
return false;
25482548

25492549
return true;
25502550
}

‎clang/lib/CodeGen/CGClass.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -929,9 +929,9 @@ namespace {
929929
Qualifiers Qual = F->getType().getQualifiers();
930930
if (Qual.hasVolatile() || Qual.hasObjCLifetime())
931931
return false;
932-
if (PointerAuthQualifier Q = F->getType().getPointerAuth())
933-
if (Q.isAddressDiscriminated())
934-
return false;
932+
if (PointerAuthQualifier Q = F->getType().getPointerAuth();
933+
Q && Q.isAddressDiscriminated())
934+
return false;
935935
return true;
936936
}
937937

‎clang/lib/CodeGen/CGDebugInfo.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1068,7 +1068,7 @@ llvm::DIType *CGDebugInfo::CreateQualifiedType(QualType Ty,
10681068
Qc.getPointerAuth().authenticatesNullValues();
10691069
Qc.removePointerAuth();
10701070
assert(Qc.empty() && "Unknown type qualifier for debug info");
1071-
auto *FromTy = getOrCreateType(QualType(T, 0), Unit);
1071+
llvm::DIType *FromTy = getOrCreateType(QualType(T, 0), Unit);
10721072
return DBuilder.createPtrAuthQualifiedType(FromTy, Key, IsDiscr,
10731073
ExtraDiscr, IsaPointer,
10741074
AuthenticatesNullValues);

‎clang/lib/CodeGen/CGExpr.cpp‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -568,8 +568,8 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
568568
QualType RefType = M->getType().withoutLocalFastQualifiers();
569569
if (RefType.getPointerAuth()) {
570570
// Use the qualifier of the reference temporary to sign the pointer.
571-
auto LV = MakeRawAddrLValue(Object.getPointer(), RefType,
572-
Object.getAlignment());
571+
LValue LV = MakeRawAddrLValue(Object.getPointer(), RefType,
572+
Object.getAlignment());
573573
EmitScalarInit(E, M->getExtendingDecl(), LV, false);
574574
} else {
575575
EmitAnyExprToMem(E, Object, Qualifiers(), /*IsInit*/ true);
@@ -1774,7 +1774,7 @@ CodeGenFunction::tryEmitAsConstant(const DeclRefExpr *RefExpr) {
17741774
ConstantEmissionKind CEK;
17751775
if (isa<ParmVarDecl>(Value)) {
17761776
CEK = CEK_None;
1777-
} else if (auto *var = dyn_cast<VarDecl>(Value)) {
1777+
} else if (const auto *var = dyn_cast<VarDecl>(Value)) {
17781778
CEK = checkVarTypeForConstantEmission(var->getType());
17791779
} else if (isa<EnumConstantDecl>(Value)) {
17801780
CEK = CEK_AsValueOnly;
@@ -1831,8 +1831,8 @@ CodeGenFunction::tryEmitAsConstant(const DeclRefExpr *RefExpr) {
18311831
}
18321832

18331833
// Emit as a constant.
1834-
auto C = ConstantEmitter(*this).emitAbstract(RefExpr->getLocation(),
1835-
result.Val, resultType);
1834+
llvm::Constant *C = ConstantEmitter(*this).emitAbstract(
1835+
RefExpr->getLocation(), result.Val, resultType);
18361836

18371837
// Make sure we emit a debug reference to the global variable.
18381838
// This should probably fire even for
@@ -2236,7 +2236,7 @@ RValue CodeGenFunction::EmitLoadOfLValue(LValue LV, SourceLocation Loc) {
22362236
// Load from __ptrauth.
22372237
if (PointerAuthQualifier PtrAuth = LV.getQuals().getPointerAuth()) {
22382238
LV.getQuals().removePointerAuth();
2239-
auto Value = EmitLoadOfLValue(LV, Loc).getScalarVal();
2239+
llvm::Value *Value = EmitLoadOfLValue(LV, Loc).getScalarVal();
22402240
return RValue::get(EmitPointerAuthUnqualify(PtrAuth, Value, LV.getType(),
22412241
LV.getAddress(),
22422242
/*known nonnull*/ false));
@@ -5735,9 +5735,10 @@ CGCallee CodeGenFunction::EmitCallee(const Expr *E) {
57355735
// Try to remember the original __ptrauth qualifier for loads of
57365736
// function pointers.
57375737
if (ICE->getCastKind() == CK_LValueToRValue) {
5738-
const auto *SubExpr = ICE->getSubExpr();
5738+
const Expr *SubExpr = ICE->getSubExpr();
57395739
if (const auto *PtrType = SubExpr->getType()->getAs<PointerType>()) {
5740-
auto Result = EmitOrigPointerRValue(E);
5740+
std::pair<llvm::Value *, CGPointerAuthInfo> Result =
5741+
EmitOrigPointerRValue(E);
57415742

57425743
QualType FunctionType = PtrType->getPointeeType();
57435744
assert(FunctionType->isFunctionType());

‎clang/lib/CodeGen/CGExprConstant.cpp‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2154,12 +2154,11 @@ llvm::Constant *ConstantLValueEmitter::tryEmit() {
21542154
}
21552155

21562156
// Apply pointer-auth signing from the destination type.
2157-
if (PointerAuthQualifier PointerAuth = DestType.getPointerAuth()) {
2158-
if (!result.HasDestPointerAuth) {
2159-
value = Emitter.tryEmitConstantSignedPointer(value, PointerAuth);
2160-
if (!value)
2161-
return nullptr;
2162-
}
2157+
if (PointerAuthQualifier PointerAuth = DestType.getPointerAuth();
2158+
PointerAuth && !result.HasDestPointerAuth) {
2159+
value = Emitter.tryEmitConstantSignedPointer(value, PointerAuth);
2160+
if (!value)
2161+
return nullptr;
21632162
}
21642163

21652164
// Convert to the appropriate type; this could be an lvalue for

‎clang/lib/CodeGen/CGPointerAuth.cpp‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ CGPointerAuthInfo CodeGenModule::getPointerAuthInfoForType(QualType T) {
196196
static std::pair<llvm::Value *, CGPointerAuthInfo>
197197
emitLoadOfOrigPointerRValue(CodeGenFunction &CGF, const LValue &LV,
198198
SourceLocation Loc) {
199-
auto *Value = CGF.EmitLoadOfScalar(LV, Loc);
199+
llvm::Value *Value = CGF.EmitLoadOfScalar(LV, Loc);
200200
CGPointerAuthInfo AuthInfo;
201201
if (PointerAuthQualifier PtrAuth = LV.getQuals().getPointerAuth())
202202
AuthInfo = CGF.EmitPointerAuthInfo(PtrAuth, LV.getAddress());
@@ -218,26 +218,26 @@ CodeGenFunction::EmitOrigPointerRValue(const Expr *E) {
218218

219219
// We're semantically required to not emit loads of certain DREs naively.
220220
if (const auto *RefExpr = dyn_cast<DeclRefExpr>(E)) {
221-
if (auto Result = tryEmitAsConstant(RefExpr)) {
221+
if (ConstantEmission Result = tryEmitAsConstant(RefExpr)) {
222222
// Fold away a use of an intermediate variable.
223223
if (!Result.isReference())
224224
return {Result.getValue(),
225225
getPointerAuthInfoForType(CGM, RefExpr->getType())};
226226

227227
// Fold away a use of an intermediate reference.
228-
auto LV = Result.getReferenceLValue(*this, RefExpr);
228+
LValue LV = Result.getReferenceLValue(*this, RefExpr);
229229
return emitLoadOfOrigPointerRValue(*this, LV, RefExpr->getLocation());
230230
}
231231
}
232232

233233
// Otherwise, load and use the pointer
234-
auto LV = EmitCheckedLValue(E, CodeGenFunction::TCK_Load);
234+
LValue LV = EmitCheckedLValue(E, CodeGenFunction::TCK_Load);
235235
return emitLoadOfOrigPointerRValue(*this, LV, E->getExprLoc());
236236
}
237237
}
238238

239239
// Fallback: just use the normal rules for the type.
240-
auto *Value = EmitScalarExpr(E);
240+
llvm::Value *Value = EmitScalarExpr(E);
241241
return {Value, getPointerAuthInfoForType(CGM, E->getType())};
242242
}
243243

@@ -248,7 +248,8 @@ CodeGenFunction::EmitPointerAuthQualify(PointerAuthQualifier DestQualifier,
248248
assert(DestQualifier);
249249
auto [Value, CurAuthInfo] = EmitOrigPointerRValue(E);
250250

251-
auto DestAuthInfo = EmitPointerAuthInfo(DestQualifier, DestStorageAddress);
251+
CGPointerAuthInfo DestAuthInfo =
252+
EmitPointerAuthInfo(DestQualifier, DestStorageAddress);
252253
return emitPointerAuthResign(Value, E->getType(), CurAuthInfo, DestAuthInfo,
253254
isPointerKnownNonNull(E));
254255
}
@@ -258,8 +259,9 @@ llvm::Value *CodeGenFunction::EmitPointerAuthQualify(
258259
QualType PointerType, Address DestStorageAddress, bool IsKnownNonNull) {
259260
assert(DestQualifier);
260261

261-
auto CurAuthInfo = getPointerAuthInfoForType(CGM, PointerType);
262-
auto DestAuthInfo = EmitPointerAuthInfo(DestQualifier, DestStorageAddress);
262+
CGPointerAuthInfo CurAuthInfo = getPointerAuthInfoForType(CGM, PointerType);
263+
CGPointerAuthInfo DestAuthInfo =
264+
EmitPointerAuthInfo(DestQualifier, DestStorageAddress);
263265
return emitPointerAuthResign(Value, PointerType, CurAuthInfo, DestAuthInfo,
264266
IsKnownNonNull);
265267
}
@@ -269,8 +271,9 @@ llvm::Value *CodeGenFunction::EmitPointerAuthUnqualify(
269271
Address CurStorageAddress, bool IsKnownNonNull) {
270272
assert(CurQualifier);
271273

272-
auto CurAuthInfo = EmitPointerAuthInfo(CurQualifier, CurStorageAddress);
273-
auto DestAuthInfo = getPointerAuthInfoForType(CGM, PointerType);
274+
CGPointerAuthInfo CurAuthInfo =
275+
EmitPointerAuthInfo(CurQualifier, CurStorageAddress);
276+
CGPointerAuthInfo DestAuthInfo = getPointerAuthInfoForType(CGM, PointerType);
274277
return emitPointerAuthResign(Value, PointerType, CurAuthInfo, DestAuthInfo,
275278
IsKnownNonNull);
276279
}

‎clang/lib/Sema/SemaType.cpp‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8413,10 +8413,8 @@ static void HandleNeonVectorTypeAttr(QualType &CurType, const ParsedAttr &Attr,
84138413
/// Handle the __ptrauth qualifier.
84148414
static void HandlePtrAuthQualifier(ASTContext &Ctx, QualType &T,
84158415
const ParsedAttr &Attr, Sema &S) {
8416-
auto AttributeName = Attr.getAttrName()->getName();
84178416
if (Attr.getNumArgs() < 1 || Attr.getNumArgs() > 3) {
8418-
S.Diag(Attr.getLoc(), diag::err_ptrauth_qualifier_bad_arg_count)
8419-
<< AttributeName;
8417+
S.Diag(Attr.getLoc(), diag::err_ptrauth_qualifier_bad_arg_count);
84208418
Attr.setInvalid();
84218419
return;
84228420
}

0 commit comments

Comments
 (0)