Summary: | Global functions should be initialized as JSFunctions in byte code | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||
Component: | JavaScriptCore | Assignee: | Saam Barati <saam> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, fpizlo, ggaren, mark.lam, mmirman, msaboff, oliver | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 144265, 144620, 144753 | ||||||||||
Bug Blocks: | 142944 | ||||||||||
Attachments: |
|
Description
Saam Barati
2015-04-24 17:49:08 PDT
Created attachment 251651 [details]
work in progress
All but two tests are passing. There are the failing tests:
1. A profiler test is failing (I haven't investigated).
2. An API test is failing; specifically: globalStaticFunction() test.
'globalStaticFunction' is resolving to undefined and not the globalStaticFunction defined inside apitests.js.
I've spent some a good amount of time investigating this and every idea I have about what is wrong seems to be incorrect.
If anyone has an idea about why this may be failing, I'd appreciate the help.
(In reply to comment #1) > Created attachment 251651 [details] > 2. An API test is failing; specifically: globalStaticFunction() test. > 'globalStaticFunction' is resolving to undefined and not the > globalStaticFunction defined inside apitests.js. > I've spent some a good amount of time investigating this and every idea I > have about what is wrong seems to be incorrect. > If anyone has an idea about why this may be failing, I'd appreciate the help. Okay, after some digging I think I now know what's wrong. JSScope::abstractAccess won't respect the GlobalObject's Structure's propertyAccessesAreCacheable() result if the identifier being resolved is in the GlobalObject's symbol table. This seems wrong to me. I think propertyAccessesAreCacheable() should be checked before checking the GlobalObject's symbol table. Can anyone confirm? (In reply to comment #2) > (In reply to comment #1) > > Created attachment 251651 [details] > > > 2. An API test is failing; specifically: globalStaticFunction() test. > > 'globalStaticFunction' is resolving to undefined and not the > > globalStaticFunction defined inside apitests.js. > > I've spent some a good amount of time investigating this and every idea I > > have about what is wrong seems to be incorrect. > > If anyone has an idea about why this may be failing, I'd appreciate the help. > > Okay, after some digging I think I now know what's wrong. > JSScope::abstractAccess won't respect the GlobalObject's Structure's > propertyAccessesAreCacheable() result if the identifier being resolved is > in the GlobalObject's symbol table. This seems wrong to me. I think > propertyAccessesAreCacheable() should be checked before checking the > GlobalObject's symbol table. > > Can anyone confirm? This is wrong. SymbolTablePut/Get should have priority and is cacheable. Created attachment 252339 [details]
patch
Comment on attachment 252339 [details]
patch
r=me
Comment on attachment 252339 [details]
patch
I'm seeing some flakiness on exceptionFuzz still.
Investigating.
Comment on attachment 252339 [details] patch Clearing flags on attachment: 252339 Committed r183789: <http://trac.webkit.org/changeset/183789> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 144620 (In reply to comment #6) > Comment on attachment 252339 [details] > patch > > I'm seeing some flakiness on exceptionFuzz still. > > Investigating. I've found out the reason: We now emit put_to_scope for these global functions. They will increment the exceptionFuzz count and if the target count is low enough, will cause the VM to throw when initializing the global functions. This thrown exceptionFuzz exception won't be caught because this code for emitting global functions is emitted before the code for the actual "program" and so the big try block won't have the proper range that includes this pre-"program" initialization. The idea of throwing an exception fuzz when initializing global functions doesn't make sense because if the count is reached, we will invariably get an uncaught exception. I'll see if there is a nice way to get around this. Created attachment 252666 [details]
patch
for CQ
Comment on attachment 252666 [details] patch Clearing flags on attachment: 252666 Committed r183972: <http://trac.webkit.org/changeset/183972> All reviewed patches have been landed. Closing bug. |