Add comments and more tests

Author: ojhunt

clang/include/clang/AST/ASTContext.h

@@ -628,11 +628,25 @@ class ASTContext : public RefCountedBase<ASTContext> {
   getRelocationInfoForCXXRecord(const CXXRecordDecl *) const;
   void setRelocationInfoForCXXRecord(const CXXRecordDecl *,
                                      CXXRecordDeclRelocationInfo);
+
+  /// Examines a given type, and returns whether the T 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);
 
 private:
+  // A simple helper function to short circuit pointer auth checks.
+  bool isPointerAuthenticationAvailable() const {
+    return LangOpts.PointerAuthCalls || LangOpts.PointerAuthIntrinsics ||
+           LangOpts.PointerAuthVTPtrAddressDiscrimination;
+  }
+
   llvm::DenseMap<const CXXRecordDecl *, CXXRecordDeclRelocationInfo>
       RelocatableClasses;
+
+  // FIXME: store in RecordDeclBitfields in future?
   llvm::DenseMap<const RecordDecl *, bool>
       RecordContainsAddressDiscriminatedPointerAuth;
 
@@ -3671,7 +3685,13 @@ OPT_LIST(V)
   /// authentication policy for the specified record.
   const CXXRecordDecl *
   baseForVTableAuthentication(const CXXRecordDecl *ThisClass);
+
+  /// If this class is polymorphic, returns true if any of this class's
+  /// vtable pointers have an address discriminated pointer authentication
+  /// schema.
+  /// This does not check fields of the class or base classes.
   bool hasAddressDiscriminatedVTableAuthentication(const CXXRecordDecl *Class);
+
   bool useAbbreviatedThunkName(GlobalDecl VirtualMethodDecl,
                                StringRef MangledName);
 

clang/lib/AST/ASTContext.cpp

@@ -1706,7 +1706,7 @@ void ASTContext::setRelocationInfoForCXXRecord(
 }
 
 bool ASTContext::containsAddressDiscriminatedPointerAuth(QualType T) {
-  if (!LangOpts.PointerAuthCalls && !LangOpts.PointerAuthIntrinsics)
+  if (!isPointerAuthenticationAvailable())
     return false;
 
   T = T.getCanonicalType();
@@ -1716,13 +1716,14 @@ bool ASTContext::containsAddressDiscriminatedPointerAuth(QualType T) {
   if (!RD)
     return false;
 
+  if (auto Existing = RecordContainsAddressDiscriminatedPointerAuth.find(RD);
+      Existing != RecordContainsAddressDiscriminatedPointerAuth.end())
+    return Existing->second;
+
   auto SaveReturn = [this, RD](bool Result) {
     RecordContainsAddressDiscriminatedPointerAuth.insert({RD, Result});
     return Result;
   };
-  if (auto Existing = RecordContainsAddressDiscriminatedPointerAuth.find(RD);
-      Existing != RecordContainsAddressDiscriminatedPointerAuth.end())
-    return Existing->second;
   if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
     if (CXXRD->isPolymorphic() &&
         hasAddressDiscriminatedVTableAuthentication(CXXRD))
@@ -15157,10 +15158,11 @@ ASTContext::baseForVTableAuthentication(const CXXRecordDecl *ThisClass) {
 
 bool ASTContext::hasAddressDiscriminatedVTableAuthentication(
     const CXXRecordDecl *Class) {
-  assert(Class->isPolymorphic());
+  if (!isPointerAuthenticationAvailable() || !Class->isPolymorphic())
+    return false;
   const CXXRecordDecl *BaseType = baseForVTableAuthentication(Class);
   using AuthAttr = VTablePointerAuthenticationAttr;
-  const auto *ExplicitAuth = BaseType->getAttr<AuthAttr>();
+  const AuthAttr *ExplicitAuth = BaseType->getAttr<AuthAttr>();
   if (!ExplicitAuth)
     return LangOpts.PointerAuthVTPtrAddressDiscrimination;
   AuthAttr::AddressDiscriminationMode AddressDiscrimination =

clang/lib/Sema/SemaTypeTraits.cpp

@@ -190,17 +190,19 @@ static bool IsEligibleForTrivialRelocation(Sema &SemaRef,
 
   bool IsUnion = D->isUnion();
   for (const FieldDecl *Field : D->fields()) {
-    QualType FieldType = Field->getType();
-    if (FieldType->isDependentType())
+    if (Field->getType()->isDependentType())
       continue;
-    if (FieldType->isReferenceType())
+    if (Field->getType()->isReferenceType())
       continue;
     // ... has a non-static data member of an object type that is not
     // of a trivially relocatable type
     if (!SemaRef.IsCXXTriviallyRelocatableType(Field->getType()))
       return false;
-    if (IsUnion &&
-        SemaRef.Context.containsAddressDiscriminatedPointerAuth(FieldType))
+
+    // A union contains values with address discriminated pointer auth
+    // cannot be relocated.
+    if (IsUnion && SemaRef.Context.containsAddressDiscriminatedPointerAuth(
+                       Field->getType()))
       return false;
   }
   return !D->hasDeletedDestructor();
@@ -318,7 +320,6 @@ bool Sema::IsCXXTriviallyRelocatableType(const CXXRecordDecl &RD) {
 }
 
 bool Sema::IsCXXTriviallyRelocatableType(QualType Type) {
-
   QualType BaseElementType = getASTContext().getBaseElementType(Type);
 
   if (Type->isVariableArrayType())
@@ -672,6 +673,9 @@ static bool IsTriviallyRelocatableType(Sema &SemaRef, QualType T) {
   if (!BaseElementType->isObjectType())
     return false;
 
+  // The deprecated __builtin_is_trivially_relocatable does not have
+  // an equivalent to __builtin_trivially_relocate, so there is no
+  // safe way to use it if there are any address discriminated values.
   if (SemaRef.getASTContext().containsAddressDiscriminatedPointerAuth(T))
     return false;
 

clang/test/SemaCXX/ptrauth-triviality.cpp

@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++20 -fptrauth-calls -fptrauth-intrinsics -verify -fsyntax-only %s
-// RUN: %clang_cc1 -triple aarch64-linux-gnu -std=c++20 -fptrauth-calls -fptrauth-intrinsics -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple arm64-apple-ios -std=c++26 -fptrauth-calls -fptrauth-intrinsics -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -std=c++26 -fptrauth-calls -fptrauth-intrinsics -verify -fsyntax-only %s
 
 #define AQ __ptrauth(1,1,50)
 #define IQ __ptrauth(1,0,50)
@@ -99,7 +99,6 @@ static_assert(!__is_trivially_assignable(S4, const S4&));
 static_assert(__is_trivially_destructible(S4));
 static_assert(!__is_trivially_copyable(S4));
 static_assert(!__is_trivially_relocatable(S4)); // expected-warning{{deprecated}}
-//FIXME
 static_assert(__builtin_is_cpp_trivially_relocatable(S4));
 static_assert(!__is_trivially_equality_comparable(S4));
 
@@ -124,7 +123,6 @@ static_assert(!__is_trivially_assignable(S5, const S5&));
 static_assert(__is_trivially_destructible(S5));
 static_assert(!__is_trivially_copyable(S5));
 static_assert(!__is_trivially_relocatable(S5)); // expected-warning{{deprecated}}
-//FIXME
 static_assert(__builtin_is_cpp_trivially_relocatable(S5));
 static_assert(!__is_trivially_equality_comparable(S5));
 
@@ -182,3 +180,39 @@ static_assert(__is_trivially_copyable(Holder<S7>));
 static_assert(__is_trivially_relocatable(Holder<S7>)); // expected-warning{{deprecated}}
 static_assert(__builtin_is_cpp_trivially_relocatable(Holder<S7>));
 static_assert(__is_trivially_equality_comparable(Holder<S7>));
+
+template <class... Bases> struct MultipleInheriter : Bases... {
+};
+
+template <class T> static const bool test_is_trivially_relocatable_v = __builtin_is_cpp_trivially_relocatable(T);
+template <class... Types> static const bool multiple_inheritance_is_relocatable = test_is_trivially_relocatable_v<MultipleInheriter<Types...>>;
+template <class... Types> static const bool inheritance_relocatability_matches_bases_v =
+  (test_is_trivially_relocatable_v<Types> && ...) == multiple_inheritance_is_relocatable<Types...>;
+
+static_assert(multiple_inheritance_is_relocatable<S4, S5> == multiple_inheritance_is_relocatable<S5, S4>);
+static_assert(inheritance_relocatability_matches_bases_v<S4, S5>);
+static_assert(inheritance_relocatability_matches_bases_v<S5, S4>);
+
+struct AA AddressDiscriminatedPolymorphicBase trivially_relocatable_if_eligible {
+  virtual void foo();
+};
+
+struct IA NoAddressDiscriminatedPolymorphicBase trivially_relocatable_if_eligible {
+  virtual void bar();
+};
+
+template <class T> struct UnionWrapper trivially_relocatable_if_eligible {
+  union U {
+    T field1;
+  } u;
+};
+
+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>);
+
+static_assert(!test_is_trivially_relocatable_v<UnionWrapper<AddressDiscriminatedPolymorphicBase>>);
+static_assert(test_is_trivially_relocatable_v<UnionWrapper<NoAddressDiscriminatedPolymorphicBase>>);
+static_assert(!test_is_trivially_relocatable_v<UnionWrapper<MultipleInheriter<NoAddressDiscriminatedPolymorphicBase, AddressDiscriminatedPolymorphicBase>>>);
+static_assert(!test_is_trivially_relocatable_v<UnionWrapper<MultipleInheriter<AddressDiscriminatedPolymorphicBase, NoAddressDiscriminatedPolymorphicBase>>>);