On Tue, Nov 27, 2012 at 12:23 PM, Jakub Jelinek <ja...@redhat.com> wrote: > On Tue, Nov 27, 2012 at 12:13:42PM +0400, Dmitry Vyukov wrote: >> I've added 128-bit atomic ops: >> http://llvm.org/viewvc/llvm-project?view=rev&revision=168683 > > Thanks. >> >> Refactored atomic ops so that the atomic operation itself is done >> under the mutex: >> http://llvm.org/viewvc/llvm-project?view=rev&revision=168682 > > This is IMHO very wrong. It is fine if you hold some mutex over the atomic > operation, but you do need to perform the operation atomicly, e.g. you can't > rely that all atomic accesses to that memory location are performed from > -fsanitize=thread compiled code (it could be from non-instrumented code > from e.g. other libraries, could be from user written assembly, etc.). > By not doing the operation atomicly, the other code might observe > inconsistent state, or the tsan non-atomic code might be doing the wrong > thing. One thing is that the tsan analysis will be right, but another > thing is that the program itself might misbehave.
Yes, you are right. I think I've done them atomically initially because of things like FUTEX_WAKE_OP. I will fix that. >> And added atomic nand operation: >> http://llvm.org/viewvc/llvm-project?view=rev&revision=168584 > > Thanks. Can you please also add the memory range read/write functions? > That could be used both for builtins (e.g. if user writes > __builtin_memcpy (&a, &b, sizeof (a)); > in his code, no -fno-builtin or similar actually disables that), compiler > generated builtins - e.g. structure copies > struct large a, b; > ... > a = b; > and also for accesses of sizes that aren't supported by the library > (consider 32-byte or 64-byte vectors etc.). Done: http://llvm.org/viewvc/llvm-project?view=rev&revision=168692
