56262 - crash on "Edit as New" from draft: 32bit value cast to ptr

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P3)

Description

[harbaugh]

When attempting "Edit as New" on a draft in Messenger, mozilla crashes
with the following stack trace:

(ladebug) tstack
Stack trace for thread 1
>0  0x3ff805a7d68 in __nxm_thread_kill(0x3ffc0183918, 0xb, 0x1, 0x0, 0x0, 0x0)
in /usr/shlib/libpthread.so
#1  0x3ff8058d8f8 in pthread_kill(0x3ffc0183918, 0xb, 0x1, 0x0, 0x0, 0x0) in
/usr/shlib/libpthread.so
#2  0x3ff8057718c in UnknownProcedure3FromFile92(0x3ffc0183918, 0xb, 0x1, 0x0,
0x0, 0x0) in /usr/shlib/libpthread.so
#3  0x3ff807b22d8 in UnknownProcedure4FromFile1(0x3ffc0183918, 0xb, 0x1, 0x0,
0x0, 0x0) in /usr/shlib/libexc.so
#4  0x3ff807b3824 in UnknownProcedure17FromFile1(0x3ffc0183918, 0xb, 0x1, 0x0,
0x0, 0x0) in /usr/shlib/libexc.so
#5  0x3ff807b3864 in exc_unwind(0x3ffc0183918, 0xb, 0x1, 0x0, 0x0, 0x0) in
/usr/shlib/libexc.so
#6  0x3ff807b3af0 in exc_raise_signal_exception(0x3ffc0183918, 0xb, 0x1, 0x0,
0x0, 0x0) in /usr/shlib/libexc.so
#7  0x3ff8058f5f0 in UnknownProcedure10FromFile104(0x3ffc0183918, 0xb, 0x1, 0x0,
0x0, 0x0) in /usr/shlib/libpthread.so
#8  0x3ff81487730 in __sigtramp(0x3ffc0183918, 0xb, 0x1, 0x0, 0x0, 0x0) in
/usr/shlib/libc.so
#9  0x3ffbe859eb8 in
((nsMsgCompFields*)0x142f6fa00)->nsMsgCompFields::Copy(pMsgCompFields=0x4334f800)
"nsMsgCompFields.cpp":110
#10 0x3ffbe8b2298 in
((nsMsgCompose*)0x142f6f900)->nsMsgCompose::CreateMessage(originalMsgURI=0x0,
type=9, format=2, compFields=0x4334f800) "nsMsgCompose.cpp":994
#11 0x3ffbe8b0198 in
((nsMsgCompose*)0x142f6f900)->nsMsgCompose::Initialize(aWindow=0x142c9dc08,
originalMsgURI=0x0, type=9, format=2, compFields=0x4334f800,
identity=0x1417dae30) "nsMsgCompose.cpp":439
#12 0x3ffbe8a680c in
((nsMsgComposeService*)0x141304280)->nsMsgComposeService::InitCompose(aWindow=0x142c9dc08,
originalMsgURI=0x0, type=9, format=2, compFieldsAddr=1127544832,
identity=0x1417dae30, _retval=0x11fffdb80) "nsMsgComposeService.cpp":327
#13 0x3ffbff5f81c in XPTC_InvokeByIndex() "xptcinvoke_asm_osf1_alpha.s":73
#14 0x3ffbfe05814 in
((nsXPCWrappedNativeClass*)0x1412c9820)->nsXPCWrappedNativeClass::CallWrappedMethod(cx=0x142db6b00,
wrapper=0x1427bf280, desc=0x141304680, callMode=CALL_METHOD, argc=6,
argv=0x142f7a300, vp=0x11fffdd30) "xpcwrappednativeclass.cpp":912
#15 0x3ffbfe082c8 in WrappedNative_CallMethod(cx=0x142db6b00, obj=0x142b84070,
argc=6, argv=0x142f7a300, vp=0x11fffdd30) "xpcwrappednativejsops.cpp":226
#16 0x3ffbffb52d8 in js_Invoke() "jsinterp.c":820
#17 0x3ffbffbf2b0 in js_Interpret(cx=0x142db6b00, result=0x11fffe048)
"jsinterp.c":2621
#18 0x3ffbffb5354 in js_Invoke() "jsinterp.c":837
#19 0x3ffbffb5650 in js_InternalInvoke(rval=0x11fffe350) "jsinterp.c":909
#20 0x3ffbff8fdc8 in JS_CallFunctionValue() "jsapi.c":3193
#21 0x3ffbf57f134 in
((nsJSContext*)0x142b63100)->nsJSContext::CallEventHandler(aTarget=0x141767a50,
aHandler=0x141457980, argc=1, argv=0x11fffe3b8, aBoolResult=0x11fffe390,
aReverseReturnResult=0) "nsJSEnvironment.cpp":906
#22 0x3ffbf5f4f70 in
((nsJSEventListener*)0x142c7a6e0)->nsJSEventListener::HandleEvent(aEvent=0x142e6c788)
"nsJSEventListener.cpp":149
#23 0x30003acffc4 in
((nsEventListenerManager*)0x1431f2180)->nsEventListenerManager::HandleEventSubType(aListenerStruct=0x142a6b940,
aDOMEvent=0x142e6c788, aCurrentTarget=0x142c9dc20, aSubType=1, aPhaseFlags=7)
"nsEventListenerManager.cpp":788
#24 0x30003ad14fc in
((nsEventListenerManager*)0x1431f2180)->nsEventListenerManager::HandleEvent(aPresContext=0x142c0f880,
aEvent=0x11fffe9d8, aDOMEvent=0x11fffe900, aCurrentTarget=0x142c9dc20, aFlags=7,
aEventStatus=0x11fffe9c0) "nsEventListenerManager.cpp":1365
#25 0x3ffbf5a4194 in
((GlobalWindowImpl*)0x142c9dc00)->GlobalWindowImpl::HandleDOMEvent(aPresContext=0x142c0f880,
aEvent=0x11fffe9d8, aDOMEvent=0x11fffe900, aFlags=1, aEventStatus=0x11fffe9c0)
"nsGlobalWindow.cpp":499
#26 0x30003fb5218 in
((DocumentViewerImpl*)0x14194aa80)->DocumentViewerImpl::LoadComplete(aStatus=0)
"nsDocumentViewer.cpp":668
#27 0x3ffbf208174 in
((nsWebShell*)0x142bdaf00)->nsWebShell::OnEndDocumentLoad(loader=0x142e77f00,
channel=0x142ca4c30, aStatus=0) "nsWebShell.cpp":951
#28 0x3ffbfafa900 in
((nsDocLoaderImpl*)0x142e77f00)->nsDocLoaderImpl::FireOnEndDocumentLoad(aLoadInitiator=0x142e77f00,
aDocChannel=0x142ca4c30, aStatus=0) "nsDocLoader.cpp":804
#29 0x3ffbfafa260 in
((nsDocLoaderImpl*)0x142e77f00)->nsDocLoaderImpl::DocLoaderIsEmpty(aStatus=0)
"nsDocLoader.cpp":610
#30 0x3ffbfafa274 in
((nsDocLoaderImpl*)0x142cec000)->nsDocLoaderImpl::DocLoaderIsEmpty(aStatus=0)
"nsDocLoader.cpp":613
#31 0x3ffbfafa07c in
((nsDocLoaderImpl*)0x142cec000)->nsDocLoaderImpl::OnStopRequest(aChannel=0x143221fc0,
aCtxt=0x0, aStatus=0, aMsg=0x3ffbfe7efe8) "nsDocLoader.cpp":552
#32 0x3ffbfc1ee58 in
((nsLoadGroup*)0x143368b00)->nsLoadGroup::RemoveChannel(channel=0x143221fc0,
ctxt=0x0, aStatus=0, aStatusArg=0x3ffbfe7efe8) "nsLoadGroup.cpp":566
#33 0x3ffbfc23320 in
((nsStreamIOChannel*)0x143221fc0)->nsStreamIOChannel::OnStopRequest(transportChannel=0x14188d000,
context=0x0, aStatus=0, aStatusArg=0x3ffbfe7efe8) "nsInputStreamChannel.cpp":624
#34 0x3ffbfbf4ed4 in
((nsOnStopRequestEvent*)0x143067c40)->nsOnStopRequestEvent::HandleEvent()
"nsAsyncStreamListener.cpp":301
#35 0x3ffbfbf40f8 in nsStreamListenerEvent::HandlePLEvent(aEvent=0x14322ba20)
"nsAsyncStreamListener.cpp":97

