381315 - crash below nsINIParser::InitFromFILE() when I try to import Opera settings

Status: VERIFIED FIXED

(Core :: XPCOM, defect, P1)

Description

[(not reading, please use [e-mail] instead)]

crash below nsINIParser::InitFromFILE() when I try to import Opera settings

I found this while testing a fix for bug #381298.  

Note, firefox 2 doesn't crash when I import opera settings.  

Also note, a recent trunk optimize build (before places bookmarks was enabled) also demonstrates the crash.

Here's a stack from my "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070518 Minefield/3.0a5pre" trunk debug build.

 	ntdll.dll!7c918fea() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]	
 	xpcom_core.dll!nsAString_internal::~nsAString_internal()  Line 59	C++
 	brwsrcmp.dll!_nh_malloc_dbg(unsigned int nSize=0x000000c4, int nhFlag=0x00000000, int nBlockUse=0x00000001, const char * szFileName=0x00000000, int nLine=0x000002c4)  Line 266 + 0x15 bytes	C++
 	brwsrcmp.dll!malloc(unsigned int nSize=0x0129729b)  Line 152 + 0x15 bytes	C++
 	xpcom_core.dll!xptiFile::xptiFile()  Line 54 + 0x30 bytes	C++
 	brwsrcmp.dll!fseek(_iobuf * stream=0x10310c50, long offset=0x00000000, int whence=0x00000002)  Line 103 + 0x9 bytes	C
>	brwsrcmp.dll!nsINIParser::InitFromFILE(_iobuf * fd=0x10310c50)  Line 105 + 0xd bytes	C++
 	brwsrcmp.dll!nsINIParser::Init(nsILocalFile * aFile=0x04024758)  Line 79 + 0x11 bytes	C++
 	brwsrcmp.dll!nsOperaProfileMigrator::CopyPreferences(int aReplace=0x00000001)  Line 414 + 0x11 bytes	C++
 	brwsrcmp.dll!nsOperaProfileMigrator::Migrate(unsigned short aItems=0x0027, nsIProfileStartup * aStartup=0x0012fee0, const unsigned short * aProfile=0x03c54178)  Line 136 + 0x70 bytes	C++
 	xpcom_core.dll!NS_InvokeByIndex_P(nsISupports * that=0x0012e814, unsigned int methodIndex=0x0012e9f8, unsigned int paramCount=0x030a96a1, nsXPTCVariant * params=0x01366798)  Line 102	C++
 	xpc3250.dll!AutoJSSuspendRequest::SuspendRequest()  Line 3249 + 0xd bytes	C++
 	xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=CALL_METHOD)  Line 2245 + 0x1e bytes	C++
 	xpc3250.dll!XPC_WN_CallMethod(JSContext * cx=0x03e1d510, JSObject * obj=0x00f41b60, unsigned int argc=0x00000003, long * argv=0x040839c4, long * vp=0x0012eb18)  Line 1467 + 0xe bytes	C++
 	js3250.dll!js_Invoke(JSContext * cx=0x03e1d510, unsigned int argc=0x00000003, unsigned int flags=0x00000000)  Line 1332 + 0x20 bytes	C
 	js3250.dll!js_Interpret(JSContext * cx=0x03e1d510, unsigned char * pc=0x04081898, long * result=0x0012f1d0)  Line 4025 + 0xf bytes	C
 	js3250.dll!js_Invoke(JSContext * cx=0x03e1d510, unsigned int argc=0x00000002, unsigned int flags=0x00000002)  Line 1351 + 0x13 bytes	C
 	js3250.dll!js_InternalInvoke(JSContext * cx=0x03e1d510, JSObject * obj=0x03e2e820, long fval=0x03e802c0, unsigned int flags=0x00000000, unsigned int argc=0x00000002, long * argv=0x03b228d0, long * rval=0x0012f34c)  Line 1426 + 0x14 bytes	C
 	js3250.dll!JS_CallFunctionValue(JSContext * cx=0x03e1d510, JSObject * obj=0x03e2e820, long fval=0x03e802c0, unsigned int argc=0x00000002, long * argv=0x03b228d0, long * rval=0x0012f34c)  Line 4855 + 0x1f bytes	C
 	gklayout.dll!nsJSContext::CallEventHandler(nsISupports * aTarget=0x03fe2ca8, void * aScope=0x03e2e820, void * aHandler=0x03e802c0, nsIArray * aargv=0x03b2288c, nsIVariant * * arv=0x0012f408)  Line 1794 + 0x24 bytes	C++
 	gklayout.dll!nsGlobalWindow::RunTimeout(nsTimeout * aTimeout=0x03b22908)  Line 6841 + 0xab bytes	C++
 	gklayout.dll!nsGlobalWindow::TimerCallback(nsITimer * aTimer=0x03b0f878, void * aClosure=0x03b22908)  Line 7172	C++
 	xpcom_core.dll!nsTimerImpl::Fire()  Line 383 + 0x13 bytes	C++
 	xpcom_core.dll!nsTimerEvent::Run()  Line 458	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0012f560)  Line 483	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00bacf98, int mayWait=0x00000001)  Line 227 + 0x16 bytes	C++
 	appshell.dll!nsXULWindow::ShowModal()  Line 402 + 0xc bytes	C++
 	appshell.dll!nsContentTreeOwner::ShowAsModal()  Line 522	C++
 	embedcomponents.dll!nsWindowWatcher::OpenWindowJSInternal(nsIDOMWindow * aParent=0x00000000, const char * aUrl=0x01317adc, const char * aName=0x01317ad4, const char * aFeatures=0x01317aa8, int aDialog=0x00000001, nsIArray * argv=0x03dcf450, int aCalledFromJS=0x00000000, nsIDOMWindow * * _retval=0x0012fb30)  Line 898	C++
 	embedcomponents.dll!nsWindowWatcher::OpenWindow(nsIDOMWindow * aParent=0x00000000, const char * aUrl=0x01317adc, const char * aName=0x01317ad4, const char * aFeatures=0x01317aa8, nsISupports * aArguments=0x03dd0b60, nsIDOMWindow * * _retval=0x0012fb30)  Line 415 + 0x2b bytes	C++
 	brwsrcmp.dll!nsProfileMigrator::Migrate(nsIProfileStartup * aStartup=0x0012fee0)  Line 140 + 0x57 bytes	C++
 	xul.dll!XRE_main(int argc=0x00000001, char * * argv=0x00ba9710, const nsXREAppData * aAppData=0x004036e0)  Line 2706	C++
 	firefox.exe!main(int argc=0x00000001, char * * argv=0x00ba9710)  Line 65 + 0x13 bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 586 + 0x19 bytes	C
 	firefox.exe!mainCRTStartup()  Line 403	C
 	kernel32.dll!7c816fd7() 	
 	ntdll.dll!7c911dac()

