238219 - nsIDocument changes cause Adobe SVG plugin to crash [was: crash [@ nsCSubstring::SetCapacity]]

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[chris hofmann]

#8 top crash in early 1.7b data

not much to do on here, but could it be related to string defrag landing?

notice svg on the stack too.  might need it to understand the bug better.

     Count   Offset    Real Signature
[ 3   nsCSubstring::SetCapacity 6d2d286b -
[ 1   nsCSubstring::SetCapacity ee83ea6e - 
[ 1   nsCSubstring::SetCapacity 5db5f36b - 

 
     Crash date range: 19-MAR-04 to 21-MAR-04
     Min/Max Seconds since last crash: 27 - 4461
     Min/Max Runtime: 418 - 4461
 
     Count   Platform List 
     3   [Windows NT 5.1 build 2600]   
     1   [Windows NT 5.0 build 2195]   
     1   [Windows 98 4.90 build 73010104]   
 
     Count   Build Id List 
     5   2004031615
 
     No of Unique Users         3
 
 Stack trace(Frame) 

	 nsCSubstring::SetCapacity
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpcom/string/src/nsTSubstring.cpp
 line 445] 
	 nsCSubstring::SetLength
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpcom/string/src/nsTSubstring.cpp
 line 481] 
	 nsCSubstring::Assign
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpcom/string/src/nsTSubstring.cpp
 line 278] 
	 nsCSubstring::Assign
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpcom/string/src/nsTSubstring.cpp
 line 330] 
	 nsCSubstring::Assign
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpcom/string/src/nsTSubstring.cpp
 line 358] 
	 nsDocument::SetDocumentCharacterSet
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/content/base/src/nsDocument.cpp
 line 963] 
	 NPSVG3.dll + 0x4194 (0x53004194)  
	 NPSVG3.dll + 0x3c23 (0x53003c23)  
	 NPSVG3.dll + 0x5058 (0x53005058)  
	 nsPluginStreamListenerPeer::SetUpStreamListener
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp
 line 2456] 
	 nsPluginStreamListenerPeer::OnStartRequest
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/modules/plugin/base/src/nsPluginHostImpl.cpp
 line 2101] 
	 nsHttpChannel::CallOnStartRequest
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp
 line 638] 
	 nsHttpChannel::ProcessNormal
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp
 line 767] 
	 nsHttpChannel::ProcessResponse
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp
 line 722] 
	 nsHttpChannel::OnStartRequest
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp
 line 3315] 
	 nsInputStreamPump::OnStateStart
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/netwerk/base/src/nsInputStreamPump.cpp
 line 381] 
	 nsInputStreamPump::OnInputStreamReady
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/netwerk/base/src/nsInputStreamPump.cpp
 line 343] 
	 nsInputStreamReadyEvent::EventHandler
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpcom/io/nsStreamUtils.cpp
 line 119] 
	 PL_HandleEvent
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpcom/threads/plevent.c
 line 672] 
	 PL_ProcessPendingEvents
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpcom/threads/plevent.c
 line 610] 
	 _md_EventReceiverProc
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpcom/threads/plevent.c
 line 1413] 
	 USER32.dll + 0x3d79 (0x77cf3d79)  
	 USER32.dll + 0x3ddf (0x77cf3ddf)  
	 nsAppShellService::Run
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpfe/appshell/src/nsAppShellService.cpp
 line 524] 
	 main1
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp
 line 1308] 
	 main
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp
 line 1712] 
	 WinMain
[c:/builds/tinderbox/Mozilla1.7b/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp
 line 1734] 
	 WinMainCRTStartup()  
	 kernel32.dll + 0x214c7 (0x77e414c7)

Comment 1

[Darin Fisher]

hmm... the stack trace indicates that this is the code path:

   void
   nsTSubstring_CharT::Assign( const char_type* data, size_type length )
     {
       // unfortunately, some callers pass null :-(
       if (!data)
         {
=>         Truncate();
           return;
         }
                                                                                
but, the stack trace also indicates that we entered the string code here:

   void
   nsTSubstring_CharT::Assign( const abstract_string_type& readable )
     {
       // promote to string if possible to take advantage of sharing
       if (readable.mVTable == nsTObsoleteAString_CharT::sCanonicalVTable)
         Assign(*readable.AsSubstring());
       else
=>       Assign(readable.ToSubstring());
     }

and found that the |readable| abstract string was not implemented internally,
which makes sense since this plugin probably linked in its own copy of the
string code.

what's odd here is that the stack trace indicates that GetReadableFragment on
the abstract string returned a fragment with a null valued data pointer.  that
shouldn't have ever happened in the old string code, so i think something else
is going on.

i'll need to test out the plugin for myself to learn more.

Comment 2

[Darin Fisher]

i'm able to reproduce a crash with the latest SVG (Version 3) plugin from
adobe's website.  the crash occurs when i try to load any SVG document.

Comment 3

[R.K.Aa.]

An Adobe SVG plugin crash is in popular bug 133567 - it used to be in the
release notes.

Comment 4

[Darin Fisher]

well, it appears from the comments in that bug that this latest SVG plugin
should work with mozilla.

Comment 5

[Darin Fisher]

sweetness!  i have this exact crash in the debugger right now...

Comment 6

[Darin Fisher]

Ok, this is fallout from nsIDocument deCOMtamination.  in fact, i think bug
222134 was the one that changed nsIDocument sufficiently to break the Adobe SVG
plugin.

in the case of this bug, the Adobe SVG plugin was not written to call
SetDocumentCharacterSet here.  since the vtable for nsIDocument changed, the
Adobe SVG plugin is no longer valid.

this is the result of Adobe using non-frozen gecko APIs.

if we rev NS_IDOCUMENT_IID that might be enough to prevent this crash.  i
haven't confirmed yet, but i think that if we've learned anything from the past
it is that we should always rev IIDs when we make interface changes.

Comment 7

[Darin Fisher]

changing NS_IDOCUMENT_IID solved the crash, and the Adobe SVG seems to be
functional.  i'm not sure what part isn't working (presuming that it needed
nsIDocument for something).

Comment 8

[Darin Fisher]

Attachment: v1 patch

Comment 9

[Darin Fisher]

i presume other deCOMtaminated interfaces need to be similarly updated.  anyone
have a list of updated interfaces?

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 144494 [details] [diff] [review]
v1 patch

I don't have a list of all interfaces that have been changed... But I bet
nsIScriptContext and nsIScriptGlobalObject need the same treatment here. And
maybe nsIContent too.

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Definitly nsIContent, nsIHTMLContent and nsIStyledContent too. Maybe nsINodeInfo
and nsINodeInfoManager too.

Comment 12

[Brendan Eich [:brendan]]

shaver had this crazy idea to extend xpidl to generate uuids for unfrozen
interfaces by hashing the signatures, noticing changes.  We could use some tool
support in enforcing, or warning, about unfrozen API usage -- and in enforcing
frozen API UUID invariance.  Thoughts?

Is someone going to file bugs on the other interfaces?

/be

Comment 13

[chris hofmann]

should we get "team deComification" together to brainstorm what areas need to
look at.  how about 4:00p friday before the cantina?

Comment 14

[Darin Fisher]

patch checked in for 1.7 final

marking fixed, filing a new bug for similar work.