On line #9 of the trace, the argument passed to nsMsgCompFields::Copy() is 
supposed to be a pointer, but the actual value passed
[pMsgCompFields=0x4334f800] is
not a valid 64bit pointer.

It turns out that the problem originates in 

   nsMsgComposeService::InitCompose()

where the argument 'compFieldsPtr' as a 32BIT VALUE, which is then CAST to a
pointer on line 326 of nsMsgComposeService.cpp.

The propogation of this invalid pointer down to nsMsgCompFields::Copy()
causes the crash at the dereference on line 110 of nsMsgCompFields.cpp

The solution is to change the argument 'compFieldsPtr' to a 64bit type,
but I don't know how to implement the patch since 'xpcwrappednativejsops.cpp'
and 'xpcwrappednativeclass.cpp' handle setting up the argment list.

How should this change be implemented?

Comment 1

[Jean-Francois Ducarroz]

Let me look at it...

Comment 2

[Jean-Francois Ducarroz]

I think the best way to solve this problem is to not pass anymore the address or
the compFields object but rather to store it temporary in the
nsMsgComposeService and pass a key to it that can be later use by the
InitCompose function to retrieve the real pointer. We should add a new queue
similar to the one use to store the nsMsgCompose object.

Comment 3

[Jean-Francois Ducarroz]

