Last Comment Bug 387572 - Invalid free deserializing JS component script
: Invalid free deserializing JS component script
Status: RESOLVED FIXED
: fixed1.8.1.8
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: x86 Linux
: -- blocker (vote)
: ---
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks: 379491
  Show dependency treegraph
 
Reported: 2007-07-10 11:39 PDT by Boris Zbarsky [:bz]
Modified: 2011-06-11 06:46 PDT (History)
6 users (show)
bzbarsky: blocking1.9?
jwalden+bmo: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
This should fix it (3.59 KB, patch)
2007-07-10 11:50 PDT, Boris Zbarsky [:bz]
brendan: review+
brendan: superreview+
dveditz: approval1.8.1.8+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2007-07-10 11:39:28 PDT
This is related to bug 379491.  Might even be the same.

valgrind is telling me:

==5993== Invalid free() / delete / delete[]
==5993==    at 0x400525D: free (vg_replace_malloc.c:233)
==5993==    by 0x41088C9: free (nsTraceMalloc.c:1813)
==5993==    by 0x470CBA2: PR_Free (prmem.c:490)
==5993==    by 0x469EDC1: NS_Free_P (nsMemoryImpl.cpp:303)
==5993==    by 0x4F6126B: nsMemory::Free(void*) (nsMemory.h:74)
==5993==    by 0x4FC9F62: ReadScriptFromStream(JSContext*, nsIObjectInputStream*, JSScript**) (mozJSComponentLoader.cpp:406)
==5993==    by 0x4FCD245: mozJSComponentLoader::ReadScript(nsIFastLoadService*, char const*, nsIURI*, JSContext*, JSScript**) (mozJSComponentLoader.cpp:974)
==5993==    by 0x4FCE028: mozJSComponentLoader::GlobalForLocation(nsILocalFile*, JSObject**, char**) (mozJSComponentLoader.cpp:1114)
==5993==    by 0x4FCB4C8: mozJSComponentLoader::LoadModule(nsILocalFile*, nsIModule**) (mozJSComponentLoader.cpp:598)
==5993==    by 0x46853CE: nsFactoryEntry::GetFactory(nsIFactory**) (nsComponentManager.cpp:3577)
==5993==    by 0x4680643: nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) (nsComponentManager.cpp:1795)
==5993==    by 0x460FAA1: CallCreateInstance(char const*, nsISupports*, nsID const&, void**) (nsComponentManagerUtils.cpp:167)
==5993==  Address 0x66AAE58 is 0 bytes inside a block of size 824 free'd
==5993==    at 0x400525D: free (vg_replace_malloc.c:233)
==5993==    by 0x41088C9: free (nsTraceMalloc.c:1813)
==5993==    by 0x470CBA2: PR_Free (prmem.c:490)
==5993==    by 0x469EDC1: NS_Free_P (nsMemoryImpl.cpp:303)
==5993==    by 0x5129131: nsMemory::Free(void*) (nsMemory.h:74)
==5993==    by 0x512BB28: nsTranscodeJSPrincipals(JSXDRState*, JSPrincipals**) (nsJSPrincipals.cpp:152)
==5993==    by 0x45483B0: js_XDRScript (jsscript.c:674)
==5993==    by 0x44E2CD7: fun_xdrObject (jsfun.c:1452)
==5993==    by 0x4516609: js_XDRObject (jsobj.c:4730)
==5993==    by 0x4554A61: XDRValueBody (jsxdrapi.c:560)
==5993==    by 0x4554CB9: js_XDRAtom (jsxdrapi.c:635)
==5993==    by 0x4547DD2: XDRAtomMap (jsscript.c:488)

it looks like the failure to deserialize a principal (which might need its own bug) causes JS_XDRScript to return false under ReadScriptFromStream.  So we never reset |data| to whatever is in the xdr, andthen free it.  Except it's already been freed by the nsTranscodeJSPrincipals code.

This is basically blocking debugging for me, because my build simply won't start.
Comment 1 Boris Zbarsky [:bz] 2007-07-10 11:50:22 PDT
Created attachment 271705 [details] [diff] [review]
This should fix it
Comment 2 Brendan Eich [:brendan] 2007-07-10 11:55:39 PDT
Comment on attachment 271705 [details] [diff] [review]
This should fix it

Shaver and I were r+sr tweedle-dee and -dum (I was -dum) on bryner's original patch. Sigh.

/be
Comment 3 Boris Zbarsky [:bz] 2007-07-10 20:10:53 PDT
Fixed.
Comment 4 Boris Zbarsky [:bz] 2007-07-10 20:17:58 PDT
Comment on attachment 271705 [details] [diff] [review]
This should fix it

I think it's worth taking this on branch.  This is a fix to properly clean up when fastload fails for some reason; the main benefit is avoiding the double-free.  I doubt this happens much on branch, but just to be safe....

I think this is a very safe fix.
Comment 5 Daniel Veditz [:dveditz] 2007-08-29 16:09:38 PDT
Comment on attachment 271705 [details] [diff] [review]
This should fix it

approved for 1.8.1.7, a=dveditz for release-drivers
Comment 6 Boris Zbarsky [:bz] 2007-08-30 08:57:14 PDT
Fixed on branch.

Note You need to log in before you can comment on or make changes to this bug.