Building 0.8.28 on Arch Linux with CMAKE_BUILD_TYPE=None hits an assert in std::optional: Assertion 'this->_M_is_engaged()' failed

[Spixmaster]

I get a lot of these error when running the tests, ./build/test/soltest -p true -- --testpath ./test/.

/usr/src/debug/solidity/solidity_0.8.28/test/soltest.cpp(120): error: in "semanticTests/viaYul/return_storage_pointers": Exception during extracted test: /usr/src/debug/solidity/solidity_0.8.28/libsolidity/interface/CompilerStack.cpp(119): Throw in function solidity::frontend::CompilerStack::CompilerStack(solidity::frontend::ReadCallback::Callback)
Dynamic exception type: boost::wrapexcept<solidity::langutil::InternalCompilerError>
std::exception::what: You shall not have another CompilerStack aside me.
[solidity::util::tag_comment*] = You shall not have another CompilerStack aside me.

It only happens when installing via makepkg on Arch Linux but not when executing the same commands on a cloned repository manually. Additional information, I do not have installed z3 or cvc5.

The build script can be inspected here.

The issue has to be introduced with version 0.8.27 or 0.8.28 because 0.8.26 worked. I have only tested 0.8.28 now which does not work.

[cameel]

Can you check if there's additional detail stuck to the first of the errors that are being reported? Like in #15788 (comment).

If there is, that's the only relevant bit of the output and all the You shall not have another CompilerStack aside me errors are just fallout from that first error. This happens when the test suite hits an internal error in a callback it passes to evmc/evmone. There's no good way to report such an error there without either affecting all subsequent tests or losing detailed info about the error. When I hit it last time I assumed this would be a pretty rare thing, not worth wild workarounds, but if you can just hit simply by running tests on a release version built on Arch, perhaps it's worth a bit more effort to get more friendly output.

As to the error itself, from these effects I highly suspect it's triggered by something in evmone. The AUR package did not change recently so I'm not sure why it would suddenly start breaking tests now. Perhaps it's something in the base system that changed and affected it? If that's the case then simply rebuilding it may help.

I actually need to bump the package to version 13, though I doubt this will help the problem we're seeing here (other than by forcing a rebuild of evmone). Solidity 0.8.28 was built with evmc 12 and has been working with evmone 12 just fine for a long time. Since solidity seems to be the only thing that depends on it, I usually don't hurry updating it and keep it at the same version we use for testing in CI.

[Spixmaster]

Thanks for the response. I am going to post the error output as soon as I have. I have to increase console output history length because it exceeds my current history length.

I am the author auf the AUR package. Only the version and cmake options were changed.

The error was actually reported to me an I was able to reproduce in a clean build. It is not an error solely on my end.

[cameel]

But I meant that there were no changes in the evmone package. Does the clean build involve building that from scratch as well?

[Spixmaster]

Yes, "evmone" was rebuild too.

I see that you are a maintainer of that AUR package. Could you update it please? It is outdated. Also consider these improvement suggestions.

[cameel]

Sure. I'll do this tomorrow when I'll have some time to reproduce and investigate the bug. I'll also incorporate the suggestions.

[Spixmaster]

Here is the beginning of the error output:

[ 97%] Built target strictasm_diff_ossfuzz
[ 98%] Built target strictasm_opt_ossfuzz
[100%] Built target strictasm_assembly_ossfuzz

WARNING :: Gas cost expectations are not being enforced


WARNING :: Gas Cost Expectations are not being enforced

Running 8957 test cases...

