Summary: | SquirrelFish: RegisterFile can be corrupted if implictly reenter global scope with no declared vars | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Oliver Hunt <oliver> | ||||||
Component: | JavaScriptCore | Assignee: | Oliver Hunt <oliver> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Blocker | CC: | ggaren, mjs, zwarich | ||||||
Priority: | P1 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Oliver Hunt
2008-05-15 05:02:23 PDT
Created attachment 21161 [details]
a fix
This fixes the bug by not using the symboltable for the global this reference, so we don't need to track initialisation as carefully. My system registers it as a 0.1-0.2% regression though :-/
Now that Oliver rolled out r33483, this patch is a 0.5% progression for me, bringing us to our best time in quite a while. Comment on attachment 21161 [details]
a fix
My system also registers this as a progression so r?
Comment on attachment 21161 [details]
a fix
It's a little squirrely to have a variable named "m_thisRegister" that's only valid if we happen to be compiling program code.
I'd recommend setting m_thisRegister when compiling function code and eval code as well. Then, "thisRegister()" can always return m_thisRegister, which makes a lot more sense.
Eventually, we'll won't put "this" in the symbol table at all.
r=me
(In reply to comment #4) > (From update of attachment 21161 [details] [edit]) > It's a little squirrely to have a variable named "m_thisRegister" that's only > valid if we happen to be compiling program code. > > I'd recommend setting m_thisRegister when compiling function code and eval code > as well. Then, "thisRegister()" can always return m_thisRegister, which makes a > lot more sense. > > Eventually, we'll won't put "this" in the symbol table at all. > > r=me > I agree, but that cost of retrieving the this register for function/evalnode codegenerator was a regression -- but that was before we rolled out the patch o' badness Created attachment 21196 [details]
A better fix, which despite being technically superior is slower. I hate GCC.
Test case: <body onload="document.write('<script' + '>' + 'var x = 0; var y = 1; var z = 2;' + '</script' + '>');"> </body> I have a fix which ggaren is okay with, but it's causing a webkit regression, i'm trying to work out how. M JavaScriptCore/ChangeLog M JavaScriptCore/VM/CodeGenerator.cpp M JavaScriptCore/VM/CodeGenerator.h M JavaScriptCore/VM/Machine.cpp M JavaScriptCore/kjs/ExecState.h M JavaScriptCore/kjs/JSGlobalObject.cpp M JavaScriptCore/kjs/JSGlobalObject.h M JavaScriptCore/kjs/nodes.cpp M LayoutTests/ChangeLog A LayoutTests/fast/js/direct-entry-to-function-code-expected.txt A LayoutTests/fast/js/direct-entry-to-function-code.html Committed r33541 |