Merge branch 'fix_race_condition' into 'derivgrind'

Fix a race condition related to an internal state of Derivgrind, 
by which certain Derivgrind actions can be temporarily disabled.
This internal state is now stored separately for each thread of
the client program. 

See merge request aehle/valgrind!27

derivgrind/bar/dg_bar_tape.c

@@ -61,7 +61,7 @@ static Int fd_tape;
 static Int fd_values;
 static VgFile *fp_inputs, *fp_outputs;
 
-extern Long dg_disable;
+extern Long* dg_disable;
 extern Bool typegrind;
 extern Bool bar_record_values;
 extern Bool tape_in_ram;
@@ -75,7 +75,7 @@ ULong tapeAddStatement(ULong index1,ULong index2,double diff1,double diff2){
 }
 
 ULong tapeAddStatement_noActivityAnalysis(ULong index1,ULong index2,double diff1,double diff2){
-  if(dg_disable!=0) return typegrind ? 0xffffffffffffffff : 0;
+  if(dg_disable[VG_(get_running_tid)()]!=0) return typegrind ? 0xffffffffffffffff : 0;
   ULong pos = (nextindex%BUFSIZE);
   buffer_tape[4*pos] = index1;
   buffer_tape[4*pos+1] = index2;

derivgrind/derivgrind.h

@@ -105,13 +105,13 @@ typedef enum {
 #define DERIVGRIND_SET_DOTVALUE(_qzz_addr,_qzz_daddr,_qzz_size) DG_SET_DOTVALUE(_qzz_addr,_qzz_daddr,_qzz_size)
 #define VALGRIND_SET_DERIVATIVE(_qzz_addr,_qzz_daddr,_qzz_size) DG_SET_DOTVALUE(_qzz_addr,_qzz_daddr,_qzz_size)
 
-/* Disable Derivgrind on specific code sections by putting the section
- * into a DG_DISABLE(1,0) ... DG_DISABLE(0,1) bracket. Used by the math function
- * wrappers.
+/* Disable certain Derivgrind actions on specific sections of user code
+ * by putting the section into a DG_DISABLE(1,0) ... DG_DISABLE(0,1) bracket.
  *
  * The macro DG_DISABLE(plus,minus) will add plus and subtract minus from
  * a Derivgrind-internal counter dg_disable. When the counter is non-zero,
- * certain actions are disabled.
+ * the following Derivgrind actions are disabled. The macro evaluates to
+ * the previous content of dg_disable.
  *
  * In forward mode, this will enable/disable outputting of values and
  * dot values for difference quotient debugging, but dot values will still
@@ -121,6 +121,12 @@ typedef enum {
  * will assign the index 0 or 0xff..f (if --typegrind=yes) instead of the
  * block index and not write to the tape. Also, typgrind warnings about an
  * index 0xff..f will be suppressed.
+ *
+ * In bit-trick-finding mode, warning messages will be suppressed.
+ *
+ * We use this client request in the math replacement wrappers, because we
+ * do not want to see Derivgrind messages for, and recordings of, the
+ * original math library code and the derivative computation.
  */
 #define DG_DISABLE(_qzz_plus,_qzz_minus) \
     VALGRIND_DO_CLIENT_REQUEST_EXPR(0 /* default return */,      \

derivgrind/dg_main.c

@@ -35,6 +35,7 @@
 #include "pub_tool_mallocfree.h"
 #include "pub_tool_libcassert.h"
 #include "pub_tool_gdbserver.h"
+#include "pub_tool_threadstate.h"
 #include "pub_tool_libcbase.h"
 #include "pub_tool_options.h"
 #include "valgrind.h"
@@ -81,9 +82,12 @@ Bool warn_about_unwrapped_expressions = False;
 Bool diffquotdebug = False;
 const HChar* diffquotdebug_directory = NULL;
 
-/*! If nonzero, do not print difference quotient debugging information.
+/*! If dg_disable[threadID] is non-zero, certain Derivgrind actions
+ *  are disabled for the thread.
+ *
+ *  See DG_DISABLE in derivgrind.h.
  */
-Long dg_disable = 0;
+Long* dg_disable;
 
 /*! Mode: d=dot/forward, b=bar/reverse/recording
  */
@@ -143,6 +147,11 @@ static void dg_post_clo_init(void)
     VG_(free)(recording_stop_indices_str_copy);
   }
 
+  dg_disable = VG_(malloc)("dg-disable",(VG_N_THREADS+1)*sizeof(Long));
+  for(UInt i=0; i<VG_N_THREADS+1; i++){
+    dg_disable[i] = 0;
+  }
+
   if(mode=='d'){
     dg_dot_initialize();
   } else if (mode=='b') {
@@ -374,8 +383,9 @@ Bool dg_handle_client_request(ThreadId tid, UWord* arg, UWord* ret){
     dg_dot_shadowSet(addr,daddr,size);
     *ret = 1; return True;
   } else if(arg[0]==VG_USERREQ__DISABLE) {
-    dg_disable += (Long)(arg[1]) - (Long)(arg[2]);
-    *ret = 1; return True;
+    *ret = dg_disable[tid]; // return previous value
+    dg_disable[tid] += (Long)(arg[1]) - (Long)(arg[2]);
+    return True;
   } else if(arg[0]==VG_USERREQ__GET_INDEX) {
     if(mode!='b') return True;
     void* addr = (void*) arg[1];

derivgrind/dot/dg_dot_diffquotdebug.c

@@ -1,6 +1,6 @@
 #include "dot/dg_dot_diffquotdebug.h"
 
-extern Bool dg_disable;
+extern Long* dg_disable;
 
 //! Next index in the buffers to be written to
 static ULong dg_dot_nextindex = 0;
@@ -64,7 +64,7 @@ void dg_dot_diffquotdebug_finalize(void){
 
 
 static VG_REGPARM(0) void dg_add_diffquotdebug_helper(ULong value, ULong dotvalue){
-  if(dg_disable==0){
+  if(dg_disable[VG_(get_running_tid)()]==0){
     dg_dot_buffer_val[dg_dot_nextindex] = value;
     dg_dot_buffer_dot[dg_dot_nextindex] = dotvalue;
     dg_dot_nextindex++;

derivgrind/gen_replace_math.py

@@ -97,26 +97,22 @@ def c_code(self):
 {self.type} I_WRAP_SONAME_FNNAME_ZU(libmZdsoZa, {self.name}) ({self.type} x) {{
   OrigFn fn;
   VALGRIND_GET_ORIG_FN(fn);
-  DG_DISABLE(1,0);
+  bool already_disabled = DG_DISABLE(1,0)!=0;
   {self.type} ret;
   CALL_FN_{self.T}_{self.T}(ret, fn, x);
   double ret_d = ret;
-  if(!called_from_within_wrapper) {{
+  if(!already_disabled) {{
     if(DG_GET_MODE=='d'){{ /* forward mode */
       {self.type} x_d;
       DG_GET_DOTVALUE(&x, &x_d, {self.size});
-      called_from_within_wrapper = true;
-        {self.type} ret_d = ({self.deriv}) * x_d;
-      called_from_within_wrapper = false;
+      {self.type} ret_d = ({self.deriv}) * x_d;
       DG_SET_DOTVALUE(&ret, &ret_d, {self.size});
       DG_DISABLE(0,1);
     }} else if(DG_GET_MODE=='b') {{ /* recording mode */
       unsigned long long x_i, y_i=0;
       DG_GET_INDEX(&x, &x_i);
       double x_pdiff, y_pdiff=0.;
-      called_from_within_wrapper = true;
-        x_pdiff = ({self.deriv});
-      called_from_within_wrapper = false;
+      x_pdiff = ({self.deriv});
       unsigned long long ret_i;
       DG_DISABLE(0,1);
       DG_NEW_INDEX(&x_i,&y_i,&x_pdiff,&y_pdiff,&ret_i,&ret_d);
@@ -151,29 +147,25 @@ def c_code(self):
 {self.type} I_WRAP_SONAME_FNNAME_ZU(libmZdsoZa, {self.name}) ({self.type} x, {self.type} y) {{
   OrigFn fn;
   VALGRIND_GET_ORIG_FN(fn);
-  DG_DISABLE(1,0);
+  bool already_disabled = DG_DISABLE(1,0);
   {self.type} ret;
   CALL_FN_{self.T}_{self.T}{self.T}(ret, fn, x, y);
   double ret_d = ret;
-  if(!called_from_within_wrapper) {{
+  if(!already_disabled) {{
     if(DG_GET_MODE=='d'){{ /* forward mode */
       {self.type} x_d, y_d;
       DG_GET_DOTVALUE(&x, &x_d, {self.size});
       DG_GET_DOTVALUE(&y, &y_d, {self.size});
-      called_from_within_wrapper = true;
-        {self.type} ret_d = ({self.derivX}) * x_d + ({self.derivY}) * y_d;
-      called_from_within_wrapper = false;
+      {self.type} ret_d = ({self.derivX}) * x_d + ({self.derivY}) * y_d;
       DG_SET_DOTVALUE(&ret, &ret_d, {self.size});
       DG_DISABLE(0,1);
     }} else if(DG_GET_MODE=='b') {{ /* recording mode */
       unsigned long long x_i, y_i;
       DG_GET_INDEX(&x,&x_i);
       DG_GET_INDEX(&y,&y_i);
       double x_pdiff, y_pdiff;
-      called_from_within_wrapper = true;
-        x_pdiff = ({self.derivX});
-        y_pdiff = ({self.derivY});
-      called_from_within_wrapper = false;
+      x_pdiff = ({self.derivX});
+      y_pdiff = ({self.derivY});
       unsigned long long ret_i;
       DG_DISABLE(0,1);
       DG_NEW_INDEX(&x_i,&y_i,&x_pdiff,&y_pdiff,&ret_i,&ret_d);
@@ -212,26 +204,22 @@ def c_code(self):
 {self.type} I_WRAP_SONAME_FNNAME_ZU(libmZdsoZa, {self.name}) ({self.type} x, {self.extratype} e) {{
   OrigFn fn;
   VALGRIND_GET_ORIG_FN(fn);
-  DG_DISABLE(1,0);
+  bool already_disabled = DG_DISABLE(1,0);
   {self.type} ret;
   CALL_FN_{self.T}_{self.T}{self.extratypeletter}(ret, fn, x, e);
   double ret_d = ret;
-  if(!called_from_within_wrapper) {{
+  if(!already_disabled) {{
     if(DG_GET_MODE=='d'){{ /* forward mode */
       {self.type} x_d;
       DG_GET_DOTVALUE(&x, &x_d, {self.size});
-      called_from_within_wrapper = true;
-        {self.type} ret_d = ({self.deriv}) * x_d;
-      called_from_within_wrapper = false;
+      {self.type} ret_d = ({self.deriv}) * x_d;
       DG_SET_DOTVALUE(&ret, &ret_d, {self.size});
       DG_DISABLE(0,1);
     }} else if(DG_GET_MODE=='b') {{ /* recording mode */
       unsigned long long x_i, y_i=0;
       DG_GET_INDEX(&x, &x_i);
       double x_pdiff, y_pdiff=0.;
-      called_from_within_wrapper = true;
-        x_pdiff = ({self.deriv});
-      called_from_within_wrapper = false;
+      x_pdiff = ({self.deriv});
       unsigned long long ret_i;
       DG_DISABLE(0,1);
       DG_NEW_INDEX(&x_i,&y_i,&x_pdiff,&y_pdiff,&ret_i,&ret_d);
@@ -397,7 +385,6 @@ def c_code(self):
   }
 }
 
-static bool called_from_within_wrapper = false;
 """)
   for function in functions:
     f.write(function.c_code())

derivgrind/trick/dg_trick.c

@@ -53,7 +53,7 @@
 #include "dg_trick_bitwise.h"
 #include "dg_utils.h"
 
-extern Long dg_disable;
+extern Long* dg_disable;
 
 //! Data is copied to/from shadow memory via this buffer of 2x V256.
 V256* dg_trick_shadow_mem_buffer;
@@ -141,7 +141,7 @@ static void dg_trick_dirty_loadF80le(DiffEnv* diffenv, IRExpr* addr, IRTemp temp
 
 ULong dg_trick_warn_dirtyhelper( ULong fLo, ULong fHi, ULong size ){
   ULong mask = (size==4) ? 0x00000000fffffffful : 0xfffffffffffffffful;
-  if((dg_disable==0) && (fLo & fHi & mask)){
+  if((dg_disable[VG_(get_running_tid)()]==0) && (fLo & fHi & mask)){
     VG_(message)(Vg_UserMsg, "Active discrete data used as floating-point operand.\n");
     VG_(message)(Vg_UserMsg, "Activity bits: %llu. Discreteness bits: %llu.\n", fLo, fHi);
     VG_(message)(Vg_UserMsg, "At\n");