Comment 1

[timeless]

seth: could you look into .symfix+ c:\symbols (or equivalently using it long enough to get symbols for ntdll and friends)

done right, this: 
        [Frames below may be incorrect and/or missing, no symbols loaded for
ntdll.dll] 

will go away. what's bothering me is this:
        xpcom_core.dll!xptiFile::xptiFile()  Line 54 + 0x30 bytes       C++

the xpcom code is bogus, openAnsiFileDescriptor isn't legal. It can't be safely used on windows because you get a random CRT which is not necessarily the one you have handy.

Comment 2

[(not reading, please use [e-mail] instead)]

I'm not certain I did the ".symfix+" thing right, but I manage to get this from WinDbg:


(c7c.1084): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=00000000 ebx=00000000 ecx=000005e8 edx=10310c80 esi=10310c70 edi=00000000
eip=7c918fea esp=0012c35c ebp=0012c3d0 iopl=0         nv up ei pl zr na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00210246
ntdll!RtlpWaitForCriticalSection+0x8c:
7c918fea ff4010          inc     dword ptr [eax+10h]  ds:0023:00000010=????????
0:000> k
ChildEBP RetAddr  
0012c3d0 7c90104b ntdll!RtlpWaitForCriticalSection+0x8c
*** WARNING: Unable to verify checksum for C:\builds\trunk-no-places\mozilla\ff-debug\dist\bin\components\brwsrcmp.dll
0012c3d8 0182727b ntdll!RtlEnterCriticalSection+0x46
0012c3e4 0181f4ea brwsrcmp!_lock_file+0x3b [f:\rtm\vctools\crt_bld\self_x86\crt\src\_file.c @ 238]
0012c428 018113ce brwsrcmp!fseek+0x11a [f:\rtm\vctools\crt_bld\self_x86\crt\src\fseek.c @ 103]
0012c474 018112a4 brwsrcmp!nsINIParser::InitFromFILE+0x2e [c:\builds\trunk-no-places\mozilla\xpcom\glue\nsiniparser.cpp @ 105]
0012c494 017d6320 brwsrcmp!nsINIParser::Init+0x64 [c:\builds\trunk-no-places\mozilla\xpcom\glue\nsiniparser.cpp @ 79]
0012c534 017d5456 brwsrcmp!nsOperaProfileMigrator::CopyPreferences+0x120 [c:\builds\trunk-no-places\mozilla\browser\components\migration\src\nsoperaprofilemigrator.cpp @ 414]
0012c5b4 00303b27 brwsrcmp!nsOperaProfileMigrator::Migrate+0x136 [c:\builds\trunk-no-places\mozilla\browser\components\migration\src\nsoperaprofilemigrator.cpp @ 136]
*** WARNING: Unable to verify checksum for C:\builds\trunk-no-places\mozilla\ff-debug\dist\bin\components\xpc3250.dll
0012c5d8 00ff96a1 xpcom_core!NS_InvokeByIndex_P+0x27 [c:\builds\trunk-no-places\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp @ 102]
0012c8ac 01004ee7 xpc3250!XPCWrappedNative::CallMethod+0xe41 [c:\builds\trunk-no-places\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp @ 2245]
0012c960 004cc91c xpc3250!XPC_WN_CallMethod+0x177 [c:\builds\trunk-no-places\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp @ 1467]
0012ca64 004da16b js3250!js_Invoke+0xb3c [c:\builds\trunk-no-places\mozilla\js\src\jsinterp.c @ 1332]
0012cf98 004cc98f js3250!js_Interpret+0xbafb [c:\builds\trunk-no-places\mozilla\js\src\jsinterp.c @ 4025]
0012d094 004cd4f8 js3250!js_Invoke+0xbaf [c:\builds\trunk-no-places\mozilla\js\src\jsinterp.c @ 1351]
0012d128 0048ab03 js3250!js_InternalInvoke+0x118 [c:\builds\trunk-no-places\mozilla\js\src\jsinterp.c @ 1426]
*** WARNING: Unable to verify checksum for C:\builds\trunk-no-places\mozilla\ff-debug\dist\bin\components\gklayout.dll
0012d150 02bf3ff9 js3250!JS_CallFunctionValue+0x23 [c:\builds\trunk-no-places\mozilla\js\src\jsapi.c @ 4855]
0012d214 02be5061 gklayout!nsJSContext::CallEventHandler+0x409 [c:\builds\trunk-no-places\mozilla\dom\src\base\nsjsenvironment.cpp @ 1794]
0012d354 02be5ae8 gklayout!nsGlobalWindow::RunTimeout+0x611 [c:\builds\trunk-no-places\mozilla\dom\src\base\nsglobalwindow.cpp @ 6841]
0012d364 002f15b3 gklayout!nsGlobalWindow::TimerCallback+0x28 [c:\builds\trunk-no-places\mozilla\dom\src\base\nsglobalwindow.cpp @ 7172]
0012d3b0 002f1711 xpcom_core!nsTimerImpl::Fire+0x233 [c:\builds\trunk-no-places\mozilla\xpcom\threads\nstimerimpl.cpp @ 383]