0%   10   20   30   40   50   60   70   80   90   100%
|----|----|----|----|----|----|----|----|----|----|
***sr/include/c++/14.2.1/optional:482: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = unsigned char; _Dp = std::_Optional_base<unsigned char, true, true>]: Assertion 'this->_M_is_engaged()' failed.
9munknown location(0): fatal error: in "StandardCompiler/linking_yul": signal: SIGABRT (application abort requested)
/usr/src/debug/solidity/solidity_0.8.28/test/libsolidity/StandardCompiler.cpp(882): last checkpoint: "linking_yul" test entry
/usr/include/c++/14.2.1/optional:482: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = unsigned char; _Dp = std::_Optional_base<unsigned char, true, true>]: Assertion 'this->_M_is_engaged()' failed.
unknown location(0): fatal error: in "StandardCompiler/linking_yul_empty_link_reference": signal: SIGABRT (application abort requested)
/usr/src/debug/solidity/solidity_0.8.28/test/libsolidity/StandardCompiler.cpp(914): last checkpoint: "linking_yul_empty_link_reference" test entry
/usr/include/c++/14.2.1/optional:482: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = unsigned char; _Dp = std::_Optional_base<unsigned char, true, true>]: Assertion 'this->_M_is_engaged()' failed.
unknown location(0): fatal error: in "StandardCompiler/linking_yul_no_filename_in_link_reference": signal: SIGABRT (application abort requested)
/usr/src/debug/solidity/solidity_0.8.28/test/libsolidity/StandardCompiler.cpp(946): last checkpoint: "linking_yul_no_filename_in_link_reference" test entry
/usr/include/c++/14.2.1/optional:482: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = unsigned char; _Dp = std::_Optional_base<unsigned char, true, true>]: Assertion 'this->_M_is_engaged()' failed.
unknown location(0): fatal error: in "StandardCompiler/linking_yul_same_library_name_different_files": signal: SIGABRT (application abort requested)
/usr/src/debug/solidity/solidity_0.8.28/test/libsolidity/StandardCompiler.cpp(978): last checkpoint: "linking_yul_same_library_name_different_files" test entry
*sr/include/c++/14.2.1/optional:482: constexpr const _Tp& std::_Optional_base_impl<_Tp, _Dp>::_M_get() const [with _Tp = unsigned char; _Dp = std::_Optional_base<unsigned char, true, true>]: Assertion 'this->_M_is_engaged()' failed.
39;49munknown location(0): fatal error: in "SolidityEndToEndTest/creation_code_optimizer": signal: SIGABRT (application abort requested)
/usr/src/debug/solidity/solidity_0.8.28/test/libsolidity/SolidityEndToEndTest.cpp(113): last checkpoint
/usr/src/debug/solidity/solidity_0.8.28/libsolidity/interface/CompilerStack.cpp(119): fatal error: in "solidity::frontend::CompilerStack::CompilerStack(solidity::frontend::ReadCallback::Callback)": /usr/src/debug/solidity/solidity_0.8.28/libsolidity/interface/CompilerStack.cpp(119): Throw in function solidity::frontend::CompilerStack::CompilerStack(solidity::frontend::ReadCallback::Callback)
Dynamic exception type: boost::wrapexcept<solidity::langutil::InternalCompilerError>
std::exception::what: You shall not have another CompilerStack aside me.
[solidity::util::tag_comment*] = You shall not have another CompilerStack aside me.

/usr/src/debug/solidity/solidity_0.8.28/test/libsolidity/SolidityEndToEndTest.cpp(122): last checkpoint: "exp_operator" fixture ctor
/usr/src/debug/solidity/solidity_0.8.28/libsolidity/interface/CompilerStack.cpp(119): fatal error: in "solidity::frontend::CompilerStack::CompilerStack(solidity::frontend::ReadCallback::Callback)": /usr/src/debug/solidity/solidity_0.8.28/libsolidity/interface/CompilerStack.cpp(119): Throw in function solidity::frontend::CompilerStack::CompilerStack(solidity::frontend::ReadCallback::Callback)
Dynamic exception type: boost::wrapexcept<solidity::langutil::InternalCompilerError>
std::exception::what: You shall not have another CompilerStack aside me.
[solidity::util::tag_comment*] = You shall not have another CompilerStack aside me.

[cameel]

Thanks! Looks like it likely has nothing to do with the evmone package, just has a similar effect. We're hitting an assert in the implementation of std::optional, which results in SIGABRT (just like throwing exceptions in the evmc callback):

