-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[AArch64][PAC] Sign block addresses used in indirectbr. #97647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5195708
cdf8b86
c21435b
46274c3
6cf9845
ecac22c
b91e109
f9cf7d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10909,6 +10909,14 @@ QualType Sema::CheckAdditionOperands(ExprResult &LHS, ExprResult &RHS, | |
if (isObjCPointer && checkArithmeticOnObjCPointer(*this, Loc, PExp)) | ||
return QualType(); | ||
|
||
// Arithmetic on label addresses is normally allowed, except when we add | ||
// a ptrauth signature to the addresses. | ||
if (isa<AddrLabelExpr>(PExp) && getLangOpts().PointerAuthIndirectGotos) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this catches all the relevant cases: there are various ways you could "hide" an address-of-label from this check. Parentheses, a conditional operator, a constexpr variable, etc. Maybe the check should be in IntExprEvaluator::VisitBinaryOperator. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this only catches the obvious cases to start. I did try having this in IntExprEvaluator, but only got it to behave as I expected (without dupes) with what I'm pretty sure is a gross misuse of |
||
Diag(Loc, diag::err_ptrauth_indirect_goto_addrlabel_arithmetic) | ||
<< /*addition*/ 1; | ||
return QualType(); | ||
} | ||
|
||
// Check array bounds for pointer arithemtic | ||
CheckArrayAccess(PExp, IExp); | ||
|
||
|
@@ -10983,6 +10991,15 @@ QualType Sema::CheckSubtractionOperands(ExprResult &LHS, ExprResult &RHS, | |
checkArithmeticOnObjCPointer(*this, Loc, LHS.get())) | ||
return QualType(); | ||
|
||
// Arithmetic on label addresses is normally allowed, except when we add | ||
// a ptrauth signature to the addresses. | ||
if (isa<AddrLabelExpr>(LHS.get()) && | ||
getLangOpts().PointerAuthIndirectGotos) { | ||
Diag(Loc, diag::err_ptrauth_indirect_goto_addrlabel_arithmetic) | ||
<< /*subtraction*/ 0; | ||
return QualType(); | ||
} | ||
|
||
// The result type of a pointer-int computation is the pointer type. | ||
if (RHS.get()->getType()->isIntegerType()) { | ||
// Subtracting from a null pointer should produce a warning. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
// RUN: %clang_cc1 -triple arm64e-apple-darwin -fsyntax-only -verify %s -fptrauth-indirect-gotos | ||
|
||
int f() { | ||
static void *addrs[] = { &&l1, &&l2 }; | ||
|
||
static int diffs[] = { | ||
&&l1 - &&l1, // expected-error{{subtraction of address-of-label expressions is not supported with ptrauth indirect gotos}} | ||
&&l1 - &&l2 // expected-error{{subtraction of address-of-label expressions is not supported with ptrauth indirect gotos}} | ||
}; | ||
|
||
int diff_32 = &&l1 - &&l2; // expected-error{{subtraction of address-of-label expressions is not supported with ptrauth indirect gotos}} | ||
goto *(&&l1 + diff_32); // expected-error{{addition of address-of-label expressions is not supported with ptrauth indirect gotos}} | ||
|
||
l1: | ||
return 0; | ||
l2: | ||
return 1; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: there is no empty line between previous
Args.addOptInFlag(...)
invocations, so probably this new line should also be deleted.Feel free to ignore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whitespaces help differentiate the different classes of fptrauth features (here frontendy vs backendy); TBH we should probably have more, not less