Crash in KJS::ArrayInstance::inlineGetOwnPropertySlot viewing enhanced Wikipedia diff

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Derk-Jan Hartman]

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

[Derk-Jan Hartman]

Created attachment 20435 [details]
Crash report of Safari 3.1

[Derk-Jan Hartman]

Created attachment 20436 [details]
System configuration

[Matt Lilek]

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)

[Derk-Jan Hartman]

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

[Derk-Jan Hartman]

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.

[Derk-Jan Hartman]

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. 

[Cameron Zwarich (cpst)]

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?

[Derk-Jan Hartman]

Confirmed, no longer crashes for me either.