[clang] [clang] Fix PointerAuth semantics of cpp_trivially_relocatable (PR #143796)

[Oliver Hunt via cfe-commits]

================
@@ -292,77 +294,129 @@ Sema::CheckCXX2CRelocatableAndReplaceable(const CXXRecordDecl *D) {
       return HasSuitableSMP();
 
     // is a union with no user-declared special member functions, or
-    if (IsUnion())
+    if (IsTrivialUnion())
       return HasSuitableSMP();
 
     // is default-movable.
     return IsDefaultMovable();
   }();
 
+  bool PtrauthMatters = LangOpts.PointerAuthIntrinsics ||
+                        LangOpts.PointerAuthVTPtrAddressDiscrimination;
+  if (PtrauthMatters) {
+    bool IsUnion = D->isUnion();
+    auto RecordPointerAuth = [&](bool HasAddressDiscrimination) {
+      if (HasAddressDiscrimination && IsUnion) {
+        Info.IsRelocatable = false;
+        Info.IsReplaceable = false;
+      }
+    };
+    auto IsBottomRelocationInfo = [](const CXXRecordDeclRelocationInfo &Info) {
+      return !Info.IsReplaceable && !Info.IsRelocatable;
+    };
+
+    if (D->isPolymorphic())
+      RecordPointerAuth(Context.hasAddressDiscriminatedVTableAuthentication(D));
+    for (auto Base : D->bases()) {
+      if (IsBottomRelocationInfo(Info))
+        break;
+      bool BaseHasPtrauth =
+          Context.containsAddressDiscriminatedPointerAuth(Base.getType());
+      RecordPointerAuth(BaseHasPtrauth);
+    }
+    for (auto *FieldDecl : D->fields()) {
+      if (IsBottomRelocationInfo(Info))
+        break;
+      bool FieldHasPtrauth =
+          Context.containsAddressDiscriminatedPointerAuth(FieldDecl->getType());
+      RecordPointerAuth(FieldHasPtrauth);
+    }
+  }
+  Context.setRelocationInfoForCXXRecord(D, Info);
   return Info;
 }
 
-bool Sema::IsCXXTriviallyRelocatableType(const CXXRecordDecl &RD) {
-  if (std::optional<ASTContext::CXXRecordDeclRelocationInfo> Info =
-          getASTContext().getRelocationInfoForCXXRecord(&RD))
-    return Info->IsRelocatable;
-  ASTContext::CXXRecordDeclRelocationInfo Info =
-      CheckCXX2CRelocatableAndReplaceable(&RD);
-  getASTContext().setRelocationInfoForCXXRecord(&RD, Info);
-  return Info.IsRelocatable;
-}
+ASTContext::CXXRecordDeclRelocationInfo
+Sema::CheckCXX2CRelocatableAndReplaceable(QualType T) {
+  T = T.getCanonicalType();
+  enum class DirectRelocationInformation { Yes, No, Unknown };
+  DirectRelocationInformation Relocatable =
+      DirectRelocationInformation::Unknown;
+  DirectRelocationInformation Replaceable =
+      DirectRelocationInformation::Unknown;
+  DirectRelocationInformation ContainsAddressDiscriminatedValues =
+      DirectRelocationInformation::Unknown;
+
+  auto UpdateRelocatable = [&](DirectRelocationInformation DRI) {
+    if (Relocatable == DirectRelocationInformation::Unknown ||
+        Relocatable == DirectRelocationInformation::Yes)
+      Relocatable = DRI;
+  };
+  auto UpdateReplaceable = [&](DirectRelocationInformation DRI) {
+    if (Replaceable == DirectRelocationInformation::Unknown ||
+        Replaceable == DirectRelocationInformation::Yes)
+      Replaceable = DRI;
+  };
+  auto UpdateAddressDiscrimination = [&](DirectRelocationInformation DRI) {
+    if (ContainsAddressDiscriminatedValues == DirectRelocationInformation::Yes)
+      Replaceable = DirectRelocationInformation::No;
+  };
 
-bool Sema::IsCXXTriviallyRelocatableType(QualType Type) {
+  if (T->isVariableArrayType()) {
+    UpdateRelocatable(DirectRelocationInformation::No);
+    UpdateReplaceable(DirectRelocationInformation::No);
+  }
 
-  QualType BaseElementType = getASTContext().getBaseElementType(Type);
+  if (T.isConstQualified() || T.isVolatileQualified())
+    UpdateReplaceable(DirectRelocationInformation::No);
 
-  if (Type->isVariableArrayType())
-    return false;
+  if (T.hasAddressDiscriminatedPointerAuth())
+    UpdateAddressDiscrimination(DirectRelocationInformation::Yes);
+
+  QualType BaseElementType =
+      SemaRef.getASTContext().getBaseElementType(T.getUnqualifiedType());
+
+  if (BaseElementType->isIncompleteType()) {
+    Relocatable = DirectRelocationInformation::No;
+    Replaceable = DirectRelocationInformation::No;
+  }
 
   if (BaseElementType.hasNonTrivialObjCLifetime())
-    return false;
+    UpdateRelocatable(DirectRelocationInformation::No);
 
-  if (BaseElementType.hasAddressDiscriminatedPointerAuth())
-    return false;
+  if (BaseElementType->isScalarType()) {
+    UpdateRelocatable(DirectRelocationInformation::Yes);
+    UpdateReplaceable(DirectRelocationInformation::Yes);
+    UpdateAddressDiscrimination(DirectRelocationInformation::No);
+  }
 
-  if (BaseElementType->isIncompleteType())
-    return false;
+  if (BaseElementType->isVectorType())
+    UpdateRelocatable(DirectRelocationInformation::Yes);
 
-  if (BaseElementType->isScalarType() || BaseElementType->isVectorType())
-    return true;
+  auto CreateInfo = [=]() -> CXXRecordDeclRelocationInfo {
+    return {Relocatable == DirectRelocationInformation::Yes,
+            Replaceable == DirectRelocationInformation::Yes};
+  };
 
-  if (const auto *RD = BaseElementType->getAsCXXRecordDecl())
-    return IsCXXTriviallyRelocatableType(*RD);
+  if (BaseElementType->isIncompleteType())
+    return CreateInfo();
 
-  return false;
+  const CXXRecordDecl *RD = BaseElementType->getAsCXXRecordDecl();
+  if (!RD)
+    return CreateInfo();
+
+  CXXRecordDeclRelocationInfo Info = CheckCXX2CRelocatableAndReplaceable(RD);
+  Info.IsRelocatable &= Relocatable != DirectRelocationInformation::No;
+  Info.IsReplaceable &= Replaceable != DirectRelocationInformation::No;
+  return Info;
----------------
ojhunt wrote:

I think I agree - when the merging happened I though the same logic also needed to track pointer auth characteristics, but later on I separated that into its own separate query via the context, which calls the earlier merge into question.

As noted on discord I may just revert Sema.h and SemaTypeTraits.cpp and just add the required union checks now that the context checks are an option

https://github.com/llvm/llvm-project/pull/143796