M1RC3 crash [@ nsCharTraits<unsigned short>::length]

VERIFIED FIXED in mozilla1.0.1

Status

()

--
critical
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: ngh114, Assigned: jag-mozilla)

Tracking

(4 keywords)

Trunk
mozilla1.0.1
x86
Windows 2000
crash, intl, qawanted, topcrash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2 RTM], crash signature)

Attachments

(3 attachments)

(Reporter)

Description

17 years ago
i've tried to reproduce the crash using rc3 and user comments as a guide but was
unsuccessful. below is the latest info from talkback:

     Count   Offset    Real Signature
[ 4   nsCharTraits<unsigned short>::length 00a06a8e - nsCharTraits<unsigned
short>::length ]
[ 1   nsCharTraits<unsigned short>::length e1d36f92 - nsCharTraits<unsigned
short>::length ]
 
     Crash date range: 2002-05-24 to 2002-05-27
     Min/Max Seconds since last crash: 2010 - 84541
     Min/Max Runtime: 7428 - 84541
     Keyword List :  
     Count   Platform List 
     4   Windows NT 5.1 build 2600
     1   Windows NT 5.0 build 2195
 
     Count   Build Id List 
     5   2002052308
 
     No of Unique Users         4
 
 Stack trace(Frame) 

	 nsCharTraits<unsigned short>::length	[..\..\dist/include/string\nsCharTraits.h  line 201] 
	 nsDependentString::Rebind
[..\..\dist/include/string\nsDependentString.h  line 60] 
	 nsAString::do_AppendFromElementPtr
[d:\builds\seamonkey\mozilla\string\src\nsAString.cpp  line 360] 
	 nsAutoString::nsAutoString
[d:\builds\seamonkey\mozilla\string\obsolete\nsString2.cpp  line 1202] 
	 nsMetaCharsetObserver::Notify
[d:\builds\seamonkey\mozilla\intl\chardet\src\nsMetaCharsetObserver.cpp  line 280] 
	 nsMetaCharsetObserver::Notify
[d:\builds\seamonkey\mozilla\intl\chardet\src\nsMetaCharsetObserver.cpp  line 165] 
	 nsObserverEntry::Notify
[d:\builds\seamonkey\mozilla\htmlparser\src\nsDTDUtils.cpp  line 1578] 
	 HTMLContentSink::NotifyTagObservers
[d:\builds\seamonkey\mozilla\content\html\document\src\nsHTMLContentSink.cpp 
line 3953] 
	 CNavDTD::WillHandleStartTag
[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp  line 1400] 
	 CNavDTD::HandleStartToken
[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp  line 1649] 
	 CNavDTD::HandleToken
[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp  line 908] 
	 CNavDTD::BuildModel
[d:\builds\seamonkey\mozilla\htmlparser\src\CNavDTD.cpp  line 521] 
	 nsParser::BuildModel
[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp  line 1869] 
	 nsParser::ResumeParse
[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp  line 1733] 
	 nsParser::OnDataAvailable
[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp  line 2369] 
	 nsDocumentOpenInfo::OnDataAvailable
[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp  line 245] 
	 nsStreamListenerTee::OnDataAvailable
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamListenerTee.cpp  line 98] 
	 nsHttpChannel::OnDataAvailable
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp  line
2965] 
	 nsOnDataAvailableEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamListenerProxy.cpp  line 203] 
	 PL_HandleEvent
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line 597] 
	 PL_ProcessPendingEvents
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line 530] 
	 _md_EventReceiverProc
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line 1078] 
	 USER32.dll + 0x3c076 (0x77d7c076)  
	 USER32.dll + 0x3c076 (0x77d7c076)  
	 _except_handler3()  
	 kernel32.dll + 0x3bb86 (0x77e9bb86)   
 
     (6693358)	URL: http://www.nytimes.com
     (6693358)	Comments: I had several tabs open and I clicked on a new link inside one of
the tabbed windows to load a new page.
     (6685112)	URL: freshmeat.net
     (6685112)	Comments: Opening URL  from freshmeat.net causes crash
(Reporter)

Updated

17 years ago
Keywords: crash, qawanted, topcrash

Comment 1

17 years ago
.
Assignee: jaggernaut → yokoyama
Component: String → Internationalization
QA Contact: scc → ruixu
Summary: M1RC3 crash [@ nsCharTraits<unsigned ] → M1RC3 crash [@ nsCharTraits<unsigned short>::length]
(Reporter)

Comment 2

17 years ago
Created attachment 85336 [details]
another stack trace for [@ nsCharTraits<unsigned ]
(Assignee)

Comment 3

17 years ago
nsCharTraits<PRunichar>::length(ptr) can crash for a few reasons.

One is that |ptr| is |0|, another is that |ptr| points to memory we're not
allowed to read. In this case I suspect it's the latter that's happening.

