Skip to content

[clang] Fix PointerAuth semantics of cpp_trivially_relocatable #143969

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jun 16, 2025
Merged

[clang] Fix PointerAuth semantics of cpp_trivially_relocatable #143969

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f476803
[clang] Fix PointerAuth semantics of cpp_trivially_relocatable
ojhunt Jun 12, 2025
3190a52
Add comments and more tests
ojhunt Jun 13, 2025
15b1a81
Address corentin comments
ojhunt Jun 15, 2025
3ba1e39
__unused doesn't work on the linux and windows bots
ojhunt Jun 15, 2025
6b5de16
Address discriminated __ptrauth should not be relocatable
ojhunt Jun 15, 2025
ea3af8a
sigh, formatting
ojhunt Jun 15, 2025
54f93cb
remove extraneous check
ojhunt Jun 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions clang/include/clang/AST/ASTContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,25 +629,46 @@ class ASTContext : public RefCountedBase<ASTContext> {
void setRelocationInfoForCXXRecord(const CXXRecordDecl *,
CXXRecordDeclRelocationInfo);

/// Examines a given type, and returns whether the T itself
/// Examines a given type, and returns whether the type itself
/// is address discriminated, or any transitively embedded types
/// contain data that is address discriminated. This includes
/// implicitly authenticated values like vtable pointers, as well as
/// explicitly qualified fields.
bool containsAddressDiscriminatedPointerAuth(QualType T);
// A simple helper function to short circuit pointer auth checks.
bool containsAddressDiscriminatedPointerAuth(QualType T) {
if (!isPointerAuthenticationAvailable())
return false;
return findPointerAuthContent(T) != PointerAuthContent::None;
}
Comment on lines +637 to +641
Copy link
Contributor

@cor3ntin cor3ntin Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that still used?

Copy link
Contributor Author

@ojhunt ojhunt Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, it's possible I updated it without verifying it was needed.

Copy link
Contributor Author

@ojhunt ojhunt Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, this is used for the union check - in a union it does not matter what the source of the address discrimination is, a union containing anything address discriminated isn't valid. In principle with this change the only thing that would get here is a vtable pointer but it seems reasonable to be safe.


bool isPointerAuthenticationAvailable() const {
return LangOpts.PointerAuthCalls || LangOpts.PointerAuthIntrinsics ||
LangOpts.PointerAuthVTPtrAddressDiscrimination;
/// Examines a given type, and returns whether the type itself
/// or any data it transitively contains has a pointer authentication
/// schema that is not safely relocatable. e.g. any data or fields
/// with address discrimination other than any otherwise similar
/// vtable pointers.
bool containsNonRelocatablePointerAuth(QualType T) {
if (!isPointerAuthenticationAvailable())
return false;
return findPointerAuthContent(T) == PointerAuthContent::AddressDiscriminatedData;
}

private:
llvm::DenseMap<const CXXRecordDecl *, CXXRecordDeclRelocationInfo>
RelocatableClasses;

// FIXME: store in RecordDeclBitfields in future?
llvm::DenseMap<const RecordDecl *, bool>
enum class PointerAuthContent : uint8_t {
None,
AddressDiscriminatedVTable,
AddressDiscriminatedData
};

// A simple helper function to short circuit pointer auth checks.
bool isPointerAuthenticationAvailable() const {
return LangOpts.PointerAuthCalls || LangOpts.PointerAuthIntrinsics ||
LangOpts.PointerAuthVTPtrAddressDiscrimination;
}
Copy link
Contributor

@cor3ntin cor3ntin Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 questions; Why do we have 3 options, and why do we care about anything but PointerAuthVTPtrAddressDiscriminationhere?

Copy link
Contributor Author

@ojhunt ojhunt Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first is historical sadness, we can probably make this better when I move the pointer auth options and similar into the target info rather than the obtuse combination of codegen and langopts we currently have. When we do that we can probably just have a single "the target supports pointer auth" flag which is what this is trying to approximate.

The reason that PointerAuthVTPtrAddressDiscrimination is not sufficient is that there is a qualifier that can be specified explicitly.

That said as we discussed (for people following at home @cor3ntin and I are talking at wg21) if someone had PointerAuthVTPtrAddressDiscrimination enabled without PointerAuthCalls and PointerAuthIntrinsics would likely be in a world of hurt, so I'll remove that separate check.

PointerAuthContent findPointerAuthContent(QualType T);
llvm::DenseMap<const RecordDecl *, PointerAuthContent>
RecordContainsAddressDiscriminatedPointerAuth;

ImportDecl *FirstLocalImport = nullptr;
Expand Down
40 changes: 24 additions & 16 deletions clang/lib/AST/ASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1704,10 +1704,11 @@ void ASTContext::setRelocationInfoForCXXRecord(
assert(RelocatableClasses.find(D) == RelocatableClasses.end());
RelocatableClasses.insert({D, Info});
}

static bool
hasAddressDiscriminatedVTableAuthentication(ASTContext &Context,
primaryBaseHaseAddressDiscriminatedVTableAuthentication(ASTContext &Context,
const CXXRecordDecl *Class) {
if (!Context.isPointerAuthenticationAvailable() || !Class->isPolymorphic())
if (!Class->isPolymorphic())
return false;
const CXXRecordDecl *BaseType = Context.baseForVTableAuthentication(Class);
using AuthAttr = VTablePointerAuthenticationAttr;
Expand All @@ -1721,43 +1722,50 @@ hasAddressDiscriminatedVTableAuthentication(ASTContext &Context,
return AddressDiscrimination == AuthAttr::AddressDiscrimination;
}

bool ASTContext::containsAddressDiscriminatedPointerAuth(QualType T) {
if (!isPointerAuthenticationAvailable())
return false;
ASTContext::PointerAuthContent ASTContext::findPointerAuthContent(QualType T) {
assert(isPointerAuthenticationAvailable());

T = T.getCanonicalType();
if (T.hasAddressDiscriminatedPointerAuth())
return true;
return PointerAuthContent::AddressDiscriminatedData;
const RecordDecl *RD = T->getAsRecordDecl();
if (!RD)
return false;
return PointerAuthContent::None;

if (auto Existing = RecordContainsAddressDiscriminatedPointerAuth.find(RD);
Existing != RecordContainsAddressDiscriminatedPointerAuth.end())
return Existing->second;

auto SaveReturn = [this, RD](bool Result) {
PointerAuthContent Result = PointerAuthContent::None;

auto SaveResultAndReturn = [&]() -> PointerAuthContent {
auto [ResultIter, DidAdd] =
RecordContainsAddressDiscriminatedPointerAuth.try_emplace(RD, Result);
(void)ResultIter;
(void)DidAdd;
assert(DidAdd);
return Result;
};
auto ShouldContinueAfterUpdate = [&](PointerAuthContent NewResult) {
static_assert(PointerAuthContent::None < PointerAuthContent::AddressDiscriminatedVTable);
static_assert(PointerAuthContent::AddressDiscriminatedVTable < PointerAuthContent::AddressDiscriminatedData);
if (NewResult > Result)
Result = NewResult;
return Result != PointerAuthContent::AddressDiscriminatedData;
};
if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
if (CXXRD->isPolymorphic() &&
hasAddressDiscriminatedVTableAuthentication(*this, CXXRD))
return SaveReturn(true);
if (primaryBaseHaseAddressDiscriminatedVTableAuthentication(*this, CXXRD) && !ShouldContinueAfterUpdate(PointerAuthContent::AddressDiscriminatedVTable))
return SaveResultAndReturn();
for (auto Base : CXXRD->bases()) {
if (containsAddressDiscriminatedPointerAuth(Base.getType()))
return SaveReturn(true);
if (!ShouldContinueAfterUpdate(findPointerAuthContent(Base.getType())))
return SaveResultAndReturn();
}
}
for (auto *FieldDecl : RD->fields()) {
if (containsAddressDiscriminatedPointerAuth(FieldDecl->getType()))
return SaveReturn(true);
if (!ShouldContinueAfterUpdate(findPointerAuthContent(FieldDecl->getType())))
return SaveResultAndReturn();
}
return SaveReturn(false);
return SaveResultAndReturn();
}

void ASTContext::addedLocalImportDecl(ImportDecl *Import) {
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Sema/SemaTypeTraits.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,9 @@ bool Sema::IsCXXTriviallyRelocatableType(QualType Type) {
if (BaseElementType->isIncompleteType())
return false;

if (Context.containsNonRelocatablePointerAuth(Type))
return false;

if (BaseElementType->isScalarType() || BaseElementType->isVectorType())
return true;

Expand Down
8 changes: 4 additions & 4 deletions clang/test/SemaCXX/ptrauth-triviality.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ static_assert(!__is_trivially_assignable(S1, const S1&));
static_assert(__is_trivially_destructible(S1));
static_assert(!__is_trivially_copyable(S1));
static_assert(!__is_trivially_relocatable(S1)); // expected-warning{{deprecated}}
static_assert(__builtin_is_cpp_trivially_relocatable(S1));
static_assert(!__builtin_is_cpp_trivially_relocatable(S1));
static_assert(!__is_trivially_equality_comparable(S1));

static_assert(__is_trivially_constructible(Holder<S1>));
Expand All @@ -35,7 +35,7 @@ static_assert(!__is_trivially_assignable(Holder<S1>, const Holder<S1>&));
static_assert(__is_trivially_destructible(Holder<S1>));
static_assert(!__is_trivially_copyable(Holder<S1>));
static_assert(!__is_trivially_relocatable(Holder<S1>)); // expected-warning{{deprecated}}
static_assert(__builtin_is_cpp_trivially_relocatable(Holder<S1>));
static_assert(!__builtin_is_cpp_trivially_relocatable(Holder<S1>));
static_assert(!__is_trivially_equality_comparable(Holder<S1>));

struct S2 {
Expand Down Expand Up @@ -146,7 +146,7 @@ static_assert(!__is_trivially_assignable(S6, const S6&));
static_assert(__is_trivially_destructible(S6));
static_assert(!__is_trivially_copyable(S6));
static_assert(!__is_trivially_relocatable(S6)); // expected-warning{{deprecated}}
static_assert(__builtin_is_cpp_trivially_relocatable(S6));
static_assert(!__builtin_is_cpp_trivially_relocatable(S6));
static_assert(!__is_trivially_equality_comparable(S6));

static_assert(__is_trivially_constructible(Holder<S6>));
Expand All @@ -155,7 +155,7 @@ static_assert(!__is_trivially_assignable(Holder<S6>, const Holder<S6>&));
static_assert(__is_trivially_destructible(Holder<S6>));
static_assert(!__is_trivially_copyable(Holder<S6>));
static_assert(!__is_trivially_relocatable(Holder<S6>)); // expected-warning{{deprecated}}
static_assert(__builtin_is_cpp_trivially_relocatable(Holder<S6>));
static_assert(!__builtin_is_cpp_trivially_relocatable(Holder<S6>));
static_assert(!__is_trivially_equality_comparable(Holder<S6>));

struct S7 {
Expand Down
15 changes: 11 additions & 4 deletions clang/test/SemaCXX/trivially-relocatable-ptrauth.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,27 @@ struct AddressDiscPtrauth {
void * __ptrauth(1, 1, 1234) p;
};

static_assert(__builtin_is_cpp_trivially_relocatable(AddressDiscPtrauth));
static_assert(!__builtin_is_cpp_trivially_relocatable(AddressDiscPtrauth));

struct MultipleBaseClasses : NonAddressDiscPtrauth, AddressDiscPtrauth {

};

static_assert(__builtin_is_cpp_trivially_relocatable(MultipleBaseClasses));
static_assert(!__builtin_is_cpp_trivially_relocatable(MultipleBaseClasses));

struct MultipleMembers {
struct MultipleMembers1 {
NonAddressDiscPtrauth field0;
AddressDiscPtrauth field1;
};

static_assert(__builtin_is_cpp_trivially_relocatable(MultipleMembers));
static_assert(!__builtin_is_cpp_trivially_relocatable(MultipleMembers1));

struct MultipleMembers2 {
NonAddressDiscPtrauth field0;
NonAddressDiscPtrauth field1;
};

static_assert(__builtin_is_cpp_trivially_relocatable(MultipleMembers2));

struct UnionOfPtrauth {
union {
Expand Down