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 2c30a5aa569a973c44d25f91ca811990ef06a627..23eb4b70e62403f098857226a275ea888b7c2ae1 100644 (file)
--- 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 9f2d503de7c998b66745e4ace3844f16f02d7a5c..74f78fb575079cb5959635e44891809aa646e2cb 100644 (file)
--- 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