@@ -2135,6 +2135,11 @@ bool
ConstantPtrAuth::hasSpecialAddressDiscriminator(uint64_t Value) const {
bool ConstantPtrAuth::isKnownCompatibleWith(const Value *Key,
const Value *Discriminator,
ojhunt wrote:
> > I have checked in with @ahmedbougacha and his feeling is that this is fine
> > as it requires a bunch of work to opt in, and for places where the security
> > is important enough that we don't want people using this it's easy enough
> > to block.
>
> Thanks for checking.
as
ojhunt wrote:
> > > I'm concerned about this - I initially thought this was for the purpose
> > > of the structure field protection in the frontend, but this is modifying
> > > the actual pointer auth intrinsics in the backend which is very
> > > concerning given the work we need to do to merg
ojhunt wrote:
> @pcc and I have been discussing this.
>
> * The perf issues I was concerned about were predicated on access to a
> pointer loaded from a field continuing to be checked after the original field
> load, this is not the case (and in hindsight doing so would imply passing the
> po
@@ -2135,6 +2135,11 @@ bool
ConstantPtrAuth::hasSpecialAddressDiscriminator(uint64_t Value) const {
bool ConstantPtrAuth::isKnownCompatibleWith(const Value *Key,
const Value *Discriminator,
ojhunt wrote:
> This isn't possible, the symbols are resolved at static link time. See the
> RFC for more information:
> https://discourse.llvm.org/t/rfc-deactivation-symbols/85556
Oh wait, I have completely misunderstood that - I have always assumed dynamic
link and that's the reason for a b
@@ -1274,6 +1274,12 @@ def AllocaWithAlignUninitialized : Builtin {
let Prototype = "void*(size_t, _Constant size_t)";
}
+def AllocTokenInfer : Builtin {
+ let Spellings = ["__builtin_alloc_token_infer"];
ojhunt wrote:
I think `__builtin_infer_alloc_token`
@@ -3352,10 +3352,15 @@ class CodeGenFunction : public CodeGenTypeCache {
SanitizerAnnotateDebugInfo(ArrayRef
Ordinals,
SanitizerHandler Handler);
- /// Emit additional metadata used by the AllocToken instrumentation.
+ /// Emit metadata used
@@ -3352,10 +3352,15 @@ class CodeGenFunction : public CodeGenTypeCache {
SanitizerAnnotateDebugInfo(ArrayRef
Ordinals,
SanitizerHandler Handler);
- /// Emit additional metadata used by the AllocToken instrumentation.
+ /// Emit metadata used
@@ -846,6 +836,22 @@ static void addSanitizers(const Triple &TargetTriple,
}
}
+static void addAllocTokenPass(const Triple &TargetTriple,
ojhunt wrote:
I'd rather separate sema changes from codegen
https://github.com/llvm/llvm-project/pull/156842
_
@@ -5760,6 +5764,24 @@ bool Sema::BuiltinAllocaWithAlign(CallExpr *TheCall) {
return false;
}
+bool Sema::BuiltinAllocTokenInfer(CallExpr *TheCall) {
ojhunt wrote:
I would prefer this not be a Sema member, and would prefer the static function
with a `Sema&
https://github.com/ojhunt edited
https://github.com/llvm/llvm-project/pull/156842
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/ojhunt milestoned
https://github.com/llvm/llvm-project/pull/155513
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
ojhunt wrote:
Superseded by https://github.com/llvm/llvm-project/pull/155513
https://github.com/llvm/llvm-project/pull/155492
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-b
https://github.com/ojhunt closed
https://github.com/llvm/llvm-project/pull/155492
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/155513
Backporting
https://github.com/llvm/llvm-project/commit/d66b53738a29fb3a0551167efcb8d35320a539b7
Cherry-picked d66b53 with fixes to avoid ABI breakage.
A number of builtins report some variation of "this type i
@@ -634,7 +634,7 @@ class ASTContext : public RefCountedBase {
/// contain data that is address discriminated. This includes
/// implicitly authenticated values like vtable pointers, as well as
/// explicitly qualified fields.
- bool containsAddressDiscriminatedPointerAu
@@ -634,7 +634,7 @@ class ASTContext : public RefCountedBase {
/// contain data that is address discriminated. This includes
/// implicitly authenticated values like vtable pointers, as well as
/// explicitly qualified fields.
- bool containsAddressDiscriminatedPointerAu
ojhunt wrote:
> I had to squash and merge because of how stupid github is - I hope I got it
> right :(
it really is terrible :-/
https://github.com/llvm/llvm-project/pull/154608
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://github.com/ojhunt approved this pull request.
https://github.com/llvm/llvm-project/pull/155319
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
ojhunt wrote:
> @ojhunt What do you think about merging this PR to the release branch?
LGTM
https://github.com/llvm/llvm-project/pull/155319
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman
ojhunt wrote:
> ping @AaronBallman for review
@tru
I've reached out as well, but i believe the original cherrypick request was
approved
(https://github.com/llvm/llvm-project/pull/153725#pullrequestreview-3123667711)
and this PR is just the manual merge of the required commits
https://github.
ojhunt wrote:
> > Dammit, yeah I wish I could still see the conflict to work out how I
> > misread it - I suspect the addition of "A new flag -
> > `-static-libclosure`..." to new flags didn't conflict :-/
>
> You can actually, `git show --remerge-diff
> 30401b1f918ea359334b507a79118938ffe3c1
ojhunt wrote:
Dammit, yeah I wish I could still see the conflict to work out how I misread it
- I suspect the addition of "A new flag - `-static-libclosure`..." to new
flags didn't conflict :-/
https://github.com/llvm/llvm-project/pull/154608
___
l
ojhunt wrote:
@EugeneZelenko @tru
Once the last of the functional changes is merged I'll do a last sweep through
the docs and release notes to ensure that the release notes and documentation
are complete
https://github.com/llvm/llvm-project/pull/154240
ojhunt wrote:
> It probably makes sense to backport this to the release branch, but @ojhunt
> would be much better to make that decision. @ojhunt : What do you think about
> merging this back onto the LLVM 21 release branch?
@kbeyls I initiated this back port request :D :D :D
https://github.c
ojhunt wrote:
Oh, the bot already asked. Sigh.
https://github.com/llvm/llvm-project/pull/154198
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
ojhunt wrote:
This back port needs to be approved by @AaronBallman - I believe we're on the
same page, but want to be safe
https://github.com/llvm/llvm-project/pull/154198
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https:/
ojhunt wrote:
> Not opposed but this is a pretty significant amount of change for being this
> late in the rc cycles, and the changes haven't been upstream for very long.
> How risky are these changes?
This is the upstreaming of code we've had deployed for a few years at this
point, the only
ojhunt wrote:
This fixes #153726 -- (multiple comments to try and see if I can get the bot
infra to understand)
https://github.com/llvm/llvm-project/pull/153725
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm
https://github.com/ojhunt edited
https://github.com/llvm/llvm-project/pull/153725
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
ojhunt wrote:
Ah, I see - the bot cherry-pick commands are not a requirement, it can be
approved by a release manager manually.
https://github.com/llvm/llvm-project/pull/153725
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
ht
https://github.com/ojhunt reopened
https://github.com/llvm/llvm-project/pull/153725
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/ojhunt closed
https://github.com/llvm/llvm-project/pull/153665
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
ojhunt wrote:
@tru this was me misunderstanding the back porting automation sorry :D
https://github.com/llvm/llvm-project/pull/152587
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinf
https://github.com/ojhunt closed
https://github.com/llvm/llvm-project/pull/152271
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
ojhunt wrote:
/cherry-pick 726847829553079a13b1b7104f2c2db9dcda9c1d
https://github.com/llvm/llvm-project/pull/152271
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-com
https://github.com/ojhunt milestoned
https://github.com/llvm/llvm-project/pull/152271
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/152271
The codegen for the final class dynamic_cast optimization fails to consider
pointer authentication. This change resolves this be simply disabling the
optimization when pointer authentication enabled.
>From 01bf
@@ -0,0 +1,142 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
UTC_ARGS: -p --version 5
ojhunt wrote:
Tests look fine but I'd like a counter example with non-constant discriminator
to track that behavior.
https://github.com/llvm/l
@@ -8309,6 +8316,74 @@ bool
CodeGenPrepare::optimizeExtractElementInst(Instruction *Inst) {
return false;
}
+// Given the instruction Inst, rewrite its discriminator operand Disc if it is
+// a PHI node with all incoming values being @llvm.ptrauth.blend(addr, imm)
+// with
@@ -8309,6 +8316,74 @@ bool
CodeGenPrepare::optimizeExtractElementInst(Instruction *Inst) {
return false;
}
+// Given the instruction Inst, rewrite its discriminator operand Disc if it is
+// a PHI node with all incoming values being @llvm.ptrauth.blend(addr, imm)
+// with
https://github.com/ojhunt approved this pull request.
r=me with additional tests for variable discriminators
https://github.com/llvm/llvm-project/pull/150226
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org
https://github.com/ojhunt edited
https://github.com/llvm/llvm-project/pull/150226
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/ojhunt requested changes to this pull request.
https://github.com/llvm/llvm-project/pull/140312
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commi
@@ -8,7 +8,7 @@ struct S trivially_relocatable_if_eligible {
};
// CHECK: @_Z4testP1SS0_
-// CHECK: call void @llvm.memmove.p0.p0.i64
+// CHECK: call void @llvm.memmove.p0.p0.i64({{.*}}, i64 8
ojhunt wrote:
given this was a size issue, I think it's important
ojhunt wrote:
@pcc and I have been discussing this.
* The perf issues I was concerned about were predicated on access to a pointer
loaded from a field continuing to be checked after the original field load,
this is not the case (and in hindsight doing so would imply passing the pointer
as a p
ojhunt wrote:
> Hi Oliver, thanks for your comments! I'll address them below.
>
> > Thoughts:
> > This should be opt-in on a field or struct granularity, not just a global
> > behavior.
>
> This would certainly be easier if it were an opt-in behavior, as it would
> allow avoiding a substantia
@@ -441,6 +445,254 @@ bool
PreISelIntrinsicLowering::expandMemIntrinsicUses(Function &F) const {
return Changed;
}
+namespace {
+
+enum class PointerEncoding {
+ Rotate,
+ PACCopyable,
+ PACNonCopyable,
+};
+
+bool expandProtectedFieldPtr(Function &Intr) {
+ Module &M =
@@ -7538,6 +7538,14 @@ static bool IsEligibleForTrivialRelocation(Sema &SemaRef,
if (!SemaRef.IsCXXTriviallyRelocatableType(Field->getType()))
return false;
}
+
+ // FIXME: PFP should not affect trivial relocatability, instead it should
+ // affect the implementat
@@ -2268,13 +2293,22 @@ CodeGenFunction::EmitNullInitialization(Address
DestPtr, QualType Ty) {
// Get and call the appropriate llvm.memcpy overload.
Builder.CreateMemCpy(DestPtr, SrcPtr, SizeVal, false);
-return;
+ } else {
+// Otherwise, just memset the who
@@ -1319,14 +1319,66 @@ static llvm::Value
*CoerceIntOrPtrToIntOrPtr(llvm::Value *Val, llvm::Type *Ty,
/// This safely handles the case when the src type is smaller than the
/// destination type; in this situation the values of bits which not
/// present in the src are undefin
@@ -544,6 +544,7 @@ TYPE_TRAIT_2(__is_pointer_interconvertible_base_of,
IsPointerInterconvertibleBas
#include "clang/Basic/TransformTypeTraits.def"
// Clang-only C++ Type Traits
+TYPE_TRAIT_1(__has_non_relocatable_fields, HasNonRelocatableFields, KEYCXX)
ojhu
@@ -544,6 +544,7 @@ TYPE_TRAIT_2(__is_pointer_interconvertible_base_of,
IsPointerInterconvertibleBas
#include "clang/Basic/TransformTypeTraits.def"
// Clang-only C++ Type Traits
+TYPE_TRAIT_1(__has_non_relocatable_fields, HasNonRelocatableFields, KEYCXX)
ojhu
@@ -1415,6 +1469,52 @@ void CodeGenFunction::CreateCoercedStore(llvm::Value
*Src, Address Dst,
}
}
+ // Coercion directly through memory does not work if the structure has
pointer
+ // field protection because the struct passed by value has a different bit
+ // patt
@@ -441,6 +445,254 @@ bool
PreISelIntrinsicLowering::expandMemIntrinsicUses(Function &F) const {
return Changed;
}
+namespace {
+
+enum class PointerEncoding {
+ Rotate,
+ PACCopyable,
+ PACNonCopyable,
+};
+
+bool expandProtectedFieldPtr(Function &Intr) {
+ Module &M =
https://github.com/ojhunt edited
https://github.com/llvm/llvm-project/pull/133538
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/ojhunt commented:
unfortunately I really don't know enough about actual backend codegen to
comment on the actual codegen implementation here, but I think there's some
design issues with having the backend determine the discriminators rather than
having those selected in the
@@ -2201,6 +2215,22 @@ void CodeGenFunction::EmitCXXConstructorCall(
EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall, Loc, This,
getContext().getRecordType(ClassDecl), CharUnits::Zero());
+ // When initializing an object that has pointer field protect
@@ -2268,13 +2293,22 @@ CodeGenFunction::EmitNullInitialization(Address
DestPtr, QualType Ty) {
// Get and call the appropriate llvm.memcpy overload.
Builder.CreateMemCpy(DestPtr, SrcPtr, SizeVal, false);
-return;
+ } else {
+// Otherwise, just memset the who
@@ -928,6 +936,11 @@ namespace {
if (PointerAuthQualifier Q = F->getType().getPointerAuth();
Q && Q.isAddressDiscriminated())
return false;
+ // Non-trivially-copyable fields with pointer field protection need to be
ojhunt wrote:
T
@@ -362,6 +362,17 @@ class LangOptionsBase {
BKey
};
+ enum class PointerFieldProtectionKind {
ojhunt wrote:
I'm not sure I like this being solely a global decision - it makes custom
allocators much harder, and it makes it hard for allocators that hav
@@ -2513,6 +2513,12 @@ def CountedByOrNull : DeclOrTypeAttr {
let LangOpts = [COnly];
}
+def NoPointerFieldProtection : DeclOrTypeAttr {
ojhunt wrote:
This an ABI break so I don't think it can reasonably an on by default for all
structs - we can already se
@@ -3011,6 +3011,12 @@ defm experimental_omit_vtable_rtti :
BoolFOption<"experimental-omit-vtable-rtti"
NegFlag,
BothFlags<[], [CC1Option], " the RTTI component from virtual tables">>;
+def experimental_pointer_field_protection_EQ : Joined<["-"],
"fexperimental-pointer-f
@@ -2976,7 +3006,15 @@ void CodeGenFunction::EmitForwardingCallToLambda(
QualType resultType = FPT->getReturnType();
ReturnValueSlot returnSlot;
if (!resultType->isVoidType() &&
- calleeFnInfo->getReturnInfo().getKind() == ABIArgInfo::Indirect &&
+ (calleeFnInfo
https://github.com/ojhunt requested changes to this pull request.
Thoughts:
This should be opt-in on a field or struct granularity, not just a global
behavior.
In the RFC I think you mentioned not applying PFP to C types, but I'm unsure
how you're deciding what is a C type?
There are a lot o
@@ -1319,14 +1319,66 @@ static llvm::Value
*CoerceIntOrPtrToIntOrPtr(llvm::Value *Val, llvm::Type *Ty,
/// This safely handles the case when the src type is smaller than the
/// destination type; in this situation the values of bits which not
/// present in the src are undefin
@@ -7538,6 +7538,14 @@ static bool IsEligibleForTrivialRelocation(Sema &SemaRef,
if (!SemaRef.IsCXXTriviallyRelocatableType(Field->getType()))
return false;
}
+
+ // FIXME: PFP should not affect trivial relocatability, instead it should
+ // affect the implementat
https://github.com/ojhunt edited
https://github.com/llvm/llvm-project/pull/133538
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
ojhunt wrote:
> > I agree, it's literally just a warning correction, and people building this
> > release of llvm with a newer clang can configure it to disable this warning
> > - I just wasn't sure what the general policy around this kind of thing was
> > which is why I made a PR and asked fo
ojhunt wrote:
@zahiraam @AaronBallman I'm not sure if this warrants the cherry pick though -
because we cannot actually trigger the use of FEM_Indeterminable the fact that
the behavior would be wrong isn't relevant, the problem is just the warning
spam you get while building with trunk clang
ojhunt wrote:
@tstellar hi, I'm not sure the process for merging to a release branch, this is
a minor correctness fix but it prevents a massive amount of warning spam when
building with more current clangs. cc'ing @AaronBallman and @zahiraam so they
see this and can weigh in on whether it is r
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/138337
Remove FEM_Indeterminable as it is unused and cannot be stored safely in an
unsigned bitfield
>From f104804f217b31f6c41559fc3b388a9c8f0f7571 Mon Sep 17 00:00:00 2001
From: Oliver Hunt
Date: Fri, 2 May 2025 13:0
@@ -296,3 +296,21 @@
ConstantAggregateBuilderBase::finishStruct(llvm::StructType *ty) {
buffer.erase(buffer.begin() + Begin, buffer.end());
return constant;
}
+
ojhunt wrote:
@asl Updating to resolve conflict required bringing this function in that was
p
@@ -7037,8 +7036,64 @@ void ItaniumMangleContextImpl::mangleCXXDtorComdat(const
CXXDestructorDecl *D,
Mangler.mangle(GlobalDecl(D, Dtor_Comdat));
}
+static void mangleOverrideDiscrimination(CXXNameMangler &mangler,
+ ASTContext &conte
ojhunt wrote:
Had to do a force push to resolve merge conflicts following @ahatanak's PR, so
this change now includes `CodeGenFunction::EmitPointerAuthAuth` etc
https://github.com/llvm/llvm-project/pull/94056
___
llvm-branch-commits mailing list
llvm-
@@ -1261,6 +1262,10 @@ class ASTContext : public RefCountedBase {
/// space.
QualType removeAddrSpaceQualType(QualType T) const;
+ /// Return the "other" type-specific discriminator for the given type.
ojhunt wrote:
How would `/// Return the "other" disc
@@ -1261,6 +1262,10 @@ class ASTContext : public RefCountedBase {
/// space.
QualType removeAddrSpaceQualType(QualType T) const;
+ /// Return the "other" type-specific discriminator for the given type.
ojhunt wrote:
@asl thoughts?
https://github.com/llv
https://github.com/ojhunt ready_for_review
https://github.com/llvm/llvm-project/pull/94056
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/ojhunt closed https://github.com/llvm/llvm-project/pull/94054
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/ojhunt converted_to_draft
https://github.com/llvm/llvm-project/pull/94054
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/93984
Minor correction to match current API
>From 0262fdfddf50853d2f40ea86c37877168ad070a8 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <4691426+ojh...@users.noreply.github.com>
Date: Fri, 31 May 2024 09:36:32 -0700
Subje
ojhunt wrote:
Build fix is trivial
`diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 82c4a3c86645..e9a867ff67ba 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -9246,7 +9246,7 @@ static void handleVTablePointerAuthenticati
ojhunt wrote:
looking as well
https://github.com/llvm/llvm-project/pull/93907
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
84 matches
Mail list logo
