| Title | Assertion failure in mps_arena_roots_walk |
| Status | closed |
| Priority | essential |
| Assigned user | Gareth Rees |
| Organization | Ravenbrook |
| Description | MMQA test function/122 [1] fails with this assertion: trace.c:351: MPS ASSERTION FAILED: !TraceSetIsMember(SegWhite(seg), trace) |
| Analysis | Here's the traceback: #0 0x00007fff8c1b9d46 in __kill () #1 0x00007fff85d16df0 in abort () #2 0x000000010011d6d2 in mps_lib_assert_fail_default (file=0x10023f996 "code/trace.c", line=351, condition=0x10023fd91 "!TraceSetIsMember(SegWhite(seg), trace)") at mpsliban.c:76 #3 0x0000000100005292 in mps_lib_assert_fail (file=0x10023f996 "code/trace.c", line=351, condition=0x10023fd91 "!TraceSetIsMember(SegWhite(seg), trace)") at mpsliban.c:85 #4 0x000000010007a6bb in TraceAddWhite (trace=0x10035f598, seg=0x101ffd620) at trace.c:351 #5 0x0000000100086798 in ArenaRootsWalk (arenaGlobals=0x10035f000, f=0x100001750 <root_step>, p=0x101fbe0f0, s=342) at walk.c:315 #6 0x0000000100086307 in mps_arena_roots_walk (mps_arena=0x10035f000, f=0x100001750 <root_step>, p=0x101fbe0f0, s=342) at walk.c:361 #7 0x00000001000015f7 in test () at function/122.c:74 #8 0x0000000100002b26 in call_f (p=0x10035f000, s=4294973264) at test/testlib/testlib.c:298 #9 0x0000000100026e84 in ProtTramp (resultReturn=0x7fff5fbff910, f=0x100002b20 <call_f>, p=0x7fff5fbff918, s=0) at protix.c:132 #10 0x0000000100026b2b in mps_tramp (r_o=0x7fff5fbff910, f=0x100002b20 <call_f>, p=0x7fff5fbff918, s=0) at mpsi.c:1378 #11 0x0000000100002352 in easy_tramp2 [inlined] () at test/test/testlib/testlib.c:316 #12 0x0000000100002352 in easy_tramp (f=0x7fff5fbff910) at test/testlib/testlib.c:337 #13 0x0000000100001102 in main () at function/122.c:192 Open Dylan uses mps_arena_roots_walk in some sort of heap analysis tool. See < https://github.com/dylan-lang/opendyla...aster/sources/lib/run-time/heap-trail.c> and <https://github.com/dylan-lang/opendyla...ster/sources/lib/run-time/heap-order2.c>. We need to determine whether it's still a requirement. We deprecated the function in 1.111 but that may have been premature. We should ask them to exercise it, in any case, and see if this problem affects them.GDR 2014-04-13: The way that ArenaRootsWalk works is that it creates a trace, makes all segments white for that trace, scans the roots, and destroys the trace. But this leaves the segments white. So when you call ArenaRootsWalk again, the .start.black test in TraceAddWhite fails. So ArenaRootsWalk needs to turn all the segments black afterwards: /* 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)); } } while (SegNext(&seg, arena, seg)); } which fixes the assertion, but doesn't make the test case pass. The debugger shows that references fixed from one of the roots get walked, but references fixed from the other roots do not, because this test in RootScan rejects them: if (TraceSetInter(root->grey, ss->traces) == TraceSetEMPTY) That's because only one of the roots (the first in the rootRing) gets turned grey by this loop: res = RootsIterate(arenaGlobals, (RootIterateFn)RootGrey, (void *)trace); And that's because the cast is bogus. A function of type RootIterateFn returns a Res, which is used to continue or discontinue the iteration. But RootGrey returns void. So some random value on the stack gets interpreted as the result, and so the iteration stops. |
| How found | automated_test |
| Evidence | [1] http://www.ravenbrook.com/project/mps/master/test/function/122.c |
| Observed in | 1.111.0 |
| Created by | Gareth Rees |
| Created on | 2013-05-25 20:26:44 |
| Last modified by | Gareth Rees |
| Last modified on | 2014-04-14 23:40:25 |
| History | 2013-05-25 GDR Created. 2013-06-16 RB Added reference to Open Dylan usage. 2014-04-14 GDR Analysis and fix. |
| Change | Effect | Date | User | Description |
|---|---|---|---|---|
| 185530 | closed | 2014-04-14 23:38:54 | Gareth Rees | Fix ArenaRootsWalk: 1. Blacken the segments again after scanning the roots, so that the roots can be walked again. 2. Don't cast RootGrey to a RootIterateFn -- the types are not compatible. MMQA test function/122.c now passes. |