If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

JM64: Mochitest assert in json_worker.js

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: dvander, Assigned: cdleary)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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
(Reporter)

Comment 1

7 years ago
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

Comment 2

7 years ago
cdleary, you have a fix for this somewhere? maybe in jorendorff's queue?
Duplicate of this bug: 588356
Blocks: 563002
No longer blocks: 562996
Assignee: general → cdleary
Created attachment 467263 [details] [diff] [review]
Patch (was tracer changes for eager this patch in other bug)

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.
Blocks: 591418
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/tracemonkey/rev/1b354f3d6e5c
Whiteboard: fixed-in-tracemonkey

Comment 10

7 years ago
http://hg.mozilla.org/mozilla-central/rev/1b354f3d6e5c
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.