18746 2008-04-25 16:13:07 -0700 SQUIRRELFISH: indirect eval used when direct eval should be used 2008-04-27 22:12:15 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 18631 1 zwarich webkit-unassigned oldest_to_newest 78943 0 zwarich 2008-04-25 16:13:07 -0700 This causes the layout test fast/js/read-modify-eval to fail. However, it only fails as a layout test, and the same code works fine if I try it out in testkjs. 78948 1 zwarich 2008-04-25 18:49:32 -0700 I know this bug report is somewhat unclear, but I don't really know what is causing the problem. It throws a SyntaxError saying that x is undefined. Maciej suggests that it might be due to the global object being a window rather than a plain JSGlobalObject, and that it could be related to the split window change. 78951 2 20827 zwarich 2008-04-25 19:43:46 -0700 Created attachment 20827 Test case Here is a test case showing the bug. It should print PASS, but instead prints FAIL. This only occurs in the browser, not in testkjs. The problem seems to be that eval is being passed an incorrect scope chain. This is also causing the failure of fast/js/kde/assignments. 78952 3 zwarich 2008-04-25 19:52:23 -0700 Some quick debugging shows that while op_call_eval is being used, the following test for whether it is a direct eval fails: if (base == exec->lexicalGlobalObject() && v == exec->lexicalGlobalObject()->evalFunction()) { The first part of the conditional is false, the second part is true. This causes it to fall back to an indirect eval. The addresses of base and exec->lexicalGlobalObject() are 0x100f0000 and 0x100f0020 respectively. 78953 4 zwarich 2008-04-25 20:02:04 -0700 This is also likely the cause of the failures of fast/js/kde/eval and fast/js/kde/scope. 78957 5 zwarich 2008-04-25 23:27:42 -0700 Changing the direct eval check to if (base == exec->globalThisValue() && v == exec->lexicalGlobalObject()->evalFunction()) { fixes almost all of the eval tests. The only exceptions, which it changes from passing to failing, are tests of the form (function() { var eval = window.eval; return eval("var y; "y" in window"); })() Here, op_call_eval is emitted for the eval, and the new direct eval check erroneously returns that it is a direct eval. Since this is the same test used for direct eval on trunk with the split window patch, it seems the best way to fix this is to get the split window patch fully functional on SquirrelFish. 78989 6 20835 zwarich 2008-04-26 14:34:34 -0700 Created attachment 20835 Proposed patch Here is a proposed patch. It fixes the 4 eval layout tests as well fast/js/encode-URI-test. Unfortunately, on my machine it is a 0.8% regression on SunSpider. I'll try doing the usual tricks, but maybe somebody can check it on their machine. 78995 7 20839 zwarich 2008-04-26 14:58:19 -0700 Created attachment 20839 Revised proposed patch It seems I didn't have an up-to-date tree. This applies cleanly to the current tree, and it is now a wash on SunSpider on my machine. 78997 8 20839 mjs 2008-04-26 15:04:51 -0700 Comment on attachment 20839 Revised proposed patch r=me 79027 9 zwarich 2008-04-26 23:18:40 -0700 I didn't notice before, but this patch also fixes fast/js/for-in-exeception, for a total of 6 fixes. 20827 2008-04-25 19:43:46 -0700 2008-04-25 19:43:46 -0700 Test case eval.html text/html 127 zwarich PHNjcmlwdD4KdmFyIHByaW50ID0gYWxlcnQ7Cgp2YXIgeCA9ICJGQUlMIjsKCmZ1bmN0aW9uIGYo KQp7CiAgICB2YXIgeCA9ICJQQVNTIjsKICAgIHJldHVybiBldmFsKCJ4Iik7Cn0KCnByaW50KGYo KSk7Cjwvc2NyaXB0Pg== 20835 2008-04-26 14:34:34 -0700 2008-04-26 14:58:19 -0700 Proposed patch evalcheck.diff text/plain 2083 zwarich SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDMyNTky KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjAgQEAKKzIwMDgtMDQt MjYgIENhbWVyb24gWndhcmljaCAgPGN3endhcmljaEB1d2F0ZXJsb28uY2E+CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQnVnIDE4NzQ2OiBTUVVJUlJF TEZJU0g6IGluZGlyZWN0IGV2YWwgdXNlZCB3aGVuIGRpcmVjdCBldmFsIHNob3VsZCBiZSB1c2Vk CisgICAgICAgIDxodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTg3NDY+ CisKKyAgICAgICAgQ2hhbmdlIHRoZSBiYXNlIHRvIHRoZSBjb3JyZWN0IHZhbHVlIG9mIHRoZSAn dGhpcycgb2JqZWN0IGFmdGVyIHRoZSBkaXJlY3QKKyAgICAgICAgZXZhbCB0ZXN0IGluc3RlYWQg b2YgYmVmb3JlLgorCisgICAgICAgIEZpeGVzIDUgbGF5b3V0IHRlc3RzLgorCisgICAgICAgICog Vk0vTWFjaGluZS5jcHA6CisgICAgICAgIChLSlM6Ok1hY2hpbmU6OnByaXZhdGVFeGVjdXRlKToK KyAgICAgICAgKiBranMvbm9kZXMuY3BwOgorICAgICAgICAoS0pTOjpFdmFsRnVuY3Rpb25DYWxs Tm9kZTo6ZW1pdENvZGUpOgorCiAyMDA4LTA0LTI2ICBNYWNpZWogU3RhY2hvd2lhayAgPG1qc0Bh cHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgT2xpdmVyLgpJbmRleDogVk0vTWFjaGlu ZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gVk0vTWFjaGluZS5jcHAJKHJldmlzaW9uIDMyNTg4KQorKysg Vk0vTWFjaGluZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTEzMjksOCArMTMyOSwxMCBAQCBKU1Zh bHVlKiBNYWNoaW5lOjpwcml2YXRlRXhlY3V0ZShFeGVjdXRpCiAgICAgICAgIH0KICAgICAgICAg CiAgICAgICAgIC8vIFdlIGRpZG4ndCBmaW5kIHRoZSBibGVzc2VkIHZlcnNpb24gb2YgZXZhbCwg c28gcmVzZXQgdlBDIGFuZCBwcm9jZXNzCi0gICAgICAgIC8vIHRoaXMgaW5zdHJ1Y3Rpb24gYXMg YSBub3JtYWwgZnVuY3Rpb24gY2FsbC4KKyAgICAgICAgLy8gdGhpcyBpbnN0cnVjdGlvbiBhcyBh IG5vcm1hbCBmdW5jdGlvbiBjYWxsLCBzdXBwbHlpbmcgdGhlIHByb3BlciAndGhpcycKKyAgICAg ICAgLy8gdmFsdWUuCiAgICAgICAgIHZQQyAtPSA1OworICAgICAgICByW3IyXS51LmpzVmFsdWUg PSBiYXNlLT50b09iamVjdChleGVjKS0+dG9UaGlzT2JqZWN0KGV4ZWMpOwogCiAjaWYgSEFWRShD T01QVVRFRF9HT1RPKQogICAgICAgICAvLyBIYWNrIGFyb3VuZCBnY2MgcGVyZm9ybWFuY2UgcXVp cmsgYnkgcGVyZm9ybWluZyBhbiBpbmRpcmVjdCBnb3RvCkluZGV4OiBranMvbm9kZXMuY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIGtqcy9ub2Rlcy5jcHAJKHJldmlzaW9uIDMyNTg4KQorKysga2pzL25vZGVz LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTI5Myw3ICsxMjkzLDcgQEAgUmVnaXN0ZXJJRCogRXZh bEZ1bmN0aW9uQ2FsbE5vZGU6OmVtaXRDbwogewogICAgIFJlZlB0cjxSZWdpc3RlcklEPiByMCA9 IGdlbmVyYXRvci50ZW1wRGVzdGluYXRpb24oZHN0KTsKICAgICBSZWdpc3RlcklEKiByMSA9IGdl bmVyYXRvci5uZXdUZW1wb3JhcnkoKTsKLSAgICBnZW5lcmF0b3IuZW1pdFJlc29sdmVCYXNlQW5k RnVuYyhyMC5nZXQoKSwgcjEsIENvbW1vbklkZW50aWZpZXJzOjpzaGFyZWQoKS0+ZXZhbCk7Cisg ICAgZ2VuZXJhdG9yLmVtaXRSZXNvbHZlQmFzZUFuZFByb3BlcnR5KHIwLmdldCgpLCByMSwgQ29t bW9uSWRlbnRpZmllcnM6OnNoYXJlZCgpLT5ldmFsKTsKICAgICByZXR1cm4gZ2VuZXJhdG9yLmVt aXRDYWxsRXZhbChnZW5lcmF0b3IuZmluYWxEZXN0aW5hdGlvbihkc3QsIHIwLmdldCgpKSwgcjEs IHIwLmdldCgpLCBtX2FyZ3MuZ2V0KCkpOwogfQogCg== 20839 2008-04-26 14:58:19 -0700 2008-04-26 15:04:51 -0700 Revised proposed patch evalcheck.diff text/plain 2101 zwarich SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDMyNTky KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjAgQEAKKzIwMDgtMDQt MjYgIENhbWVyb24gWndhcmljaCAgPGN3endhcmljaEB1d2F0ZXJsb28uY2E+CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQnVnIDE4NzQ2OiBTUVVJUlJF TEZJU0g6IGluZGlyZWN0IGV2YWwgdXNlZCB3aGVuIGRpcmVjdCBldmFsIHNob3VsZCBiZSB1c2Vk CisgICAgICAgIDxodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTg3NDY+ CisKKyAgICAgICAgQ2hhbmdlIHRoZSBiYXNlIHRvIHRoZSBjb3JyZWN0IHZhbHVlIG9mIHRoZSAn dGhpcycgb2JqZWN0IGFmdGVyIHRoZSBkaXJlY3QKKyAgICAgICAgZXZhbCB0ZXN0IGluc3RlYWQg b2YgYmVmb3JlLgorCisgICAgICAgIEZpeGVzIDUgbGF5b3V0IHRlc3RzLgorCisgICAgICAgICog Vk0vTWFjaGluZS5jcHA6CisgICAgICAgIChLSlM6Ok1hY2hpbmU6OnByaXZhdGVFeGVjdXRlKToK KyAgICAgICAgKiBranMvbm9kZXMuY3BwOgorICAgICAgICAoS0pTOjpFdmFsRnVuY3Rpb25DYWxs Tm9kZTo6ZW1pdENvZGUpOgorCiAyMDA4LTA0LTI2ICBNYWNpZWogU3RhY2hvd2lhayAgPG1qc0Bh cHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgT2xpdmVyLgpJbmRleDogVk0vTWFjaGlu ZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gVk0vTWFjaGluZS5jcHAJKHJldmlzaW9uIDMyNTkyKQorKysg Vk0vTWFjaGluZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTE0MjEsOCArMTQyMSwxMCBAQCBKU1Zh bHVlKiBNYWNoaW5lOjpwcml2YXRlRXhlY3V0ZShFeGVjdXRpCiAgICAgICAgIH0KICAgICAgICAg CiAgICAgICAgIC8vIFdlIGRpZG4ndCBmaW5kIHRoZSBibGVzc2VkIHZlcnNpb24gb2YgZXZhbCwg c28gcmVzZXQgdlBDIGFuZCBwcm9jZXNzCi0gICAgICAgIC8vIHRoaXMgaW5zdHJ1Y3Rpb24gYXMg YSBub3JtYWwgZnVuY3Rpb24gY2FsbC4KKyAgICAgICAgLy8gdGhpcyBpbnN0cnVjdGlvbiBhcyBh IG5vcm1hbCBmdW5jdGlvbiBjYWxsLCBzdXBwbHlpbmcgdGhlIHByb3BlciAndGhpcycKKyAgICAg ICAgLy8gdmFsdWUuCiAgICAgICAgIHZQQyAtPSA1OworICAgICAgICByW3IyXS51LmpzVmFsdWUg PSBiYXNlLT50b09iamVjdChleGVjKS0+dG9UaGlzT2JqZWN0KGV4ZWMpOwogCiAjaWYgSEFWRShD T01QVVRFRF9HT1RPKQogICAgICAgICAvLyBIYWNrIGFyb3VuZCBnY2MgcGVyZm9ybWFuY2UgcXVp cmsgYnkgcGVyZm9ybWluZyBhbiBpbmRpcmVjdCBnb3RvCkluZGV4OiBranMvbm9kZXMuY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIGtqcy9ub2Rlcy5jcHAJKHJldmlzaW9uIDMyNTkyKQorKysga2pzL25vZGVz LmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTI5Myw3ICsxMjkzLDcgQEAgUmVnaXN0ZXJJRCogRXZh bEZ1bmN0aW9uQ2FsbE5vZGU6OmVtaXRDbwogewogICAgIFJlZlB0cjxSZWdpc3RlcklEPiBiYXNl ID0gZ2VuZXJhdG9yLnRlbXBEZXN0aW5hdGlvbihkc3QpOwogICAgIFJlZ2lzdGVySUQqIGZ1bmMg PSBnZW5lcmF0b3IubmV3VGVtcG9yYXJ5KCk7Ci0gICAgZ2VuZXJhdG9yLmVtaXRSZXNvbHZlQmFz ZUFuZEZ1bmMoYmFzZS5nZXQoKSwgZnVuYywgQ29tbW9uSWRlbnRpZmllcnM6OnNoYXJlZCgpLT5l dmFsKTsKKyAgICBnZW5lcmF0b3IuZW1pdFJlc29sdmVCYXNlQW5kUHJvcGVydHkoYmFzZS5nZXQo KSwgZnVuYywgQ29tbW9uSWRlbnRpZmllcnM6OnNoYXJlZCgpLT5ldmFsKTsKICAgICByZXR1cm4g Z2VuZXJhdG9yLmVtaXRDYWxsRXZhbChnZW5lcmF0b3IuZmluYWxEZXN0aW5hdGlvbihkc3QsIGJh c2UuZ2V0KCkpLCBmdW5jLCBiYXNlLmdldCgpLCBtX2FyZ3MuZ2V0KCkpOwogfQogCg==