llvm-project icon indicating copy to clipboard operation
llvm-project copied to clipboard

[clang] Fix an invalidate iterator in PCH with -ftime-trace enabled.

Open hokein opened this issue 3 weeks ago • 3 comments

Fixes #170421

hokein avatar Dec 17 '25 14:12 hokein

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

Fixes #170421


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

2 Files Affected:

  • (modified) clang/lib/Serialization/ASTReader.cpp (+10-5)
  • (added) clang/test/PCH/pr170421.cpp (+20)
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index aec61322fb8be..01a2240460cab 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -8529,10 +8529,15 @@ bool ASTReader::LoadExternalSpecializationsImpl(
     ArrayRef<TemplateArgument> TemplateArgs) {
   assert(D);
 
-  auto It = SpecLookups.find(D);
-  if (It == SpecLookups.end())
-    return false;
-
+  reader::LazySpecializationInfoLookupTable *LookupTable =
+      nullptr;
+  if (auto It = SpecLookups.find(D); It != SpecLookups.end())
+    LookupTable = &It->getSecond();
+  if (!LookupTable)
+    return false; 
+
+  // NOTE: The getNameForDiagnostic usage in the lambda may mutate the
+  // `SpecLookups` object.
   llvm::TimeTraceScope TimeScope("Load External Specializations for ", [&] {
     std::string Name;
     llvm::raw_string_ostream OS(Name);
@@ -8547,7 +8552,7 @@ bool ASTReader::LoadExternalSpecializationsImpl(
 
   // Get Decl may violate the iterator from SpecLookups
   llvm::SmallVector<serialization::reader::LazySpecializationInfo, 8> Infos =
-      It->second.Table.find(HashValue);
+      LookupTable->Table.find(HashValue);
 
   bool NewSpecsFound = false;
   for (auto &Info : Infos) {
diff --git a/clang/test/PCH/pr170421.cpp b/clang/test/PCH/pr170421.cpp
new file mode 100644
index 0000000000000..51d56a68e6523
--- /dev/null
+++ b/clang/test/PCH/pr170421.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -ftime-trace=%t.json -o - %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER_INCLUDED
+#define HEADER_INCLUDED
+
+inline namespace {
+template<typename T> T g(T v) { return v; }
+template<typename T> T f(T v) { return g(v); }
+template<typename T> T g();
+}
+
+#else
+
+int x;
+void i() { f(x); }
+
+#endif

llvmbot avatar Dec 17 '25 14:12 llvmbot

@llvm/pr-subscribers-clang-modules

Author: Haojian Wu (hokein)

Changes

Fixes #170421


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

2 Files Affected:

  • (modified) clang/lib/Serialization/ASTReader.cpp (+10-5)
  • (added) clang/test/PCH/pr170421.cpp (+20)
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index aec61322fb8be..01a2240460cab 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -8529,10 +8529,15 @@ bool ASTReader::LoadExternalSpecializationsImpl(
     ArrayRef<TemplateArgument> TemplateArgs) {
   assert(D);
 
-  auto It = SpecLookups.find(D);
-  if (It == SpecLookups.end())
-    return false;
-
+  reader::LazySpecializationInfoLookupTable *LookupTable =
+      nullptr;
+  if (auto It = SpecLookups.find(D); It != SpecLookups.end())
+    LookupTable = &It->getSecond();
+  if (!LookupTable)
+    return false; 
+
+  // NOTE: The getNameForDiagnostic usage in the lambda may mutate the
+  // `SpecLookups` object.
   llvm::TimeTraceScope TimeScope("Load External Specializations for ", [&] {
     std::string Name;
     llvm::raw_string_ostream OS(Name);
@@ -8547,7 +8552,7 @@ bool ASTReader::LoadExternalSpecializationsImpl(
 
   // Get Decl may violate the iterator from SpecLookups
   llvm::SmallVector<serialization::reader::LazySpecializationInfo, 8> Infos =
-      It->second.Table.find(HashValue);
+      LookupTable->Table.find(HashValue);
 
   bool NewSpecsFound = false;
   for (auto &Info : Infos) {
diff --git a/clang/test/PCH/pr170421.cpp b/clang/test/PCH/pr170421.cpp
new file mode 100644
index 0000000000000..51d56a68e6523
--- /dev/null
+++ b/clang/test/PCH/pr170421.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -ftime-trace=%t.json -o - %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER_INCLUDED
+#define HEADER_INCLUDED
+
+inline namespace {
+template<typename T> T g(T v) { return v; }
+template<typename T> T f(T v) { return g(v); }
+template<typename T> T g();
+}
+
+#else
+
+int x;
+void i() { f(x); }
+
+#endif

llvmbot avatar Dec 17 '25 14:12 llvmbot

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.

github-actions[bot] avatar Dec 17 '25 14:12 github-actions[bot]

:penguin: Linux x64 Test Results

  • 112125 tests passed
  • 4519 tests skipped

:white_check_mark: The build succeeded and all tests passed.

github-actions[bot] avatar Dec 17 '25 21:12 github-actions[bot]