Closed Bug 858097 Opened 12 years ago Closed 12 years ago

BaselineCompiler: Crash [@ Mark<js::types::TypeObject>] with OOM

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86_64
Linux
defect
Not set
major

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)

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();
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
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.
Attached patch Patch (deleted) — Splinter Review
See the analysis in comment 2. Let me know if you can think of a better fix.
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Attachment #733928 - Flags: review?(luke)
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+
...that is, using that return value as though it were real, not expecting this rare case where a MagicValue manifested itself.
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.

Attachment

General

Created:
Updated:
Size: