In mps_arena_roots_walk, don't call traceaddwhite to whiten a segment, as this calls segwhiten which has undesirable side-effects, including breaking formatting objects walking. instead, call segsetwhite to make the segment white, and set the trace's white summary to the universe.

Add cross-reference from second-stage test in _mps_fix2 so that if the white set test is changed, the roots walking code can be changed to match.
In walkt0, add a smoke test for mps_arena_roots_walk.

Copied from Perforce
 Change: 194507

mps/code/trace.c

@@ -1330,7 +1330,9 @@ mps_res_t _mps_fix2(mps_ss_t mps_ss, mps_addr_t *mps_ref_io)
   tract = PageTract(&chunk->pageTable[i]);
   if (TraceSetInter(TractWhite(tract), ss->traces) == TraceSetEMPTY) {
     /* Reference points to a tract that is not white for any of the
-     * active traces. See <design/trace/#fix.tractofaddr> */
+       active traces. See <design/trace/#fix.tractofaddr>.
+       See also <walk.c#roots-walk.second-stage> for where we arrange
+       to fool this test when walking references in the roots. */
     STATISTIC({
       if (TRACT_SEG(&seg, tract)) {
         ++ss->segRefCount;

mps/code/walk.c

@@ -229,6 +229,7 @@ static Res RootsWalkFix(Seg seg, ScanState ss, Ref *refIO)
   rootsStepClosure rsc;
   Ref ref;
 
+  AVERT(Seg, seg);
   AVERT(ScanState, ss);
   AVER(refIO != NULL);
   rsc = ScanState2rootsStepClosure(ss);
@@ -236,10 +237,6 @@ static Res RootsWalkFix(Seg seg, ScanState ss, Ref *refIO)
 
   ref = *refIO;
 
-  /* If the segment isn't GCable then the ref is not to the heap and */
-  /* shouldn't be passed to the client. */
-  AVER(PoolHasAttr(SegPool(seg), AttrGC));
-
   /* Call the client closure - .assume.rootaddr */
   rsc->f((mps_addr_t*)refIO, (mps_root_t)rsc->root, rsc->p, rsc->s);
 
@@ -312,15 +309,19 @@ static Res ArenaRootsWalk(Globals arenaGlobals, mps_roots_stepper_t f,
   if (res != ResOK)
     return res;
 
-  /* ArenaRootsWalk only passes references to GCable pools to the client. */
-  /* NOTE: I'm not sure why this is. RB 2012-07-24 */
+  /* .roots-walk.first-stage: In order to fool MPS_FIX12 into calling
+     _mps_fix2 for a reference in a root, the reference must pass the
+     first-stage test (against the summary of the trace's white
+     set), so make the summary universal. */
+  trace->white = ZoneSetUNIV;
+
+  /* .roots-walk.second-stage: In order to fool _mps_fix2 into calling
+     our fix function (RootsWalkFix), the reference must be to a
+     segment that is white for the trace, so make all segments white
+     for the trace. */
   if (SegFirst(&seg, arena)) {
     do {
-      if (PoolHasAttr(SegPool(seg), AttrGC)) {
-        res = TraceAddWhite(trace, seg);
-        if (res != ResOK)
-          goto failBegin;
-      }
+      SegSetWhite(seg, TraceSetAdd(SegWhite(seg), trace));
     } while (SegNext(&seg, arena, seg));
   }
 
@@ -340,14 +341,10 @@ static Res ArenaRootsWalk(Globals arenaGlobals, mps_roots_stepper_t f,
       break;
   }
 
-failBegin:
   /* Turn segments black again. */
   if (SegFirst(&seg, arena)) {
     do {
-      if (PoolHasAttr(SegPool(seg), AttrGC)) {
-        SegSetGrey(seg, TraceSetDel(SegGrey(seg), trace));
-        SegSetWhite(seg, TraceSetDel(SegWhite(seg), trace));
-      }
+      SegSetWhite(seg, TraceSetDel(SegWhite(seg), trace));
     } while (SegNext(&seg, arena, seg));
   }
 

mps/code/walkt0.c

@@ -16,7 +16,6 @@
 #include "mpsclo.h"
 #include "mpscsnc.h"
 #include "mpsavm.h"
-#include "mpstd.h"
 #include "mps.h"
 #include "mpm.h"
 
@@ -77,7 +76,8 @@ static mps_addr_t make(void)
     return p;
 }
 
-/* A stepper function.  Passed to mps_arena_formatted_objects_walk.
+/* A formatted objects stepper function. Passed to
+ * mps_arena_formatted_objects_walk.
  *
  * Tests the (pool, format) values that MPS passes to it for each 
  * object, by...
@@ -93,19 +93,19 @@ static mps_addr_t make(void)
  *
  * ...3: accumulating the count and size of objects found
  */
-struct stepper_data {
+typedef struct object_stepper_data {
   mps_arena_t arena;
   mps_pool_t expect_pool;
   mps_fmt_t expect_fmt;
   size_t count;                 /* number of non-padding objects found */
   size_t objSize;               /* total size of non-padding objects */
   size_t padSize;               /* total size of padding objects */
-};
+} object_stepper_data_s, *object_stepper_data_t;
 
-static void stepper(mps_addr_t object, mps_fmt_t format,
-    mps_pool_t pool, void *p, size_t s)
+static void object_stepper(mps_addr_t object, mps_fmt_t format,
+                           mps_pool_t pool, void *p, size_t s)
 {
-    struct stepper_data *sd;
+    object_stepper_data_t sd;
     mps_arena_t arena;
     mps_bool_t b;
     mps_pool_t query_pool;
@@ -137,6 +137,25 @@ static void stepper(mps_addr_t object, mps_fmt_t format,
     }      
 }
 
+
+/* A roots stepper function. Passed to mps_arena_roots_walk. */
+
+typedef struct roots_stepper_data {
+  mps_root_t exactRoot;
+  size_t count;
+} roots_stepper_data_s, *roots_stepper_data_t;
+
+static void roots_stepper(mps_addr_t *ref, mps_root_t root, void *p, size_t s)
+{
+  roots_stepper_data_t data = p;
+  Insist(ref != NULL);
+  Insist(p != NULL);
+  Insist(s == sizeof *data);
+  Insist(root == data->exactRoot);
+  ++ data->count;
+}
+
+
 /* test -- the body of the test */
 
 static void test(mps_arena_t arena, mps_pool_class_t pool_class)
@@ -148,8 +167,8 @@ static void test(mps_arena_t arena, mps_pool_class_t pool_class)
     size_t i;
     size_t totalSize, freeSize, allocSize, bufferSize;
     unsigned long objs;
-    struct stepper_data sdStruct, *sd;
-    PoolClass class;
+    object_stepper_data_s objectStepperData, *sd;
+    roots_stepper_data_s rootsStepperData, *rsd;
 
     die(dylan_fmt(&format, arena), "fmt_create");
     die(mps_chain_create(&chain, arena, genCOUNT, testChain), "chain_create");
@@ -191,23 +210,29 @@ static void test(mps_arena_t arena, mps_pool_class_t pool_class)
 
     mps_arena_park(arena);
 
-    sd = &sdStruct;
+    rsd = &rootsStepperData;
+    rsd->exactRoot = exactRoot;
+    rsd->count = 0;
+    mps_arena_roots_walk(arena, roots_stepper, rsd, sizeof *rsd);
+    printf("%lu %lu\n", (unsigned long)rsd->count, (unsigned long)exactRootsCOUNT);
+    Insist(rsd->count == exactRootsCOUNT);
+
+    sd = &objectStepperData;
     sd->arena = arena;
     sd->expect_pool = pool;
     sd->expect_fmt = format;
     sd->count = 0;
     sd->objSize = 0;
     sd->padSize = 0;
-    mps_arena_formatted_objects_walk(arena, stepper, sd, sizeof *sd);
+    mps_arena_formatted_objects_walk(arena, object_stepper, sd, sizeof *sd);
     Insist(sd->count == objs);
 
     totalSize = mps_pool_total_size(pool);
     freeSize = mps_pool_free_size(pool);
     allocSize = totalSize - freeSize;
     bufferSize = AddrOffset(ap->init, ap->limit);
-    class = ClassOfPoly(Pool, pool);
     printf("%s: obj=%lu pad=%lu total=%lu free=%lu alloc=%lu buffer=%lu\n",
-           ClassName(class),
+           ClassName(pool_class),
            (unsigned long)sd->objSize,
            (unsigned long)sd->padSize,
            (unsigned long)totalSize,