Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assertion failure with nested parameter packs and lambdas #61460

Closed
dwblaikie opened this issue Mar 16, 2023 · 5 comments · Fixed by #69224
Closed

Assertion failure with nested parameter packs and lambdas #61460

dwblaikie opened this issue Mar 16, 2023 · 5 comments · Fixed by #69224
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party crash-on-valid

Comments

@dwblaikie
Copy link
Collaborator

dwblaikie commented Mar 16, 2023

https://godbolt.org/z/4Pj4GhY9s

template <typename... Ts> void g(Ts... p1s) {
  (void)[&](auto... p2s) { ([&] { p1s; p2s; }, ...); };
}                                                       
void f1() {
  g();
}

Looks like (according to GCC and Clang without assertions) this is probably valid (though I'm not entirely sure how/why - not sure how the lambda pack applies/doesn't to the function template pack)

The crash backtrace is:

clang++: /root/llvm-project/clang/lib/Sema/SemaTemplateVariadic.cpp:407: bool clang::Sema::DiagnoseUnexpandedParameterPack(clang::Expr*, clang::Sema::UnexpandedParameterPackContext): Assertion `!Unexpanded.empty() && "Unable to find unexpanded parameter packs"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /opt/compiler-explorer/clang-assertions-trunk/bin/clang++ -gdwarf-4 -g -o /app/output.s -mllvm --x86-asm-syntax=intel -S --gcc-toolchain=/opt/compiler-explorer/gcc-snapshot -fcolor-diagnostics -fno-crash-diagnostics <source>
1.	<eof> parser at end of file
2.	<source>:1:32: instantiating function definition 'g<>'
 #0 0x00005555a36d685f llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x3fee85f)
 #1 0x00005555a36d459c llvm::sys::CleanupOnSignal(unsigned long) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x3fec59c)
 #2 0x00005555a3621d28 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x00007f04e7246420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
 #4 0x00007f04e6d1300b raise (/lib/x86_64-linux-gnu/libc.so.6+0x4300b)
 #5 0x00007f04e6cf2859 abort (/lib/x86_64-linux-gnu/libc.so.6+0x22859)
 #6 0x00007f04e6cf2729 (/lib/x86_64-linux-gnu/libc.so.6+0x22729)
 #7 0x00007f04e6d03fd6 (/lib/x86_64-linux-gnu/libc.so.6+0x33fd6)
 #8 0x00005555a69239c2 (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x723b9c2)
 #9 0x00005555a640e83e clang::Sema::ActOnFinishFullExpr(clang::Expr*, clang::SourceLocation, bool, bool, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6d2683e)
#10 0x00005555a669107e clang::Sema::ActOnExprStmt(clang::ActionResult<clang::Expr*, true>, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6fa907e)
#11 0x00005555a68af229 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt(clang::CompoundStmt*, bool) SemaTemplateInstantiate.cpp:0:0
#12 0x00005555a687863f clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformLambdaExpr(clang::LambdaExpr*) SemaTemplateInstantiate.cpp:0:0
#13 0x00005555a68791fa clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformExpr(clang::Expr*) SemaTemplateInstantiate.cpp:0:0
#14 0x00005555a688144f clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCXXFoldExpr(clang::CXXFoldExpr*) SemaTemplateInstantiate.cpp:0:0
#15 0x00005555a6879449 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformExpr(clang::Expr*) SemaTemplateInstantiate.cpp:0:0
#16 0x00005555a68adfcf clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformStmt(clang::Stmt*, clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::StmtDiscardKind) SemaTemplateInstantiate.cpp:0:0
#17 0x00005555a68af229 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt(clang::CompoundStmt*, bool) SemaTemplateInstantiate.cpp:0:0
#18 0x00005555a687863f clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformLambdaExpr(clang::LambdaExpr*) SemaTemplateInstantiate.cpp:0:0
#19 0x00005555a68791fa clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformExpr(clang::Expr*) SemaTemplateInstantiate.cpp:0:0
#20 0x00005555a68aa5a2 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCStyleCastExpr(clang::CStyleCastExpr*) SemaTemplateInstantiate.cpp:0:0
#21 0x00005555a6879374 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformExpr(clang::Expr*) SemaTemplateInstantiate.cpp:0:0
#22 0x00005555a68adfcf clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformStmt(clang::Stmt*, clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::StmtDiscardKind) SemaTemplateInstantiate.cpp:0:0
#23 0x00005555a68af229 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt(clang::CompoundStmt*, bool) SemaTemplateInstantiate.cpp:0:0
#24 0x00005555a68b48ae clang::Sema::SubstStmt(clang::Stmt*, clang::MultiLevelTemplateArgumentList const&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x71cc8ae)
#25 0x00005555a6903baa clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation, clang::FunctionDecl*, bool, bool, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x721bbaa)
#26 0x00005555a6901eff clang::Sema::PerformPendingInstantiations(bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x7219eff)
#27 0x00005555a5efdd70 clang::Sema::ActOnEndOfTranslationUnitFragment(clang::Sema::TUFragmentKind) (.part.0) Sema.cpp:0:0
#28 0x00005555a5efe45a clang::Sema::ActOnEndOfTranslationUnit() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x681645a)
#29 0x00005555a5d99913 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, clang::Sema::ModuleImportState&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x66b1913)
#30 0x00005555a5d8d49a clang::ParseAST(clang::Sema&, bool, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x66a549a)
#31 0x00005555a48fb2f8 clang::CodeGenAction::ExecuteAction() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x52132f8)
#32 0x00005555a4161839 clang::FrontendAction::Execute() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4a79839)
#33 0x00005555a40e5a46 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x49fda46)
#34 0x00005555a4245517 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4b5d517)
#35 0x00005555a0c62b66 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x157ab66)
#36 0x00005555a0c5e98a ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) driver.cpp:0:0
#37 0x00005555a3f4c53d void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const::'lambda'()>(long) Job.cpp:0:0
#38 0x00005555a3622210 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x3f3a210)
#39 0x00005555a3f4cdff clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const (.part.0) Job.cpp:0:0
#40 0x00005555a3f1413c clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x482c13c)
#41 0x00005555a3f14bdd clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&, bool) const (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x482cbdd)
#42 0x00005555a3f1c97d clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x483497d)
#43 0x00005555a0c61010 clang_main(int, char**, llvm::ToolContext const&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x1579010)
#44 0x00005555a0b6d4a5 main (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x14854a5)
#45 0x00007f04e6cf4083 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24083)
#46 0x00005555a0c596ce _start (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x15716ce)
@dwblaikie dwblaikie added clang Clang issues not falling into any other category crash-on-valid labels Mar 16, 2023
@shafik
Copy link
Collaborator

shafik commented Mar 16, 2023

Confirmed: #57082

@shafik shafik added concepts C++20 concepts confirmed Verified by a second party and removed concepts C++20 concepts labels Mar 16, 2023
@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed clang Clang issues not falling into any other category labels Mar 16, 2023
@llvmbot
Copy link

llvmbot commented Mar 16, 2023

@llvm/issue-subscribers-clang-frontend

@rnk
Copy link
Collaborator

rnk commented Jun 15, 2023

Clicking through the godbolt link shows this still repros.

@zyn0217
Copy link
Contributor

zyn0217 commented Oct 15, 2023

Upon some investigation on this issue, I'd say removing all assertions of non-empty Unexpanded probably be the optimal way to address it.

The crash happens in Sema::DiagnoseUnexpandedParameterPack, where an assertion is hit that Unexpanded is empty anyways.

bool Sema::DiagnoseUnexpandedParameterPack(Expr *E,
UnexpandedParameterPackContext UPPC) {
// C++0x [temp.variadic]p5:
// An appearance of a name of a parameter pack that is not expanded is
// ill-formed.
if (!E->containsUnexpandedParameterPack())
return false;
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseStmt(E);
assert(!Unexpanded.empty() && "Unable to find unexpanded parameter packs");
return DiagnoseUnexpandedParameterPacks(E->getBeginLoc(), UPPC, Unexpanded);
}

This is suspicious since we've validated that E must contain unexpanded packs at the beginning. Inspecting the type of E reveals FunctionParmPackExpr, which is an expression that "has been substituted but not yet expanded" per the comment below. And E represents the p1s inside the inner-most lambda ([&] { p1s; p2s; }) in this case.

/// Represents a reference to a function parameter pack or init-capture pack
/// that has been substituted but not yet expanded.
///
/// When a pack expansion contains multiple parameter packs at different levels,
/// this node is used to represent a function parameter pack at an outer level
/// which we have already substituted to refer to expanded parameters, but where
/// the containing pack expansion cannot yet be expanded.
///
/// \code
/// template<typename...Ts> struct S {
/// template<typename...Us> auto f(Ts ...ts) -> decltype(g(Us(ts)...));
/// };
/// template struct S<int, int>;
/// \endcode
class FunctionParmPackExpr final

Looking through the stack, we'd find that the FunctionParmPackExpr is an outcome of a tree transformation during the template instantiation:

// Handle references to function parameter packs.
if (VarDecl *PD = dyn_cast<VarDecl>(D))
if (PD->isParameterPack())
return TransformFunctionParmPackRefExpr(E, PD);

(which invokes TransformFunctionParmPackRefExpr)

// If this is a reference to a function parameter pack which we can
// substitute but can't yet expand, build a FunctionParmPackExpr for it.
if (getSema().ArgumentPackSubstitutionIndex == -1) {
QualType T = TransformType(E->getType());
if (T.isNull())
return ExprError();
auto *PackExpr = FunctionParmPackExpr::Create(getSema().Context, T, PD,
E->getExprLoc(), *Pack);
getSema().MarkFunctionParmPackReferenced(PackExpr);
return PackExpr;

(which transforms DeclRefExpr that refers to Ts... p1s to a FunctionParmPackExpr due to the reason in the comment above. Note that, if we remove the t2s from the expansion, i.e.,

template <typename... Ts> void g(Ts... p1s) {
  (void)[&](auto... p2s) { ([&] { p1s; }, ...); };
}                                                       
void f1() {
  g();
}

then we won't be building a FunctionParmPackExpr since the fold expression itself is independent of the undetermined p2s. Thus, the crash will disappear as we are not expecting any unexpands from FunctionParmPackExpr.)

So, one way that occurs to me is to add the (missing)FunctionParmPackExpr visit function to CollectUnexpandedParameterPacksVisitor in SemaTemplateVariadic.cpp:

/// A class that collects unexpanded parameter packs.
class CollectUnexpandedParameterPacksVisitor :

bool VisitFunctionParmPackExpr(FunctionParmPackExpr *Pack) {
  addUnexpanded(Pack->getParameterPack(), Pack->getParameterPackLocation());
  return true;
}

I patched this collector locally, and indeed it solved the crash. But unfortunately, this immediately crashes the test case clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp. Specifically,

auto v = [](auto ...a) {
  [&](auto ...b) {
    ((a = b), ...);
  }(100, 20, 3);
  return (a + ...);
}(400, 50, 6);

This time, I'm hitting the assertion inside LocalInstantiationScope::findInstantiationOf, where D is the unexpanded parameter that is a VarDecl from FunctionParmPackExpr that represents a inside the expression ((a = b), ...), and it is the one added by the new VisitFunctionParmPackExpr. Obviously, the pack a is not instantiated inside the inner but the outer lambda. So we find nothing from the current scope and the crash happens: Changing the collector is infeasible.

For this reason, we might have to accept some discrepancies inside Sema::DiagnoseUnexpandedParameterPack: Given an unexpanded expression E, it is possible that all packs are indeed expanded without FunctionParmPackExpr being considered. This could happen when we're instantiating a template, that DeclRefExpr being converted to FunctionParmPackExpr and passed into DiagnoseUnexpandedParameterPack (Aside: It should be named as MaybeDiagnoseUnexpandedParameterPack, I think.), where DiagnoseUnexpandedParameterPack (again, Maybe) might be invoked.

The other reason that makes this plausible is, that although we've affirmed non-empty Unexpanded, we've also bailed out early if the Unexpanded is empty inside Sema::DiagnoseUnexpandedParameterPacks. This is also why we're not confronted with the issue with a release version of clang.

Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
UnexpandedParameterPackContext UPPC,
ArrayRef<UnexpandedParameterPack> Unexpanded) {
if (Unexpanded.empty())
return false;

@zyn0217
Copy link
Contributor

zyn0217 commented Oct 16, 2023

Proposed fix: #69224

zyn0217 added a commit to zyn0217/llvm-project that referenced this issue Oct 31, 2023
…armPackExpr

Closes llvm#61460.

We have FunctionParmPackExpr that serves as the unexpanded
expression but from which the visitor collects none, which may
lead to assertion failure during the template instantiation.
zyn0217 added a commit to zyn0217/llvm-project that referenced this issue Nov 8, 2023
…ndedParameterPacksVisitor

Closes llvm#61460.

We have FunctionParmPackExpr that serves as unexpanded pack
expressions but from which the visitor collects none, which leads
to the assertion failure during the template instantiation.

Actually, we don't need to assert this condition since we would
do nothing in the subsequent DiagnoseUnexpandedParameterPacks call
even if unsatisfied.
zyn0217 added a commit that referenced this issue Nov 13, 2023
…armPackExpr (#69224)

Closes #61460.

We have FunctionParmPackExpr that serves as the unexpanded expression
but from which the visitor collects none, which may lead to assertion
failure during the template instantiation.
zahiraam pushed a commit to zahiraam/llvm-project that referenced this issue Nov 20, 2023
…armPackExpr (llvm#69224)

Closes llvm#61460.

We have FunctionParmPackExpr that serves as the unexpanded expression
but from which the visitor collects none, which may lead to assertion
failure during the template instantiation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party crash-on-valid
Projects
None yet
6 participants