Author: sewardj
Date: 2007-10-02 09:10:04 +0100 (Tue, 02 Oct 2007)
New Revision: 6929

Log:
Rewrite happens_before_do_dfs_from_to to use an explicit stack rather
than recursion.  This fixes sporadic segfaults resulting from stack
overflow when running Qt4 tests examples/threads/semaphores and
examples/threads/waitconditions.

Modified:
   branches/THRCHECK/thrcheck/tc_main.c


Modified: branches/THRCHECK/thrcheck/tc_main.c
===================================================================
--- branches/THRCHECK/thrcheck/tc_main.c        2007-10-01 10:33:41 UTC (rev 
6928)
+++ branches/THRCHECK/thrcheck/tc_main.c        2007-10-02 08:10:04 UTC (rev 
6929)
@@ -46,6 +46,7 @@
 #include "pub_tool_replacemalloc.h"
 #include "pub_tool_machine.h"
 #include "pub_tool_options.h"
+#include "pub_tool_xarray.h"
 
 #include "thrcheck.h"
 
@@ -1231,62 +1232,97 @@
 static UWord stats__hbefore_gsearches = 0; // # searches in graph
 static UWord stats__hbefore_gsearchFs = 0; // # fast searches in graph
 static UWord stats__hbefore_invals    = 0; // # cache invals
+static UWord stats__hbefore_stk_hwm   = 0; // stack high water mark
 
 /* Running marker for depth-first searches */
 /* NOTE: global variable */
 static UInt dfsver_current = 0;
 
+/* A stack of possibly-unexplored nodes used in the depth first search */
+/* NOTE: global variable */
+static XArray* dfsver_stack = NULL;
+
 // FIXME: check this - is it really correct?
-static Bool happens_before_do_dfs_from_to ( Segment* here, Segment* dst )
+static Bool happens_before_do_dfs_from_to ( Segment* src, Segment* dst )
 {
-   /* begin SPEEDUP HACK */
+   Segment* here;
+   Word     ssz;
+
+   /* begin SPEEDUP HACK -- the following can safely be omitted */
    /* fast track common case, without favouring either the
       ->prev or ->other links */
-   tl_assert(here);
+   tl_assert(src);
    tl_assert(dst);
-   if ((here->prev && here->prev == dst)
-       || (here->other && here->other == dst)) {
+   if ((src->prev && src->prev == dst)
+       || (src->other && src->other == dst)) {
       stats__hbefore_gsearchFs++;
       return True;
    }
    /* end SPEEDUP HACK */
 
-  again:
-   tl_assert(here);
-   tl_assert(dst);
-   if (here == dst)
-      return True;
-   tl_assert(here->dfsver <= dfsver_current);
-   if (here->dfsver == dfsver_current)
-      return False; /* We've been here before */
-   /* Mark that we've been here */
-   here->dfsver = dfsver_current;
-   /* See if we can get to 'dst' via either of our two links */
-   /* Avoiding recursion if possible */
-   /* begin SPEEDUP hack -- the following can safely be omitted */
-   if (here->prev && !here->other) {
-      here = here->prev;
-      goto again;
+   /* empty out the stack */
+   tl_assert(dfsver_stack);
+   VG_(dropTailXA)( dfsver_stack, VG_(sizeXA)( dfsver_stack ));
+   tl_assert(VG_(sizeXA)( dfsver_stack ) == 0);
+
+   /* push starting point */
+   (void) VG_(addToXA)( dfsver_stack, &src );
+
+   while (True) {
+      /* While the stack is not empty, pop the next node off it and
+         consider. */
+      ssz = VG_(sizeXA)( dfsver_stack );
+      tl_assert(ssz >= 0);
+      if (ssz == 0)
+         return False; /* stack empty ==> no path from src to dst */
+
+      if (UNLIKELY( ((UWord)ssz) > stats__hbefore_stk_hwm ))
+         stats__hbefore_stk_hwm = (UWord)ssz;
+
+      /* here = pop(stack) */
+      here = *(Segment**) VG_(indexXA)( dfsver_stack, ssz-1 );
+      VG_(dropTailXA)( dfsver_stack, 1 );
+
+     again:
+      /* consider the node 'here' */
+      if (here == dst)
+         return True; /* found a path from src and dst */
+
+      /* have we been to 'here' before? */
+      tl_assert(here->dfsver <= dfsver_current);
+      if (here->dfsver == dfsver_current)
+         continue; /* We've been 'here' before - node is not interesting*/
+
+      /* Mark that we've been here */
+      here->dfsver = dfsver_current;
+
+      /* Now push both children on the stack */
+
+      /* begin SPEEDUP hack -- the following can safely be omitted */
+      /* idea is, if there is exactly one child, avoid the overhead of
+         pushing it on the stack and immediately popping it off again.
+         Kinda like doing a tail-call. */
+      if (here->prev && !here->other) {
+         here = here->prev;
+         goto again;
+      }
+      if (here->other && !here->prev) {
+         here = here->other;
+         goto again;
+      }
+      /* end of SPEEDUP HACK */
+
+      /* Push all available children on stack.  From some quick
+         experimentation it seems like exploring ->other first leads
+         to lower maximum stack use, although getting repeatable
+         results is difficult. */
+      if (here->prev)
+         (void) VG_(addToXA)( dfsver_stack, &(here->prev) );
+      if (here->other)
+         (void) VG_(addToXA)( dfsver_stack, &(here->other) );
    }
-   if (here->other && !here->prev) {
-      here = here->other;
-      goto again;
-   }
-   /* end of SPEEDUP HACK */
-   /* GENERAL CASE -- recurse */
-   if (here->other) {
-      if (happens_before_do_dfs_from_to(here->other, dst))
-         return True;
-   }
-   if (here->prev) {
-      if (happens_before_do_dfs_from_to(here->prev, dst))
-         return True;
-   }
-   return False;
 }
 
-// FIXME: cache the results of the last few queries and hand
-// them out as appropriate
 static Bool happens_before_wrk ( SegmentID segid1, SegmentID segid2 )
 {
    Bool    reachable;
@@ -1309,6 +1345,12 @@
       .prev and .other fields, that leads from seg2 back to seg1 ? */
    tl_assert(dfsver_current < 0xFFFFFFFF);
    dfsver_current++;
+   
+   if (dfsver_stack == NULL) {
+     dfsver_stack = VG_(newXA)( tc_zalloc, tc_free, sizeof(Segment*) );
+     tl_assert(dfsver_stack);
+  }
+
    reachable = happens_before_do_dfs_from_to( seg2, seg1 );
 
    if (0)
@@ -5298,8 +5340,11 @@
       VG_(printf)(" hbefore: %10lu graph searches\n", 
stats__hbefore_gsearches);
       VG_(printf)(" hbefore: %10lu   of which slow\n",
                   stats__hbefore_gsearches - stats__hbefore_gsearchFs);
+      VG_(printf)(" hbefore: %10lu stack high water mark\n",
+                  stats__hbefore_stk_hwm);
       VG_(printf)(" hbefore: %10lu cache invals\n",   stats__hbefore_invals);
       VG_(printf)(" hbefore: %10lu probes\n",         stats__hbefore_probes);
+
       VG_(printf)("\n");
       VG_(printf)("segments:       %10lu Segment objects allocated\n", 
                   stats__mk_Segment);


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Valgrind-developers mailing list
Valgrind-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-developers

Reply via email to