Last Comment Bug 704510 - js::MaybeGC(JSContext*) (jsgc.cpp:2167): Conditional jump or move depends on uninitialised value(s)
: js::MaybeGC(JSContext*) (jsgc.cpp:2167): Conditional jump or move depends on ...
Status: RESOLVED FIXED
[qa-]
: regression
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Terrence Cole [:terrence]
:
Mentors:
Depends on:
Blocks: 700357
  Show dependency treegraph
 
Reported: 2011-11-22 09:18 PST by Josh Aas
Modified: 2012-02-22 11:44 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
affected
+
fixed
unaffected


Attachments
v1: trivial (886 bytes, patch)
2011-11-22 10:57 PST, Terrence Cole [:terrence]
wmccloskey: review+
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Josh Aas 2011-11-22 09:18:43 PST
Seen running Firefox trunk x86_64 build on Mac OS X 10.6 under valgrind:

==97971== Conditional jump or move depends on uninitialised value(s)
==97971==    at 0x108C9551F: js::MaybeGC(JSContext*) (jsgc.cpp:2167)
==97971==    by 0x108BFD3A3: JS_MaybeGC (jsapi.cpp:2767)
==97971==    by 0x107C21546: nsJSContext::ScriptEvaluated(bool) (nsJSEnvironment.cpp:3127)
==97971==    by 0x107C20667: nsJSContext::EvaluateStringWithValue(nsAString_internal const&, JSObject*, nsIPrincipal*, char const*, unsigned int, unsigned int, JS::Value*, bool*) (nsJSEnvironment.cpp:1336)
==97971==    by 0x107BC0639: nsXBLProtoImplField::InstallField(nsIScriptContext*, JSObject*, nsIPrincipal*, nsIURI*, bool*) const (nsXBLProtoImplField.cpp:151)
==97971==    by 0x107BA1139: XBLResolve(JSContext*, JSObject*, jsid, unsigned int, JSObject**) (nsXBLBinding.cpp:199)
==97971==    by 0x106FCEF02: CallResolveOp(JSContext*, JSObject*, JSObject*, jsid, unsigned int, JSObject**, JSProperty**, bool*) (jsobj.cpp:5402)
==97971==    by 0x106FCF3B4: js::LookupPropertyWithFlags(JSContext*, JSObject*, jsid, unsigned int, JSObject**, JSProperty**) (jsobj.cpp:5455)
==97971==    by 0x106FDDB4D: js_SetPropertyHelper(JSContext*, JSObject*, jsid, unsigned int, JS::Value*, int) (jsobj.cpp:6105)
==97971==    by 0x106F8F235: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:3743)
==97971==    by 0x106FBA3C6: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:584)
==97971==    by 0x106FBB6F8: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jsinterp.cpp:647)

Will mark as security sensitive to be safe, hopefully it isn't necessary.
Comment 1 Bill McCloskey (:billm) 2011-11-22 09:27:45 PST
Could you take a look at this, Terrence? It looks like gcNumFreeArenas isn't initialized. It should be done in the JSRuntime constructor, which is in jsapi.cpp.
Comment 2 Gregor Wagner [:gwagner] 2011-11-22 09:37:38 PST
At worst we perform a GC the first time we enter MaybeGC so it's not sec-sensitive.
Comment 3 Josh Aas 2011-11-22 10:51:56 PST
The pages I loaded under valgrind when this came up: the Mozilla nightly home page, w3.org, and espn.com.
Comment 4 Terrence Cole [:terrence] 2011-11-22 10:57:53 PST
Created attachment 576209 [details] [diff] [review]
v1: trivial

https://tbpl.mozilla.org/?tree=Try&rev=4b9c258934d1
Comment 5 Bill McCloskey (:billm) 2011-11-22 11:31:17 PST
Comment on attachment 576209 [details] [diff] [review]
v1: trivial

Thanks.
Comment 6 Terrence Cole [:terrence] 2011-11-22 14:23:25 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/2c13341cc1c1
Comment 7 Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]] 2011-11-23 06:27:15 PST
If your sure this is not sec-sensative then please unhide.

This appears to be resolved, so I am marking it (resolved->fixed), per request from Ed Morely who monitors mozilla-inbound > mozilla-central merges.
Comment 8 Ian Melven :imelven 2011-11-23 09:12:21 PST
adding the changeset that landed this in m-c :

https://hg.mozilla.org/mozilla-central/rev/2c13341cc1c1

and setting target milestone to FF 11
Comment 9 Daniel Veditz [:dveditz] 2011-11-23 10:36:52 PST
This is a "regression" from bug 700357 which added this property. Unfortunately that landed the day before the merge and made it into Fx 10, so we need to land this fix on Aurora as well.
Comment 10 Daniel Veditz [:dveditz] 2011-11-23 10:47:45 PST
Looking at the code I agree w/Gregor in comment 2: the only decision based on this possibly bogus value is whether to GC now or wait a bit and that's not a security problem.
Comment 11 Terrence Cole [:terrence] 2011-11-23 11:55:26 PST
Daniel, thanks for unhiding this: I don't have permission bits to do that.

If we want to go ahead and push this to FF10, there is zero risk in doing so.
Comment 12 Terrence Cole [:terrence] 2011-11-23 11:57:12 PST
*sigh* Bugzilla, why do you do these things to me?  I can't even see the fields I just unset.

Note You need to log in before you can comment on or make changes to this bug.