Summary: | SQUIRRELFISH: indirect eval used when direct eval should be used | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Cameron Zwarich (cpst) <zwarich> | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | ||||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 18631 | ||||||||||
Attachments: |
|
Description
Cameron Zwarich (cpst)
2008-04-25 16:13:07 PDT
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. Created attachment 20827 [details]
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.
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. This is also likely the cause of the failures of fast/js/kde/eval and fast/js/kde/scope. 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. Created attachment 20835 [details]
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.
Created attachment 20839 [details]
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.
Comment on attachment 20839 [details]
Revised proposed patch
r=me
I didn't notice before, but this patch also fixes fast/js/for-in-exeception, for a total of 6 fixes. |