Bug 18392 - Crash in KJS::ArrayInstance::inlineGetOwnPropertySlot viewing enhanced Wikipedia diff
Summary: Crash in KJS::ArrayInstance::inlineGetOwnPropertySlot viewing enhanced Wikipe...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 525.x (Safari 3.1)
Hardware: Macintosh Intel OS X 10.5
: P1 Normal
Assignee: Nobody
URL: http://en.wikipedia.org/w/index.php?t...
Keywords: NeedsReduction
Depends on:
Blocks:
 
Reported: 2008-04-09 11:46 PDT by Derk-Jan Hartman
Modified: 2008-10-30 14:03 PDT (History)
2 users (show)

See Also:


Attachments
Crash report of Safari 3.1 (27.67 KB, text/plain)
2008-04-09 11:47 PDT, Derk-Jan Hartman
no flags Details
System configuration (1.25 KB, text/plain)
2008-04-09 11:47 PDT, Derk-Jan Hartman
no flags Details
Crash report for Safari NB (44.58 KB, text/plain)
2008-04-09 11:55 PDT, Derk-Jan Hartman
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Derk-Jan Hartman 2008-04-09 11:46:27 PDT
This is an issue that I ran into with Safari 3.1

To reproduce:
1: Register on en.wikipedia.org
2: Go to preferences->Gadgets
3: select "Improved diff view" and save
4: Visit a diff link http://en.wikipedia.org/w/index.php?title=Juno_%28film%29&curid=11862690&diff=204473416&oldid=204458429 
5: Below the diff should be a button with a small green triangle.
6: Clicking the bug crashes Safari with attached crashreport
Comment 1 Derk-Jan Hartman 2008-04-09 11:47:05 PDT
Created attachment 20435 [details]
Crash report of Safari 3.1
Comment 2 Derk-Jan Hartman 2008-04-09 11:47:35 PDT
Created attachment 20436 [details]
System configuration
Comment 3 Matt Lilek 2008-04-09 11:52:23 PDT
Confirmed with r31753, probably a regression but it untested right now.

Top of debug stack trace:
Thread 0 Crashed:
0   com.apple.JavaScriptCore      	0x0047df6c KJS::ArrayInstance::inlineGetOwnPropertySlot(KJS::ExecState*, unsigned int, KJS::PropertySlot&) + 162 (array_instance.cpp:148)
1   com.apple.JavaScriptCore      	0x00427f42 KJS::ArrayInstance::getOwnPropertySlot(KJS::ExecState*, unsigned int, KJS::PropertySlot&) + 38 (array_instance.cpp:182)
2   com.apple.JavaScriptCore      	0x0040982f KJS::JSObject::getPropertySlot(KJS::ExecState*, unsigned int, KJS::PropertySlot&) + 51 (object.cpp:183)
3   com.apple.JavaScriptCore      	0x0042697e KJS::JSObject::get(KJS::ExecState*, unsigned int) const + 38 (object.cpp:172)
4   com.apple.JavaScriptCore      	0x0047f9b5 KJS::BracketAccessorNode::inlineEvaluate(KJS::ExecState*) + 235 (nodes.cpp:912)
5   com.apple.JavaScriptCore      	0x004351d4 KJS::BracketAccessorNode::evaluate(KJS::ExecState*) + 30 (nodes.cpp:919)
6   com.apple.JavaScriptCore      	0x0047ed75 KJS::NotEqualNode::inlineEvaluateToBoolean(KJS::ExecState*) + 37 (nodes.cpp:3143)
7   com.apple.JavaScriptCore      	0x0043308a KJS::NotEqualNode::evaluateToBoolean(KJS::ExecState*) + 30 (nodes.cpp:3158)
8   com.apple.JavaScriptCore      	0x004319b5 KJS::IfNode::execute(KJS::ExecState*) + 43 (nodes.cpp:4026)
9   com.apple.JavaScriptCore      	0x0041535d KJS::statementListExecute(WTF::Vector<WTF::RefPtr<KJS::StatementNode>, 0ul>&, KJS::ExecState*) + 85 (nodes.cpp:3946)
10  com.apple.JavaScriptCore      	0x004153ea KJS::BlockNode::execute(KJS::ExecState*) + 26 (nodes.cpp:3972)

Comment 4 Derk-Jan Hartman 2008-04-09 11:55:00 PDT
Created attachment 20437 [details]
Crash report for Safari NB

Crash report of Safari NB 5525.13 (rev 31388 ).

Confirmed in #webkit by pewtermoose and cpst
Comment 5 Derk-Jan Hartman 2008-04-10 11:23:43 PDT
Apparently the script also takes IE to 100% memory usage. So there is probably an error in the code that is at the root of this crash.
Comment 6 Derk-Jan Hartman 2008-04-12 17:40:04 PDT
I looked at the script in question today for a minute and spotted the following:

// get diff table and version link cells
	var tdArray = document.getElementsByTagName('TD');
	var tdOld;
	var tdNew;
	for (var i = 0; i < tdArray.length; i ++) {
		if (tdArray[i].className == 'diff-otitle') {
			tdOld = tdArray[i];
		}
		else if (tdArray[i].className == 'diff-ntitle') {
			tdNew = tdArray[i];
			break;
		}
	}
	if ( (tdOld == null) || (tdNew == null) ) {
		return;
	}
 
	var oldVersion = null;
	var newVersion = null;
 
	var oldUrl;
	var newUrl;

probably crashes on the line: if (tdArray[i].className == 'diff-otitle')
The problem being that getElementsByTagName returns a Nodelist and not an Array object. 
Comment 7 Cameron Zwarich (cpst) 2008-06-29 02:23:56 PDT
This doesn't crash for me with the r34824 nightly and a local debug build of r34870. As far as I can tell, it hasn't happened since SquirrelFish. Can we close this?
Comment 8 Derk-Jan Hartman 2008-10-30 14:02:41 PDT
Confirmed, no longer crashes for me either.