Bug 19844

Summary: JavaScript Switch statement modifies "this"
Product: WebKit Reporter: Fabian Jakobs <fabian.jakobs>
Component: JavaScriptCoreAssignee: Cameron Zwarich (cpst) <zwarich>
Status: RESOLVED FIXED    
Severity: Critical CC: zwarich
Priority: P2 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Test case for this bug
none
Codegen dump
none
Proposed patch (without tests) darin: review+

Fabian Jakobs
Reported 2008-07-01 07:23:29 PDT
I have encountered a strange error with the latest nightlys (rev. 34824). Special switch statements seem to modifiy the function context (aka this) and set if to the value "false". This can be observed in this simple example. <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd"> <html lang="en"> <body> <script type="text/javascript" charset="utf-8"> function test() { alert(this == switchFunction.call(this, 12)); } function switchFunction(val) { switch (val) { case this: break; default: break; } return this; } test(); </script> </body> </html>
Attachments
Test case for this bug (405 bytes, text/html)
2008-07-01 07:25 PDT, Fabian Jakobs 		no flags
Details
Codegen dump (1.18 KB, text/plain)
2008-07-01 16:19 PDT, Cameron Zwarich (cpst) 		no flags
Details
Proposed patch (without tests) (2.34 KB, patch)
2008-07-01 16:28 PDT, Cameron Zwarich (cpst) 		darin: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Fabian Jakobs
Comment 1 2008-07-01 07:25:14 PDT
Created attachment 22028 [details] Test case for this bug Example which shows this problem. Other browser including Safari 3.1.1 return "true" while the current nightly returns "false"
Mark Rowe (bdash)
Comment 2 2008-07-01 15:44:51 PDT
<rdar://problem/6048093>
Cameron Zwarich (cpst)
Comment 3 2008-07-01 16:19:32 PDT
Created attachment 22033 [details] Codegen dump The problem is that it is writing the result of stricteq to lr2, which is 'this'. I will fix this after dinner.
Cameron Zwarich (cpst)
Comment 4 2008-07-01 16:28:48 PDT
Created attachment 22034 [details] Proposed patch (without tests) Here's a patch. I will add tests to fast/js/codegen-temporaries for both the 'this' and local variable cases.
Darin Adler
Comment 5 2008-07-01 16:32:15 PDT
Comment on attachment 22034 [details] Proposed patch (without tests) Patch looks good. r=me But why not include the tests with the patch next time?
Cameron Zwarich (cpst)
Comment 6 2008-07-01 16:32:46 PDT
(In reply to comment #5) > (From update of attachment 22034 [details] [edit]) > Patch looks good. r=me > > But why not include the tests with the patch next time? Going out for a Canada Day dinner and didn't have the time.
Cameron Zwarich (cpst)
Comment 7 2008-07-01 18:09:11 PDT
Landed in r34940.
Fabian Jakobs
Comment 8 2008-07-02 01:19:50 PDT
Awesome. This was really quick! Works for me.
Note You need to log in before you can comment on or make changes to this bug.