Completely disable signal handler on 32-bit #1488
Conversation
The previous change prevented `WASM_RT_MEMCHECK_SIGNAL_HANDLER_POSIX` from being defined, but `WASM_RT_MEMCHECK_SIGNAL_HANDLER` was still defined, which would prevent the memory bounds check.
wasm2c/wasm-rt.h
Outdated
| @@ -47,6 +47,8 @@ extern "C" { | |||
| #if defined(__linux__) || defined(__unix__) || defined(__APPLE__) | |||
| #if defined(__WORDSIZE) && __WORDSIZE != 64 | |||
| #warning "Signal handler is only supported on 64-bit architectures" | |||
| #undef WASM_RT_MEMCHECK_SIGNAL_HANDLER | |||
| #define WASM_RT_MEMCHECK_SIGNAL_HANDLER 0 | |||
There was a problem hiding this comment.
Perhaps better not to enable it in the first place just above?
There was a problem hiding this comment.
Yeah, but that's only done if it's not already defined, so it could be defined externally too. Though in that case, I guess we'd want to error out instead of warn.
There was a problem hiding this comment.
I think an error would make more sense yeah. undef seems little odd.
Obviously this just a nit.
|
@sbc100 PTAL |
wasm2c/wasm-rt.h
Outdated
| @@ -39,24 +39,32 @@ extern "C" { | |||
| * | |||
| * This is usually 10%-25% faster, but requires OS-specific support. | |||
| * */ | |||
| // #define WASM_RT_MEMCHECK_SIGNAL_HANDLER 1 | |||
There was a problem hiding this comment.
I was using it as documentation of what the flag is. Maybe I could make that more clear?
There was a problem hiding this comment.
Maybe it inside the docstring? without the 1?
Something like Set the default value for WASM_RT_MEMCHECK_SIGNAL_HANDLER which determines ...?
The previous change prevented
WASM_RT_MEMCHECK_SIGNAL_HANDLER_POSIXfrom being defined, but
WASM_RT_MEMCHECK_SIGNAL_HANDLERwas stilldefined, which would prevent the memory bounds check.