Closed Bug 587809 Opened 11 years ago Closed 11 years ago
JM64: Mochitest assert in json
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
Status: ASSIGNED → NEW
cdleary, you have a fix for this somewhere? maybe in jorendorff's queue?
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).
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+
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.