The current code of js_NewGCThing in jsgc.c contains: /* * Clear thing before unlocking in case a GC run is about to scan it, * finding it via newborn. */ thing->next = NULL; thing->flagp = NULL; Such zeroing here is unnecessary since all users of js_NewGCThing initialize the thing before doing any operation that can trigger GC. Moreover, the code does not work as a defense again bugs in the initialization code. For that it would be necessary to call memset(thing, 0, nbytes) to clear the whole thing, not just first sizeof(JSGCThing) its bytes. Thus I suggest to remove this zeroing.
Created attachment 277712 [details] [diff] [review] v1 js_NewObject is the only user of js_NewGCThing that relies on JSGCThing zeroing so a potential GC called from newObjectMap would see null, not junk, as JSObject.map. Thus the patch added explicit obj->map = NULL besides removing the zeroing code in js_NewGCThing.
Comment on attachment 277712 [details] [diff] [review] v1 >+ * can potentially cause GC. This will ensure that GC tracing never see a junk >+ * values stored in the partially initialized thing. s/see a/sees/ r/a=me with that. /be
Created attachment 277750 [details] [diff] [review] v1b Patch to check in with nits addressed.
I checked in the patch from comment 4 to the trunk: http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1187816103&maxdate=1187816280&who=igor%mir2.org
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.