640085 - Debug OS X SpiderMonkey Shark builds dying in -a -m -d jit-test/tests/v8-v5/check-splay.js with "Error 2"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Phil Ringnalda (:philor)]

A decidedly unhelpful error message, but as soon as we switched the SpiderMonkey builds to --enable-debug, the 10.6 (but not the 10.5) Shark build started dying with

    -a -m -d /builds/slave/tm-osx64-dbg-spidermonkey-shark/src/js/src/jit-test/tests/v8-v5/check-splay.js
make: *** [check] Error 2

Comment 1

[Paul Biggar]

I'm taking a look at this.

Comment 2

[Phil Ringnalda (:philor)]

Oops, it's not that 10.6 is failing and 10.5 is succeeding, it's that 10.6 is failing after two hours, and 10.5 is failing after tying up a slave for 20 hours, so we're finally getting around to seeing the orange from yesterday's pushes.

Comment 3

[Andreas Gal :gal]

This worked for me locally. Maybe all tests have to run for this to trigger?

Comment 4

[Paul Biggar]

For me, any shell invocation is breaking in a memcmp in CheckHelpMessages is shell/js.cpp

Comment 5

[Paul Biggar]

Attachment: Fix fail

hg blame suggests brendan should review this.

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 518201 [details] [diff] [review]
Fix fail

I missed the bug, but it seems to me the evil came in with the patch for bug 625962. Bouncing review to Andreas.

Why do we need those help messages? What uses them? I see no uses, since they have no parallel shell_functions elements.

If we can lose them and restore parallel arrays (ignoring the terminating null pointer in shell_functions), that's a better fix than this patch.

/be

Comment 7

[Paul Biggar]

(In reply to comment #6)
> Comment on attachment 518201 [details] [diff] [review]
> Fix fail
> 
> I missed the bug, but it seems to me the evil came in with the patch for bug
> 625962. Bouncing review to Andreas.
> 
> Why do we need those help messages? What uses them? I see no uses, since they
> have no parallel shell_functions elements.

They functions are in jsdbgapi.cpp:

static JSFunctionSpec profiling_functions[] = {
    JS_FN("startProfiling",  StartProfiling,      0,0),
    JS_FN("stopProfiling",   StopProfiling,       0,0),
#ifdef MOZ_SHARK
    /* Keep users of the old shark API happy. */
    JS_FN("connectShark",    IgnoreAndReturnTrue, 0,0),
    JS_FN("disconnectShark", IgnoreAndReturnTrue, 0,0),
    JS_FN("startShark",      StartProfiling,      0,0),
    JS_FN("stopShark",       StopProfiling,       0,0),
#endif
    JS_FS_END
};



A cleaner way to do this might be to put the help message as a parameter to JS_FN.

Comment 8

[Andreas Gal :gal]

Comment on attachment 518201 [details] [diff] [review]
Fix fail

bz said that jquery hardcoded those names and we break stuff if we take those out. Trust me, I am no fan of this. But hey, I heard the jquery guy works for us, so maybe we can get him to fix this and then we take it out?

Comment 9

[Paul Biggar]

http://hg.mozilla.org/tracemonkey/rev/fdab5c480ab5

Comment 10

[Brendan Eich [:brendan]]

(In reply to comment #7)
> (In reply to comment #6)
> They functions are in jsdbgapi.cpp:
> 
> static JSFunctionSpec profiling_functions[] = {
>     JS_FN("startProfiling",  StartProfiling,      0,0),
>     JS_FN("stopProfiling",   StopProfiling,       0,0),
> #ifdef MOZ_SHARK
>     /* Keep users of the old shark API happy. */
>     JS_FN("connectShark",    IgnoreAndReturnTrue, 0,0),
>     JS_FN("disconnectShark", IgnoreAndReturnTrue, 0,0),
>     JS_FN("startShark",      StartProfiling,      0,0),
>     JS_FN("stopShark",       StopProfiling,       0,0),
> #endif
>     JS_FS_END
> };

Ok, but what indexes into shell_help_messages at those last two (modulo #ifdef) slots? IOW what needs those help strings to be there in the first place?

> A cleaner way to do this might be to put the help message as a parameter to
> JS_FN.

That's an API change and probably most functions don't want to burn space on help strings (not sure, but this isn't the place to figure that out).

/be

Comment 11

[Brendan Eich [:brendan]]

I guess the idea is to support help("startProfiling") and help("stopProfiling"). That really suggests putting the JSNative initializers at the corresponding slots in shell_functions' initializer.

If jQuery or something like it needs those too, could we duplicate JSFunctionSpec array elements a bit?

/be

Comment 12

[Paul Biggar]

I'll happily tidy this up if you can tell me specifically what I should do.

Comment 13

[Brendan Eich [:brendan]]

(In reply to comment #12)
> I'll happily tidy this up if you can tell me specifically what I should do.

No worries, it's hard to improve on this bug's solution and not really worth it right now.

/be

Comment 14

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/fdab5c480ab5