Closed Bug 852563 Opened 11 years ago Closed 11 years ago

Assertion failure: v->toGCThing(), at gc/Marking.cpp:472 with uninitialized value


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox21 --- unaffected
firefox22 --- fixed
firefox-esr17 --- unaffected
b2g18 --- unaffected


(Reporter: decoder, Assigned: Waldo)



(6 keywords, Whiteboard: [jsbugmon:])


(1 file)

The following testcase asserts on mozilla-central revision e23e43a2c14e (no options required):

x = JSON.parse('{"foo":[]}');
Object.getPrototypeOf( == Array.prototype;
x = JSON.parse('{"foo":[], "bar":[]}');
In an opt-build I see this in Valgrind:

==59247== Conditional jump or move depends on uninitialised value(s)
==59247==    at 0x8270B77: js::gc::MarkValueRoot(JSTracer*, JS::Value*, char const*) (Marking.cpp:471)
==59247==    by 0x81320C7: js::JSONParser::trace(JSTracer*) (jsonparser.cpp:47)
==59247==    by 0x8263673: JS::AutoGCRooter::trace(JSTracer*) (RootMarking.cpp:626)
==59247==    by 0x8263743: JS::AutoGCRooter::traceAll(JSTracer*) (RootMarking.cpp:639)
==59247==    by 0x82638DF: js::gc::MarkRuntime(JSTracer*, bool) (RootMarking.cpp:687)
==59247==    by 0x80CECC4: IncrementalCollectSlice(JSRuntime*, long long, JS::gcreason::Reason, js::JSGCInvocationKind) (jsgc.cpp:2873)
==59247==    by 0x80CFE9C: GCCycle(JSRuntime*, bool, long long, js::JSGCInvocationKind, JS::gcreason::Reason) (jsgc.cpp:4463)
==59247==    by 0x80D0270: _ZL7CollectP9JSRuntimebxN2js18JSGCInvocationKindEN2JS8gcreason6ReasonE.part.297 (jsgc.cpp:4591)
==59247==    by 0x80D104B: js::gc::RunDebugGC(JSContext*) (jsgc.cpp:4509)
==59247==    by 0x8075E4F: JSObject::createArray(JSContext*, js::gc::AllocKind, js::gc::InitialHeap, JS::Handle<js::Shape*>, JS::Handle<js::types::TypeObject*>, unsigned int) (jsgcinlines.h:490)
==59247==    by 0x807CDC1: js::NewDenseCopiedArray(JSContext*, unsigned int, JS::Value const*, JSObject*, js::NewObjectKind) (jsarray.cpp:2700)
==59247==    by 0x81344DE: js::JSONParser::parse(JS::MutableHandle<JS::Value>) (jsonparser.cpp:589)

Marking s-s due to GC-related assertion and use of uninitialized value.
Keywords: valgrind
Whiteboard: [jsbugmon:update,bisect]
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Ugh, I think this is kind of my fault.  I noticed that IdValuePair didn't initialize its value, then forgot to follow up on that to determine whether it was an issue.  I'm pretty sure that not initializing, then getting appended to the properties vector, then getting a trace/GC at just the right time, will trigger this easily, obviously, and reliably.  Patch for that issue, which is very likely this one, in a moment.
Assignee: general → jwalden+bmo
OS: Linux → All
Hardware: x86 → All
Attached patch PatchSplinter Review
Attachment #726746 - Flags: review?(bhackett1024)
And yes, the issue here is the issue I thought it was, fixt in the patch.
Comment on attachment 726746 [details] [diff] [review]

Review of attachment 726746 [details] [diff] [review]:

Thanks, I totally forgot that Value's constructor leaves it uninitialized.

::: js/src/tests/ecma_5/JSON/parse-array-gc.js
@@ +8,5 @@
> +  "IdValuePair::value should be initialized to avoid GC sequence-point issues";
> +
> +print(BUGNUMBER + ": " + summary);
> +
> +print('Note: You must run this test under valgrind to determine if it passes');

I don't think this print is necessary, it looks like it will bust even in (some) non-valgrind builds.
Attachment #726746 - Flags: review?(bhackett1024) → review+
If it busted in every non-valgrind build I'd remove it, but it doesn't, sadly.  On my system things run just fine; decoder got luckier (heh) with actual failure.  Given that, running under valgrind's the only way to guarantee a failure.  Alas and alack.

I'll push after I add a few other patches to my tree, and maybe after the tree reopens assuming it's not now.  (Cue sarcastic philor.)  This is only a couple-days-old issue, so no need for approvals.  I'll open this up in a few days after nightly users have had a chance to update to an unaffected build.
Blocks: 836968
Depends on: 852912
Closed: 11 years ago
Resolution: --- → FIXED
Nightly users have had five or so days to update to a build without this issue, including a weekend.  And this didn't affect any other builds.  Unhiding.
Group: core-security
You need to log in before you can comment on or make changes to this bug.