Closed Bug 587809 Opened 11 years ago Closed 11 years ago

JM64: Mochitest assert in json_worker.js

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: cdleary)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

entering trace at http://mochi.test:8888/tests/dom/src/threads/test/json_worker.js:306@9, execs: 2 code: 0x11be3a5dc
leaving trace at http://mochi.test:8888/tests/dom/src/threads/test/json_worker.js:310@28, op=getprop, lr=0x10633d9e0, exitType=BRANCH, sp=3, calldepth=0, cycles=0
args0=function<0x1350a68b8:onmessage> args1=object<0x1350a7558:DedicatedWorkerGlobalScope> args2=object<0x134cf0d38:XPCWrappedNative_NoHelper> arguments0=null scopeChain0=object<0x1350a7558:DedicatedWorkerGlobalScope> var0=int<5> var1=undefined stack0=function<0x134cf0d80:postMessage> stack1=Assertion failure: v.isNull(), at /Users/dvander/mozilla/jaegermonkey/js/src/jstracer.cpp:2898
Looks like this is most if not all of the trace-test failures, and is related to eager-this. To reproduce:

./Debug64/js -m -j trace-test/tests/basic/bug535760.js

The interpreter stores a NULL and the tracer stores the global object. This was only caught when Luke added assertions in NativeToValue(). It likely affects x86 as well but it might be harder to see with the split value encoding.
Assignee: dvander → general
Blocks: 574697
Status: ASSIGNED → NEW
cdleary, you have a fix for this somewhere? maybe in jorendorff's queue?
Duplicate of this bug: 588356
Blocks: JaegerGreen
No longer blocks: JaegerBrowser
Assignee: general → cdleary
This patch is cdleary's work; I just rebased it to JM tip. This is probably the worst outstanding JM bug blocking Sep 1 landing right now, so expedited review would be much appreciated. Jason, let us know if you are too busy and we need to find another reviewer.
Attachment #467263 - Flags: review?(jorendorff)
I "pre-landed" this on JM as:

  http://hg.mozilla.org/projects/jaegermonkey/rev/305b00ec07e7

just so we can get Tinderbox coverage on it, both as an extra test for the patch, and to find out earlier what new bugs this fix might expose (from tests that asserted early before).
Duplicate of this bug: 586922
I am too busy. I'll try and get to it anyway, maybe tomorrow.
Comment on attachment 467263 [details] [diff] [review]
Patch (was tracer changes for eager this patch in other bug)

I originally wrote this patch (for bug 556277). So I only have a few nits.

In TR::getThis:
>+        JS_ASSERT(!fp->getThisValue().isPrimitive());

Instead, assert that fp->getThisValue().isObject(), which is the same
condition. And then...

>+        JS_ASSERT(fp->getThisValue().toObjectOrNull() == obj);

...use the stricter .toObject() here...

>+        this_ins = INS_CONSTOBJ(fp->getThisValue().toObjectOrNull());

...and here.

>+    Value& thisv = fp->argv[-1];

const Value& please.

>+    if (!obj)
>+        RETURN_ERROR("getThisObject failed");
>+    JS_ASSERT(fp->argv[-1] == ObjectOrNullValue(obj));

ObjectValue, please -- obj can't be null given the previous lines.

Great patch! ;-)
Attachment #467263 - Flags: review?(jorendorff) → review+
http://hg.mozilla.org/mozilla-central/rev/1b354f3d6e5c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.