279              nsAutoString contentPart1(contentValue+10); // after
"text/html;"

Except the test above is for "text/html" so +10 may put it right after the
null-terminator. I suspect changing it to +9 will fix this. Patch coming up.
(Assignee)

Comment 4

17 years ago
Created attachment 85389 [details] [diff] [review]
Fix

Untested patch, the second chunk is the fix, the first chunk is a
simplification of the test.
(Assignee)

Comment 5

17 years ago
Nominating for nsbeta1.
Keywords: nsbeta1

Comment 6

17 years ago
while i don't think this patch is correct, i don't believe the original code 
was either (ignoring crashes).

offhand, i've seen this crumy code before, and i'm pretty sure i've complained 
about it before.

fwiw http://www.ietf.org/rfc/rfc2045.txt specifies content type rules http://
www.ietf.org/rfc/rfc2234.txt describes abnf
section 3.1 covers 'LINEAR WHITE SPACE:'
so what we should do is RFind, and then verify that the preceding character is 
a space, semicolon or tab.
Keywords: nsbeta1

Comment 7

17 years ago
examples of inputs that the current and proposed code don't handle correctly:
'text/html; charset=LATIN-1; foocharset=US-ASCII'
in fact, we should probably tokenize around ';' or scan forward for it.

Updated

17 years ago
Keywords: intl
QA Contact: ruixu → ylong
(Assignee)

Comment 8