Assertion 'this->_M_is_engaged()' failed.

This happens in StandardCompiler/linking_yul, StandardCompiler/linking_yul_empty_link_reference and a few other boost-based tests. All the other failures are spurious.

That's just the symptom though. Hard to say what's the underlying cause without some debugging. Could be just a random side-effect of an unrelated bug, like writing to already freed memory somewhere.

I suspect that the reason we're not seeing the failure in CI despite having an Arch Linux job is that we build a release binary (with debug info), and have debug asserts disabled.

BTW, I think your package may (unintentionally?) be doing a debug build. You have -D CMAKE_BUILD_TYPE=None, but I don't think None is treated specially by CMake itself and we don't have such a configuration either, so it probably ends up skipping all the build logic that explicitly check for Debug/Release. We default to Release/RelWithDebInfo, but only if the value is actually not set to anything:

solidity/CMakeLists.txt

Lines 6 to 15 in 65e0fd7

	# Set the build type, if none was specified.
	if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
	if(EXISTS "${PROJECT_SOURCE_DIR}/.git")
	set(DEFAULT_BUILD_TYPE "RelWithDebInfo")
	else()
	set(DEFAULT_BUILD_TYPE "Release")
	endif()
	set(CMAKE_BUILD_TYPE "${DEFAULT_BUILD_TYPE}" CACHE STRING "Choose the type of build, options are: Debug Release RelWithDebInfo MinSizeRel" FORCE)
	set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release" "RelWithDebInfo" "MinSizeRel")
	endif()

We should really add a validation for this.

I don't recommend running debug builds outside development, because these produce a huge binary (200-300 MB) and are very slow. Though you'd probably notice this so it's also possible that you're just getting some hybrid that's optimized but still has debug asserts compiled in.

Please either just omit the setting or set it to Release, RelWithDebInfo or MinSizeRel. This should make the test suite pass. We still need to fix the underlying bug of course, but in all likelihood it only affects something minor in the test suite rather than the functioning of the compiler itself.

[cameel]

Maybe we should just have a dedicated build type for release with debug assertions enabled. The reason we don't have a Debug build in CI is mostly the potential performance hit (#12641), but just enabling assertions would probably be fine.

[Spixmaster]

The option -D CMAKE_BUILD_TYPE=None is an Arch Linux thing. See here.

[Spixmaster]

Sure. I'll do this tomorrow when I'll have some time to reproduce and investigate the bug. I'll also incorporate the suggestions.

May I politely remind you of this.

[cameel]

Sorry for the delay. I haven't forgotten, but we're trying to get a new release out and it had a higher priority. I also had problems with reproducing it in our build environment, which made debugging this pretty tedious.

I spent some time on this today to still get it done before we release and got to the bottom of it: #15909. It was actually a simple bug due to dereferencing of an empty optional. Fortunately it does not seem to have serious consequences, at worst could degrade optimizer performance, but good to have that fixed. The fix will be included in 0.8.29, which we're planning to release next week.

There's still the matter of being able to detect these failing asserts so I reopened #12641 and will try to get it addressed in the near future. I also need to look more into those CMAKE_BUILD_TYPE=None builds. I wasn't aware of this before, but if it's a thing we should probably support it and run it in CI.

[Spixmaster]

Out of curiosity since I am a C++ developer too, why does the build type CMAKE_BUILD_TYPE=None affect dereferencing std::optional<T>?

[cameel]

Because it enables the assertion in std::optional. When it's enabled, dereferencing an empty value behaves in a sane way and just fails. When it's not, we can proceed and get into undefined behavior. Which could theoretically result in some kind of failure or a segfault too, but in this case it apparently just gives us some (probably wrong) value. The value is only used as one of the inputs in calculation of a cache key so a wrong one does not make things break. At worst it prevents the optimizer from finding an existing copy of optimized bytecode and forces it to do the optimization again.