From: Bart Van Assche <bvanassche@acm.org>
Date: Sat, 12 Mar 2011 14:26:01 +0000 (+0000)
Subject: DRD: Restored --free-is-write support and fixed the bug that was present in the previ... 
X-Git-Tag: svn/VALGRIND_3_7_0~593
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=84b87cf1e72f77b5b784d9344b877a771af984f8;p=thirdparty%2Fvalgrind.git

DRD: Restored --free-is-write support and fixed the bug that was present in the previous implementation. Still needs further testing though.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11636
---

diff --git a/drd/docs/drd-manual.xml b/drd/docs/drd-manual.xml
index 21adaa2125..4b0090a4a1 100644
--- a/drd/docs/drd-manual.xml
+++ b/drd/docs/drd-manual.xml
@@ -361,6 +361,42 @@ behavior of the DRD tool itself:</para>
       </para>
     </listitem>
   </varlistentry>
+  <varlistentry>
+    <term>
+      <option>
+        <![CDATA[--free-is-write=<yes|no> [default: no]]]>
+      </option>
+    </term>
+    <listitem>
+      <para>
+        Whether to report races between accessing memory and freeing
+        memory. Enabling this option may cause DRD to run slightly
+        slower. Notes:
+	<itemizedlist>
+	  <listitem>
+	    <para>
+	      Don't enable this option when using custom memory allocators
+	      that use
+	      the <computeroutput>VG_USERREQ__MALLOCLIKE_BLOCK</computeroutput>
+	      and <computeroutput>VG_USERREQ__FREELIKE_BLOCK</computeroutput>
+	      because that would result in false positives.
+	    </para>
+	  </listitem>
+	  <listitem>
+	    <para>Don't enable this option when using reference-counted
+	      objects because that will result in false positives, even when
+	      that code has been annotated properly with
+	      <computeroutput>ANNOTATE_HAPPENS_BEFORE</computeroutput>
+	      and <computeroutput>ANNOTATE_HAPPENS_AFTER</computeroutput>. See
+	      e.g.  the output of the following command for an example:
+	      <computeroutput>valgrind --tool=drd --free-is-write=yes
+		drd/tests/annotate_smart_pointer</computeroutput>.
+	    </para>
+	  </listitem>
+	</itemizedlist>
+      </para>
+    </listitem>
+  </varlistentry>
   <varlistentry>
     <term>
       <option>
diff --git a/drd/drd_main.c b/drd/drd_main.c
index 51fd337378..da021333fc 100644
--- a/drd/drd_main.c
+++ b/drd/drd_main.c
@@ -57,6 +57,7 @@
 
 /* Local variables. */
 
+static Bool s_free_is_write    = False;
 static Bool s_print_stats      = False;
 static Bool s_var_info         = False;
 static Bool s_show_stack_usage = False;
@@ -94,6 +95,7 @@ static Bool DRD_(process_cmd_line_option)(Char* arg)
    if      VG_BOOL_CLO(arg, "--check-stack-var",     check_stack_accesses) {}
    else if VG_BOOL_CLO(arg, "--drd-stats",           s_print_stats) {}
    else if VG_BOOL_CLO(arg, "--first-race-only",     first_race_only) {}
+   else if VG_BOOL_CLO(arg, "--free-is-write",       s_free_is_write) {}
    else if VG_BOOL_CLO(arg,"--report-signal-unlocked",report_signal_unlocked)
    {}
    else if VG_BOOL_CLO(arg, "--segment-merging",     segment_merging) {}
@@ -192,6 +194,8 @@ static void DRD_(print_usage)(void)
 "                              time (in milliseconds) [off].\n"
 "    --first-race-only=yes|no  Only report the first data race that occurs on\n"
 "                              a memory location instead of all races [no].\n"
+"    --free-is-write=yes|no    Whether to report races between freeing memory\n"
+"                              and subsequent accesses of that memory[no].\n"
 "    --report-signal-unlocked=yes|no Whether to report calls to\n"
 "                              pthread_cond_signal() where the mutex associated\n"
 "                              with the signal via pthread_cond_wait() is not\n"
@@ -299,13 +303,18 @@ static __inline__
 void drd_start_using_mem(const Addr a1, const SizeT len,
                          const Bool is_stack_mem)
 {
-   tl_assert(a1 <= a1 + len);
+   const Addr a2 = a1 + len;
+
+   tl_assert(a1 <= a2);
 
    if (!is_stack_mem && s_trace_alloc)
       VG_(message)(Vg_UserMsg, "Started using memory range 0x%lx + %ld%s\n",
                    a1, len, DRD_(running_thread_inside_pthread_create)()
                    ? " (inside pthread_create())" : "");
 
+   if (!is_stack_mem && s_free_is_write)
+      DRD_(thread_stop_using_mem)(a1, a2);
+
    if (UNLIKELY(DRD_(any_address_is_traced)()))
    {
       DRD_(trace_mem_access)(a1, len, eStart);
@@ -313,7 +322,7 @@ void drd_start_using_mem(const Addr a1, const SizeT len,
 
    if (UNLIKELY(DRD_(running_thread_inside_pthread_create)()))
    {
-      DRD_(start_suppression)(a1, a1 + len, "pthread_create()");
+      DRD_(start_suppression)(a1, a2, "pthread_create()");
    }
 }
 
@@ -348,7 +357,10 @@ void drd_stop_using_mem(const Addr a1, const SizeT len,
 
    if (!is_stack_mem || DRD_(get_check_stack_accesses)())
    {
-      DRD_(thread_stop_using_mem)(a1, a2);
+      if (is_stack_mem || !s_free_is_write)
+	 DRD_(thread_stop_using_mem)(a1, a2);
+      else if (s_free_is_write)
+	 DRD_(trace_store)(a1, len);
       DRD_(clientobj_stop_using_mem)(a1, a2);
       DRD_(suppression_stop_using_mem)(a1, a2);
    }
diff --git a/drd/drd_malloc_wrappers.c b/drd/drd_malloc_wrappers.c
index 7b594ffc70..1e6e267594 100644
--- a/drd/drd_malloc_wrappers.c
+++ b/drd/drd_malloc_wrappers.c
@@ -128,7 +128,7 @@ Bool DRD_(freelike_block)(const ThreadId tid, const Addr p, const Bool dealloc)
 
    s_cmalloc_n_frees++;
 
-   mc = VG_(HT_remove)(s_malloc_list, (UWord)p);
+   mc = VG_(HT_lookup)(s_malloc_list, (UWord)p);
    if (mc)
    {
       tl_assert(p == mc->data);
@@ -136,6 +136,7 @@ Bool DRD_(freelike_block)(const ThreadId tid, const Addr p, const Bool dealloc)
 	 VG_(cli_free)((void*)p);
       if (mc->size > 0)
          s_stop_using_mem_callback(mc->data, mc->size);
+      VG_(HT_remove)(s_malloc_list, (UWord)p);
       VG_(free)(mc);
       return True;
    }