Closed Bug 450529 Opened 11 years ago Closed 11 years ago

TM: asserts on foxnews.com in 3rd party script

Categories

(Core :: JavaScript Engine, defect)

Other
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: mrbkap)

References

()

Details

Attachments

(2 files)

Attached file trace + gdb dump
Script looks third party, "Prototype JavaScript framework, version 1.5.1.2"
That assertion doesn't like that regs->pc is out of date, and it wants to peek ahead in the bytecode stream.  We could probably parameterize match_or_replace to have the caller pass in the implied-test bit, since we can tell at record time that we hit that case.
FWIW, seeing this on more than one site -- same assert on Apple's iPhone apps store.
Builtins shouldn't reach back into the interpreter. We might want to set cx->fp to NULL when entering a trace, and back to its original value on exit in debug builds to catch these cases.
Blocks: landtm
Assignee: general → brendan
Assignee: brendan → mrbkap
Attached patch Proposed fixSplinter Review
Many thanks to Brendan and Andreas for holding my hand through the builtin jungle.
Attachment #333877 - Flags: review?(brendan)
Comment on attachment 333877 [details] [diff] [review]
Proposed fix

>+js_str_match_helper(JSContext *cx, uintN argc, jsval *vp, jsbytecode *pc)

Been using js_StringMatchHelper and the like (see js_StringReplaceHelper).

I'm hip to pc at end.

> TraceRecorder::record_JSOP_CALL()
> {
>-    uintN argc = GET_ARGC(cx->fp->regs->pc);
>+    uintN argc = GET_ARGC(pc);

We talked, you're cool with a local pc.

Need a trace-tests.js addition to cover this case. Thanks,

/be
Attachment #333877 - Flags: review?(brendan) → review+
http://hg.mozilla.org/tracemonkey/index.cgi/rev/3eaa6a428d52 includes the trace-tests.js testcase.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
in js1_8_1/trace/trace-test.js also
Flags: in-testsuite+
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.