Fix GH-9068: Conditional jump or move depends on uninitialised value(s) #10221
Conversation
|
I pushed a new version that uses inline assembly to define the function and calls a wrapper, which should be more robust because we have full control and the compiler cannot mess with our store and restore procedure. |
|
This PR might need an adjustment regarding the platforms where this wrapper is applied to. The code is currently guarded by |
|
Thanks for the review. I applied your comments and I also changed the denylist of the platforms to an allowlist. EDIT: test failure in macOS seems unrelated. |
|
I have been bitten by this a couple of times again. |
…ue(s) This patch preserves the scratch registers of the SysV x86-64 ABI by storing them to the stack and restoring them later. We need to do this to prevent the registers of the caller from being corrupted. The reason these get corrupted is because the compiler is unaware of the Valgrind replacement function and thus makes assumptions about the original function regarding registers which are not true for the replacement function. For implementation I used a GCC and Clang attribute. A more general approach would be to use inline assembly but that's also less portable and quite hacky. This attributes is supported since GCC 7.x, but the target option is only supported since 11.x. For Clang the target option does not matter.
| { | ||
| return !memcmp(ZSTR_VAL(s1), ZSTR_VAL(s2), ZSTR_LEN(s1)); | ||
| } | ||
|
|
||
| #if defined(__GNUC__) && __GNUC__ >= 11 && !defined(__clang__) && __has_attribute(no_caller_saved_registers) |
There was a problem hiding this comment.
nit: Maybe define a macro after push_options and check for this instead of repeating the condition?
Fixes GH-9068
See GH-9068 for analysis.
This patch preserves the scratch registers of the SysV x86-64 ABI by storing them to the stack and restoring them later. We need to do this to prevent the registers of the caller from being corrupted. The reason these get corrupted is because the compiler is unaware of the Valgrind replacement function and thus makes assumptions about the original function regarding registers which are not true for the replacement function.
I considered the alternative of using target function attributes, but that's not supported on GCC<7.0 and on ICC.