Closed Bug 640085 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: philor, Assigned: paul.biggar)

References

Details

(Keywords: intermittent-failure, Whiteboard: [fixed-in-tracemonkey])

Attachments

(1 file)

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
I'm taking a look at this.
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.
Summary: Debug OS X 10.6 SpiderMonkey Shark builds dying in -a -m -d jit-test/tests/v8-v5/check-splay.js with "Error 2" → Debug OS X SpiderMonkey Shark builds dying in -a -m -d jit-test/tests/v8-v5/check-splay.js with "Error 2"
This worked for me locally. Maybe all tests have to run for this to trigger?
For me, any shell invocation is breaking in a memcmp in CheckHelpMessages is shell/js.cpp
Attached patch Fix failSplinter Review
hg blame suggests brendan should review this.
Assignee: general → pbiggar
Attachment #518201 - Flags: review?(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
Attachment #518201 - Flags: review?(brendan) → review?(gal)
(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 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?
Attachment #518201 - Flags: review?(gal) → review+
http://hg.mozilla.org/tracemonkey/rev/fdab5c480ab5
Whiteboard: [orange] → [orange][fixed-in-tracemonkey]
(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
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
I'll happily tidy this up if you can tell me specifically what I should do.
(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
http://hg.mozilla.org/mozilla-central/rev/fdab5c480ab5
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [orange][fixed-in-tracemonkey] → [fixed-in-tracemonkey]
You need to log in before you can comment on or make changes to this bug.