Comment 3

[Benjamin Smedberg]

This is caused by a mismatched CRT. We should be using NSPRFileDesc or raw pathnames or something. I'll take a look.

Comment 4

[Benjamin Smedberg]

Yeah, so this is an xpcom bug of sorts.

Comment 5

[Benjamin Smedberg]

Attachment: Use local FILE*

Comment 6

[Doug Turner (:dougt)]

oh, this is bad.  It is the case that calling openAnsiFileDescriptor from one piece of code may result in a FILE* from a different CRT then the calling code is using.


Do we need to fix the other cases:
http://lxr.mozilla.org/mozilla1.8/search?string=OpenANSIFileDesc

Also we should document the IDL -- warning the developer this is a problem

Comment 7

[Benjamin Smedberg]

Yes we should deprecate the API and add warnings. It's not such a big deal on the branches because pretty much everyone shares the same CRT (only embedders who use a different compiler or statically link the CRT would be affected).

Comment 8

[Doug Turner (:dougt)]

r= on the patch above (why is it a application/octet-stream)?

bsmedberg, do you want to create a comment for nsILocalFile.idl?

Comment 9

[Benjamin Smedberg]

Fixed on trunk. I filed 382008 on the meta-issue of it being a bad API.

Comment 10

[(not reading, please use [e-mail] instead)]

after updating and rebuilding, I no longer crasher when importing from opera.

thanks for the quick fix, benjamin!

Comment 11

[Doug Turner (:dougt)]

Comment on attachment 266063 [details] [diff] [review]
Use local FILE*

(r+ previously in comments, just getting out of my review queue)