414451 - Venkman crashs on profiling after clearing profile data [@ _callHook]

Status: RESOLVED WONTFIX

(Other Applications Graveyard :: Venkman JS Debugger, defect)

Description

[Victor STINNER]

User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.5; Linux) KHTML/3.5.8 (like Gecko)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.11) Gecko/2007120410 Ubuntu/7.10 (gutsy) Firefox/2.0.0.11

When I profile my application, Firefox crashs around profiling stuff (JSD): _callHook() in jsd_step.c:170 and jsd_step.c:174. I looks like Firefox reuse free'd memory.

Reproducible: Always

Steps to Reproduce:
1. Open Firefox
2. Open Javascript Debuger (Venkman)
3. Enable profiling
4. Surf on the WWW
5. Go back to Venkman and click on Profile > Clear Profile Data
6. Continue to surf on the WWW
7. If Firefox doesn't crash, go back to step 5

Actual Results:  
gdb backtrace: don't trust gdb, bug is somewhere else (see Valgrind data below):

Program received signal SIGSEGV, Segmentation fault.
(gdb) where
#0  0xb7ef913a in js_DropObjectMap (cx=0x94d9f48, map=0x0, obj=0xb2429f8)
    at jsobj.c:2287
#1  0xb7efb9f3 in js_FinalizeObject (cx=0x94d9f48, obj=0xb2429f8)
    at jsobj.c:2753
#2  0xb7edd727 in js_GC (cx=0x94d9f48, gckind=GC_NORMAL) at jsgc.c:3028
#3  0xb7eb2a22 in JS_GC (cx=0x94d9f48) at jsapi.c:1873
(...)

-------------

Valgrind result: first Valgrind related to Javascript.

Invalid read of size 4
   at 0x78E71DD: _callHook (jsd_step.c:170)
   by 0x78E7372: jsd_FunctionCallHook (jsd_step.c:285)
   by 0x40667ED: js_Invoke (jsinterp.c:1359)
   by 0x59EC266: nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS*, unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) (xpcwrappedjsclass.cpp:1453)
   by 0x59E5EB2: nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) (xpcwrappedjs.cpp:467)
   by 0x4161592: PrepareAndDispatch (xptcstubs_gcc_x86_unix.cpp:100)
   by 0x414B190: nsTimerImpl::Fire() (nsTimerImpl.cpp:397)
   by 0x414B927: handleTimerEvent(TimerEventType*) (nsTimerImpl.cpp:459)
   by 0x4146B46: PL_HandleEvent (plevent.c:688)
   by 0x4146E3A: PL_ProcessPendingEvents (plevent.c:623)
   by 0x4148FA7: nsEventQueueImpl::ProcessPendingEvents() (nsEventQueue.cpp:448)
   by 0x5A2F334: event_processor_callback(_GIOChannel*, GIOCondition, void*) (nsAppShell.cpp:67)
 Address 0x10B95E2C is 4 bytes inside a block of size 80 free'd
   at 0x402237F: free (vg_replace_malloc.c:233)
   by 0x78E54E8: jsd_ClearScriptProfileData (jsd_scpt.c:388)
   by 0x78E3A22: jsd_ClearAllProfileData (jsd_high.c:292)
   by 0x78E33CC: JSD_ClearAllProfileData (jsdebug.c:119)
   by 0x78E9BB8: jsdService::ClearProfileData() (jsd_xpc.cpp:2722)
   by 0x41609C8: XPTC_InvokeByIndex (in /usr/lib/firefox/libxpcom_core.so)
   by 0x59EE3A6: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (xpcwrappednative.cpp:2169)
   by 0x59F4C3E: XPC_WN_CallMethod(JSContext*, JSObject*, unsigned, long*, long*) (xpcwrappednativejsops.cpp:1455)
   by 0x4066840: js_Invoke (jsinterp.c:1375)
   by 0x4068A10: js_Interpret (jsinterp.c:3944)
   by 0x4066CD3: js_Invoke (jsinterp.c:1394)
   by 0x4076B5C: js_InternalInvoke (jsinterp.c:1469)

And there is a similar bug on "_callHook (jsd_step.c:174)". The line 170 is "JSLL_SUB(ll_delta, now, callerpdata->lastCallStart);" and the line 174 is "JSLL_ADD(callerpdata->runningTime, callerpdata->runningTime, ll_delta);".

I think that callerpdata is the evil pointer to free'd memory.


Venkman version: 0.9.87.2
Firefox version: 2.0.0.11
OS: Ubuntu Gutsy
CPU: Pentium4, 32-bit, 3 GHz

Comment 1

[:Gijs (he/him)]

So unfortunately, nothing is going to happen to this bug unless you can get me working steps to reproduce, preferably on trunk. I just don't have the time to randomly surf around waiting for my build to magically crash. I haven't seen this myself, and while I'd love to do something about it, I can't until you give me more information. Sorry!

Comment 2

[Victor STINNER]

Ok, I tried to reproduce the crash on a fresh Firefox installation.

1- Start Firefox with a fresh (empty) profile
2- Install Venkman: https://addons.mozilla.org/en-US/firefox/addon/216
3- Restart Firefox to enable Venkman
4- Open Venkman (Tools > Javascript Debugger)
5- Enable Profiling (click on the big Clock icon, label Profile)
6- Go to a website using Javascript, eg. http://linuxfr.org/
7- In Venkman, clear profile data (Profile > Clear Profile Data)
8- Go to another page using Javascript, eg. http://linuxfr.org/journal/
9- Loop to step 7 until Firefox crashs

