David Anderson [:dvander] - inactive, e-mail if emergency Reporter
	Description • 14 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

	David Anderson [:dvander] - inactive, e-mail if emergency Reporter
	Comment 1 • 14 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.

	Robert Sayre
	Comment 2 • 14 years ago

cdleary, you have a fix for this somewhere? maybe in jorendorff's queue?

	David Mandelin [:dmandelin]
	Comment 4 • 14 years ago

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.

	David Mandelin [:dmandelin]
	Comment 5 • 14 years ago

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).

	Jason Orendorff [:jorendorff]
	Comment 7 • 14 years ago

I am too busy. I'll try and get to it anyway, maybe tomorrow.

	Jason Orendorff [:jorendorff]
	Comment 8 • 14 years ago

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! ;-)

	David Mandelin [:dmandelin]
	Comment 9 • 14 years ago

http://hg.mozilla.org/tracemonkey/rev/1b354f3d6e5c

	Robert Sayre
	Comment 10 • 14 years ago

http://hg.mozilla.org/mozilla-central/rev/1b354f3d6e5c