Re: [Valgrind-users] helgrind and atomic operations
On Sunday 26 August 2012 01:30:43 David Faure wrote:
On Saturday 25 August 2012 22:43:58 Julian Seward wrote:
Or maybe Qt really is racey
Bingo :)
OK, maybe not. Turns out the code runs as intended by the Qt developers.
Marc Mutz said
The standard says it's racy, but the implementation of
std::atomic::load(memory_order_acquire) won't look different. Simple reads
and writes on x86 are already sequentially consistent. Think MESI cache
coherency. Before a CPU writes to a memory location it needs to acquire
exclusive ownership (E) of the cache line, the well-known hardware mutex on
a cache line that produces False Sharing, too. This seems to hold for all
architectures, cf. threads re: Brief example ... at
http://www.decadent.org.uk/pipermail/cpp-threads/2008-December/thread.html
I attached a pure C++ testcase of the issue. Compile it with
g++ testcase_atomic_ops_helgrind.cpp -o testcase_atomic_ops_helgrind -lpthread
Thiago expects that helgrind can't autodetect this case and that helgrind-
macros markup is needed in Qt, I'm fine with adding that if you guys agree --
after you show me how by modifying the attached example :-)
Thanks.
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. KDE Frameworks 5
#include pthread.h
template typename T struct QAtomicOps // qgenericatomic.h
{
static inline
T loadAcquire(const T _q_value)
{
T tmp = *static_castconst volatile T *(_q_value);
return tmp;
}
static inline
void storeRelease(T _q_value, T newValue)
{
*static_castvolatile T *(_q_value) = newValue;
}
};
class QBasicAtomicInt // qbasicatomic.h
{
public:
typedef QAtomicOpsint Ops;
int _q_value;
int loadAcquire() const { return Ops::loadAcquire(_q_value); }
void storeRelease(int newValue) { Ops::storeRelease(_q_value, newValue); }
};
// Modelled after qt_metatype_id()
static int onDemandNumber() {
static QBasicAtomicInt sharedInt = { 0 };
if (!sharedInt.loadAcquire()) {
// some calculation goes here
sharedInt.storeRelease(42);
}
return sharedInt.loadAcquire();
}
void * threadStart(void *)
{
onDemandNumber();
onDemandNumber();
onDemandNumber();
onDemandNumber();
return 0;
}
int main( int argc, char** argv ) {
pthread_t thread1;
if ( pthread_create(thread1, 0, threadStart, 0) )
return 1;
pthread_t thread2;
if ( pthread_create(thread2, 0, threadStart, 0) )
return 1;
void* v;
pthread_join(thread1, v);
pthread_join(thread2, v);
return 0;
}
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users
Re: [Valgrind-users] helgrind and atomic operations
On Sunday, August 26, 2012, David Faure wrote:
Thiago expects that helgrind can't autodetect this case and that helgrind-
macros markup is needed in Qt, I'm fine with adding that if you guys agree
-- after you show me how by modifying the attached example :-)
IIUC then, QBasicAtomicInt::{loadAcquire,storeRelease} just does normal
loads and stores of an int. Right?
What atomicity properties are you expecting onDemandNumber() to have?
Viz, how is onDemandNumber supposed to behave when you have multiple
threads doing it at the same time on the same location?
J
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users
Re: [Valgrind-users] helgrind and atomic operations
On Sunday 26 August 2012 11:28:06 Julian Seward wrote:
On Sunday, August 26, 2012, David Faure wrote:
Thiago expects that helgrind can't autodetect this case and that helgrind-
macros markup is needed in Qt, I'm fine with adding that if you guys agree
-- after you show me how by modifying the attached example :-)
IIUC then, QBasicAtomicInt::{loadAcquire,storeRelease} just does normal
loads and stores of an int. Right?
Yep (on x86). This whole API exists so that other architectures can get
another implementation.
What atomicity properties are you expecting onDemandNumber() to have?
Viz, how is onDemandNumber supposed to behave when you have multiple
threads doing it at the same time on the same location?
static int onDemandNumber() {
static QBasicAtomicInt sharedInt = { 0 };
if (!sharedInt.loadAcquire()) {
// some calculation goes here
sharedInt.storeRelease(42);
}
return sharedInt.loadAcquire();
}
I think the point is that the first call to loadAcquire() should either return
0 or 42, but never some intermediate value due to a write in progress.
Hmm, I see the point though. This *is* racy by design, since you don't know if
you'll get 0 or 42, if another thread is running at the same time. The only
thing is, we know it's fine to get either one, since we'll simply do the
calculation twice if two threads get 0 at the same time. Overall this is more
performant than using a mutex every single time this is called.
I'm not sure what can be done then, to avoid a helgrind warning.
I presume the only solution is to annotate onDemandNumber() itself, not
QBasicAtomicInt. I.e. annotate all uses of QBasicAtomicInt where the overall
logic makes it safe, in order to still be able to detect unsafe uses...
(like load, increment, store).
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. KDE Frameworks 5
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users
Re: [Valgrind-users] helgrind and atomic operations
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.
If // some calculation goes here touches shared state then you will
have to use VALGRIND_{DISABLE,ENABLE}_ERROR_REPORTING instead. This is
a heavyweight and undesirable solution, though.
--
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.
J
--
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users
