Bug 24654 - REGRESSION (Safari 4): Incorrect function return value when using IE "try ... finally" memory leak work-around
Summary: REGRESSION (Safari 4): Incorrect function return value when using IE "try ......
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2009-03-17 14:11 PDT by Christopher Blum
Modified: 2009-03-18 00:25 PDT (History)
0 users

See Also:


Attachments
Fix codegen for return statements if there are finalisers present. (5.80 KB, patch)
2009-03-17 23:35 PDT, Oliver Hunt
zwarich: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Blum 2009-03-17 14:11:53 PDT
Hey,

I recently checked out Safari 4 Public Beta and recognized that the following piece of javascript code behaves different than in other browsers (tested in Safari 3.2, FF 2, FF 3, IE 6, IE 7, IE 8, Opera 9.6).

var foo = function() {
  var bar = {};
  try { return bar; } finally { bar = null };
};

foo(); // this returns null in Safari 4 Public Beta (Win and Mac)

Safari 4 is the only browser that doesn't return an empty object.
The "try ... finally" is a known workaround for preventing memory leaks in IE 6.
(http://www.hedgerwow.com/360/dhtml/ie6_memory_leak_fix/)

Regards,
Christopher
Comment 1 Geoffrey Garen 2009-03-17 16:11:50 PDT
<rdar://problem/6692138>
Comment 2 Oliver Hunt 2009-03-17 23:35:49 PDT
Created attachment 28718 [details]
Fix codegen for return statements if there are finalisers present.

Fairly obvious fix, added test to existing finaliser test, and converted test to newer js test form.
Comment 3 Cameron Zwarich (cpst) 2009-03-17 23:39:36 PDT
Comment on attachment 28718 [details]
Fix codegen for return statements if there are finalisers present.

I like returnRegister more than returnReg, and it matches what we use elsewhere in the JIT codegen. Other than that, r=me if perf is fine. I always like seeing tests in the newer format.
Comment 4 Oliver Hunt 2009-03-18 00:25:12 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/bytecompiler/BytecodeGenerator.h
	M	JavaScriptCore/parser/Nodes.cpp
	M	LayoutTests/ChangeLog
	M	LayoutTests/fast/js/finally-codegen-failure-expected.txt
	M	LayoutTests/fast/js/finally-codegen-failure.html
	A	LayoutTests/fast/js/resources/finally-codegen-failure.js
Committed r41806