494045 - TM: setting scopeChain to NULL in SynthesizeFrame breaks GetCallObject

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Mark Banner (:standard8)]

Attachment: Crash stack

With the landing of the tracemonkey merge on 20th May, some of the mailnews xpcshell tests have started crashing. The tests that fail are:

TEST-UNEXPECTED-FAIL | /Volumes/Build/macosx-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_imap/unit/test_bug460636.js | test failed (with xpcshell return code: -10), see following log:
TEST-UNEXPECTED-FAIL | /Volumes/Build/macosx-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_imap/unit/test_compactOfflineStore.js | test failed (with xpcshell return code: -10), see following log:
TEST-UNEXPECTED-FAIL | /Volumes/Build/macosx-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_imap/unit/test_downloadOffline.js | test failed (with xpcshell return code: -10), see following log:
TEST-UNEXPECTED-FAIL | /Users/moztest/comm/trunk/tb/mozilla/_tests/xpcshell/test_imap/unit/test_nsIMsgFolderListenerIMAP.js | test failed (with xpcshell return code: -10), see following log:

The logs aren't interesting as there are no additional warnings/errors and we don't get crash stacks at the moment.

However I've run the tests on my machine and the crash stack from one of the failures is attached.

I've verified by reverting the mozilla-central repository to the revision before the landing (a3b152e3e972) and then updated to the merge set of the landing (cb4d2ce3b5db).

Tracemonkey landing:

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-05-20+09%3A22%3A05&enddate=2009-05-20+09%3A23

Apologies, but I don't have time for further debug now, and probably won't over the next couple of days. I'm guessing the merge may be heading to 1.9.1 so wanted to raise it now to make you guys aware.

Comment 1

[Mark Banner (:standard8)]

A brief comparison of crash-stack and files changed leads me to believe this is most likely a regression from bug 493657.

Comment 2

[Andreas Gal :gal]

Attachment: patch

Comment 3

[Andreas Gal :gal]

Attachment: patch with testcase

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 378729 [details] [diff] [review]
patch with testcase

>diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp
>--- a/js/src/jstracer.cpp
>+++ b/js/src/jstracer.cpp
>@@ -1930,24 +1930,46 @@ skip:
>                 /*
>                  * We might return from trace with a different function object, but it still
>                  * has to be the same function (FIXME: bug 471425, eliminate fp->callee).

This comment confuses me -- change function object to callee object, and maybe change "same function" to "same JSFunction"?

>                  */
>                 JS_ASSERT(JSVAL_IS_OBJECT(fp->argv[-1]));
>                 JS_ASSERT(HAS_FUNCTION_CLASS(JSVAL_TO_OBJECT(fp->argv[-2])));
>                 JS_ASSERT(GET_FUNCTION_PRIVATE(cx, JSVAL_TO_OBJECT(fp->argv[-2])) ==
>                           GET_FUNCTION_PRIVATE(cx, fp->callee));
>+                JS_ASSERT(GET_FUNCTION_PRIVATE(cx, fp->callee) == fp->fun);
>                 fp->callee = JSVAL_TO_OBJECT(fp->argv[-2]);

Blank line here, while you are nearby.

r=me with these nits.

/be

Comment 5

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/8b95ee49081b

Comment 6

[Andreas Gal :gal]

Thanks for reporting this so quickly. The stack frame helped a lot.

Comment 7

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/8b95ee49081b

Comment 8

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/2a4f5b1dcef7

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Marking verified based on all tests passes on trunk and 1.9.1 and no backout so far.