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

[Clang] Crash instantiating a dependent lambda annotated with annotate_type #85154

Open
Sirraide opened this issue Mar 13, 2024 · 15 comments · May be fixed by #85325
Open

[Clang] Crash instantiating a dependent lambda annotated with annotate_type #85154

Sirraide opened this issue Mar 13, 2024 · 15 comments · May be fixed by #85325
Assignees
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party crash-on-valid lambda C++11 lambda expressions

Comments

@Sirraide
Copy link
Contributor

This crashes when we try to instantiate the lambda:

template <typename T>
void h() {
  (void) [] (T) [[clang::annotate_type("foo")]] { };
}

void f() {
  h<int>();
}
@Sirraide Sirraide added clang:frontend Language frontend issues, e.g. anything involving "Sema" crash-on-valid lambda C++11 lambda expressions labels Mar 13, 2024
@Sirraide Sirraide self-assigned this Mar 13, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 13, 2024

@llvm/issue-subscribers-clang-frontend

Author: None (Sirraide)

This crashes when we try to instantiate the lambda: ```c++ template <typename T> void h() { (void) [] (T) [[clang::annotate_type("foo")]] { }; }

void f() {
h<int>();
}

</details>

@Sirraide Sirraide changed the title [Clang] Crash instantiating a lambda annotated with annotate_type [Clang] Crash instantiating a dependent lambda annotated with annotate_type Mar 13, 2024
@shafik
Copy link
Collaborator

shafik commented Mar 14, 2024

Confirmed: https://godbolt.org/z/xcYT8K17M

Assertion:

clang++: /root/llvm-project/clang/lib/Sema/SemaTemplateInstantiate.cpp:4393:
void clang::LocalInstantiationScope::InstantiatedLocal(const clang::Decl*, clang::Decl*): 
Assertion `!Current->LocalDecls.contains(D) && "Instantiated local in inner and outer scopes"' failed.

Backtrace:

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 -std=c++20 <source>
1.	<eof> parser at end of file
2.	<source>:2:6: instantiating function definition 'h<int>'
 #0 0x00000000038fc108 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x38fc108)
 #1 0x00000000038f9dec llvm::sys::CleanupOnSignal(unsigned long) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x38f9dec)
 #2 0x0000000003841908 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x00007f008fe42520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #4 0x00007f008fe969fc pthread_kill (/lib/x86_64-linux-gnu/libc.so.6+0x969fc)
 #5 0x00007f008fe42476 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x42476)
 #6 0x00007f008fe287f3 abort (/lib/x86_64-linux-gnu/libc.so.6+0x287f3)
 #7 0x00007f008fe2871b (/lib/x86_64-linux-gnu/libc.so.6+0x2871b)
 #8 0x00007f008fe39e96 (/lib/x86_64-linux-gnu/libc.so.6+0x39e96)
 #9 0x0000000006ceae37 clang::LocalInstantiationScope::InstantiatedLocal(clang::Decl const*, clang::Decl*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6ceae37)
