[clang] Define ptrauth_sign_constant builtin. #93904
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-backend-x86 Author: Ahmed Bougacha (ahmedbougacha) ChangesThis is a constant-expression equivalent to __builtin_ptrauth_sign, allowing its usage in global initializers, but requiring constant pointers and discriminators. Patch is 25.89 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/93904.diff 14 Files Affected:
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 836697632a3bc..557b70172fc08 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -4393,6 +4393,12 @@ def PtrauthSignUnauthenticated : Builtin {
let Prototype = "void*(void*,int,void*)";
}
+def PtrauthSignConstant : Builtin {
+ let Spellings = ["__builtin_ptrauth_sign_constant"];
+ let Attributes = [CustomTypeChecking, NoThrow, Const, Constexpr];
+ let Prototype = "void*(void*,int,void*)";
+}
+
def PtrauthSignGenericData : Builtin {
let Spellings = ["__builtin_ptrauth_sign_generic_data"];
let Attributes = [CustomTypeChecking, NoThrow, Const];
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 64add46248c69..753e775ce0968 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -922,6 +922,13 @@ def err_ptrauth_value_bad_type :
Error<"%select{signed value|extra discriminator|blended pointer|blended "
"integer}0 must have %select{pointer|integer|pointer or integer}1 "
"type; type here is %2">;
+def err_ptrauth_bad_constant_pointer :
+ Error<"argument to ptrauth_sign_constant must refer to a global variable "
+ "or function">;
+def err_ptrauth_bad_constant_discriminator :
+ Error<"discriminator argument to ptrauth_sign_constant must be a constant "
+ "integer, the address of the global variable where the result "
+ "will be stored, or a blend of the two">;
def warn_ptrauth_sign_null_pointer :
Warning<"signing a null pointer will yield a non-null pointer">,
InGroup<PtrAuthNullPointers>;
diff --git a/clang/include/clang/CodeGen/CodeGenABITypes.h b/clang/include/clang/CodeGen/CodeGenABITypes.h
index fda0855dc8683..8c62d8597ecbe 100644
--- a/clang/include/clang/CodeGen/CodeGenABITypes.h
+++ b/clang/include/clang/CodeGen/CodeGenABITypes.h
@@ -104,6 +104,12 @@ llvm::Type *convertTypeForMemory(CodeGenModule &CGM, QualType T);
unsigned getLLVMFieldNumber(CodeGenModule &CGM,
const RecordDecl *RD, const FieldDecl *FD);
+/// Return a signed constant pointer.
+llvm::Constant *getConstantSignedPointer(CodeGenModule &CGM,
+ llvm::Constant *pointer,
+ unsigned key,
+ llvm::Constant *storageAddress,
+ llvm::Constant *otherDiscriminator);
/// Given the language and code-generation options that Clang was configured
/// with, set the default LLVM IR attributes for a function definition.
/// The attributes set here are mostly global target-configuration and
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index eafecfb5fe5b1..b1cb3c323074b 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2042,6 +2042,7 @@ static bool IsNoOpCall(const CallExpr *E) {
unsigned Builtin = E->getBuiltinCallee();
return (Builtin == Builtin::BI__builtin___CFStringMakeConstantString ||
Builtin == Builtin::BI__builtin___NSStringMakeConstantString ||
+ Builtin == Builtin::BI__builtin_ptrauth_sign_constant ||
Builtin == Builtin::BI__builtin_function_start);
}
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index a3c6510503324..b2e3b6fa64284 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -5273,6 +5273,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
case Builtin::BI__iso_volatile_store64:
return RValue::get(EmitISOVolatileStore(*this, E));
+ case Builtin::BI__builtin_ptrauth_sign_constant:
+ return RValue::get(ConstantEmitter(*this).emitAbstract(E, E->getType()));
+
case Builtin::BI__builtin_ptrauth_auth:
case Builtin::BI__builtin_ptrauth_auth_and_resign:
case Builtin::BI__builtin_ptrauth_blend_discriminator:
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index 4eb65b34a89f5..de9380c0e63be 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -1856,6 +1856,12 @@ class ConstantLValueEmitter : public ConstStmtVisitor<ConstantLValueEmitter,
ConstantLValue VisitMaterializeTemporaryExpr(
const MaterializeTemporaryExpr *E);
+ ConstantLValue emitPointerAuthSignConstant(const CallExpr *E);
+ llvm::Constant *emitPointerAuthPointer(const Expr *E);
+ unsigned emitPointerAuthKey(const Expr *E);
+ std::pair<llvm::Constant*, llvm::Constant*>
+ emitPointerAuthDiscriminator(const Expr *E);
+
bool hasNonZeroOffset() const {
return !Value.getLValueOffset().isZero();
}
@@ -2048,6 +2054,10 @@ ConstantLValueEmitter::VisitCallExpr(const CallExpr *E) {
if (builtin == Builtin::BI__builtin_function_start)
return CGM.GetFunctionStart(
E->getArg(0)->getAsBuiltinConstantDeclRef(CGM.getContext()));
+
+ if (builtin == Builtin::BI__builtin_ptrauth_sign_constant)
+ return emitPointerAuthSignConstant(E);
+
if (builtin != Builtin::BI__builtin___CFStringMakeConstantString &&
builtin != Builtin::BI__builtin___NSStringMakeConstantString)
return nullptr;
@@ -2061,6 +2071,58 @@ ConstantLValueEmitter::VisitCallExpr(const CallExpr *E) {
}
}
+ConstantLValue
+ConstantLValueEmitter::emitPointerAuthSignConstant(const CallExpr *E) {
+ auto unsignedPointer = emitPointerAuthPointer(E->getArg(0));
+ auto key = emitPointerAuthKey(E->getArg(1));
+ llvm::Constant *storageAddress;
+ llvm::Constant *otherDiscriminator;
+ std::tie(storageAddress, otherDiscriminator) =
+ emitPointerAuthDiscriminator(E->getArg(2));
+
+ auto signedPointer =
+ CGM.getConstantSignedPointer(unsignedPointer, key, storageAddress,
+ otherDiscriminator);
+ return signedPointer;
+}
+
+llvm::Constant *ConstantLValueEmitter::emitPointerAuthPointer(const Expr *E) {
+ Expr::EvalResult result;
+ bool succeeded = E->EvaluateAsRValue(result, CGM.getContext());
+ assert(succeeded); (void) succeeded;
+
+ // The assertions here are all checked by Sema.
+ assert(result.Val.isLValue());
+ return ConstantEmitter(CGM, Emitter.CGF)
+ .emitAbstract(E->getExprLoc(), result.Val, E->getType());
+}
+
+unsigned ConstantLValueEmitter::emitPointerAuthKey(const Expr *E) {
+ return E->EvaluateKnownConstInt(CGM.getContext()).getZExtValue();
+}
+
+std::pair<llvm::Constant*, llvm::Constant*>
+ConstantLValueEmitter::emitPointerAuthDiscriminator(const Expr *E) {
+ E = E->IgnoreParens();
+
+ if (auto call = dyn_cast<CallExpr>(E)) {
+ if (call->getBuiltinCallee() ==
+ Builtin::BI__builtin_ptrauth_blend_discriminator) {
+ auto pointer = ConstantEmitter(CGM).emitAbstract(call->getArg(0),
+ call->getArg(0)->getType());
+ auto extra = ConstantEmitter(CGM).emitAbstract(call->getArg(1),
+ call->getArg(1)->getType());
+ return { pointer, extra };
+ }
+ }
+
+ auto result = ConstantEmitter(CGM).emitAbstract(E, E->getType());
+ if (result->getType()->isPointerTy())
+ return { result, nullptr };
+ else
+ return { nullptr, result };
+}
+
ConstantLValue
ConstantLValueEmitter::VisitBlockExpr(const BlockExpr *E) {
StringRef functionName;
diff --git a/clang/lib/CodeGen/CGPointerAuth.cpp b/clang/lib/CodeGen/CGPointerAuth.cpp
new file mode 100644
index 0000000000000..756c00aa42c8c
--- /dev/null
+++ b/clang/lib/CodeGen/CGPointerAuth.cpp
@@ -0,0 +1,77 @@
+//===--- CGPointerAuth.cpp - IR generation for pointer authentication -----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains common routines relating to the emission of
+// pointer authentication operations.
+//
+//===----------------------------------------------------------------------===//
+
+#include "CGCXXABI.h"
+#include "CGCall.h"
+#include "CodeGenFunction.h"
+#include "CodeGenModule.h"
+#include "clang/AST/Attr.h"
+#include "clang/Basic/PointerAuthOptions.h"
+#include "clang/CodeGen/CodeGenABITypes.h"
+#include "clang/CodeGen/ConstantInitBuilder.h"
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/IR/ValueMap.h"
+#include "llvm/Analysis/ValueTracking.h"
+#include <vector>
+
+using namespace clang;
+using namespace CodeGen;
+
+/// Build a signed-pointer "ptrauth" constant.
+static llvm::ConstantPtrAuth *
+buildConstantAddress(CodeGenModule &CGM, llvm::Constant *pointer, unsigned key,
+ llvm::Constant *storageAddress,
+ llvm::Constant *otherDiscriminator) {
+ llvm::Constant *addressDiscriminator = nullptr;
+ if (storageAddress) {
+ addressDiscriminator = storageAddress;
+ assert(storageAddress->getType() == CGM.UnqualPtrTy);
+ } else {
+ addressDiscriminator = llvm::Constant::getNullValue(CGM.UnqualPtrTy);
+ }
+
+ llvm::ConstantInt *integerDiscriminator = nullptr;
+ if (otherDiscriminator) {
+ assert(otherDiscriminator->getType() == CGM.Int64Ty);
+ integerDiscriminator = cast<llvm::ConstantInt>(otherDiscriminator);
+ } else {
+ integerDiscriminator = llvm::ConstantInt::get(CGM.Int64Ty, 0);
+ }
+
+ return llvm::ConstantPtrAuth::get(
+ pointer, llvm::ConstantInt::get(CGM.Int32Ty, key), integerDiscriminator,
+ addressDiscriminator);
+}
+
+llvm::Constant *
+CodeGenModule::getConstantSignedPointer(llvm::Constant *pointer,
+ unsigned key,
+ llvm::Constant *storageAddress,
+ llvm::Constant *otherDiscriminator) {
+ // Unique based on the underlying value, not a signing of it.
+ auto stripped = pointer->stripPointerCasts();
+
+ // Build the constant.
+ return buildConstantAddress(*this, stripped, key, storageAddress,
+ otherDiscriminator);
+}
+
+llvm::Constant *
+CodeGen::getConstantSignedPointer(CodeGenModule &CGM,
+ llvm::Constant *pointer, unsigned key,
+ llvm::Constant *storageAddress,
+ llvm::Constant *otherDiscriminator) {
+ return CGM.getConstantSignedPointer(pointer, key, storageAddress,
+ otherDiscriminator);
+}
diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt
index 7a933d0ed0d0d..8dd9d8b54c25f 100644
--- a/clang/lib/CodeGen/CMakeLists.txt
+++ b/clang/lib/CodeGen/CMakeLists.txt
@@ -89,6 +89,7 @@ add_clang_library(clangCodeGen
CGOpenCLRuntime.cpp
CGOpenMPRuntime.cpp
CGOpenMPRuntimeGPU.cpp
+ CGPointerAuth.cpp
CGRecordLayoutBuilder.cpp
CGStmt.cpp
CGStmtOpenMP.cpp
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 0f68418130ead..194ac180171e0 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -938,6 +938,11 @@ class CodeGenModule : public CodeGenTypeCache {
// Return the function body address of the given function.
llvm::Constant *GetFunctionStart(const ValueDecl *Decl);
+ llvm::Constant *getConstantSignedPointer(llvm::Constant *pointer,
+ unsigned key,
+ llvm::Constant *storageAddress,
+ llvm::Constant *extraDiscrim);
+
// Return whether RTTI information should be emitted for this target.
bool shouldEmitRTTI(bool ForEH = false) {
return (ForEH || getLangOpts().RTTI) && !getLangOpts().CUDAIsDevice &&
diff --git a/clang/lib/Headers/ptrauth.h b/clang/lib/Headers/ptrauth.h
index 3e58af1802084..069bd6ad03b03 100644
--- a/clang/lib/Headers/ptrauth.h
+++ b/clang/lib/Headers/ptrauth.h
@@ -78,12 +78,30 @@ typedef __UINTPTR_TYPE__ ptrauth_generic_signature_t;
On arm64e, the integer must fall within the range of a uint16_t;
other bits may be ignored.
+ For the purposes of ptrauth_sign_constant, the result of calling
+ this function is considered a constant expression if the arguments
+ are constant. Some restrictions may be imposed on the pointer.
+
The first argument must be an expression of pointer type.
The second argument must be an expression of integer type.
The result will have type uintptr_t. */
#define ptrauth_blend_discriminator(__pointer, __integer) \
__builtin_ptrauth_blend_discriminator(__pointer, __integer)
+/* Add a signature to the given pointer value using a specific key,
+ using the given extra data as a salt to the signing process.
+
+ The value must be a constant expression of pointer type.
+ The key must be a constant expression of type ptrauth_key.
+ The extra data must be a constant expression of pointer or integer type;
+ if an integer, it will be coerced to ptrauth_extra_data_t.
+ The result will have the same type as the original value.
+
+ This is a constant expression if the extra data is an integer or
+ null pointer constant. */
+#define ptrauth_sign_constant(__value, __key, __data) \
+ __builtin_ptrauth_sign_constant(__value, __key, __data)
+
/* Add a signature to the given pointer value using a specific key,
using the given extra data as a salt to the signing process.
@@ -183,6 +201,13 @@ typedef __UINTPTR_TYPE__ ptrauth_generic_signature_t;
((ptrauth_extra_data_t)0); \
})
+#define ptrauth_sign_constant(__value, __key, __data) \
+ ({ \
+ (void)__key; \
+ (void)__data; \
+ __value; \
+ })
+
#define ptrauth_sign_unauthenticated(__value, __key, __data) \
({ \
(void)__key; \
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index f6012ef4b3601..52eba53cd2053 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2030,8 +2030,26 @@ bool Sema::checkConstantPointerAuthKey(Expr *Arg, unsigned &Result) {
return false;
}
+static std::pair<const ValueDecl *, CharUnits>
+findConstantBaseAndOffset(Sema &S, Expr *E) {
+ // Must evaluate as a pointer.
+ Expr::EvalResult result;
+ if (!E->EvaluateAsRValue(result, S.Context) ||
+ !result.Val.isLValue())
+ return std::make_pair(nullptr, CharUnits());
+
+ // Base must be a declaration and can't be weakly imported.
+ auto baseDecl =
+ result.Val.getLValueBase().dyn_cast<const ValueDecl *>();
+ if (!baseDecl || baseDecl->hasAttr<WeakRefAttr>())
+ return std::make_pair(nullptr, CharUnits());
+
+ return std::make_pair(baseDecl, result.Val.getLValueOffset());
+}
+
static bool checkPointerAuthValue(Sema &S, Expr *&Arg,
- PointerAuthOpKind OpKind) {
+ PointerAuthOpKind OpKind,
+ bool RequireConstant = false) {
if (Arg->hasPlaceholderType()) {
ExprResult R = S.CheckPlaceholderExpr(Arg);
if (R.isInvalid())
@@ -2074,16 +2092,91 @@ static bool checkPointerAuthValue(Sema &S, Expr *&Arg,
if (convertArgumentToType(S, Arg, ExpectedTy))
return true;
- // Warn about null pointers for non-generic sign and auth operations.
- if ((OpKind == PAO_Sign || OpKind == PAO_Auth) &&
- Arg->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNull)) {
- S.Diag(Arg->getExprLoc(), OpKind == PAO_Sign
- ? diag::warn_ptrauth_sign_null_pointer
- : diag::warn_ptrauth_auth_null_pointer)
- << Arg->getSourceRange();
+ if (!RequireConstant) {
+ // Warn about null pointers for non-generic sign and auth operations.
+ if ((OpKind == PAO_Sign || OpKind == PAO_Auth) &&
+ Arg->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNull)) {
+ S.Diag(Arg->getExprLoc(), OpKind == PAO_Sign
+ ? diag::warn_ptrauth_sign_null_pointer
+ : diag::warn_ptrauth_auth_null_pointer)
+ << Arg->getSourceRange();
+ }
+
+ return false;
}
- return false;
+ // Perform special checking on the arguments to ptrauth_sign_constant.
+
+ // The main argument.
+ if (OpKind == PAO_Sign) {
+ // Require the value we're signing to have a special form.
+ auto result = findConstantBaseAndOffset(S, Arg);
+ bool invalid;
+
+ // Must be rooted in a declaration reference.
+ if (!result.first) {
+ invalid = true;
+
+ // If it's a function declaration, we can't have an offset.
+ } else if (isa<FunctionDecl>(result.first)) {
+ invalid = !result.second.isZero();
+
+ // Otherwise we're fine.
+ } else {
+ invalid = false;
+ }
+
+ if (invalid) {
+ S.Diag(Arg->getExprLoc(), diag::err_ptrauth_bad_constant_pointer);
+ }
+ return invalid;
+ }
+
+ // The discriminator argument.
+ assert(OpKind == PAO_Discriminator);
+
+ // Must be a pointer or integer or blend thereof.
+ Expr *pointer = nullptr;
+ Expr *integer = nullptr;
+ if (auto call = dyn_cast<CallExpr>(Arg->IgnoreParens())) {
+ if (call->getBuiltinCallee() ==
+ Builtin::BI__builtin_ptrauth_blend_discriminator) {
+ pointer = call->getArg(0);
+ integer = call->getArg(1);
+ }
+ }
+ if (!pointer && !integer) {
+ if (Arg->getType()->isPointerType())
+ pointer = Arg;
+ else
+ integer = Arg;
+ }
+
+ // Check the pointer.
+ bool invalid = false;
+ if (pointer) {
+ assert(pointer->getType()->isPointerType());
+
+ // TODO: if we're initializing a global, check that the address is
+ // somehow related to what we're initializing. This probably will
+ // never really be feasible and we'll have to catch it at link-time.
+ auto result = findConstantBaseAndOffset(S, pointer);
+ if (!result.first || !isa<VarDecl>(result.first)) {
+ invalid = true;
+ }
+ }
+
+ // Check the integer.
+ if (integer) {
+ assert(integer->getType()->isIntegerType());
+ if (!integer->isEvaluatable(S.Context))
+ invalid = true;
+ }
+
+ if (invalid) {
+ S.Diag(Arg->getExprLoc(), diag::err_ptrauth_bad_constant_discriminator);
+ }
+ return invalid;
}
static ExprResult PointerAuthStrip(Sema &S, CallExpr *Call) {
@@ -2126,14 +2219,16 @@ static ExprResult PointerAuthSignGenericData(Sema &S, CallExpr *Call) {
}
static ExprResult PointerAuthSignOrAuth(Sema &S, CallExpr *Call,
- PointerAuthOpKind OpKind) {
+ PointerAuthOpKind OpKind,
+ bool RequireConstant) {
if (S.checkArgCount(Call, 3))
return ExprError();
if (checkPointerAuthEnabled(S, Call))
return ExprError();
- if (checkPointerAuthValue(S, Call->getArgs()[0], OpKind) ||
+ if (checkPointerAuthValue(S, Call->getArgs()[0], OpKind, RequireConstant) ||
checkPointerAuthKey(S, Call->getArgs()[1]) ||
- checkPointerAuthValue(S, Call->getArgs()[2], PAO_Discriminator))
+ checkPointerAuthValue(S, Call->getArgs()[2], PAO_Discriminator,
+ RequireConstant))
return ExprError();
Call->setType(Call->getArgs()[0]->getType());
@@ -2932,10 +3027,15 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
return PointerAuthStrip(*this, TheCall);
case Builtin::BI__builtin_ptrauth_blend_discriminator:
return PointerAuthBlendDiscriminator(*this, TheCall);
+ case Builtin::BI__builtin_ptrauth_sign_constant:
+ return PointerAuthSignOrAuth(*this, TheCall, PAO_Sign,
+ ...
[truncated]
|
|
Why do we want a separate builtin, as opposed to just constant-folding calls to __builtin_ptrauth_sign? |
That's a good question. Mechanically, I assumed constant-evaluating Conceptually though, the distinction does seem useful, because we treat For the naughty users we could and maybe should do the opportunistic folding of |
kovdan01
requested changes
Jun 3, 2024
There was a problem hiding this comment.
The changes themself in terms of functionality look OK to me - but I want someone else with deeper understanding of the context to take a look before this gets merged.
In terms of style, here are some of the categories of minor issues:
- unneeded includes
- wrong case for variables (need
PascalCase, the surrounding code also uses that, so no need for not following the global convention https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly) - missing
constqualifiers andauto->auto *
As for discussion above:
Conceptually though, the distinction does seem useful, because we treat
__builtin_ptrauth_sign_unauthenticatedas dangerous (because in general we can't guarantee that it won't get lowered to a signing oracle), and only to be used carefully, when absolutely necessary (e.g., in a dynamic loader.)__builtin_ptrauth_sign_constantis safe, because we do always have safe ways to materialize signed constants (with the constants in IR, and either relocations or safe pseudos in the backend.)
I agree that having such a distinction seems useful.
asl
reviewed
Jun 3, 2024
| if (S.checkArgCount(Call, 3)) | ||
| return ExprError(); | ||
| if (checkPointerAuthEnabled(S, Call)) | ||
| return ExprError(); | ||
| if (checkPointerAuthValue(S, Call->getArgs()[0], OpKind) || | ||
| if (checkPointerAuthValue(S, Call->getArgs()[0], OpKind, RequireConstant) || |
There was a problem hiding this comment.
Can we check code like Call->getArgs()[N] to Call->getArg(N) while here? The latter does assert for out of bound arguments. Certainly not everywhere, but on changed lines?
There was a problem hiding this comment.
This one is actually interesting: we can't do that as-is, because the helper functions take an Expr *&, to do type conversion and placeholder expansion in-place; see e.g., convertArgumentToType.
Another existing user makes the modification explicit using a local and setArg, but we have a lot of these calls in the checking code here.
We can rename the helper to something explicit though, perhaps checkAndConvertPointerAuthValue, and pass the call expr and arg index separately (same for key etc.)
There was a problem hiding this comment.
Yes, checkAndConvertPointerAuthValue looks better to me as otherwise it is not pretty clear there are side effects.
There was a problem hiding this comment.
Let's do that in a separate PR and adopt it for the other builtins there
kovdan01
requested changes
Jun 7, 2024
kovdan01
requested changes
Jun 10, 2024
ahmedbougacha
force-pushed
the
users/ahmedbougacha/ptrauth-string-discriminator
branch
from
8e7d4bc to
11e7dcc
Compare
ahmedbougacha
force-pushed
the
users/ahmedbougacha/ptrauth-sign-constant
branch
from
33cdfdd to
6e7ba73
Compare
kovdan01
approved these changes
Jun 19, 2024
Base automatically changed from
users/ahmedbougacha/ptrauth-string-discriminator
to
main
June 20, 2024 18:55
This is constant-expression equivalent to __builtin_ptrauth_sign, allowing its usage in global initializers, but requiring constant pointers and discriminators. Co-Authored-By: John McCall <[email protected]>
- test elf as well - rename every variable.
- don't reject weakref base pointers (allowed in quals now anyway) - fold buildConstantAddress into getConstantSignedPointer (originally to provide a caching layer for global wrappers but not needed anymore) - remove stripPointerCasts from it (originally for typed pointers) - use ConstantInt for the integer/other component of discriminators - remove CodeGen:: helper (originally for swift but not required) - rejigger tests; try for parity in fn body and global init - add a couple missing tests - tweak doc/header comment - adopt 'can be used in cst exprs' phrasing from LangExts doc - various nits, naming, and format
ahmedbougacha
force-pushed
the
users/ahmedbougacha/ptrauth-sign-constant
branch
from
1201821 to
68d0bb2
Compare
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
This is a constant-expression equivalent to ptrauth_sign_unauthenticated. Its constant nature lets us guarantee a non-attackable sequence is generated, unlike ptrauth_sign_unauthenticated which we generally discourage using. It being a constant also allows its usage in global initializers, though requiring constant pointers and discriminators. The value must be a constant expression of pointer type which evaluates to a non-null pointer. The key must be a constant expression of type ptrauth_key. The extra data must be a constant expression of pointer or integer type; if an integer, it will be coerced to ptrauth_extra_data_t. The result will have the same type as the original value. This can be used in constant expressions. Co-authored-by: John McCall <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a constant-expression equivalent to
ptrauth_sign_unauthenticated. Its constant nature lets us guarantee
a non-attackable sequence is generated, unlike
ptrauth_sign_unauthenticated which we generally discourage using.*
It being a constant also allows its usage in global initializers, though
requiring constant pointers and discriminators.