599400 - Firefox crashes when jsdScript::ClearBreakpoint(0) is called

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Firefox crashes when clearBreakpoint is called. Here is the callstack.

mozjs.dll!js::Shape::search(js::Shape * * startp=0x0ae98c60, int id=179134256, bool adding=false)  Line 834 + 0x5 bytes	C++
mozjs.dll!JSFunction::lookupLocal(JSContext * cx=0x0964d398, JSAtom * atom=0x0aad5f30, unsigned int * indexp=0x00000000)  Line 3236 + 0x1b bytes	C++
mozjs.dll!js::mjit::Compiler::generateMethod()  Line 1362 + 0x23 bytes	C++
mozjs.dll!js::mjit::Compiler::Compile()  Line 143 + 0x8 bytes	C++
mozjs.dll!js::mjit::Recompiler::recompile()  Line 156 + 0xb bytes	C++
mozjs.dll!JS_ClearTrap(JSContext * cx=0x0964d398, JSScript * script=0x0ae98c08, unsigned char * pc=0x0ae98cc4, JSTrapStatus (JSContext *, JSScript *, unsigned char *, unsigned __int64 *, unsigned __int64)* * handlerp=0x00000000, unsigned __int64 * closurep=0x00000000)  Line 334	C++
jsd3250.dll!jsd_ClearExecutionHook(JSDContext * jsdc=0x0964b608, JSDScript * jsdscript=0x05369780, unsigned int pc=183078084)  Line 833 + 0x1c bytes	C
jsd3250.dll!JSD_ClearExecutionHook(JSDContext * jsdc=0x0964b608, JSDScript * jsdscript=0x05369780, unsigned int pc=183078084)  Line 552 + 0x11 bytes	C
jsd3250.dll!jsdScript::ClearBreakpoint(unsigned int aPC=0)  Line 1502 + 0x17 bytes	C++

The method is called by Firebug from within a onBreakpoint hook (jsdIDebuggerService.breakpointHook)

The parameter passed into the clearBreakpoint method is |0|.

Here is how you can reproduce that:

1) Install Firebug 1.6b1
http://getfirebug.com/releases/firebug/1.6X/firebug-1.6X.0b1.xpi

2) Load:
http://getfirebug.com/tests/content/script/debuggerKeyword/testPage.html

3) Open Firebug (press F12) and reload -> CRASH

The place where Firebug calls clearBreakpoint is here:
http://code.google.com/p/fbug/source/browse/branches/firebug1.6/modules/firebug-service.js#1460

Firefox shouldn't crash even if Firebug is not using the method properly.

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I have also a simple extension that can be also used to reproduce the problem. Let me know I should attach it to this bug.

Just to note, the Firebug logic (how JSD is used) is:

1) when onScriptCreated is executed,
2) set a breakpoint by calling: script.setBreakpoint(0);
3) when onBreakpoint (breakpoint hook) is executed
4) remove it by calling: frame.script.clearBreakpoint(0); -> CRASH

Honza

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: A test extension for bug reproduction.

This crash is critical for Firebug and prevents further testing. So, I am
attaching a simple extension (using JSD service) so, it's easier to reproduce
and see the logic of the crash.

STR:

1) Install the extension
2) Load
http://getfirebug.com/tests/content/script/debuggerKeyword/testPage.html
3) There is a "Simple Debugger" label in the status bar, right click on it and
pick "Enable"
4) Reload the page -> CRASH.

Honza

Comment 3

[David Mandelin [:dmandelin]]

Attachment: Patch

This fixes the STR with the test extension, at least. The bug was probably introduced by a merge with one of the API changes, where someone (probably me) forgot that fp->fun turns to fp->maybeFun() for this usage, not fp->fun().

Comment 4

[David Mandelin [:dmandelin]]

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

Comment 5

[Robert Sayre]

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

Comment 6

[Mozilla RelEng Bot]