#10 0x0000000006d1a052 clang::Sema::SubstParmVarDecl(clang::ParmVarDecl*, clang::MultiLevelTemplateArgumentList const&, int, std::optional<unsigned int>, bool, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6d1a052)
#11 0x0000000006d1af10 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformFunctionTypeParams(clang::SourceLocation, llvm::ArrayRef<clang::ParmVarDecl*>, clang::QualType const*, clang::FunctionType::ExtParameterInfo const*, llvm::SmallVectorImpl<clang::QualType>&, llvm::SmallVectorImpl<clang::ParmVarDecl*>*, clang::Sema::ExtParameterInfoBuilder&, unsigned int*) SemaTemplateInstantiate.cpp:0:0
#12 0x0000000006d20734 clang::QualType clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformFunctionProtoType<clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformFunctionProtoType(clang::TypeLocBuilder&, clang::FunctionProtoTypeLoc)::'lambda'(clang::FunctionProtoType::ExceptionSpecInfo&, bool&)>(clang::TypeLocBuilder&, clang::FunctionProtoTypeLoc, clang::CXXRecordDecl*, clang::Qualifiers, clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformFunctionProtoType(clang::TypeLocBuilder&, clang::FunctionProtoTypeLoc)::'lambda'(clang::FunctionProtoType::ExceptionSpecInfo&, bool&)) (.constprop.0) SemaTemplateInstantiate.cpp:0:0
#13 0x0000000006d00d05 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformType(clang::TypeLocBuilder&, clang::TypeLoc) SemaTemplateInstantiate.cpp:0:0
#14 0x0000000006d21a7c clang::QualType clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformAttributedType<clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformLambdaExpr(clang::LambdaExpr*)::'lambda1'(clang::TypeLocBuilder&, clang::TypeLoc)>(clang::TypeLocBuilder&, clang::AttributedTypeLoc, clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformLambdaExpr(clang::LambdaExpr*)::'lambda1'(clang::TypeLocBuilder&, clang::TypeLoc)) SemaTemplateInstantiate.cpp:0:0
#15 0x0000000006cf08e9 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformLambdaExpr(clang::LambdaExpr*) SemaTemplateInstantiate.cpp:0:0
#16 0x0000000006cf1c8f clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformExpr(clang::Expr*) SemaTemplateInstantiate.cpp:0:0
#17 0x0000000006d26e3e clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCStyleCastExpr(clang::CStyleCastExpr*) SemaTemplateInstantiate.cpp:0:0
#18 0x0000000006cf1a8c clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformExpr(clang::Expr*) SemaTemplateInstantiate.cpp:0:0
#19 0x0000000006d29fef clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformStmt(clang::Stmt*, clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::StmtDiscardKind) SemaTemplateInstantiate.cpp:0:0
#20 0x0000000006d2ade4 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt(clang::CompoundStmt*, bool) SemaTemplateInstantiate.cpp:0:0
#21 0x0000000006d3062a clang::Sema::SubstStmt(clang::Stmt*, clang::MultiLevelTemplateArgumentList const&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6d3062a)
#22 0x0000000006d821ca clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation, clang::FunctionDecl*, bool, bool, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6d821ca)
#23 0x0000000006d8052f clang::Sema::PerformPendingInstantiations(bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6d8052f)
#24 0x00000000062ff0a9 clang::Sema::ActOnEndOfTranslationUnitFragment(clang::Sema::TUFragmentKind) (.part.0) Sema.cpp:0:0
#25 0x00000000062ff892 clang::Sema::ActOnEndOfTranslationUnit() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x62ff892)
#26 0x0000000006182d61 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, clang::Sema::ModuleImportState&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6182d61)
#27 0x0000000006175902 clang::ParseAST(clang::Sema&, bool, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6175902)
#28 0x0000000004193908 clang::CodeGenAction::ExecuteAction() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4193908)
#29 0x000000000440efc9 clang::FrontendAction::Execute() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x440efc9)
#30 0x000000000438c6fe clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x438c6fe)
#31 0x00000000044f199e clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x44f199e)
#32 0x0000000000c27226 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0xc27226)
#33 0x0000000000c1ef8a ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&, llvm::ToolContext const&) driver.cpp:0:0
#34 0x00000000041d5099 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
#35 0x0000000003841db4 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x3841db4)
#36 0x00000000041d568f 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
#37 0x000000000419d195 clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x419d195)
#38 0x000000000419dbfd 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+++0x419dbfd)
#39 0x00000000041a5b35 clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x41a5b35)
#40 0x0000000000c2470d clang_main(int, char**, llvm::ToolContext const&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0xc2470d)
#41 0x0000000000b198e4 main (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0xb198e4)
#42 0x00007f008fe29d90 (/lib/x86_64-linux-gnu/libc.so.6+0x29d90)
#43 0x00007f008fe29e40 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e40)
#44 0x0000000000c1ea4e _start (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0xc1ea4e)
clang++: error: clang frontend command failed with exit code 134 (use -v to see invocation)
Compiler returned: 134

Adding assertions and backtrace helps to find duplicate for those screening bugs in the future. Adding godbolt link removes one extra step for those who want to reproduce, I usually try to add gcc/edg/MSVC as well depending on the specifics of the bug.

@shafik shafik added the confirmed Verified by a second party label Mar 14, 2024
@Sirraide
Copy link
Contributor Author

