[Clang] Fix an iterator invalidation bug in concept normalization cache

[zyn0217]

The NormalizationCache may be inserted recursively when normalizing template arguments with non-dependent default arguments. Since the ADT doesn't preserve iterator validity, this caused undefined behavior.

This is a regression on trunk so there is no release note.

Fixes #165238

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

The NormalizationCache may be inserted recursively when normalizing template arguments with non-dependent default arguments. Since the ADT doesn't preserve iterator validity, this caused undefined behavior.

We convert it to std::map where the insert operation doesn't invalidate iterators, than calling find() twice for performance concerns.

This is a regression on trunk so there is no release note.

Fixes #165238

---

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

2 Files Affected:

• (modified) clang/include/clang/Sema/Sema.h (+1-1)
• (modified) clang/test/SemaTemplate/concepts.cpp (+26)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 52904c72d1cfc..031d1811c5c27 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -14941,7 +14941,7 @@ class Sema final : public SemaBase {
   /// constrained declarations). If an error occurred while normalizing the
   /// associated constraints of the template or concept, nullptr will be cached
   /// here.
-  llvm::DenseMap<ConstrainedDeclOrNestedRequirement, NormalizedConstraint *>
+  std::map<ConstrainedDeclOrNestedRequirement, NormalizedConstraint *>
       NormalizationCache;
 
   /// Cache whether the associated constraint of a declaration
diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp
index becf5467a1b61..c90af41a09468 100644
--- a/clang/test/SemaTemplate/concepts.cpp
+++ b/clang/test/SemaTemplate/concepts.cpp
@@ -1632,3 +1632,29 @@ void fn3() {
 }
 
 }
+
+namespace GH165238 {
+
+namespace std {
+template <typename, typename _Tp>
+concept output_iterator = requires(_Tp __t) { __t; };
+template <typename _Out> struct basic_format_context {
+  static_assert(output_iterator<_Out, int>);
+  using char_type = _Out;
+};
+template <typename> class basic_format_parse_context;
+template <typename, typename _Context, typename _Formatter,
+          typename = basic_format_parse_context<typename _Context::char_type>>
+concept __parsable_with = requires(_Formatter __f) { __f; };
+template <typename _Tp, typename _CharT,
+          typename _Context = basic_format_context<_CharT>>
+concept __formattable_impl = __parsable_with<_Tp, _Context, _Context>;
+template <typename _Tp, typename _CharT>
+concept formattable = __formattable_impl<_Tp, _CharT>;
+} // namespace std
+struct {
+  void operator()(std::formattable<char> auto);
+} call;
+void foo() { call(""); }
+
+}

[shafik]

Next time with a regression, please label the bug report as a regression. Also try to identify the PR that caused the regression and link that PR in your fix.