19844 2008-07-01 07:23:29 -0700 JavaScript Switch statement modifies "this" 2008-07-02 01:19:50 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED InRadar, Regression P2 Critical --- 1 fabian.jakobs zwarich zwarich oldest_to_newest 84869 0 fabian.jakobs 2008-07-01 07:23:29 -0700 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> 84870 1 22028 fabian.jakobs 2008-07-01 07:25:14 -0700 Created attachment 22028 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" 84914 2 mrowe 2008-07-01 15:44:51 -0700 <rdar://problem/6048093> 84920 3 22033 zwarich 2008-07-01 16:19:32 -0700 Created attachment 22033 Codegen dump The problem is that it is writing the result of stricteq to lr2, which is 'this'. I will fix this after dinner. 84921 4 22034 zwarich 2008-07-01 16:28:48 -0700 Created attachment 22034 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. 84923 5 22034 darin 2008-07-01 16:32:15 -0700 Comment on attachment 22034 Proposed patch (without tests) Patch looks good. r=me But why not include the tests with the patch next time? 84924 6 zwarich 2008-07-01 16:32:46 -0700 (In reply to comment #5) > (From update of attachment 22034 [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. 84929 7 zwarich 2008-07-01 18:09:11 -0700 Landed in r34940. 84954 8 fabian.jakobs 2008-07-02 01:19:50 -0700 Awesome. This was really quick! Works for me. 22028 2008-07-01 07:25:14 -0700 2008-07-01 07:25:14 -0700 Test case for this bug webkit-switch.html text/html 405 fabian.jakobs PCFET0NUWVBFIGh0bWwgUFVCTElDICItLy9XM0MvL0RURCBIVE1MIDQuMDEvL0VOIgogICAiaHR0 cDovL3d3dy53My5vcmcvVFIvaHRtbDQvc3RyaWN0LmR0ZCI+Cgo8aHRtbCBsYW5nPSJlbiI+Cjxi b2R5Pgo8c2NyaXB0IHR5cGU9InRleHQvamF2YXNjcmlwdCIgY2hhcnNldD0idXRmLTgiPgogICAg CmZ1bmN0aW9uIHRlc3QoKSB7CiAgICBhbGVydCh0aGlzID09IHN3aXRjaEZ1bmN0aW9uLmNhbGwo dGhpcywgMTIpKTsKfQoKZnVuY3Rpb24gc3dpdGNoRnVuY3Rpb24odmFsKQp7CiAgc3dpdGNoICh2 YWwpCiAgewogICAgY2FzZSB0aGlzOgogICAgICBicmVhazsKCiAgICBkZWZhdWx0OgogICAgICBi cmVhazsKICB9CiAgcmV0dXJuIHRoaXM7Cn0KdGVzdCgpOwoKPC9zY3JpcHQ+Cgo8L2JvZHk+Cjwv aHRtbD4K 22033 2008-07-01 16:19:32 -0700 2008-07-01 16:19:32 -0700 Codegen dump codegen.txt text/plain 1208 zwarich ZnVuY3Rpb24gdGVzdCgpCnsKICAgIHByaW50KHRoaXMgPT0gc3dpdGNoRnVuY3Rpb24uY2FsbCh0 aGlzLCAxMikpOwp9CgpmdW5jdGlvbiBzd2l0Y2hGdW5jdGlvbih2YWwpCnsKICAgIHN3aXRjaCAo dmFsKSB7CiAgICBjYXNlIHRoaXM6CiAgICAgICAgYnJlYWs7CiAgICBkZWZhdWx0OgogICAgICAg IGJyZWFrOwogICAgfQogICAgCiAgICByZXR1cm4gdGhpczsKfQoKdGVzdCgpOwoKNSBpbnN0cnVj dGlvbnM7IDY4IGJ5dGVzIGF0IDB4NjBhMjUwOyAxIGxvY2FscyAoMCBwYXJhbWV0ZXJzKTsgMTIg dGVtcG9yYXJpZXMKClsgICAwXSBuZXdfZnVuYwkJIGxyMTYsIGYwClsgICAzXSBuZXdfZnVuYwkJ IGxyMTcsIGYxClsgICA2XSBsb2FkCQkgdHIwLCB1bmRlZmluZWQoQGswKQkJClsgICA5XSBjYWxs CQkgdHIwLCBscjE2LCB0cjIxNDc0ODM2NDcsIDExLCAxClsgIDE1XSBlbmQJCSB0cjAKCkNvbnN0 YW50czoKICBrMCA9IHVuZGVmaW5lZAoKMTAgaW5zdHJ1Y3Rpb25zOyAxNTYgYnl0ZXMgYXQgMHg2 MGExZDA7IDEgbG9jYWxzICgxIHBhcmFtZXRlcnMpOyAyOSB0ZW1wb3JhcmllcwoKWyAgIDBdIHJl c29sdmVfZnVuYwkgdHIwLCB0cjEsIHByaW50KEBpZDApClsgICA0XSBnZXRfc2NvcGVkX3ZhcgkJ IHRyMTQsIC02LCAwClsgICA4XSBnZXRfYnlfaWQJIHRyMTUsIHRyMTQsIGNhbGwoQGlkMSkKWyAg MTJdIG1vdgkJIHRyMjcsIGxyMQpbICAxNV0gbG9hZAkJIHRyMjgsIDEyKEBrMCkJCQpbICAxOF0g Y2FsbAkJIHRyMTQsIHRyMTUsIHRyMTQsIDI2LCAzClsgIDI0XSBlcQkJIHRyMTMsIGxyMSwgdHIx NApbICAyOF0gY2FsbAkJIHRyMCwgdHIxLCB0cjAsIDEyLCAyClsgIDM0XSBsb2FkCQkgdHIwLCB1 bmRlZmluZWQoQGsxKQkJClsgIDM3XSByZXQJCSB0cjAKCklkZW50aWZpZXJzOgogIGlkMCA9IHBy aW50CiAgaWQxID0gY2FsbAoKQ29uc3RhbnRzOgogIGswID0gMTIKICBrMSA9IHVuZGVmaW5lZAoK NiBpbnN0cnVjdGlvbnM7IDYwIGJ5dGVzIGF0IDB4NjBhM2IwOyAyIGxvY2FscyAoMiBwYXJhbWV0 ZXJzKTsgMCB0ZW1wb3JhcmllcwoKWyAgIDBdIHN0cmljdGVxCQkgbHIyLCBscjIsIGxyMQpbICAg NF0ganRydWUJCSBscjIsIDMoLT45KQpbICAgN10gam1wCQkgMygtPjExKQpbICAgOV0gam1wCQkg MygtPjEzKQpbICAxMV0gam1wCQkgMSgtPjEzKQpbICAxM10gcmV0CQkgbHIyCgpmYWxzZQpFbmQ6 IHVuZGVmaW5lZAo= 22034 2008-07-01 16:28:48 -0700 2008-07-01 16:32:15 -0700 Proposed patch (without tests) switch.diff text/plain 2401 zwarich SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDM0OTM1 KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAKKzIwMDgtMDct MDEgIENhbWVyb24gWndhcmljaCAgPGN3endhcmljaEB1d2F0ZXJsb28uY2E+CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQnVnIDE5ODQ0OiBKYXZhU2Ny aXB0IFN3aXRjaCBzdGF0ZW1lbnQgbW9kaWZpZXMgInRoaXMiCisgICAgICAgIDxodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTk4NDQ+CisKKyAgICAgICAgVXNlIGEgdGVt cG9yYXJ5IHdoZW4gZ2VuZXJhdGluZyBjb2RlIGZvciBzd2l0Y2ggY2xhdXNlcyB0byBhdm9pZAor ICAgICAgICBvdmVyd3JpdGluZyAndGhpcycgb3IgYSBsb2NhbCB2YXJpYWJsZS4KKworICAgICAg ICAqIGtqcy9ub2Rlcy5jcHA6CisgICAgICAgIChLSlM6OkNhc2VCbG9ja05vZGU6OmVtaXRDb2Rl Rm9yQmxvY2spOgorCiAyMDA4LTA3LTAxICBEYW4gQmVybnN0ZWluICA8bWl0ekBhcHBsZS5jb20+ CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQW5kZXJzIENhcmxzc29uLgpJbmRleDoga2pzL25vZGVz LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09Ci0tLSBranMvbm9kZXMuY3BwCShyZXZpc2lvbiAzNDkzNCkKKysrIGtq cy9ub2Rlcy5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE1MDksMTcgKzE1MDksMTkgQEAgUmVnaXN0 ZXJJRCogQ2FzZUJsb2NrTm9kZTo6ZW1pdENvZGVGb3JCbAogCiAgICAgLy8gU2V0dXAganVtcHMK ICAgICBmb3IgKENsYXVzZUxpc3ROb2RlKiBsaXN0ID0gbV9saXN0MS5nZXQoKTsgbGlzdDsgbGlz dCA9IGxpc3QtPmdldE5leHQoKSkgewotICAgICAgICBSZWdpc3RlcklEKiBjbGF1c2VWYWwgPSBn ZW5lcmF0b3IuZW1pdE5vZGUobGlzdC0+Z2V0Q2xhdXNlKCktPmV4cHIoKSk7Ci0gICAgICAgIGdl bmVyYXRvci5lbWl0QmluYXJ5T3Aob3Bfc3RyaWN0ZXEsIGNsYXVzZVZhbCwgY2xhdXNlVmFsLCBz d2l0Y2hFeHByZXNzaW9uKTsKKyAgICAgICAgUmVmUHRyPFJlZ2lzdGVySUQ+IGNsYXVzZVZhbCA9 IGdlbmVyYXRvci5uZXdUZW1wb3JhcnkoKTsKKyAgICAgICAgZ2VuZXJhdG9yLmVtaXROb2RlKGNs YXVzZVZhbC5nZXQoKSwgbGlzdC0+Z2V0Q2xhdXNlKCktPmV4cHIoKSk7CisgICAgICAgIGdlbmVy YXRvci5lbWl0QmluYXJ5T3Aob3Bfc3RyaWN0ZXEsIGNsYXVzZVZhbC5nZXQoKSwgY2xhdXNlVmFs LmdldCgpLCBzd2l0Y2hFeHByZXNzaW9uKTsKICAgICAgICAgbGFiZWxWZWN0b3IuYXBwZW5kKGdl bmVyYXRvci5uZXdMYWJlbCgpKTsKLSAgICAgICAgZ2VuZXJhdG9yLmVtaXRKdW1wSWZUcnVlKGNs YXVzZVZhbCwgbGFiZWxWZWN0b3JbbGFiZWxWZWN0b3Iuc2l6ZSgpIC0gMV0uZ2V0KCkpOworICAg ICAgICBnZW5lcmF0b3IuZW1pdEp1bXBJZlRydWUoY2xhdXNlVmFsLmdldCgpLCBsYWJlbFZlY3Rv cltsYWJlbFZlY3Rvci5zaXplKCkgLSAxXS5nZXQoKSk7CiAgICAgfQogCiAgICAgZm9yIChDbGF1 c2VMaXN0Tm9kZSogbGlzdCA9IG1fbGlzdDIuZ2V0KCk7IGxpc3Q7IGxpc3QgPSBsaXN0LT5nZXRO ZXh0KCkpIHsKLSAgICAgICAgUmVnaXN0ZXJJRCogY2xhdXNlVmFsID0gZ2VuZXJhdG9yLmVtaXRO b2RlKGxpc3QtPmdldENsYXVzZSgpLT5leHByKCkpOwotICAgICAgICBnZW5lcmF0b3IuZW1pdEJp bmFyeU9wKG9wX3N0cmljdGVxLCBjbGF1c2VWYWwsIGNsYXVzZVZhbCwgc3dpdGNoRXhwcmVzc2lv bik7CisgICAgICAgIFJlZlB0cjxSZWdpc3RlcklEPiBjbGF1c2VWYWwgPSBnZW5lcmF0b3IubmV3 VGVtcG9yYXJ5KCk7CisgICAgICAgIGdlbmVyYXRvci5lbWl0Tm9kZShjbGF1c2VWYWwuZ2V0KCks IGxpc3QtPmdldENsYXVzZSgpLT5leHByKCkpOworICAgICAgICBnZW5lcmF0b3IuZW1pdEJpbmFy eU9wKG9wX3N0cmljdGVxLCBjbGF1c2VWYWwuZ2V0KCksIGNsYXVzZVZhbC5nZXQoKSwgc3dpdGNo RXhwcmVzc2lvbik7CiAgICAgICAgIGxhYmVsVmVjdG9yLmFwcGVuZChnZW5lcmF0b3IubmV3TGFi ZWwoKSk7Ci0gICAgICAgIGdlbmVyYXRvci5lbWl0SnVtcElmVHJ1ZShjbGF1c2VWYWwsIGxhYmVs VmVjdG9yW2xhYmVsVmVjdG9yLnNpemUoKSAtIDFdLmdldCgpKTsKKyAgICAgICAgZ2VuZXJhdG9y LmVtaXRKdW1wSWZUcnVlKGNsYXVzZVZhbC5nZXQoKSwgbGFiZWxWZWN0b3JbbGFiZWxWZWN0b3Iu c2l6ZSgpIC0gMV0uZ2V0KCkpOwogICAgIH0KIAogICAgIFJlZlB0cjxMYWJlbElEPiBkZWZhdWx0 TGFiZWw7Cg==