17 years ago
This patch fixes the crash, and throws in a simplification of code for free. No,
it doesn't make Mozilla perfect, but I never said it would do that. Yes, we
should probably be a little smarter in the parsing, but right now the code works
with what the webpages provide, since there is no site out there that'll have a
"foocharset" (because there's no point in putting that in). File another bug on
the parsing if you must, leave this bug to address this crash.

Updated

16 years ago
Attachment #85389 - Flags: review+

Comment 9

16 years ago
Lots of instances of this crash showing up for nsCharTraits::length in each 
of the mozilla release candidates, and continuing on the branch and trunk.

If this patch is good, then let's move forward on landing it to trunk. 
Keywords: mozilla1.0.1, patch

Comment 10

16 years ago
Comment on attachment 85389 [details] [diff] [review]
Fix

sr=blake
Attachment #85389 - Flags: superreview+
Comment on attachment 85389 [details] [diff] [review]
Fix

r=dbaron.  If you want to be pedantic, the following is incorrect:

>+         Substring(httpEquivValue,
>+                   httpEquivValue+contenttype.Length()).Equals(contenttype,
>+                                                               nsCaseInsensitiveStringComparator())
>+         &&
>+         Substring(contentValue,
>+                   contentValue+texthtml.Length()).Equals(texthtml,
>+                                                          nsCaseInsensitiveStringComparator())

Since you don't know the lengths of contentValue or httpEquivValue, but you'll
probably get away without crashing.  It's worth testing with short strings,
though.
(Assignee)

Comment 12

16 years ago
Yeah, I saw that when I read the code. It's ugly, but it works as is because
contentValue and httpEquivValue are null terminated, which'll guarantee we won't
try to read beyond the null (since we know that the strings we're comparing
against don't have embedded nulls).
(Assignee)

Comment 13

16 years ago
Created attachment 88592 [details] [diff] [review]
Suggested fix for branch
(Assignee)

Comment 14

16 years ago
Comment on attachment 88592 [details] [diff] [review]
Suggested fix for branch

Carrying over r and sr since this is exactly the second hunk of the trunk patch
Attachment #88592 - Flags: superreview+
Attachment #88592 - Flags: review+
(Assignee)

Comment 15

16 years ago
Taking bug. Adding nsbeta1 and adt1.0.1, I think we really want the small fix on 
the branch, if it indeed fixes this crash (I'm pretty sure it does).

Since we don't have a testcase, let's see if the fix on the trunk removes this 
stack from trunk talkback data. John, can you keep an eye on that?
Assignee: yokoyama → jaggernaut
Keywords: adt1.0.1, nsbeta1
(Assignee)

Comment 16

16 years ago
And marking fixed since I checked it in on the trunk today.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 17

16 years ago
will do.

Updated

16 years ago
Blocks: 143047
Whiteboard: [adt2 RTM]
Target Milestone: --- → mozilla1.0.1

Comment 18

16 years ago
Change QA contact to jrgm@netscape.com per comment #15, cause there is not way
for me to verify it.
QA Contact: ylong → jrgm

Comment 19

16 years ago
adding adt1.0.1+.  Please get drivers approval and land before the 6/25 branch
build.
Keywords: adt1.0.1 → adt1.0.1+

Updated

16 years ago
Attachment #88592 - Flags: approval+

Comment 20

16 years ago
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+

Comment 21

16 years ago
Verified fixed for the trunk. No crashes reported for trunk builds
with this stack trace since 06/18, after 29 crashes reported 06/09 to
06/18 (3 per day).
Status: RESOLVED → VERIFIED
(Assignee)

Updated

16 years ago
Keywords: mozilla1.0.1+ → fixed1.0.1

Comment 22

16 years ago
*** Bug 154664 has been marked as a duplicate of this bug. ***

Comment 23

16 years ago
What is the signature to look at for the branch builds in talkback?
nsCharTraits<unsigned short>::length ?

Comment 24

16 years ago
talkback gets a little baffled by C++ template syntax. I use just 
'nsCharTraits' to get a match. There are sporadic cases of a crash 
ending in nsCharTraits::<unsigned short>length on the trunk, although
not apparently the same crash as this one (the bug is really with the
callers, not 'length'). There weren't any crashes on the branch in 'length'
until a couple of days ago when junruh had 8 crashes while doing something
with certificates (according to the comments). I don't know if a bug was 
opened for those crashes, but it would be a different bug than this one.
As for this bug, per talkback and the checkin log (1.64.2.3 of 
nsMetaCharsetObserver.cpp), verified for the branch.
Keywords: fixed1.0.1 → verified1.0.1

Comment 25

16 years ago
Doh, meant to cc: junruh on the previous comment just to ask if there was a bug 
on a file for a crash in |nsCharTraits::<unsigned short>length| when working 
with certificates (since you had those talkback crashes in the past few days).

Comment 26

16 years ago
I believe that bug 152382 causes the crash in the cert manager.

Comment 27

16 years ago
Most recent build:  2003011305, reopening. This is currently #19 on the trunk
topcrash list.

nsCharTraits<unsigned   9 
Crash data range: 2003-01-12 to 2003-01-13
Build ID range: 2003011204 to 2003011305
Stack Trace: 

	 nsCharTraits<unsigned short>::length	[../../dist/include/string\nsCharTraits.h
 line 200]
	 nsDependentString::Rebind	[../../dist/include/string\nsDependentString.h  line 60]
	 nsAString::do_AssignFromElementPtr
[c:/builds/seamonkey/mozilla/string/src/nsAString.cpp  line 303]
	 nsMIMEInfoImpl::SetDefaultDescription
[c:/builds/seamonkey/mozilla/netwerk/mime/src/nsMIMEInfoImpl.cpp  line 295]
	 XPTC_InvokeByIndex
[c:/builds/seamonkey/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp 
line 102]
	 XPCWrappedNative::CallMethod
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp  line 2025]
	 XPC_WN_GetterSetter
[c:/builds/seamonkey/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp 
line 1317]
	 js_Invoke	[c:/builds/seamonkey/mozilla/js/src/jsinterp.c  line 841]
	 js_InternalInvoke	[c:/builds/seamonkey/mozilla/js/src/jsinterp.c  line 932]
	 js_Interpret	[c:/builds/seamonkey/mozilla/js/src/jsinterp.c  line 2648]
	 js_Invoke	[c:/builds/seamonkey/mozilla/js/src/jsinterp.c  line 857]
	 js_InternalInvoke	[c:/builds/seamonkey/mozilla/js/src/jsinterp.c  line 932]
	 JS_CallFunctionValue	[c:/builds/seamonkey/mozilla/js/src/jsapi.c  line 3433]
	 nsJSContext::CallEventHandler
[c:/builds/seamonkey/mozilla/dom/src/base/nsJSEnvironment.cpp  line 1043]
	 GlobalWindowImpl::RunTimeout
[c:/builds/seamonkey/mozilla/dom/src/base/nsGlobalWindow.cpp  line 4749]
	 GlobalWindowImpl::TimerCallback
[c:/builds/seamonkey/mozilla/dom/src/base/nsGlobalWindow.cpp  line 5105]
	 nsTimerImpl::Fire	[c:/builds/seamonkey/mozilla/xpcom/threads/nsTimerImpl.cpp 
line 383]
	 nsAppShell::Run	[c:/builds/seamonkey/mozilla/widget/src/windows/nsAppShell.cpp
 line 176]
	 nsAppShellService::Run
[c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp  line 471]
	 main1	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1559]
	 main	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1907]
	 WinMain	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1927]
	 WinMainCRTStartup()
	 kernel32.dll + 0x214c7 (0x77e814c7)
 
 	Source File : ../../dist/include/string/nsCharTraits.h line : 200


Status: VERIFIED → REOPENED
Resolution: FIXED → ---

Comment 28

16 years ago
Closing this bug again as fixed.   Jan, the stack looks a bit different.  I
think we should log a new bug if we continue to see these crashes.
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Comment 29

16 years ago
v.
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsCharTraits<unsigned short>::length]
You need to log in before you can comment on or make changes to this bug.