Bug 19844

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

Description Fabian Jakobs 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"

<html lang="en">
<script type="text/javascript" charset="utf-8">
function test() {
    alert(this == switchFunction.call(this, 12));

function switchFunction(val)
  switch (val)
    case this:

  return this;


Comment 1 Fabian Jakobs 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"
Comment 2 Mark Rowe (bdash) 2008-07-01 15:44:51 PDT
Comment 3 Cameron Zwarich (cpst) 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.
Comment 4 Cameron Zwarich (cpst) 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.
Comment 5 Darin Adler 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?
Comment 6 Cameron Zwarich (cpst) 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.
Comment 7 Cameron Zwarich (cpst) 2008-07-01 18:09:11 PDT
Landed in r34940.
Comment 8 Fabian Jakobs 2008-07-02 01:19:50 PDT
Awesome. This was really quick!

Works for me.