[clang] [clang] Fix nondeterminism in MemberPointerType (PR #137910)

[Maksim Ivanov via cfe-commits]

https://github.com/emaxx-google updated https://github.com/llvm/llvm-project/pull/137910

>From b31a3f86d3427405f1182d59c381e90c93e521b8 Mon Sep 17 00:00:00 2001
From: Maksim Ivanov <emaxx at google.com>
Date: Wed, 30 Apr 2025 00:31:30 +0000
Subject: [PATCH 1/2] [clang] Fix nondeterminism in MemberPointerType

This commit fixes the nondeterminism issue in C++ header module enabled
builds which were observed after
https://github.com/llvm/llvm-project/pull/132401.

The issue was related to the fact that the hash set operation in
MemberPointerType::Profile() was triggering getMostRecentDecl(). The
root cause seems to be that the latter was leading to the reentrant
modification of the hash set, with some probability (likely depending
on the actual values of hashes).

We haven't been able to come up with a deterministic regression test for
this fix.
---
 clang/include/clang/AST/Type.h |  3 ++-
 clang/lib/AST/Type.cpp         | 10 +++++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 3e1fb05ad537c..c93cbae767db8 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -3602,6 +3602,7 @@ class MemberPointerType : public Type, public llvm::FoldingSetNode {
   }
 
   NestedNameSpecifier *getQualifier() const { return Qualifier; }
+  CXXRecordDecl *getCXXRecordDecl() const;
   CXXRecordDecl *getMostRecentCXXRecordDecl() const;
 
   bool isSugared() const;
@@ -3610,7 +3611,7 @@ class MemberPointerType : public Type, public llvm::FoldingSetNode {
   }
 
   void Profile(llvm::FoldingSetNodeID &ID) {
-    Profile(ID, getPointeeType(), getQualifier(), getMostRecentCXXRecordDecl());
+    Profile(ID, getPointeeType(), getQualifier(), getCXXRecordDecl());
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID, QualType Pointee,
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 111a642173418..a149eac7e4555 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -5275,10 +5275,14 @@ void MemberPointerType::Profile(llvm::FoldingSetNodeID &ID, QualType Pointee,
     ID.AddPointer(Cls->getCanonicalDecl());
 }
 
+CXXRecordDecl *MemberPointerType::getCXXRecordDecl() const {
+  return dyn_cast<MemberPointerType>(getCanonicalTypeInternal())
+      ->getQualifier()
+      ->getAsRecordDecl();
+}
+
 CXXRecordDecl *MemberPointerType::getMostRecentCXXRecordDecl() const {
-  auto *RD = dyn_cast<MemberPointerType>(getCanonicalTypeInternal())
-                 ->getQualifier()
-                 ->getAsRecordDecl();
+  auto *RD = getCXXRecordDecl();
   if (!RD)
     return nullptr;
   return RD->getMostRecentNonInjectedDecl();

>From a8c958e79b7595b1d741b61357e3954af84affdf Mon Sep 17 00:00:00 2001
From: Maksim Ivanov <emaxx at google.com>
Date: Wed, 30 Apr 2025 14:29:28 +0000
Subject: [PATCH 2/2] add comment

---
 clang/include/clang/AST/Type.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index c93cbae767db8..6d08a2f49f9f9 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -3603,6 +3603,9 @@ class MemberPointerType : public Type, public llvm::FoldingSetNode {
 
   NestedNameSpecifier *getQualifier() const { return Qualifier; }
   CXXRecordDecl *getCXXRecordDecl() const;
+  /// Note: this can trigger extra deserialization when external AST sources are
+  /// used. Prefer `getCXXRecordDecl()` unless you really need the most recent
+  /// decl.
   CXXRecordDecl *getMostRecentCXXRecordDecl() const;
 
   bool isSugared() const;