Closed
Bug 858097
Opened 12 years ago
Closed 12 years ago
BaselineCompiler: Crash [@ Mark<js::types::TypeObject>] with OOM
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: decoder, Assigned: jandem)
References
(Blocks 1 open bug)
Details
(Keywords: crash, testcase, Whiteboard: [jsbugmon:])
Crash Data
Attachments
(1 file)
(deleted),
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on baseline compiler branch revision 5fd27c1b3943 (run with --ion-eager):
function MyObject( value ) {}
gcparam("maxBytes", gcparam("gcBytes") + 4*(1));
gczeal(4);
function test() {}
var obj = new test();
Reporter | ||
Updated•12 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:]
Reporter | ||
Comment 1•12 years ago
|
||
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Assignee | ||
Comment 2•12 years ago
|
||
I was able to reproduce this on decoder's fuzzing machine. Baseline may have exposed the problem in this case, but I think it's a pre-existing issue.
We call js::InvokeConstructorKernel:
args.setThis(MagicValue(JS_IS_CONSTRUCTING));
Then in StackFrame::prologue:
if (isConstructing()) {
RootedObject callee(cx, &this->callee());
JSObject *obj = CreateThisForFunction(cx, callee, useNewType());
if (!obj)
return false;
functionThis() = ObjectValue(*obj);
}
If the CreateThisForFunction OOM's this will leave |this| == MagicValue(JS_IS_CONSTRUCTING).
Then in StackFrame::epilogue:
if (isConstructing() && returnValue().isPrimitive())
setReturnValue(ObjectValue(constructorThis()));
This sets the frame's return value slot to ObjectValue(0xa). (JS_IS_CONSTRUCTING == 0xa). With a debug build this would assert isObject() I think, but this is an opt build.
Assignee | ||
Comment 3•12 years ago
|
||
See the analysis in comment 2. Let me know if you can think of a better fix.
Comment 4•12 years ago
|
||
Comment on attachment 733928 [details] [diff] [review]
Patch
Review of attachment 733928 [details] [diff] [review]:
-----------------------------------------------------------------
Stealing, as Luke's out and this is clearly fine.
::: js/src/vm/Stack.cpp
@@ +411,1 @@
> setReturnValue(ObjectValue(constructorThis()));
Looking at this, we might also consider making constructorThis() just return a Value, and then there's no need for this extra check. The return value of a function *should be* considered meaningless in cases like these, but I could imagine mistakes being made.
Attachment #733928 -
Flags: review?(luke) → review+
Comment 5•12 years ago
|
||
...that is, using that return value as though it were real, not expecting this rare case where a MagicValue manifested itself.
Assignee | ||
Comment 6•12 years ago
|
||
Thanks for stealing the r? Waldo.
https://hg.mozilla.org/integration/mozilla-inbound/rev/25323c442d1a
Comment 7•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in
before you can comment on or make changes to this bug.
Description
•