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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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


gczeal(2,1);
x = JSON.parse('{"foo":[]}');
Object.getPrototypeOf(x.foo) == Array.prototype;
x = JSON.parse('{"foo":[], "bar":[]}');

Comment 1

[Christian Holler (:decoder)]

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.

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 3

[Jeff Walden [:Waldo]]

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.

Comment 4

[Jeff Walden [:Waldo]]

Attachment: Patch

Comment 5

[Jeff Walden [:Waldo]]

And yes, the issue here is the issue I thought it was, fixt in the patch.

Comment 6

[Brian Hackett [Laid off!]]

Comment on attachment 726746 [details] [diff] [review]
Patch

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.

Comment 7

[Jeff Walden [:Waldo]]

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.

Comment 8

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa248c4df2f1

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/fa248c4df2f1

Comment 10

[Jeff Walden [:Waldo]]

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.