[clang][ASTImporter] Fix possible crash "given incorrect InsertPos for specialization".

[balazske]

In some situations a new VarTemplateSpecializationDecl (for the same template) can be added during import of another one. The "insert position" that is used to insert the current object into the list of specializations is stored at start of the import and is used later. If the list changes before the insertion the position is not valid any more.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Balázs Kéri (balazske)

Changes

In some situations a new VarTemplateSpecializationDecl (for the same template) can be added during import of another one. The "insert position" that is used to insert the current object into the list of specializations is stored at start of the import and is used later. If the list changes before the insertion the position is not valid any more.

---

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

1 Files Affected:

• (modified) clang/lib/AST/ASTImporter.cpp (+5-2)

diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 023aaa7f0572b4..0036e506b63653 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6504,6 +6504,11 @@ ExpectedDecl ASTNodeImporter::VisitVarTemplateSpecializationDecl(
       return D2;
   }
 
+  // Update InsertPos, because preceding import calls may have invalidated
+  // it by adding new specializations.
+  if (!VarTemplate->findSpecialization(TemplateArgs, InsertPos))
+    VarTemplate->AddSpecialization(D2, InsertPos);
+
   QualType T;
   if (Error Err = importInto(T, D->getType()))
     return std::move(Err);
@@ -6540,8 +6545,6 @@ ExpectedDecl ASTNodeImporter::VisitVarTemplateSpecializationDecl(
   if (FoundSpecialization)
     D2->setPreviousDecl(FoundSpecialization->getMostRecentDecl());
 
-  VarTemplate->AddSpecialization(D2, InsertPos);
-
   addDeclToContexts(D, D2);
 
   // Import the rest of the chain. I.e. import all subsequent declarations.

[balazske]

A test is needed to make the change acceptable but I could not find an easy case to provoke the situation. The problem looks to be related to his code:

using size_t = int;
template<typename... _Types> class tuple;

template<class T, T v>
struct integral_constant
{
    static constexpr T value = v;
    using value_type = T;
    using type = integral_constant; // using injected-class-name
    constexpr operator value_type() const noexcept { return value; }
    constexpr value_type operator()() const noexcept { return value; } // since c++14
};

using true_type = integral_constant<bool, true>;
using false_type = integral_constant<bool, false>;

template<class T, class U>
struct is_same : false_type {};
 
template<class T>
struct is_same<T, T> : true_type {};

template< class T, class U >
inline constexpr bool is_same_v = is_same<T, U>::value;

namespace A {
  template<typename _Tp, typename _Tuple>
  struct __tuple_count;

  template<typename _Tp, typename _Tuple>
    inline constexpr size_t __tuple_count_v =
      __tuple_count<_Tp, _Tuple>::value;

  template<typename _Tp, typename... _Types>
    struct __tuple_count<_Tp, tuple<_Types...>>
    : integral_constant<size_t, 0> { };

  template<typename _Tp, typename _First, typename... _Rest>
    struct __tuple_count<_Tp, tuple<_First, _Rest...>>
    : integral_constant<
	size_t,
	__tuple_count_v<_Tp, tuple<_Rest...>> + is_same_v<_Tp, _First>> { };
};


size_t x = A::__tuple_count_v<int, tuple<bool, int, float>>;

[balazske]

I could reproduce this assertion (with CTU analysis on project "contour"):

clang-19: llvm-project/clang/lib/AST/DeclTemplate.cpp:370: void clang::RedeclarableTemplateDecl::addSpecializationImpl(llvm::FoldingSetVector<EntryType>&, EntryType*, void*) [with Derived = clang::VarTemplateDecl; EntryType = clang::VarTemplateSpeciali
zationDecl]: Assertion `!findSpecializationImpl(Specializations, CorrectInsertPos, SETraits::getTemplateArgs(Entry)) && InsertPos == CorrectInsertPos && "given incorrect InsertPos for specialization"' failed.

With the latest clang (debug or release) the assertion does not show up any more (when the same test is run). Still I think that this change is an improvement in the code and could be merged, even if no test can be found that triggers the assertion.

[balazske]

The problem is that there is a distance between getting the "InsertPos" and the insetion into the list. Between getting the InsertPos (VarTemplate->findSpecialization) and the insertion further AST import statements can occur and probably it can cause the list of specializations to change. I have a test that can cause the same situation (change of list before the insertion) but still the assertion did not show up.

[NagyDonat]

This seems to be a reasonable and straightforward improvement which rules out a hard-to test corner case. I think it's acceptable to merge this as it is now, because crafting a testcase would require a disproportionate amount of work.

[jcsxky]

A test is needed to make the change acceptable but I could not find an easy case to provoke the situation. The problem looks to be related to his code:

using size_t = int;
template<typename... _Types> class tuple;

template<class T, T v>
struct integral_constant
{
    static constexpr T value = v;
    using value_type = T;
    using type = integral_constant; // using injected-class-name
    constexpr operator value_type() const noexcept { return value; }
    constexpr value_type operator()() const noexcept { return value; } // since c++14
};

using true_type = integral_constant<bool, true>;
using false_type = integral_constant<bool, false>;

template<class T, class U>
struct is_same : false_type {};
 
template<class T>
struct is_same<T, T> : true_type {};

template< class T, class U >
inline constexpr bool is_same_v = is_same<T, U>::value;

namespace A {
  template<typename _Tp, typename _Tuple>
  struct __tuple_count;

  template<typename _Tp, typename _Tuple>
    inline constexpr size_t __tuple_count_v =
      __tuple_count<_Tp, _Tuple>::value;

  template<typename _Tp, typename... _Types>
    struct __tuple_count<_Tp, tuple<_Types...>>
    : integral_constant<size_t, 0> { };

  template<typename _Tp, typename _First, typename... _Rest>
    struct __tuple_count<_Tp, tuple<_First, _Rest...>>
    : integral_constant<
	size_t,
	__tuple_count_v<_Tp, tuple<_Rest...>> + is_same_v<_Tp, _First>> { };
};


size_t x = A::__tuple_count_v<int, tuple<bool, int, float>>;

Could you please show your commands which reproduced this crash? I tested locally with the following commands and it runs OK.

clang++ -cc1 -std=c++17 -emit-pch -o test.cpp.ast test.cpp
clang++ -cc1 -x c++ -ast-merge test.cpp.ast  /dev/null -ast-dump

[balazske]

Could you please show your commands which reproduced this crash? I tested locally with the following commands and it runs OK.

clang++ -cc1 -std=c++17 -emit-pch -o test.cpp.ast test.cpp
clang++ -cc1 -x c++ -ast-merge test.cpp.ast  /dev/null -ast-dump

That code is only an example, it differs not much of the real code that caused the crash. But it is not enough to use this code for the problem reproduction. This code can be used to get the case when the specialization list is changed before the insertion, but even then no crash happens:

namespace N {
template <unsigned X>
int B = B<X - 1> + B<X - 2>;
template <>
int B<0> = 0;
template <>
int B<1> = 1;
}
int A = N::B<5>;

With clang version 18.1.6 the original crash does not occur any more.
The "original crash" was reproduced on specific source files of project "contour". Probably I can attach the files and command, but some changes can be required to make it work.

[jcsxky]

Could you please show your commands which reproduced this crash? I tested locally with the following commands and it runs OK.

clang++ -cc1 -std=c++17 -emit-pch -o test.cpp.ast test.cpp
clang++ -cc1 -x c++ -ast-merge test.cpp.ast  /dev/null -ast-dump

That code is only an example, it differs not much of the real code that caused the crash. But it is not enough to use this code for the problem reproduction. This code can be used to get the case when the specialization list is changed before the insertion, but even then no crash happens:

namespace N {
template <unsigned X>
int B = B<X - 1> + B<X - 2>;
template <>
int B<0> = 0;
template <>
int B<1> = 1;
}
int A = N::B<5>;

With clang version 18.1.6 the original crash does not occur any more. The "original crash" was reproduced on specific source files of project "contour". Probably I can attach the files and command, but some changes can be required to make it work.

Could you please show me how to reproduce the crash in detail if possible? Then I can debug this issue in my spare time.

[balazske]

I have not enough resources to create a reproducer and it is not trivial, so I would merge this change now.