wasm-opt(Windows): child not found in parent, UNREACHABLE executed #6639
Comments
|
Very strange that there would be a windows-specific bug. Perhaps undefined behavior in some C++ STL stuff? I don't have a windows machine so this would be hard for me to investigate. Can you perhaps reduce this with wasm-reduce? A tiny testcase might be much easier to reason about. |
|
|
|
I don't have a guess as to what could be going wrong in the code, very strange - there should be no way for the child not to have a proper parent. That this only happens on windows makes me wonder if this is a compiler bug. Which compilers do you see this with? I'd be especially interested in comparing MSVC, clang, and clang+libc++ (the last would not be using the system C++ libraries). Reducing could still help here, but if this is a compiler bug then the more important thing might be to narrow this down to which compiler and version it happens on. |
|
Yes, very strange! maybe we can wait for more information. |
|
I see it when using |
the |
15 tasks
|
Just to make sure there is no miscommunication, the issue happens with :
So I guess this isn't a compiler issue since official release is built with mingw(?) and my version with MSVC. I tried my best using wasm-reduce but I wasn't able to produce any useful result. So I went through the rabbit hole of trying to debug this issue : (Disclaimer my C++ and Windows skills are a bit Rust-y and I might be missing things here)
Patch : (activate thread sanitizer + change the builtin trap for msvc compatibility ) diff --git a/CMakeLists.txt b/CMakeLists.txt
index fef45bfca..ec7375d46 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -204,6 +204,7 @@ if(MSVC)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index fef45bfca..ec7375d46 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -204,6 +204,7 @@ if(MSVC)
if(NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang")
# multi-core build.
add_compile_flag("/MP")
+ add_compile_flag("/fsanitize=address")
if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "19.0")
# VS2013 and older explicitly need /arch:sse2 set, VS2015 no longer has that option, but always enabled.
add_compile_flag("/arch:sse2")
diff --git a/src/parser/CMakeLists.txt b/src/parser/CMakeLists.txt
index 9c54646e7..86c5f5a52 100644
--- a/src/parser/CMakeLists.txt
+++ b/src/parser/CMakeLists.txt
@@ -1,3 +1,6 @@
+if(MSVC)
+ add_compile_options(/bigobj)
+endif()
FILE(GLOB parser_HEADERS *.h)
set(parser_SOURCES
context-decls.cpp
diff --git a/src/support/utilities.cpp b/src/support/utilities.cpp
index f051a1871..0e80b3ede 100644
--- a/src/support/utilities.cpp
+++ b/src/support/utilities.cpp
@@ -37,7 +37,8 @@ wasm::handle_unreachable(const char* msg, const char* file, unsigned line) {
std::cerr << "!\n";
#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
__sanitizer_print_stack_trace();
- __builtin_trap();
+ //__builtin_trap();
+ __debugbreak();
#endif
#endif
abort();Result : |
mtb0x1
added a commit
to mtb0x1/binaryen
that referenced
this issue
Jun 4, 2024
create a tmp var to store the childiterator to avoid use of free.
|
Interesting, thanks for investigating @mtb0x1 ... Perhaps this is UB due to how the stack works on windows somehow, I'm really not sure. I don't get an error with ASan on Linux on that file with the command from the top comment, at least. But the ASan error line does point to a for loop with a stack allocation, so maybe that is the issue? I'm not an expert on that but I recall the rules are subtle. Does this diff help? diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp
index 295d86b40..434d5a486 100644
--- a/src/passes/Precompute.cpp
+++ b/src/passes/Precompute.cpp
@@ -857,8 +857,11 @@ private:
return &func->body;
}
+ assert(index < stack.size());
auto* child = stack[index];
- for (auto** currChild : ChildIterator(stack[index - 1]).children) {
+ assert(index - 1 < stack.size());
+ auto parentChildren = ChildIterator(stack[index - 1]).children;
+ for (auto** currChild : parentChildren) {
if (*currChild == child) {
return currChild;
} |
|
I submitted a PR before seeing your comment, so yeah I confirm the use after free is on ChildIterator. |
|
Sounds good, thanks for the PR. I guess in C++ the stack allocation really does not live til the end of the loop... mysterious that only on windows does this end up noticeable. |
radekdoulik
pushed a commit
to dotnet/binaryen
that referenced
this issue
Jun 5, 2024
Create a temp var to store the ChildIterator. Fixes WebAssembly#6639
radekdoulik
pushed a commit
to dotnet/binaryen
that referenced
this issue
Jul 12, 2024
Create a temp var to store the ChildIterator. Fixes WebAssembly#6639
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I am facing a bug I can reproduce only on Windows (on Linux no warning and everything seems to be fine).
Binaryen version : 117
Command to reproduce :
Result :
The text was updated successfully, but these errors were encountered: