300858 - crash on Windows 2000 product documentation page [@ js_CompareStrings]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Mikel Ward]

In Deer Park Alpha 2, viewing the Microsoft Windows 2000 Product Documentation
page causes the browser to crash.

I tried to report this via the crash tool, but it doesn't work properly behind
proxies. :-(

Comment 1

[Ryan Flint [:rflint] (ping via IRC for reviews)]

WFM: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b3) Gecko/20050714
Firefox/1.0+  on XP

Comment 2

[Mikel Ward]

Sorry, the link I previously supplied was my bookmark.  It actually crashes when
you go to the Server Help section.

Updating URL accordingly.

Comment 3

[Ryan Flint [:rflint] (ping via IRC for reviews)]

There we go... thanks for updating the URL ;)
Talkback ID: TB7493297G

Comment 4

[timeless]

Incident ID: 7493297
Stack Signature	js_CompareStrings 935eb65c
Product ID	FirefoxTrunk
Build ID	2005071406
Trigger Time	2005-07-14 18:25:53.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	js3250.dll + (00046ca6)
URL visited	
User Comments	
Since Last Crash	131 sec
Total Uptime	131 sec
Trigger Reason	Access violation
Source File, Line No.
c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsstr.c, line 2785
Stack Trace 	
js_CompareStrings 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsstr.c, line 2785]
sort_compare_strings 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsarray.c, line 848]
js_HeapSort 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsarray.c, line 762]
array_sort 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsarray.c, line 936]
js_Invoke 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1173]
js_Interpret 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3464]
js_Execute 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1404]
JS_EvaluateUCScriptForPrincipals 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 3855]
nsJSContext::EvaluateString 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1060]
nsScriptLoader::EvaluateScript 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsScriptLoader.cpp,
line 757]
nsScriptLoader::ProcessRequest 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsScriptLoader.cpp,
line 658]
nsScriptLoader::ProcessScriptElement 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsScriptLoader.cpp,
line 593]
nsHTMLScriptElement::MaybeProcessScript 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/content/src/nsHTMLScriptElement.cpp,
line 662]
nsHTMLScriptElement::BindToTree 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/content/src/nsHTMLScriptElement.cpp,
line 455]
nsGenericElement::AppendChildTo 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsGenericElement.cpp,
line 2728]
HTMLContentSink::ProcessSCRIPTTag 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 4121]
HTMLContentSink::AddLeaf 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 2987]
CNavDTD::AddLeaf 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/parser/htmlparser/src/CNavDTD.cpp,
line 3568]
CNavDTD::HandleDefaultStartToken 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/parser/htmlparser/src/CNavDTD.cpp,
line 1283]
CNavDTD::HandleStartToken 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/parser/htmlparser/src/CNavDTD.cpp,
line 1664]
CNavDTD::HandleToken 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/parser/htmlparser/src/CNavDTD.cpp,
line 955]
CNavDTD::BuildModel 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/parser/htmlparser/src/CNavDTD.cpp,
line 458]
nsParser::BuildModel 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/parser/htmlparser/src/nsParser.cpp,
line 2116]

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Taking.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v1

We don't update all_strings in array_sort in the case of a hole, so we can end
up in sort_compare_strings with a hole for one of the strings (which
js_CompareStrings doesn't account for). I decided that the extra check in
sort_compare_strings was better than penalizing the sort and sending it through
sort_compare (and updating all_strings in the hole case).

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: penalize fewer cases

Brendan says to avoid the extra branches, and just penalize the cases with
holes in them instead of all string comparisons.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 189845 [details] [diff] [review]
penalize fewer cases

Cool, r+a=me.

/be

Comment 9

[Brendan Eich [:brendan]]

This should get fixed for 1.8b4, no doubt about it! ;-)

/be

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

I checked this in last night.

Might be good to have a testcase for this.

Comment 11

[Bob Clary [:bc] (inactive)]

Checking in regress-300858.js;
/cvsroot/mozilla/js/tests/js1_5/Array/regress-300858.js,v  <--  regress-300858.js
initial revision: 1.1

Comment 12

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20060818 win/mac*/linux