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 v15.0.2: crash #58207

Closed
RipleyTom opened this issue Oct 6, 2022 · 28 comments · Fixed by llvm/llvm-project-release-prs#456
Closed

Clang v15.0.2: crash #58207

RipleyTom opened this issue Oct 6, 2022 · 28 comments · Fixed by llvm/llvm-project-release-prs#456
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party consteval C++20 consteval crash Prefer [crash-on-valid] or [crash-on-invalid] release:backport release:merged

Comments

@RipleyTom
Copy link

RipleyTom commented Oct 6, 2022

Clang v15.0.2 seems to fail to compile rpcs3 ( https://github.com/RPCS3/rpcs3 ):

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: /usr/lib/llvm/15/bin/clang++ -DDATADIR=\"/usr/local/share/rpcs3\" -DGLX_GLXEXT_PROTOTYPES -DGL_GLEXT_PROTOTYPES -DHAVE_FAUDIO -DHAVE_LIBEVDEV -DHAVE_VULKAN -DHAVE_X11 -DLLVM_AVAILABLE -DSOUNDTOUCH_ALLOW_SSE -DSOUNDTOUCH_FLOAT_SAMPLES -DST_NO_EXCEPTION_HANDLING -DUSE_MULTICH_ALWAYS -DWC_NO_HARDEN -I/home/ripley/Repos/rpcs3/rpcs3 -I/home/ripley/Repos/rpcs3 -I/home/ripley/Repos/rpcs3/3rdparty/yaml-cpp/yaml-cpp/include -I/home/ripley/Repos/rpcs3/3rdparty/flatbuffers/include -I/home/ripley/Repos/rpcs3/3rdparty/pugixml/src -I/home/ripley/Repos/rpcs3/3rdparty/FAudio/src -I/home/ripley/Repos/rpcs3/3rdparty/FAudio/include -I/usr/include/AL -I/home/ripley/Repos/rpcs3/3rdparty/cubeb/cubeb/include -I/home/ripley/Repos/rpcs3_build/build_clang/exports -I/home/ripley/Repos/rpcs3/3rdparty/SoundTouch/soundtouch/include -I/home/ripley/Repos/rpcs3/3rdparty/stblib/include -I/home/ripley/Repos/rpcs3_build/build_clang/3rdparty/libpng/libpng -I/home/ripley/Repos/rpcs3/3rdparty/libpng/libpng -I/home/ripley/Repos/rpcs3/llvm/include -I/home/ripley/Repos/rpcs3_build/build_clang/3rdparty/llvm_build/include -I/home/ripley/Repos/rpcs3/3rdparty/asmjit/asmjit/src -I/home/ripley/Repos/rpcs3/3rdparty/ffmpeg/include -I/home/ripley/Repos/rpcs3/3rdparty/GL -I/home/ripley/Repos/rpcs3/3rdparty/glslang/glslang/SPIRV/.. -I/home/ripley/Repos/rpcs3_build/build_clang/include -I/home/ripley/Repos/rpcs3/3rdparty/SPIRV/SPIRV-Tools/include -I/home/ripley/Repos/rpcs3/3rdparty/SPIRV/SPIRV-Headers/include -I/home/ripley/Repos/rpcs3/3rdparty/libusb/libusb/libusb -I/home/ripley/Repos/rpcs3/3rdparty/wolfssl/wolfssl -I/home/ripley/Repos/rpcs3_build/build_clang/3rdparty/wolfssl/wolfssl -I/home/ripley/Repos/rpcs3/3rdparty/xxHash/cmake_unofficial/.. -I/home/ripley/Repos/rpcs3/3rdparty/xxHash -isystem /usr/include/libevdev-1.0 -isystem /usr/include/SDL2 -Wl,--exclude-libs,ALL -O3 -DNDEBUG -pthread -Wall -fno-exceptions -fstack-protector -msse -msse2 -mcx16 -Werror=old-style-cast -Werror=sign-compare -Werror=reorder -Werror=return-type -Werror=overloaded-virtual -Werror=missing-noreturn -Wunused-parameter -Wignored-qualifiers -Wredundant-move -Wcast-qual -Wdeprecated-copy -Wtautological-compare -Wempty-body -Wredundant-decls -Wstrict-aliasing=1 -Werror=inconsistent-missing-override -fconstexpr-steps=16777216 -Wno-unused-lambda-capture -Wno-unused-private-field -Wno-delete-non-virtual-dtor -Wno-unused-command-line-argument -march=native -std=gnu++20 -MD -MT rpcs3/Emu/CMakeFiles/rpcs3_emu.dir/cache_utils.cpp.o -MF CMakeFiles/rpcs3_emu.dir/cache_utils.cpp.o.d -o CMakeFiles/rpcs3_emu.dir/cache_utils.cpp.o -c /home/ripley/Repos/rpcs3/rpcs3/Emu/cache_utils.cpp
1.      <eof> parser at end of file
 #0 0x00007fdf5f2a98a1 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/usr/lib/llvm/15/bin/../lib64/libLLVM-15.so+0xef38a1)
 #1 0x00007fdf5f2a72f4 llvm::sys::RunSignalHandlers() (/usr/lib/llvm/15/bin/../lib64/libLLVM-15.so+0xef12f4)
 #2 0x00007fdf5f1b7bb8 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x00007fdf5dfaec10 __restore_rt libc_sigaction.c:0:0
 #4 0x00007fdf5e00a8c5 cfree@GLIBC_2.2.5 (/lib64/libc.so.6+0x938c5)
 #5 0x00007fdf66d45693 clang::Sema::PopExpressionEvaluationContext() (/usr/lib/llvm/15/bin/../lib64/libclang-cpp.so.15+0x1643693)
 #6 0x00007fdf662aa7e6 clang::ParseAST(clang::Sema&, bool, bool) (/usr/lib/llvm/15/bin/../lib64/libclang-cpp.so.15+0xba87e6)
 #7 0x00007fdf67dead19 clang::FrontendAction::Execute() (/usr/lib/llvm/15/bin/../lib64/libclang-cpp.so.15+0x26e8d19)
 #8 0x00007fdf67d83e62 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/usr/lib/llvm/15/bin/../lib64/libclang-cpp.so.15+0x2681e62)
 #9 0x00007fdf67e5e517 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/usr/lib/llvm/15/bin/../lib64/libclang-cpp.so.15+0x275c517)
#10 0x000055e6b2f6e05c cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/usr/lib/llvm/15/bin/clang+++0x1805c)
#11 0x000055e6b2f69661 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) driver.cpp:0:0
#12 0x00007fdf67a55ed5 void llvm::function_ref<void ()>::callback_fn<clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const::'lambda'()>(long) Job.cpp:0:0
#13 0x00007fdf5f1b7cc3 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/usr/lib/llvm/15/bin/../lib64/libLLVM-15.so+0xe01cc3)
#14 0x00007fdf67a57974 clang::driver::CC1Command::Execute(llvm::ArrayRef<llvm::Optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const (/usr/lib/llvm/15/bin/../lib64/libclang-cpp.so.15+0x2355974)
#15 0x00007fdf67a22463 clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/usr/lib/llvm/15/bin/../lib64/libclang-cpp.so.15+0x2320463)
#16 0x00007fdf67a22bfd clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&, bool) const (/usr/lib/llvm/15/bin/../lib64/libclang-cpp.so.15+0x2320bfd)
#17 0x00007fdf67a3211c clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) (/usr/lib/llvm/15/bin/../lib64/libclang-cpp.so.15+0x233011c)
#18 0x000055e6b2f6b84c clang_main(int, char**) (/usr/lib/llvm/15/bin/clang+++0x1584c)
#19 0x00007fdf5df9a34a __libc_start_call_main libc-start.c:0:0
#20 0x00007fdf5df9a3fc __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x233fc)
#21 0x000055e6b2f66a41 _start (/usr/lib/llvm/15/bin/clang+++0x10a41)
clang-15: error: clang frontend command failed with exit code 139 (use -v to see invocation)
clang version 15.0.2
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm/15/bin
Configuration file: /etc/clang/clang++.cfg
clang-15: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-15: note: diagnostic msg: /tmp/cache_utils-93deb0.cpp
clang-15: note: diagnostic msg: /tmp/cache_utils-93deb0.sh
clang-15: note: diagnostic msg: 

********************
make[2]: *** [rpcs3/Emu/CMakeFiles/rpcs3_emu.dir/build.make:76: rpcs3/Emu/CMakeFiles/rpcs3_emu.dir/cache_utils.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:29773: rpcs3/Emu/CMakeFiles/rpcs3_emu.dir/all] Error 2
make: *** [Makefile:156: all] Error 2

I wish I could give you a small sample but all I can give you are those 5MB preprocessed sources:

cache_utils-93deb0..zip

Clang15 seems to compile all the submodules(including llvm14 itself) just fine but fails on rpcs3 itself.
Of note is that Rpcs3 uses C++20 extensively.

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" crash Prefer [crash-on-valid] or [crash-on-invalid] and removed new issue labels Oct 6, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 6, 2022

@llvm/issue-subscribers-clang-frontend

@royjacobson
Copy link
Contributor

Reduced to

struct channel {
    consteval channel(const char* name) noexcept { }
};
consteval const char* make_channel_name(const char* name) { return name;}

channel rsx_log(make_channel_name("rsx_log"));
channel sys_log(make_channel_name("sys_log"));

https://godbolt.org/z/n19hcEPYv

@shafik
Copy link
Collaborator

shafik commented Feb 3, 2023

Assertion:

clang++: /root/llvm-project/clang/lib/Sema/SemaExpr.cpp:17866: 
void RemoveNestedImmediateInvocation(clang::Sema&, clang::Sema::ExpressionEvaluationContextRecord&, llvm::SmallVectorTemplateCommon<llvm::PointerIntPair<clang::ConstantExpr*, 1>, void>::reverse_iterator)::ComplexRemove::RemoveImmediateInvocation(clang::ConstantExpr*):
Assertion `It != IISet.rend() && "ConstantExpr marked IsImmediateInvocation should " "be present"' failed.

Backtrace:

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++2b <source>
1.	<eof> parser at end of file
 #0 0x000056318d1625cf llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x40d25cf)
 #1 0x000056318d16056c llvm::sys::CleanupOnSignal(unsigned long) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x40d056c)
 #2 0x000056318d0ad248 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x00007f3a77ef8420 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
 #4 0x00007f3a779c500b raise (/lib/x86_64-linux-gnu/libc.so.6+0x4300b)
 #5 0x00007f3a779a4859 abort (/lib/x86_64-linux-gnu/libc.so.6+0x22859)
 #6 0x00007f3a779a4729 (/lib/x86_64-linux-gnu/libc.so.6+0x22729)
 #7 0x00007f3a779b5fd6 (/lib/x86_64-linux-gnu/libc.so.6+0x33fd6)
 #8 0x000056318fcb4255 (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6c24255)
 #9 0x000056318fdd34f2 RemoveNestedImmediateInvocation(clang::Sema&, clang::Sema::ExpressionEvaluationContextRecord&, std::reverse_iterator<llvm::PointerIntPair<clang::ConstantExpr*, 1u, unsigned int, llvm::PointerLikeTypeTraits<clang::ConstantExpr*>, llvm::PointerIntPairInfo<clang::ConstantExpr*, 1u, llvm::PointerLikeTypeTraits<clang::ConstantExpr*>>>*>)::ComplexRemove::TransformInitializer(clang::Expr*, bool) SemaExpr.cpp:0:0
#10 0x000056318fdd375e clang::TreeTransform<RemoveNestedImmediateInvocation(clang::Sema&, clang::Sema::ExpressionEvaluationContextRecord&, std::reverse_iterator<llvm::PointerIntPair<clang::ConstantExpr*, 1u, unsigned int, llvm::PointerLikeTypeTraits<clang::ConstantExpr*>, llvm::PointerIntPairInfo<clang::ConstantExpr*, 1u, llvm::PointerLikeTypeTraits<clang::ConstantExpr*>>>*>)::ComplexRemove>::TransformExprs(clang::Expr* const*, unsigned int, bool, llvm::SmallVectorImpl<clang::Expr*>&, bool*) SemaExpr.cpp:0:0
#11 0x000056318fdd4a9e clang::TreeTransform<RemoveNestedImmediateInvocation(clang::Sema&, clang::Sema::ExpressionEvaluationContextRecord&, std::reverse_iterator<llvm::PointerIntPair<clang::ConstantExpr*, 1u, unsigned int, llvm::PointerLikeTypeTraits<clang::ConstantExpr*>, llvm::PointerIntPairInfo<clang::ConstantExpr*, 1u, llvm::PointerLikeTypeTraits<clang::ConstantExpr*>>>*>)::ComplexRemove>::TransformCXXConstructExpr(clang::CXXConstructExpr*) SemaExpr.cpp:0:0
#12 0x000056318fd45f28 clang::Sema::PopExpressionEvaluationContext() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6cb5f28)
#13 0x000056318f7f2a06 clang::ParseAST(clang::Sema&, bool, bool) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x6762a06)
#14 0x000056318e4e66c8 clang::CodeGenAction::ExecuteAction() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x54566c8)
#15 0x000056318dd50db9 clang::FrontendAction::Execute() (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4cc0db9)
#16 0x000056318dcd656e clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4c4656e)
#17 0x000056318de38443 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4da8443)
#18 0x000056318a5e05cd cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x15505cd)
#19 0x000056318a5dc6e7 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) driver.cpp:0:0
#20 0x000056318db3c4d9 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
#21 0x000056318d0ad730 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x401d730)
#22 0x000056318db3cd8f 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
#23 0x000056318db052ec clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4a752ec)
#24 0x000056318db05d6d 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+++0x4a75d6d)
#25 0x000056318db0ef3c clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x4a7ef3c)
#26 0x000056318a5ded32 clang_main(int, char**) (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x154ed32)
#27 0x00007f3a779a6083 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24083)
#28 0x000056318a5d73ce _start (/opt/compiler-explorer/clang-assertions-trunk/bin/clang+++0x15473ce)
clang-17: error: clang frontend command failed with exit code 134 (use -v to see invocation)
Compiler returned: 134

@DimitryAndric
Copy link
Collaborator

This starts appearing after 1c55f05 ("Properly diagnose constant evaluation issues at TU scope") by @AaronBallman.

It looks like this change is pretty minimal, but add a line EnterExpressionEvaluationContext PotentiallyEvaluated(S, Sema::ExpressionEvaluationContext::PotentiallyEvaluated); at a pretty early stage in parsing, so maybe this uncovered a problem in the expression evaluator...

@Fznamznon
Copy link
Contributor

struct channel {
    consteval channel(const char* name) noexcept { }
};
consteval const char* make_channel_name(const char* name) { return name;}

channel rsx_log(make_channel_name("rsx_log"));
channel sys_log(make_channel_name("sys_log"));

I've been looking into this. Looks like when immediate invocations are handled, it it expected that if there was several immediate invocations they all belong to the same expression evaluation context, so the function call below:

RemoveNestedImmediateInvocation(SemaRef, Rec, It);

tries to remove subexpressions and fails to find one. It starts with a call to constructor of channel class, then tries to remove its subexpression which is call to make_channel_name.

The thing is, during parsing of non local variable initializer a new evaluation context is pushed:

ExpressionEvaluationContext::PotentiallyEvaluated, D);

so the call to make_channel_name ends up being attached to another expression evaluation context. Immediate invocations are handled per evaluation context, so we get an assertion since we can't find call make_channel_name in the same context as the constructor call and get an assertion.

@shafik
Copy link
Collaborator

shafik commented Mar 8, 2023

Great work, that makes sense, maybe I missing this but this does not happen if we comment out the sys_log declaration line: https://godbolt.org/z/dY6xW8xWv

What is the difference here?

@Fznamznon
Copy link
Contributor

Great work, that makes sense, maybe I missing this but this does not happen if we comment out the sys_log declaration line: https://godbolt.org/z/dY6xW8xWv

The thing is, we start "removing" expressions only if there is more than one of them. See:

if (Rec.ImmediateInvocationCandidates.size() > 1) {

Once you comment sys_log you get one expression in each context, so the code removing expressions is never entered.

@AaronBallman
Copy link
Collaborator

Immediate invocations are handled per evaluation context, so we get an assertion since we can't find call make_channel_name in the same context as the constructor call and get an assertion.

Ooofda, so in 1c55f05, I added an expression evaluation context for the entire TU, which is why we now get the immediate invocation in different evaluation contexts. Ugh. Notionally, I think that was the correct approach, but I didn't realize I was introducing this issue where we then add declarations to different contexts.

@AaronBallman
Copy link
Collaborator

The thing is, during parsing of non local variable initializer a new evaluation context is pushed:

I wonder if one approach worth exploring is to only do this if the non-local variable is not at global scope; the evaluation context at TU scope should suffice now, shouldn't it?

@Fznamznon
Copy link
Contributor

I wonder if one approach worth exploring is to only do this if the non-local variable is not at global scope; the evaluation context at TU scope should suffice now, shouldn't it?

Static variables at function scope seems to have the same problem. It seems adding evaluation context at TU scope just revealed the problem that's been there for a while. https://godbolt.org/z/5eh998qdP

@Fznamznon
Copy link
Contributor

I wonder if should just stop assuming that an immediate expression and its subexpression belong to the same evaluation context and remove the failed assertion. Clearly with non-local variables whose initializers are nested immediate invocations that is not the case. It seems that the "removal" which fails now only prevents double diagnosing for some cases (like checking if address of consteval function is taken outside of another consteval function, or checking that result of immediate invocation produces constexpr result). I actually can't figure out a case that would trigger such a diagnostic from subexpression of consteval initializer of global variable.

@AaronBallman
Copy link
Collaborator

I think that could be worth exploring -- it seems like that assumption is no longer true. However, we do need to be careful not to issue duplicate diagnostics, as well. Please tell me we have existing test coverage that breaks if you remove the assumption? ;-)

@Fznamznon
Copy link
Contributor

Please tell me we have existing test coverage that breaks if you remove the assumption? ;-)

I'm not seeing any tests failing, so we likely don't have tests that actually run into the situation with separate evaluation contexts. Hence the bug report, I guess.

@AaronBallman
Copy link
Collaborator

Please tell me we have existing test coverage that breaks if you remove the assumption? ;-)

I'm not seeing any tests failing, so we likely don't have tests that actually run into the situation with separate evaluation contexts. Hence the bug report, I guess.

Oh no. :-( If we can't find any duplicate diagnostics generated by removing that check, it's possible that duplicates are filtered through some other means. CC @usx95, @ilya-biryukov, and @cor3ntin in case this rings any bells for others who've worked in this area recently.

@Fznamznon
Copy link
Contributor

I wonder if should just stop assuming that an immediate expression and its subexpression belong to the same evaluation context and remove the failed assertion.

That is probably not the right way to fix it. I figured the test case which produces duplicate errors. BTW, it gives two errors even now without any changes due to separate evaluation contexts. See https://godbolt.org/z/r41E98nff .
I'll explore a bit more.

@Fznamznon
Copy link
Contributor

I posted https://reviews.llvm.org/D146234 as possible solution.

@shafik
Copy link
Collaborator

shafik commented Mar 17, 2023

@Fznamznon we have another case here: #61463

@Fznamznon
Copy link
Contributor

@Fznamznon we have another case here: #61463

Seems to be the same thing. https://reviews.llvm.org/D146234 helps.

gysit pushed a commit to nextsilicon/llvm-project that referenced this issue Apr 27, 2023
Before this patch it was expected that if there was several immediate
invocations they all belong to the same expression evaluation context.
During parsing of non local variable initializer a new evaluation context is
pushed, so code like this
```
namespace scope {
struct channel {
    consteval channel(const char* name) noexcept { }
};
consteval const char* make_channel_name(const char* name) { return name;}

channel rsx_log(make_channel_name("rsx_log"));
}
```
produced a nested immediate invocation whose subexpressions are attached
to different expression evaluation contexts. The constructor call
belongs to TU context and `make_channel_name` call to context of
variable initializer.

This patch removes this assumption and adds tracking of previously
failed immediate invocations, so it is possible when handling an
immediate invocation th check that its subexpressions from possibly another
evaluation context contains errors and not produce duplicate
diagnostics.

Fixes llvm#58207

Reviewed By: aaron.ballman, shafik

Differential Revision: https://reviews.llvm.org/D146234
freebsd-git pushed a commit to freebsd/freebsd-src that referenced this issue Apr 28, 2023
Merge commit a5e1a93ea10f from llvm-project (by Mariya Podchishchaeva):

  [clang] Fix crash when handling nested immediate invocations

  Before this patch it was expected that if there was several immediate
  invocations they all belong to the same expression evaluation context.
  During parsing of non local variable initializer a new evaluation context is
  pushed, so code like this
  ```
  namespace scope {
  struct channel {
      consteval channel(const char* name) noexcept { }
  };
  consteval const char* make_channel_name(const char* name) { return name;}

  channel rsx_log(make_channel_name("rsx_log"));
  }
  ```
  produced a nested immediate invocation whose subexpressions are attached
  to different expression evaluation contexts. The constructor call
  belongs to TU context and `make_channel_name` call to context of
  variable initializer.

  This patch removes this assumption and adds tracking of previously
  failed immediate invocations, so it is possible when handling an
  immediate invocation th check that its subexpressions from possibly another
  evaluation context contains errors and not produce duplicate
  diagnostics.

  Fixes llvm/llvm-project#58207

  Reviewed By: aaron.ballman, shafik

  Differential Revision: https://reviews.llvm.org/D146234

PR:		269489
MFC after:	3 days
freebsd-git pushed a commit to freebsd/freebsd-src that referenced this issue May 3, 2023
Merge commit a5e1a93ea10f from llvm-project (by Mariya Podchishchaeva):

  [clang] Fix crash when handling nested immediate invocations

  Before this patch it was expected that if there was several immediate
  invocations they all belong to the same expression evaluation context.
  During parsing of non local variable initializer a new evaluation context is
  pushed, so code like this
  ```
  namespace scope {
  struct channel {
      consteval channel(const char* name) noexcept { }
  };
  consteval const char* make_channel_name(const char* name) { return name;}

  channel rsx_log(make_channel_name("rsx_log"));
  }
  ```
  produced a nested immediate invocation whose subexpressions are attached
  to different expression evaluation contexts. The constructor call
  belongs to TU context and `make_channel_name` call to context of
  variable initializer.

  This patch removes this assumption and adds tracking of previously
  failed immediate invocations, so it is possible when handling an
  immediate invocation th check that its subexpressions from possibly another
  evaluation context contains errors and not produce duplicate
  diagnostics.

  Fixes llvm/llvm-project#58207

  Reviewed By: aaron.ballman, shafik

  Differential Revision: https://reviews.llvm.org/D146234

PR:		269489
MFC after:	3 days

(cherry picked from commit 56f2446)
freebsd-git pushed a commit to freebsd/freebsd-src that referenced this issue May 3, 2023
Merge commit a5e1a93ea10f from llvm-project (by Mariya Podchishchaeva):

  [clang] Fix crash when handling nested immediate invocations

  Before this patch it was expected that if there was several immediate
  invocations they all belong to the same expression evaluation context.
  During parsing of non local variable initializer a new evaluation context is
  pushed, so code like this
  ```
  namespace scope {
  struct channel {
      consteval channel(const char* name) noexcept { }
  };
  consteval const char* make_channel_name(const char* name) { return name;}

  channel rsx_log(make_channel_name("rsx_log"));
  }
  ```
  produced a nested immediate invocation whose subexpressions are attached
  to different expression evaluation contexts. The constructor call
  belongs to TU context and `make_channel_name` call to context of
  variable initializer.

  This patch removes this assumption and adds tracking of previously
  failed immediate invocations, so it is possible when handling an
  immediate invocation th check that its subexpressions from possibly another
  evaluation context contains errors and not produce duplicate
  diagnostics.

  Fixes llvm/llvm-project#58207

  Reviewed By: aaron.ballman, shafik

  Differential Revision: https://reviews.llvm.org/D146234

PR:		269489
MFC after:	3 days

(cherry picked from commit 56f2446)
@ormris ormris added this to the LLVM 16.0.X Release milestone May 19, 2023
@ormris
Copy link
Collaborator

ormris commented May 19, 2023

/cherry-pick a5e1a93

@ormris ormris reopened this May 19, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented May 19, 2023

Failed to cherry-pick: a5e1a93

https://github.com/llvm/llvm-project/actions/runs/5026430589

Please manually backport the fix and push it to your github fork. Once this is done, please add a comment like this:

/branch <user>/<repo>/<branch>

ormris pushed a commit to ormris/llvm-project that referenced this issue May 23, 2023
Before this patch it was expected that if there was several immediate
invocations they all belong to the same expression evaluation context.
During parsing of non local variable initializer a new evaluation context is
pushed, so code like this
```
namespace scope {
struct channel {
    consteval channel(const char* name) noexcept { }
};
consteval const char* make_channel_name(const char* name) { return name;}

channel rsx_log(make_channel_name("rsx_log"));
}
```
produced a nested immediate invocation whose subexpressions are attached
to different expression evaluation contexts. The constructor call
belongs to TU context and `make_channel_name` call to context of
variable initializer.

This patch removes this assumption and adds tracking of previously
failed immediate invocations, so it is possible when handling an
immediate invocation th check that its subexpressions from possibly another
evaluation context contains errors and not produce duplicate
diagnostics.

Fixes llvm#58207

Reviewed By: aaron.ballman, shafik

Differential Revision: https://reviews.llvm.org/D146234

(cherry picked from commit a5e1a93)
@ormris
Copy link
Collaborator

ormris commented May 23, 2023

/branch ormris/llvm-project/release/16.x

@llvmbot
Copy link
Collaborator

llvmbot commented May 24, 2023

Failed to create pull request for release/16.x https://github.com/llvm/llvm-project/actions/runs/5063148022

ormris pushed a commit to ormris/llvm-project that referenced this issue May 24, 2023
Before this patch it was expected that if there was several immediate
invocations they all belong to the same expression evaluation context.
During parsing of non local variable initializer a new evaluation context is
pushed, so code like this
```
namespace scope {
struct channel {
    consteval channel(const char* name) noexcept { }
};
consteval const char* make_channel_name(const char* name) { return name;}

channel rsx_log(make_channel_name("rsx_log"));
}
```
produced a nested immediate invocation whose subexpressions are attached
to different expression evaluation contexts. The constructor call
belongs to TU context and `make_channel_name` call to context of
variable initializer.

This patch removes this assumption and adds tracking of previously
failed immediate invocations, so it is possible when handling an
immediate invocation th check that its subexpressions from possibly another
evaluation context contains errors and not produce duplicate
diagnostics.

Fixes llvm#58207

Reviewed By: aaron.ballman, shafik

Differential Revision: https://reviews.llvm.org/D146234

(cherry picked from commit a5e1a93)
@ormris
Copy link
Collaborator

ormris commented May 24, 2023

/branch ormris/llvm-project/pick/release/16.x

@llvmbot
Copy link
Collaborator

llvmbot commented May 24, 2023

/pull-request llvm/llvm-project-release-prs#456

@tstellar
Copy link
Collaborator

@erichkeane What do you think about backporting this?

@erichkeane
Copy link
Collaborator

I believe there ended up being additional fallout from this, right @Fznamznon ? So we couldn't get away with just cherry-picking this, we'd need at least 1 or 2 follow-up patches.

Additionally while this does seem to be a regression, this is a C++20 only feature, and I believe we consider all of C++20 to be experimental for Clang 16, so I don't believe it meets the cherry-pick criteria for 16.

@Fznamznon
Copy link
Contributor

I believe there ended up being additional fallout from this, right @Fznamznon ? So we couldn't get away with just cherry-picking this, we'd need at least 1 or 2 follow-up patches.

No, I don't think so. Hope I didn't forget anything. https://reviews.llvm.org/D146234 turned out pretty self-contained. Though there was several other serious consteval issues I was working on and we didn't cherry pick them since those are not really regressions.

@erichkeane
Copy link
Collaborator

I believe there ended up being additional fallout from this, right @Fznamznon ? So we couldn't get away with just cherry-picking this, we'd need at least 1 or 2 follow-up patches.

No, I don't think so. Hope I didn't forget anything. https://reviews.llvm.org/D146234 turned out pretty self-contained. Though there was several other serious consteval issues I was working on and we didn't cherry pick them since those are not really regressions.

Thanks! Then @tstellar : this is a low-risk patch, so this I would think backporting would be acceptable. I think the motivation isn't huge (since C++20, particularly consteval/concepts support is experimental in Clang16) .

tstellar pushed a commit to llvm/llvm-project-release-prs that referenced this issue Jun 1, 2023
Before this patch it was expected that if there was several immediate
invocations they all belong to the same expression evaluation context.
During parsing of non local variable initializer a new evaluation context is
pushed, so code like this
```
namespace scope {
struct channel {
    consteval channel(const char* name) noexcept { }
};
consteval const char* make_channel_name(const char* name) { return name;}

channel rsx_log(make_channel_name("rsx_log"));
}
```
produced a nested immediate invocation whose subexpressions are attached
to different expression evaluation contexts. The constructor call
belongs to TU context and `make_channel_name` call to context of
variable initializer.

This patch removes this assumption and adds tracking of previously
failed immediate invocations, so it is possible when handling an
immediate invocation th check that its subexpressions from possibly another
evaluation context contains errors and not produce duplicate
diagnostics.

Fixes llvm/llvm-project#58207

Reviewed By: aaron.ballman, shafik

Differential Revision: https://reviews.llvm.org/D146234

(cherry picked from commit a5e1a93)
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this issue Jun 21, 2023
Merge commit a5e1a93ea10f from llvm-project (by Mariya Podchishchaeva):

  [clang] Fix crash when handling nested immediate invocations

  Before this patch it was expected that if there was several immediate
  invocations they all belong to the same expression evaluation context.
  During parsing of non local variable initializer a new evaluation context is
  pushed, so code like this
  ```
  namespace scope {
  struct channel {
      consteval channel(const char* name) noexcept { }
  };
  consteval const char* make_channel_name(const char* name) { return name;}

  channel rsx_log(make_channel_name("rsx_log"));
  }
  ```
  produced a nested immediate invocation whose subexpressions are attached
  to different expression evaluation contexts. The constructor call
  belongs to TU context and `make_channel_name` call to context of
  variable initializer.

  This patch removes this assumption and adds tracking of previously
  failed immediate invocations, so it is possible when handling an
  immediate invocation th check that its subexpressions from possibly another
  evaluation context contains errors and not produce duplicate
  diagnostics.

  Fixes llvm/llvm-project#58207

  Reviewed By: aaron.ballman, shafik

  Differential Revision: https://reviews.llvm.org/D146234

PR:		269489
MFC after:	3 days
laffer1 pushed a commit to MidnightBSD/src that referenced this issue Jun 27, 2023
Merge commit a5e1a93ea10f from llvm-project (by Mariya Podchishchaeva):

  [clang] Fix crash when handling nested immediate invocations

  Before this patch it was expected that if there was several immediate
  invocations they all belong to the same expression evaluation context.
  During parsing of non local variable initializer a new evaluation context is
  pushed, so code like this
  ```
  namespace scope {
  struct channel {
      consteval channel(const char* name) noexcept { }
  };
  consteval const char* make_channel_name(const char* name) { return name;}

  channel rsx_log(make_channel_name("rsx_log"));
  }
  ```
  produced a nested immediate invocation whose subexpressions are attached
  to different expression evaluation contexts. The constructor call
  belongs to TU context and `make_channel_name` call to context of
  variable initializer.

  This patch removes this assumption and adds tracking of previously
  failed immediate invocations, so it is possible when handling an
  immediate invocation th check that its subexpressions from possibly another
  evaluation context contains errors and not produce duplicate
  diagnostics.

  Fixes llvm/llvm-project#58207

  Reviewed By: aaron.ballman, shafik

  Differential Revision: https://reviews.llvm.org/D146234

PR:		269489
MFC after:	3 days

(cherry picked from commit 56f2446575c78d962b6dda5e3310bec078622f3d)
DianQK pushed a commit to DianQK/llvm-project that referenced this issue Oct 10, 2023
Before this patch it was expected that if there was several immediate
invocations they all belong to the same expression evaluation context.
During parsing of non local variable initializer a new evaluation context is
pushed, so code like this
```
namespace scope {
struct channel {
    consteval channel(const char* name) noexcept { }
};
consteval const char* make_channel_name(const char* name) { return name;}

channel rsx_log(make_channel_name("rsx_log"));
}
```
produced a nested immediate invocation whose subexpressions are attached
to different expression evaluation contexts. The constructor call
belongs to TU context and `make_channel_name` call to context of
variable initializer.

This patch removes this assumption and adds tracking of previously
failed immediate invocations, so it is possible when handling an
immediate invocation th check that its subexpressions from possibly another
evaluation context contains errors and not produce duplicate
diagnostics.

Fixes llvm#58207

Reviewed By: aaron.ballman, shafik

Differential Revision: https://reviews.llvm.org/D146234
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 consteval C++20 consteval crash Prefer [crash-on-valid] or [crash-on-invalid] release:backport release:merged
Projects
Status: Done
Archived in project
Development

Successfully merging a pull request may close this issue.