Adding assertions and backtrace helps to find duplicate for those screening bugs in the future. Adding godbolt link removes one extra step for those who want to reproduce, I usually try to add gcc/edg/MSVC as well depending on the specifics of the bug.

Ah, my bad; I ran into this while I was already working on another bug, and I’m a bit tired so I apparently forgot about that.

@Sirraide
Copy link
Contributor Author

This is an educated guess at most, but looking at the code, I think we’re trying to instantiate the parameter twice, once for the modified type and once for the equivalent type of the AttributedType, and it’s asserting on that.

@Sirraide
Copy link
Contributor Author

Looks like that was the issue.

However, we’re also dropping the AttributedType and replacing it with the underlying FunctionProtoType right after; since there’s also another issue open for that, I’ll try and see if I can fix that as well reasonably quickly so either fix doesn’t have to wait for the other to get merged, but if that takes too long, I’ll open a pr just for this first.

@zyn0217
Copy link
Contributor

zyn0217 commented Mar 14, 2024

Fwiw, I remembered there’s a similar patch working on lambda’s attributes by @yuxuanchen1997: #78801

@Sirraide
Copy link
Contributor Author

Fwiw, I remembered there’s a similar patch working on lambda’s attributes by @yuxuanchen1997: #78801

That one looks interesting; I’ll take another look at this later today. The author of that pr mentioned they’re not entirely sure what’s causing the problem, so I’ll try to look into this a bit more to see if I can figure out what the problem(s) are here.

So far, I’ve found that we’re basically attempting to instantiate the same FunctionProtoTypeLoc twice, but instantiating a FunctionProtoTypeLoc has side-effects in that it instantiates the ParmVarDecls for that type loc and establishes a mapping between them and the original ParmVarDecls; if there already is a mapping, this causes an assertion. I think it makes sense to see if we can’t do this differently (e.g. instantiate the ParmVarDecls elsewhere), but irrespective of that, it still makes sense to not try and instantiate the same type twice.

@Sirraide
Copy link
Contributor Author

I’ll also try and see if that pr fixes the same issues, and since it hasn’t had any activity in over a month, I may end up adding some of the changes that they’ve made to a new pr and add them as a co-author for those commits. If anything, I do think we need more tests for this than that pr adds.

@yuxuanchen1997
Copy link
Contributor

Hey! Happy to get back to this. Would appreciate any feedback on the PR.

@Sirraide
Copy link
Contributor Author

Hey! Happy to get back to this. Would appreciate any feedback on the PR.

I’ll try and see if I can use your pr and what I’ve found so far to try and figure out witw is actually the underlying issue here, because the code that is instantiating lambdas seems to be a bit of a mess atm (e.g. we have an overload of TransformAttributedType that takes a callback that is only used to transform the ModifiedType of an AttributedType, and it’s only ever called in one location with one and the same callback—do we really need that?), and your pr looks like it would end up cleaning that up quite considerably.

@Sirraide
Copy link
Contributor Author

it’s only ever called in one location

Actually, it is called in two places, but one of the callbacks just forwards to getDerived().TransformType

@yuxuanchen1997
Copy link
Contributor

we have an overload of TransformAttributedType that takes a callback that is only used to transform the ModifiedType of an AttributedType, and it’s only ever called in one location with one and the same callback—do we really need that?

I wrote that code too. I didn't like it so I came up with a newer one. Just wasn't sure of the approach.

@Sirraide
Copy link
Contributor Author

Update: this entire situation is fairly complicated: TransformLambdaExpr takes care to call TreeTransform’s TransformFunctionProtoType, and not TemplateInstantiatior’s, because the latter creates a new LocalInstantiationScope, which would cause problems here.

@Sirraide
Copy link
Contributor Author

So, I see now why you added a field to keep track of whether we’re instantiating a lambda.

@Sirraide
Copy link
Contributor Author

Ok, I’ve manged to implement a fix for this that performs a cleanup of TransformLambdaExpr similar to #78801, but I’ve moved the variable that tracks whether we’re instantiating a lambda into the LocalInstantiationScope.

I still need to add some more tests for attributes, then I’ll open a pr for this so people can take a look at it.

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 lambda C++11 lambda expressions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants