From: Julian Seward Date: Fri, 11 Mar 2011 18:38:12 +0000 (+0000) Subject: Change the semantics of ANNOTATE_HAPPENS_BEFORE from 'overwrite' to X-Git-Tag: svn/VALGRIND_3_7_0~605 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3cc6fcac8c47289f0c39dc673f5ad43b26b5546f;p=thirdparty%2Fvalgrind.git Change the semantics of ANNOTATE_HAPPENS_BEFORE from 'overwrite' to 'add' behaviour, w.r.t. any h-b edges associated with the synchronisation object prior to the call. This brings the behaviour into line with DRD and TSan, and is required for correct annotation of thread safe reference counting. It fixes #243935 -- at least, the original bug as discussed in comments 0 and 2. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11624 --- diff --git a/helgrind/helgrind.h b/helgrind/helgrind.h index 2c30a5aa56..23eb4b70e6 100644 --- a/helgrind/helgrind.h +++ b/helgrind/helgrind.h @@ -433,13 +433,15 @@ typedef /* ---------------------------------------------------------------- Create completely arbitrary happens-before edges between threads. - If thread T1 does ANNOTATE_HAPPENS_BEFORE(obj) and later (w.r.t. - some notional global clock for the computation) thread T2 does - ANNOTATE_HAPPENS_AFTER(obj), then Helgrind will regard all memory - accesses done by T1 before the ..BEFORE.. call as happening-before - all memory accesses done by T2 after the ..AFTER.. call. Hence - Helgrind won't complain about races if T2's accesses afterwards are - to the same locations as T1's accesses before. + + If threads T1 .. Tn all do ANNOTATE_HAPPENS_BEFORE(obj) and later + (w.r.t. some notional global clock for the computation) thread Tm + does ANNOTATE_HAPPENS_AFTER(obj), then Helgrind will regard all + memory accesses done by T1 .. Tn before the ..BEFORE.. call as + happening-before all memory accesses done by Tm after the + ..AFTER.. call. Hence Helgrind won't complain about races if Tm's + accesses afterwards are to the same locations as accesses before by + any of T1 .. Tn. OBJ is a machine word (unsigned long, or void*), is completely arbitrary, and denotes the identity of some synchronisation object @@ -453,6 +455,9 @@ typedef take the time to understand these two. They form the very essence of describing arbitrary inter-thread synchronisation events to Helgrind. You can get a long way just with them alone. + + See also, extensive discussion on semantics of this in + https://bugs.kde.org/show_bug.cgi?id=243935 ---------------------------------------------------------------- */ #define ANNOTATE_HAPPENS_BEFORE(obj) \ diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c index 9f2d503de7..74f78fb575 100644 --- a/helgrind/hg_main.c +++ b/helgrind/hg_main.c @@ -3040,9 +3040,12 @@ void evh__HG_USERSO_SEND_PRE ( ThreadId tid, UWord usertag ) /* TID is just about to notionally sent a message on a notional abstract synchronisation object whose identity is given by USERTAG. Bind USERTAG to a real SO if it is not already so - bound, and do a 'strong send' on the SO. This is later used by + bound, and do a 'weak send' on the SO. This joins the vector + clocks from this thread into any vector clocks already present + in the SO. The resulting SO vector clocks are later used by other thread(s) which successfully 'receive' from the SO, - thereby acquiring a dependency on this signalling event. */ + thereby acquiring a dependency on all the events that have + previously signalled on this SO. */ Thread* thr; SO* so; @@ -3056,7 +3059,7 @@ void evh__HG_USERSO_SEND_PRE ( ThreadId tid, UWord usertag ) so = map_usertag_to_SO_lookup_or_alloc( usertag ); tl_assert(so); - libhb_so_send( thr->hbthr, so, True/*strong_send*/ ); + libhb_so_send( thr->hbthr, so, False/*!strong_send*/ ); } static