A changeset from this bug was associated with a XP Ts, Cold MED Dirty Profile regression on Firefox. boo-urns :(

  Previous: avg 420.356 stddev 3.679 of 30 runs up to a9d1ad0bc386
  New     : avg 430.832 stddev 1.905 of 5 runs since a60414d076b5
  Change  : +10.475 (2.49% / z=2.847)
  Graph   : http://mzl.la/bZFaB3

The regression occurred from changesets in the following range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5

The tag [suspect-regress-ts_cold_generated_med] has been added to the status whiteboard;
please remove it only once you have confirmed this bug is not the cause
of the regression.

Comment 7

[Mozilla RelEng Bot]

A changeset from this bug was associated with a Win7 Dromaeo (CSS) regression on Firefox. boo-urns :(

  Previous: avg 2014.419 stddev 40.480 of 30 runs up to a9d1ad0bc386
  New     : avg 1901.610 stddev 12.432 of 5 runs since a60414d076b5
  Change  : -112.809 (-5.6% / z=2.787)
  Graph   : http://mzl.la/9gFu4n

The regression occurred from changesets in the following range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5

The tag [suspect-regress-dromaeo_css] has been added to the status whiteboard;
please remove it only once you have confirmed this bug is not the cause
of the regression.

Comment 8

[Mozilla RelEng Bot]

A changeset from this bug was associated with a Win7 Dromaeo (jslib) regression on Firefox. boo-urns :(

  Previous: avg 127.610 stddev 4.222 of 30 runs up to a9d1ad0bc386
  New     : avg 116.384 stddev 0.751 of 5 runs since a60414d076b5
  Change  : -11.226 (-8.8% / z=2.659)
  Graph   : http://mzl.la/bZu5UP

The regression occurred from changesets in the following range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5

The tag [suspect-regress-dromaeo_jslib] has been added to the status whiteboard;
please remove it only once you have confirmed this bug is not the cause
of the regression.

Comment 9

[Mozilla RelEng Bot]

A changeset from this bug was associated with a XP Dromaeo (CSS) regression on Firefox. boo-urns :(

  Previous: avg 2045.275 stddev 49.676 of 30 runs up to a9d1ad0bc386
  New     : avg 1936.120 stddev 13.937 of 5 runs since a60414d076b5
  Change  : -109.155 (-5.34% / z=2.197)
  Graph   : http://mzl.la/b0dlou

The regression occurred from changesets in the following range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5

The tag [suspect-regress-dromaeo_css] has been added to the status whiteboard;
please remove it only once you have confirmed this bug is not the cause
of the regression.

Comment 10

[Mozilla RelEng Bot]

A changeset from this bug was associated with a XP Dromaeo (jslib) regression on Firefox. boo-urns :(

  Previous: avg 129.703 stddev 4.099 of 30 runs up to a9d1ad0bc386
  New     : avg 117.954 stddev 0.660 of 5 runs since a60414d076b5
  Change  : -11.749 (-9.06% / z=2.866)
  Graph   : http://mzl.la/avZij4

The regression occurred from changesets in the following range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5

The tag [suspect-regress-dromaeo_jslib] has been added to the status whiteboard;
please remove it only once you have confirmed this bug is not the cause
of the regression.

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I am seeing yet one test case that causes crash and is also related to clearBreakpoints(), here is a stack trace:

mozjs.dll!js::StackSegment::getCurrentFrame()  Line 86 + 0x8 bytes	C++
mozjs.dll!js::AllFramesIter::operator++()  Line 474 + 0x12 bytes	C++
mozjs.dll!js::mjit::Recompiler::recompile()  Line 125 + 0x21 bytes	C++
mozjs.dll!JS_ClearTrap(JSContext * cx=0x08307f08, JSScript * script=0x0d271df8, unsigned char * pc=0x0d271e60, JSTrapStatus (JSContext *, JSScript *, unsigned char *, unsigned __int64 *, unsigned __int64)* * handlerp=0x00000000, unsigned __int64 * closurep=0x00000000)  Line 335	C++
jsd3250.dll!jsd_ClearExecutionHook(JSDContext * jsdc=0x039f5ec8, JSDScript * jsdscript=0x0d274b00, unsigned int pc=220667488)  Line 833 + 0x1c bytes	C
jsd3250.dll!JSD_ClearExecutionHook(JSDContext * jsdc=0x039f5ec8, JSDScript * jsdscript=0x0d274b00, unsigned int pc=220667488)  Line 552 + 0x11 bytes	C
jsd3250.dll!jsdScript::ClearBreakpoint(unsigned int aPC=0)  Line 1502 + 0x17 bytes	C++

I have modified getCurrentFrame() as follows:

JS_REQUIRES_STACK JS_ALWAYS_INLINE JSStackFrame *
StackSegment::getCurrentFrame() const
{
    JSFrameRegs* regs = getCurrentRegs();
    return regs ? regs->fp : NULL;
}

and I am seeing that |regs| is an invalid pointer.


STR:
1) Install Firebug 1.6b1
2) Open: http://getfirebug.com/tests/content/console/2914/issue2914.html
3) CRASH

A glue could be to know that the test is trying to access a parent frame (from a child frame) and use its window property. Something like as follows:

parent.window.console.error("iframe error");

Honza

Comment 12

[David Mandelin [:dmandelin]]

I'm gonna spin off the new failure, because it requires a separate patch.

Comment 13

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Do you want me to file separate bug for it?
Honza

Comment 14

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Ah, this one has been reported for it:
https://bugzilla.mozilla.org/show_bug.cgi?id=601197

Honza