Attachment: Proposed fix

Comment 4

[Jean-Francois Ducarroz]

I have a fix, no more joggling with pointer address. harbaugh can you test it?

Comment 5

[harbaugh]

Yes, I can test it.

Comment 6

[pmock]

Jeff,

Is this problem specific to the mozilla build?  I could not reproduce problem on
commercial branch build.
 win32 commercial seamonkey build 2000-101213-mn6 installed on P500 Win98
 linux commercial seamonkey build 2000-101211-mn6 installed on P200 RedHat 6.2
 macos commercial seamonkey build 2000-101110-mn6 installed on G3/400 OS 9.04

Comment 7

[Jean-Francois Ducarroz]

It's only on 64bits processor like the DEC OSF/1

Comment 8

[pmock]

Thanks Jeff for the explaination.

Comment 9

[harbaugh]

The patch works.  :D

Comment 10

[Jean-Francois Ducarroz]

Thanks for testing it. Do you have permission to check in code in the tree? I won't be able to check it in myself for a 
while as I am under Netscape employees rule :-(

Comment 11

[harbaugh]

No, I don't have permission to check code into the tree.  How do I get
permission?

Comment 12

[Jean-Francois Ducarroz]

due to some modification with content handler, the original fix doesn't work
anymore with mailto url. I'll post soon a new fix...

Comment 13

[Jean-Francois Ducarroz]

I have a new fix. The change is that I had to create a new class to play the
role of mailto content handler instead of using nsMsgComposeService. This
because nsMsgComposeService is a service and therefore should be created as a
service and not as an instance like does the URI dispatcher.

Comment 14

[Jean-Francois Ducarroz]

Attachment: part of second fix. New file to add to the tree

Comment 15

[Jean-Francois Ducarroz]

Attachment: part of second fix. nsMsgComposeContentHandler.cpp to add to the tree

Comment 16

[Jean-Francois Ducarroz]

Attachment: second fix.

Comment 17

[Jean-Francois Ducarroz]

Fixed and checked in

Comment 18

[Blake Ross]

marking fixed...(?)

Comment 19

[Blake Ross]

er...

Comment 20

[Jean-Francois Ducarroz]

oops, thanks...

The solution that was checked in to fix this problem is different that the
original one proposed. Instead of continuing passsing the arguments as a string
which cause problem when passing object (identity and composeFields), I now pass
the argument using a nsISupports object (nsIMsgComposeParam) which contains all
the needed arguments

Comment 21

[[e-mail] (gone - use [e-mail] instead)]

verified. I haven't received any reports of this crashing again on DEC and it
worksforme using 2001042820 on win2k.