Re: [Valgrind-users] helgrind and atomic operations

[David F. <fa...@kd...>]

Hi, I'm finally coming back to this issue.

On Sunday 26 August 2012 12:49:27 Julian Seward wrote:
> > I'm not sure what can be done then, to avoid a helgrind warning.
> 
> If you can guarantee that "// some calculation goes here" touches only
> thread-local state, then there is only a race on sharedInt itself.  In
> which case, use VALGRIND_HG_{DISABLE,ENABLE}_CHECKING to disable reporting
> on the relevant specific instances of the sharedInt.

This seems to be what I need.
        VALGRIND_HG_DISABLE_CHECKING(&_q_value, sizeof(_q_value));
in loadAcquire silences the warning.

Surprisingly, VALGRIND_HG_ENABLE_CHECKING doesn't appear to work, though.
All races are suppressed, even obvious races that warn if disable+enable was 
never used before. Testcase attached, see the call to oops(). In my tests, 
ENABLE_CHECKING basically behaves like DISABLE_CHECKING (for instance if you 
simply put a ENABLE_CHECKING at the beginning of loadAcquire and nothing else, 
then there's no warning at all anymore).


> ----------
> 
> My understanding of this is that it is in violation of the C++11 memory
> model, which says that "the implementation" may deliver stores from one
> core to another in any order, in the absence of any C++11 mandated inter-
> thread synchronisation.
> 
> You can argue that for x86 the hardware's TSO guarantees make this
> harmless ...
> 
> > Marc Mutz said "
> > The standard says it's racy, but the implementation of
> 
> ... but AIUI, "the implementation" also includes the compiler, and I
> believe it has been observed that GCC will indeed break your code in
> unexpected ways, in some cases.  In short you need to prove that not
> only the hardware won't reorder stores between cores -- which for x86
> happens to be true -- but also the compiler won't.

Yes but AFAIU that's what the "volatile" does -- prevent the compiler from 
reordering.

However valgrind can't possibly find out that "volatile" was used, if all that 
does is disable compiler optimizations, so I agree that this cannot all work 
out of the box, valgrind-specific markup is definitely needed in the Qt atomics 
class. Current version of my patch attached -- no re-ENABLE for now, since it 
doesn't work anyway ;)

-- 
David Faure, fa...@kd..., http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5