Add free-is-write functionality (experimental, not enabled by default).

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11627 a5019735-40e9-0310-863c-91ae7b9d1cf9

9 files changed, 112 insertions, 1 deletions

diff --git a/helgrind/docs/hg-manual.xml b/helgrind/docs/hg-manual.xml
index bbb3f562..25c2a69d 100644
--- a/helgrind/docs/hg-manual.xml
+++ b/helgrind/docs/hg-manual.xml
@@ -955,6 +955,28 @@ unlock(mx)                             unlock(mx)
 <!-- start of xi:include in the manpage -->
 <variablelist id="hg.opts.list">
 
+  <varlistentry id="opt.free-is-write"
+                xreflabel="--free-is-write">
+    <term>
+      <option><![CDATA[--free-is-write=no|yes
+      [default: no] ]]></option>
+    </term>
+    <listitem>
+      <para>When enabled (not the default), Helgrind treats freeing of
+        heap memory as if the memory was written immediately before
+        the free.  This exposes races where memory is referenced by
+        one thread, and freed by another, but there is no observable
+        synchronisation event to ensure that the reference happens
+        before the free.
+      </para>
+      <para>This functionality is new in Valgrind 3.7.0, and is
+        regarded as experimental.  It is not enabled by default
+        because its interaction with custom memory allocators is not
+        well understood at present.  User feedback is welcomed.
+      </para>
+    </listitem>
+  </varlistentry>
+
   <varlistentry id="opt.track-lockorders"
                 xreflabel="--track-lockorders">
     <term>
diff --git a/helgrind/hg_basics.c b/helgrind/hg_basics.c
index 7cef39db..172b1dae 100644
--- a/helgrind/hg_basics.c
+++ b/helgrind/hg_basics.c
@@ -80,6 +80,8 @@ UWord HG_(clo_conflict_cache_size) = 1000000;
 
 Word  HG_(clo_sanity_flags) = 0;
 
+Bool  HG_(clo_free_is_write) = False;
+
 
 /*--------------------------------------------------------------------*/
 /*--- end                                              hg_basics.c ---*/
diff --git a/helgrind/hg_basics.h b/helgrind/hg_basics.h
index fc34a39e..edf05e83 100644
--- a/helgrind/hg_basics.h
+++ b/helgrind/hg_basics.h
@@ -103,6 +103,12 @@ extern UWord HG_(clo_conflict_cache_size);
    SCE_{THREADS,LOCKS,BIGRANGE,ACCESS,LAOG}. */
 extern Word HG_(clo_sanity_flags);
 
+/* Treat heap frees as if the memory was written immediately prior to
+   the free.  This shakes out races in which memory is referenced by
+   one thread, and freed by another, and there's no observable
+   synchronisation event to guarantee that the reference happens
+   before the free. */
+extern Bool HG_(clo_free_is_write);
 
 #endif /* ! __HG_BASICS_H */
 
diff --git a/helgrind/hg_main.c b/helgrind/hg_main.c
index a09e18c1..8baffcad 100644
--- a/helgrind/hg_main.c
+++ b/helgrind/hg_main.c
@@ -1705,9 +1705,20 @@ void evh__new_mem_heap ( Addr a, SizeT len, Bool is_inited ) {
 
 static
 void evh__die_mem_heap ( Addr a, SizeT len ) {
+   Thread* thr;
    if (SHOW_EVENTS >= 1)
       VG_(printf)("evh__die_mem_heap(%p, %lu)\n", (void*)a, len );
-   shadow_mem_make_NoAccess( get_current_Thread(), a, len );
+   thr = get_current_Thread();
+   tl_assert(thr);
+   if (HG_(clo_free_is_write)) {
+      /* Treat frees as if the memory was written immediately prior to
+         the free.  This shakes out more races, specifically, cases
+         where memory is referenced by one thread, and freed by
+         another, and there's no observable synchronisation event to
+         guarantee that the reference happens before the free. */
+      shadow_mem_cwrite_range(thr, a, len);
+   }
+   shadow_mem_make_NoAccess( thr, a, len );
    if (len >= SCE_BIGRANGE_T && (HG_(clo_sanity_flags) & SCE_BIGRANGE))
       all__sanity_check("evh__pre_mem_read-post");
 }
@@ -4580,6 +4591,8 @@ static Bool hg_process_cmd_line_option ( Char* arg )
       if (0) VG_(printf)("XXX sanity flags: 0x%lx\n", HG_(clo_sanity_flags));
    }
 
+   else if VG_BOOL_CLO(arg, "--free-is-write",
+                            HG_(clo_free_is_write)) {}
    else 
       return VG_(replacement_malloc_process_cmd_line_option)(arg);
 
