Skip to content

release/21.x: [clang][PAC] Fix builtins that claim address discriminated types are bitwise compatible (#154490) #155492

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

Closed
wants to merge 1 commit into from
Closed

release/21.x: [clang][PAC] Fix builtins that claim address discriminated types are bitwise compatible (#154490) #155492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 5 additions & 6 deletions clang/include/clang/AST/ASTContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
/// contain data that is address discriminated. This includes
/// implicitly authenticated values like vtable pointers, as well as
/// explicitly qualified fields.
bool containsAddressDiscriminatedPointerAuth(QualType T) {
bool containsAddressDiscriminatedPointerAuth(QualType T) const {
Copy link
Contributor

@nikic nikic Aug 26, 2025

Choose a reason for hiding this comment

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

This looks like an ABI breaking change.

Copy link
Contributor

@ojhunt ojhunt Aug 26, 2025

Choose a reason for hiding this comment

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

This has not shipped - llvm21 is the first version of llvm to include any of the pointer auth work.

That said it's irksome that we don't have a way to indicate that functions are not intended to be API, or that there is no abstraction layer for the API given that ASTContext is used for both internal semantic checks and apparently is part of the API (it did not occur to me that anything here would be directly exposed as API as none of the changes I have made have ever triggered any kind of API review) :-/

Copy link
Contributor

@nikic nikic Aug 26, 2025

Choose a reason for hiding this comment

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

This has not shipped - llvm21 is the first version of llvm to include any of the pointer auth work.

LLVM 21.1.0 has been released, so all backports from this point on must be ABI compatible.

That said it's irksome that we don't have a way to indicate that functions are not intended to be API, or that there is no abstraction layer for the API given that ASTContext is used for both internal semantic checks and apparently is part of the API (it did not occur to me that anything here would be directly exposed as API as none of the changes I have made have ever triggered any kind of API review) :-/

This may be possible in the future using ABI annotations, but the clang-side work for that hasn't really started yet.

For the purposes of this backport, you'll have to rewrite this without the const qualifiers.

Copy link
Contributor

@ojhunt ojhunt Aug 26, 2025

Choose a reason for hiding this comment

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

@nikic this was targeting that release - I thought we had until the 28th?

Copy link
Contributor

@ojhunt ojhunt Aug 26, 2025

Choose a reason for hiding this comment

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

anyway, if that has shipped there's nothing we can do now except publish errata on broken behavior here

Copy link
Contributor

@nikic nikic Aug 26, 2025

Choose a reason for hiding this comment

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

@nikic this was targeting that release - I thought we had until the 28th?

You can find the release schedule in the sidebar on https://llvm.org/. The general rule is that LLVM always releases on Tuesdays.

anyway, if that has shipped there's nothing we can do now except publish errata on broken behavior here

Why can't you just drop all the const qualifiers from this patch? That should be sufficient to avoid changing the symbol mangling.

if (!isPointerAuthenticationAvailable())
return false;
return findPointerAuthContent(T) != PointerAuthContent::None;
Expand All @@ -648,8 +648,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
bool containsNonRelocatablePointerAuth(QualType T) {
if (!isPointerAuthenticationAvailable())
return false;
return findPointerAuthContent(T) ==
PointerAuthContent::AddressDiscriminatedData;
return findPointerAuthContent(T) != PointerAuthContent::None;
}

private:
Expand All @@ -667,8 +666,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
bool isPointerAuthenticationAvailable() const {
return LangOpts.PointerAuthCalls || LangOpts.PointerAuthIntrinsics;
}
PointerAuthContent findPointerAuthContent(QualType T);
llvm::DenseMap<const RecordDecl *, PointerAuthContent>
PointerAuthContent findPointerAuthContent(QualType T) const;
mutable llvm::DenseMap<const RecordDecl *, PointerAuthContent>
RecordContainsAddressDiscriminatedPointerAuth;

ImportDecl *FirstLocalImport = nullptr;
Expand Down Expand Up @@ -3698,7 +3697,7 @@ OPT_LIST(V)
/// Resolve the root record to be used to derive the vtable pointer
/// authentication policy for the specified record.
const CXXRecordDecl *
baseForVTableAuthentication(const CXXRecordDecl *ThisClass);
baseForVTableAuthentication(const CXXRecordDecl *ThisClass) const;

bool useAbbreviatedThunkName(GlobalDecl VirtualMethodDecl,
StringRef MangledName);
Expand Down
9 changes: 5 additions & 4 deletions clang/lib/AST/ASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1706,7 +1706,7 @@ void ASTContext::setRelocationInfoForCXXRecord(
}

static bool primaryBaseHaseAddressDiscriminatedVTableAuthentication(
ASTContext &Context, const CXXRecordDecl *Class) {
const ASTContext &Context, const CXXRecordDecl *Class) {
if (!Class->isPolymorphic())
return false;
const CXXRecordDecl *BaseType = Context.baseForVTableAuthentication(Class);
Expand All @@ -1721,7 +1721,8 @@ static bool primaryBaseHaseAddressDiscriminatedVTableAuthentication(
return AddressDiscrimination == AuthAttr::AddressDiscrimination;
}

ASTContext::PointerAuthContent ASTContext::findPointerAuthContent(QualType T) {
ASTContext::PointerAuthContent
ASTContext::findPointerAuthContent(QualType T) const {
assert(isPointerAuthenticationAvailable());

T = T.getCanonicalType();
Expand Down Expand Up @@ -3032,7 +3033,7 @@ bool ASTContext::hasUniqueObjectRepresentations(
return true;
}

// All other pointers (except __ptrauth pointers) are unique.
// All other pointers are unique.
if (Ty->isPointerType())
return !Ty.hasAddressDiscriminatedPointerAuth();

Expand Down Expand Up @@ -15106,7 +15107,7 @@ StringRef ASTContext::getCUIDHash() const {
}

const CXXRecordDecl *
ASTContext::baseForVTableAuthentication(const CXXRecordDecl *ThisClass) {
ASTContext::baseForVTableAuthentication(const CXXRecordDecl *ThisClass) const {
assert(ThisClass);
assert(ThisClass->isPolymorphic());
const CXXRecordDecl *PrimaryBase = ThisClass;
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/AST/DeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,13 @@ void CXXRecordDecl::addedMember(Decl *D) {
data().StructuralIfLiteral = false;
}

// If this type contains any address discriminated values we should
// have already indicated that the only special member functions that
// can possibly be trivial are the default constructor and destructor.
if (T.hasAddressDiscriminatedPointerAuth())
data().HasTrivialSpecialMembers &=
SMF_DefaultConstructor | SMF_Destructor;

// C++14 [meta.unary.prop]p4:
// T is a class type [...] with [...] no non-static data members other
// than subobjects of zero size
Expand Down
20 changes: 20 additions & 0 deletions clang/lib/AST/Type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2715,6 +2715,11 @@ bool QualType::isCXX98PODType(const ASTContext &Context) const {
return false;

QualType CanonicalType = getTypePtr()->CanonicalType;

// Any type that is, or contains, address discriminated data is never POD.
if (Context.containsAddressDiscriminatedPointerAuth(CanonicalType))
return false;

switch (CanonicalType->getTypeClass()) {
// Everything not explicitly mentioned is not POD.
default:
Expand Down Expand Up @@ -2773,6 +2778,11 @@ bool QualType::isTrivialType(const ASTContext &Context) const {
if (CanonicalType->isDependentType())
return false;

// Any type that is, or contains, address discriminated data is never a
// trivial type.
if (Context.containsAddressDiscriminatedPointerAuth(CanonicalType))
return false;

// C++0x [basic.types]p9:
// Scalar types, trivial class types, arrays of such types, and
// cv-qualified versions of these types are collectively called trivial
Expand Down Expand Up @@ -2870,6 +2880,12 @@ bool QualType::isBitwiseCloneableType(const ASTContext &Context) const {

if (CanonicalType->isIncompleteType())
return false;

// Any type that is, or contains, address discriminated data is never
// bitwise clonable.
if (Context.containsAddressDiscriminatedPointerAuth(CanonicalType))
return false;

const auto *RD = CanonicalType->getAsRecordDecl(); // struct/union/class
if (!RD)
return true;
Expand Down Expand Up @@ -3115,6 +3131,10 @@ bool QualType::isCXX11PODType(const ASTContext &Context) const {
if (BaseTy->isIncompleteType())
return false;

// Any type that is, or contains, address discriminated data is non-POD.
if (Context.containsAddressDiscriminatedPointerAuth(*this))
return false;

// As an extension, Clang treats vector types as Scalar types.
if (BaseTy->isScalarType() || BaseTy->isVectorType())
return true;
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Sema/SemaDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19651,6 +19651,7 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
Q && Q.isAddressDiscriminated()) {
Record->setArgPassingRestrictions(
RecordArgPassingKind::CanNeverPassInRegs);
Record->setNonTrivialToPrimitiveCopy(true);
}
}

Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Sema/SemaTypeTraits.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1768,7 +1768,10 @@ static bool EvaluateBinaryTypeTrait(Sema &Self, TypeTrait BTT,
// Objective-C lifetime, this is a non-trivial assignment.
if (LhsT.getNonReferenceType().hasNonTrivialObjCLifetime())
return false;

const ASTContext &Context = Self.getASTContext();
if (Context.containsAddressDiscriminatedPointerAuth(LhsT) ||
Context.containsAddressDiscriminatedPointerAuth(RhsT))
return false;
return !Result.get()->hasNonTrivialCall(Self.Context);
}

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


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

struct IA S4 {
Expand Down Expand Up @@ -207,7 +207,7 @@ template <class T> struct UnionWrapper trivially_relocatable_if_eligible {
} u;
};

static_assert(test_is_trivially_relocatable_v<AddressDiscriminatedPolymorphicBase>);
static_assert(!test_is_trivially_relocatable_v<AddressDiscriminatedPolymorphicBase>);
static_assert(test_is_trivially_relocatable_v<NoAddressDiscriminatedPolymorphicBase>);
static_assert(inheritance_relocatability_matches_bases_v<AddressDiscriminatedPolymorphicBase, NoAddressDiscriminatedPolymorphicBase>);
static_assert(inheritance_relocatability_matches_bases_v<NoAddressDiscriminatedPolymorphicBase, AddressDiscriminatedPolymorphicBase>);
Expand Down
Loading
Loading