[PAC][clang] Do not attempt to get definition of non-instantiated template

[kovdan01]

This patch fixes a crash which we hit after #154490. The reproducer is provided in a test case.

The root cause of the crash is as follows.
primaryBaseHasAddressDiscriminatedVTableAuthentication requires a non-null CXX record decl definition which might not be available if we are dealing with a non-instantiated template. It might be the case if the template was not instantiated due to a prior fatal error. See the corresponding check in the Sema::InstantiatingTemplate constructor.

Previously, we tried to call isPolymorphic() on a CXXRecordDecl with null DefinitionData, which led to a crash. With this patch, we abort execution of ASTContext::findPointerAuthContent before call to primaryBaseHasAddressDiscriminatedVTableAuthentication if we have a prior fatal error which might have led to non-instantiated templates.

[kovdan01]

@ojhunt It would be nice to see your feedback on this since it's related to your previous changes from #154490

[llvmbot]

@llvm/pr-subscribers-clang

Author: Daniil Kovalev (kovdan01)

Changes

This patch fixes a crash which we hit after #154490. The reproducer is provided in a test case.

The root cause of the crash is as follows.
primaryBaseHasAddressDiscriminatedVTableAuthentication requires a non-null CXX record decl definition which might not be available if we are dealing with a non-instantiated template. It might be the case if the template was not instantiated due to a prior fatal error. See the corresponding check in the Sema::InstantiatingTemplate constructor.

Previously, we tried to call isPolymorphic() on a CXXRecordDecl with null DefinitionData, which led to a crash. With this patch, we abort execution of ASTContext::findPointerAuthContent before call to primaryBaseHasAddressDiscriminatedVTableAuthentication if we have a prior fatal error which might have led to non-instantiated templates.

---

Full diff: https://github.com/llvm/llvm-project/pull/164505.diff

2 Files Affected:

• (modified) clang/lib/AST/ASTContext.cpp (+9-2)
• (added) clang/test/SemaCXX/ptrauth-template-instantiation-aborted.cpp (+28)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 32c8f6209a693..087fd53ff3659 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1618,7 +1618,7 @@ void ASTContext::setRelocationInfoForCXXRecord(
   RelocatableClasses.insert({D, Info});
 }
 
-static bool primaryBaseHaseAddressDiscriminatedVTableAuthentication(
+static bool primaryBaseHasAddressDiscriminatedVTableAuthentication(
     const ASTContext &Context, const CXXRecordDecl *Class) {
   if (!Class->isPolymorphic())
     return false;
@@ -1672,7 +1672,14 @@ ASTContext::findPointerAuthContent(QualType T) const {
     return Result != PointerAuthContent::AddressDiscriminatedData;
   };
   if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
-    if (primaryBaseHaseAddressDiscriminatedVTableAuthentication(*this, CXXRD) &&
+    // `primaryBaseHasAddressDiscriminatedVTableAuthentication` requires a
+    // non-null CXX record decl definition which might not be available if we
+    // are dealing with a non-instantiated template. It might be the case if the
+    // template was not instantiated due to a prior fatal error. See the
+    // corresponding check in the `Sema::InstantiatingTemplate` constructor.
+    if (getDiagnostics().hasFatalErrorOccurred())
+      return PointerAuthContent::None;
+    if (primaryBaseHasAddressDiscriminatedVTableAuthentication(*this, CXXRD) &&
         !ShouldContinueAfterUpdate(
             PointerAuthContent::AddressDiscriminatedVTable))
       return SaveResultAndReturn();
diff --git a/clang/test/SemaCXX/ptrauth-template-instantiation-aborted.cpp b/clang/test/SemaCXX/ptrauth-template-instantiation-aborted.cpp
new file mode 100644
index 0000000000000..2d400c6d6f439
--- /dev/null
+++ b/clang/test/SemaCXX/ptrauth-template-instantiation-aborted.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fptrauth-intrinsics -fsyntax-only -ferror-limit 1 -verify -std=c++03 %s
+
+/// Force two errors so we hit the error limit leading to skip of template instantiation
+# "" // expected-error {{invalid preprocessing directive}}
+# ""
+// expected-error@* {{too many errors emitted}}
+
+template <typename>
+struct a {};
+
+struct b {
+  b(int) {}
+  void c() {
+    /// Trigger the following call stack:
+    ///   ...
+    ///   clang::ASTContext::findPointerAuthContent(clang::QualType) const /path/to/llvm-project/clang/lib/AST/ASTContext.cpp
+    ///   clang::ASTContext::containsAddressDiscriminatedPointerAuth(clang::QualType) const /path/to/llvm-project/clang/lib/AST/ASTContext.cpp
+    ///   clang::QualType::isCXX{11|98}PODType(clang::ASTContext const&) const /path/to/llvm-project/clang/lib/AST/Type.cpp
+    ///   clang::QualType::isPODType(clang::ASTContext const&) const /path/to/llvm-project/clang/lib/AST/Type.cpp
+    ///   SelfReferenceChecker /path/to/llvm-project/clang/lib/Sema/SemaDecl.cpp
+    ///   CheckSelfReference /path/to/llvm-project/clang/lib/Sema/SemaDecl.cpp
+    ///   clang::Sema::AddInitializerToDecl(clang::Decl*, clang::Expr*, bool) /path/to/llvm-project/clang/lib/Sema/SemaDecl.cpp
+    ///   ...
+    b d(0);
+  }
+  a<int> e;
+};
+

[ojhunt]

I would swear I fixed this, please hold.

[ojhunt]

not sure that this is the correct approach, but will need to look at what's happening locally.

[ojhunt]

I've put #164528 up

[kovdan01]

I've put #164528 up

Thanks, closing in favor of #164528