On my computer, it does crash at the step 8, sometimes I need to open more pages.

If it doesn't crash, go to Venkman and click on Profile > Display Profile Data, to use Javascript core functions.

The bug is an Heisenbug since it uses free'd memory. So it's hard to find an easy procedure to reproduce it...

Comment 3

[Victor STINNER]

I tried to reproduce the bug on Windows XP SP2 with Firefox 2.0.11 and Vekman 0.9.87.2, but it never crashed. The bug is specific to Linux or it's harder to reproduce it on Windows.

So, bug workaround: use Windows instead of Linux ;-)

Comment 4

[timeless]

jsd_ClearAllProfileData seems /relatively/ sound. it calls lock scripts.
 290 jsd_ClearAllProfileData(JSDContext* jsdc)
 294     JSD_LOCK_SCRIPTS(jsdc);
 298         jsd_ClearScriptProfileData(jsdc, current);
 302     JSD_UNLOCK_SCRIPTS(jsdc);

however, the code to get script profile data doesn't seem to call lock scripts.
 140         JSD_UNLOCK_SCRIPTS(jsdc);
 147                 pdata = jsd_GetScriptProfileData (jsdc, jsdscript);

 290 jsd_GetScriptProfileData(JSDContext* jsdc, JSDScript *script)
 292     if (!script->profileData)
 293         script->profileData = (JSDProfileData*)calloc(1, sizeof(JSDProfileData));
 295     return script->profileData;

i think this:
 147                 pdata = jsd_GetScriptProfileData (jsdc, jsdscript);

needs to be done while holding a lock.

while we're at it,

 238 JSD_ClearScriptProfileData(JSDContext* jsdc, JSDScript *script)
 241     jsd_ClearScriptProfileData(jsdc, script);

that's a public function which isn't holding a lock.

 116 JSD_ClearAllProfileData(JSDContext *jsdc)
 119     jsd_ClearAllProfileData(jsdc);    
this is the public function exercised by venkman (see below).

ClearProfileData
    * js/jsd/jsd_xpc.cpp, line 1360 (View change log or Blame annotations)
    * js/jsd/jsd_xpc.cpp, line 2740 (View change log or Blame annotations) 
Defined as a function prototype in:
    * js/jsd/idl/jsdIDebuggerService.idl, line 236 (View change log or Blame annotations)
    * js/jsd/idl/jsdIDebuggerService.idl, line 790 (View change log or Blame annotations) 
Referenced (in 2 files total) in:
    * js/jsd/jsd_xpc.cpp (View change log or Blame annotations)
          o line 1361
          o line 2741 
    * extensions/venkman/resources/content/venkman-commands.js (View change log or Blame annotations)
          o line 621
          o line 625
          o line 629 

summary:
_callHook needs to JSD_LOCK_SCRIPTS for its call to jsd_GetScriptProfileData
JSD_ClearScriptProfileData needs to JSD_LOCK_SCRIPTS for its call to jsd_ClearScriptProfileData.

Comment 5

[:Gijs (he/him)]

Attachment: Possible patch

Like this?

I'll run a tryserver build with this patch and post URLs for the reporter to try when I have them.

Comment 6

[:Gijs (he/him)]

Egh, looks like the tryserver builds aren't going to work out until tomorrow night. Sorry! In the meantime you can build on your own, of course, if you feel up to it. :-)

Comment 7

[timeless]

Comment on attachment 302321 [details] [diff] [review]
Possible patch

yeah, this is what i had in mind, but when i looked at the code, i found that it's not really sufficient to make jsd not broken. it might be sufficient to fix this crash.

I have a much bigger (yet unfortunately incomplete changeset) on my computer.

Comment 8

[:Gijs (he/him)]

(In reply to comment #7)
> (From update of attachment 302321 [details] [diff] [review])
> yeah, this is what i had in mind, but when i looked at the code, i found that
> it's not really sufficient to make jsd not broken. it might be sufficient to
> fix this crash.
> 
> I have a much bigger (yet unfortunately incomplete changeset) on my computer.
> 

OK, so should I still check this in, or obsolete and wait for yours?

Comment 9

[:Gijs (he/him)]

(In reply to comment #6)
> Egh, looks like the tryserver builds aren't going to work out until tomorrow
> night. Sorry! In the meantime you can build on your own, of course, if you feel
> up to it. :-)
> 

TryServer builds should now appear in a couple of hours in https://build.mozilla.org/tryserver-builds/?C=M;O=D (in a subdir which has my committer ID ([e-mail]) in it). I'd give a direct link, but they're not there yet and I'm going to catch some sleep, so. :-)

Comment 10

[:Gijs (he/him)]

Reassigning per previous comments.

Comment 11

[John J. Barton]

I'm not sure what the state is here, but does not seem to involve Firebug, removing priority

Comment 12

[Phil Ringnalda (:philor)]

The state is that assigning to timeless without also nagging him on IRC every time he seems to be goofing off when he could be patching jsd is the same as not assigning.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Closing because no crash reported since 12 weeks.