llvmorg-github-actions[bot] wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: mzukovec

<details>
<summary>Changes</summary>

Fixes a use-after-free in `ASTReader::LoadExternalSpecializationsImpl` when 
loading external specializations with `-ftime-trace` enabled.

This resolves the https://github.com/llvm/llvm-project/issues/196482 and builds 
upon the https://github.com/llvm/llvm-project/pull/172658

The function kept a pointer into `SpecLookups`:

```cpp
LookupTable = &amp;It-&gt;getSecond();
```

Then it constructed a `TimeTraceScope` whose name callback calls 
`getNameForDiagnostic`. That call may deserialize additional AST state and 
mutate `SpecLookups`, invalidating the saved pointer before it is later used 
for:

```cpp
LookupTable-&gt;Table.find(HashValue);
```

This is observed as an ASAN `heap-use-after-free` in:

```text
MultiOnDiskHashTable&lt;LazySpecializationInfoLookupTrait&gt;::find
ASTReader::LoadExternalSpecializationsImpl
```

The fix computes the template-argument hash and copies the lazy specialization 
lookup result before constructing the time-trace scope. This avoids retaining a 
`DenseMap` iterator or pointer into `SpecLookups` across code that may trigger 
deserialization.

Validation from a local reproducer:

- Unpatched Clang + PCH + `-ftime-trace`: ASAN heap-use-after-free.
- Unpatched Clang + same PCH without `-ftime-trace`: passes.
- Patched Clang + PCH + `-ftime-trace`: full downstream wasm build passes.

I cannot attach the original PCH-based reproducer publicly because the PCH 
contains private project paths and serialized private headers, but the reduced 
failing source no longer contains project logic and the ASAN trace points 
directly at invalidation inside `LoadExternalSpecializationsImpl`.

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


1 Files Affected:

- (modified) clang/lib/Serialization/ASTReader.cpp (+9-9) 


``````````diff
diff --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 7e8bb6509e84b..c731335185aea 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -8715,12 +8715,17 @@ bool ASTReader::LoadExternalSpecializationsImpl(
     ArrayRef<TemplateArgument> TemplateArgs) {
   assert(D);
 
-  reader::LazySpecializationInfoLookupTable *LookupTable = nullptr;
-  if (auto It = SpecLookups.find(D); It != SpecLookups.end())
-    LookupTable = &It->getSecond();
-  if (!LookupTable)
+  auto It = SpecLookups.find(D);
+  if (It == SpecLookups.end())
     return false;
 
+  auto HashValue = StableHashForTemplateArguments(TemplateArgs);
+
+  // The time-trace diagnostic name and later deserialization may mutate
+  // SpecLookups, invalidating DenseMap iterators and pointers into map 
storage.
+  llvm::SmallVector<serialization::reader::LazySpecializationInfo, 8> Infos =
+      It->second.Table.find(HashValue);
+
   // NOTE: The getNameForDiagnostic usage in the lambda may mutate the
   // `SpecLookups` object.
   llvm::TimeTraceScope TimeScope("Load External Specializations for ", [&] {
@@ -8733,11 +8738,6 @@ bool ASTReader::LoadExternalSpecializationsImpl(
   });
 
   Deserializing LookupResults(this);
-  auto HashValue = StableHashForTemplateArguments(TemplateArgs);
-
-  // Get Decl may violate the iterator from SpecLookups
-  llvm::SmallVector<serialization::reader::LazySpecializationInfo, 8> Infos =
-      LookupTable->Table.find(HashValue);
 
   bool NewSpecsFound = false;
   for (auto &Info : Infos) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/196533
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to