search

richgel999 / lzham_codec_devel

LZHAM codec - unstable/experimental repo. Much faster compression and higher ratios in extreme mode. This is well tested but isn't the official version just yet, see lzham_codec instead. Still 100% backwards compatible with lzham v1.0.
MIT License
70 stars 6 forks source link

ThreadSanitizer detects races due to non-atomic loads #1

Open nemequ opened 8 years ago

nemequ commented 8 years ago

ThreadSanitizer freaks out when running lzham, which is unfortunate because it makes it difficult to run software which uses lzham through it. I'd really like to run my unit tests with tsan in CI.

The issue is that lzham uses atomic operations to store some values, but assumes that loads are atomic. IMHO lzham should be using C11-style atomics when available, including atomic_load (or maybe atomic_load_explicit).

stdatomic.h is available with gcc >= 4.9, or there are similar builtins for 4.8. Not sure about clang. I think just checking for stdatomic.h in cmake (and using it if it is available, of course) should be enough, though; no need to deal with all the corner cases.

richgel999 commented 8 years ago

Hi nemequ, Thanks for pointing this out. I can switch the lib to use these macros. I'm going to be porting LZHAM to a lot more platforms very soon, so this sounds like the right thing to do.