@@ -4589,6 +4602,7 @@ static Bool hg_process_cmd_line_option ( Char* arg )
 static void hg_print_usage ( void )
 {
    VG_(printf)(
+"    --free-is-write=no|yes    treat heap frees as writes [no]\n"
 "    --track-lockorders=no|yes show lock ordering errors? [yes]\n"
 "    --history-level=none|approx|full [full]\n"
 "       full:   show both stack traces for a data race (can be very slow)\n"
diff --git a/helgrind/tests/Makefile.am b/helgrind/tests/Makefile.am
index 9fd18c9d..f48293a7 100644
--- a/helgrind/tests/Makefile.am
+++ b/helgrind/tests/Makefile.am
@@ -12,6 +12,8 @@ EXTRA_DIST = \
 		annotate_smart_pointer.stderr.exp \
 	bar_bad.vgtest bar_bad.stdout.exp bar_bad.stderr.exp \
 	bar_trivial.vgtest bar_trivial.stdout.exp bar_trivial.stderr.exp \
+	free_is_write.vgtest free_is_write.stdout.exp \
+		free_is_write.stderr.exp \
 	hg01_all_ok.vgtest hg01_all_ok.stdout.exp hg01_all_ok.stderr.exp \
 	hg02_deadlock.vgtest hg02_deadlock.stdout.exp hg02_deadlock.stderr.exp \
 	hg03_inherit.vgtest hg03_inherit.stdout.exp hg03_inherit.stderr.exp \
@@ -79,6 +81,7 @@ EXTRA_DIST = \
 # should be conditionally compiled like tc20_verifywrap is.
 check_PROGRAMS = \
 	annotate_hbefore \
+	free_is_write \
 	hg01_all_ok \
 	hg02_deadlock \
 	hg03_inherit \
diff --git a/helgrind/tests/free_is_write.c b/helgrind/tests/free_is_write.c
new file mode 100644
index 00000000..0dc53f8d
--- /dev/null
+++ b/helgrind/tests/free_is_write.c
@@ -0,0 +1,42 @@
+
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+#include <unistd.h>
+
+static char* s_mem;
+
+/* wait a second, so as to guarantee that the parent access
+   the malloc'd area, then free it. */
+static void* thread_func(void* arg)
+{
+    sleep(1);
+    free(s_mem);
+    return NULL;
+}
+
+int main(int argc, char** argv)
+{
+    pthread_t tid;
+    int quiet;
+
+    fprintf(stderr, "Start.\n");
+
+    quiet = argc > 1;
+
+    s_mem = malloc(10);
+    if (0 && !quiet)
+        fprintf(stderr, "Pointer to allocated memory: %p\n", s_mem);
+    assert(s_mem);
+    pthread_create(&tid, NULL, thread_func, NULL);
+
+    /* Write, which isn't coordinated with the free ==> a race
+       should be reported. */
+    char c = s_mem[5];
+    __asm__ __volatile__("" : : "r"((long)c) );
+
+    pthread_join(tid, NULL);
+    fprintf(stderr, "Done.\n");
+    return 0;
+}
diff --git a/helgrind/tests/free_is_write.stderr.exp b/helgrind/tests/free_is_write.stderr.exp
new file mode 100644
index 00000000..672915d7
--- /dev/null
+++ b/helgrind/tests/free_is_write.stderr.exp
@@ -0,0 +1,20 @@
+
+Start.
+Thread #x was created
+   ...
+   by 0x........: pthread_create@* (hg_intercepts.c:...)
+   by 0x........: main (free_is_write.c:32)
+
+Thread #x is the program's root thread
+
+Possible data race during write of size 1 at 0x........ by thread #x
+   at 0x........: free (vg_replace_malloc.c:...)
+   by 0x........: thread_func (free_is_write.c:15)
+   by 0x........: mythread_wrapper (hg_intercepts.c:...)
+   ...
+ This conflicts with a previous read of size 1 by thread #x
+   at 0x........: main (free_is_write.c:36)
+
+Done.
+
+ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
diff --git a/helgrind/tests/free_is_write.stdout.exp b/helgrind/tests/free_is_write.stdout.exp
new file mode 100644
index 00000000..e69de29b
--- /dev/null
+++ b/helgrind/tests/free_is_write.stdout.exp
diff --git a/helgrind/tests/free_is_write.vgtest b/helgrind/tests/free_is_write.vgtest
new file mode 100644
index 00000000..5ba9d342
--- /dev/null
+++ b/helgrind/tests/free_is_write.vgtest
@@ -0,0 +1,2 @@
+prog: free_is_write
+vgopts: --free-is-write=yes