-
Notifications
You must be signed in to change notification settings - Fork 122
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
tsan also needs sanitizer nerf for crc64 #122
Comments
Observing a tsan fault depends on the state of your heap (some other previously free'd memory block needs to be in the vestigial space before or after the buffer) |
Perhaps with Clang it could be |
It's not enough to silence the address sanitizer. Also memory and thread sanitizers would need to be silenced. They, at least currently, aren't smart enough to see that the extra bytes are discarded from the xmm registers by later instructions. Valgrind is smarter, possibly because this kind of code isn't weird to write in assembly. Agner Fog's optimizing_assembly.pdf even mentions this idea of doing an aligned read and then discarding the extra bytes. The sanitizers don't instrument assembly code but Valgrind checks all code. It's better to change the implementation to avoid the sanitization attributes which also look scary in the code. (Somehow they can look more scary than __asm__ which is implictly unsanitized.) See also: #112 #122
It's not enough to silence the address sanitizer. Also memory and thread sanitizers would need to be silenced. They, at least currently, aren't smart enough to see that the extra bytes are discarded from the xmm registers by later instructions. Valgrind is smarter, possibly because this kind of code isn't weird to write in assembly. Agner Fog's optimizing_assembly.pdf even mentions this idea of doing an aligned read and then discarding the extra bytes. The sanitizers don't instrument assembly code but Valgrind checks all code. It's better to change the implementation to avoid the sanitization attributes which also look scary in the code. (Somehow they can look more scary than __asm__ which is implictly unsanitized.) See also: #112 #122
It's not enough to silence the address sanitizer. Also memory and thread sanitizers would need to be silenced. They, at least currently, aren't smart enough to see that the extra bytes are discarded from the xmm registers by later instructions. Valgrind is smarter, possibly because this kind of code isn't weird to write in assembly. Agner Fog's optimizing_assembly.pdf even mentions this idea of doing an aligned read and then discarding the extra bytes. The sanitizers don't instrument assembly code but Valgrind checks all code. It's better to change the implementation to avoid the sanitization attributes which also look scary in the code. (Somehow they can look more scary than __asm__ which is implictly unsanitized.) See also: #112 #122
It's faster with both tiny and large buffers and doesn't require disabling any sanitizers. With large buffers the extra speed is from folding four 16-byte chunks in parallel. It is unknown if the MSVC workaround on 32-bit x86 is still needed. I omitted it from this commit. Thanks to Sam James for the feedback. Fixes: #112 Fixes: #122
See #127. |
It's not enough to silence the address sanitizer. Also memory and thread sanitizers would need to be silenced. They, at least currently, aren't smart enough to see that the extra bytes are discarded from the xmm registers by later instructions. Valgrind is smarter, possibly because this kind of code isn't weird to write in assembly. Agner Fog's optimizing_assembly.pdf even mentions this idea of doing an aligned read and then discarding the extra bytes. The sanitizers don't instrument assembly code but Valgrind checks all code. It's better to change the implementation to avoid the sanitization attributes which also look scary in the code. (Somehow they can look more scary than __asm__ which is implictly unsanitized.) See also: #112 #122
It's faster with both tiny and large buffers and doesn't require disabling any sanitizers. With large buffers the extra speed is from folding four 16-byte chunks in parallel. The 32-bit x86 with MSVC reportedly still needs a workaround. Now the simpler "__asm mov ebx, ebx" trick is enough but it needs to be in lzma_crc64() instead of crc64_arch_optimized(). Thanks to Iouri Kharon for testing and the fix. Thanks to Sam James for general feedback. Fixes: #112 Fixes: #122
The
no_sanitize_address
nerf for crc64 also needsno_sanitize_thread
since the tsan checker also looks for heap-use-after-free.The text was updated successfully, but these errors were encountered: