Address discriminated __ptrauth should not be relocatable

After consideration, while "supportable" by relocation, it
seems much more reasonable for people to understand that
explicitly qualified types can have different relocatability
semantics.

Implicitly address discriminated fields should be relocatable
as developers in general cannot control those behaviours, hence
the trivial_relocation language explicitly acknowledges that
this impacts unions.

Explicitly address discriminated fields do however drastically
change the developer aware model of a type, and the practical
requirements for efficiently handling __ptrauth qualified types
mean that you would want

   some_addr_discriminated_t[100];

to not be reported as trivially relocatable, while

   struct {
     ...
     some_addr_discriminated_t field;
     ...
   }

to be reported as such, and the actual decision would regress
to what proportion of the object being relocated is subject to
such a policy.

Having is_trivially_relocatable change behaviour according to
some hypothetical internal heuristic is clearly a terrible
idea, and so I think for now we should instead simply limit
the application of trivial relocation to address discriminated
data to those fields that outside of the control of developers.

Author: ojhunt

clang/include/clang/AST/ASTContext.h

@@ -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;
+  }
 
-  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;
+  }
+  PointerAuthContent findPointerAuthContent(QualType T);
+  llvm::DenseMap<const RecordDecl *, PointerAuthContent>
       RecordContainsAddressDiscriminatedPointerAuth;
 
   ImportDecl *FirstLocalImport = nullptr;

clang/lib/AST/ASTContext.cpp

@@ -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;
@@ -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) {

clang/lib/Sema/SemaTypeTraits.cpp

@@ -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;
 

clang/test/SemaCXX/ptrauth-triviality.cpp

@@ -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>));
@@ -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 {
@@ -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>));
@@ -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 {

clang/test/SemaCXX/trivially-relocatable-ptrauth.cpp

@@ -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 {