Summary: | Crash in operationNewFunction when scrolling on Google+ | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||
Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | Keywords: | InRadar | ||||||
Priority: | P2 | ||||||||
Version: | 312.x | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
URL: | http://plus.google.com | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 140145 | ||||||||
Attachments: |
|
Description
Michael Saboff
2015-01-01 15:54:25 PST
Consider a function that creates another function that is never used. The DFG will dead code eliminate the creation of the inner function. If the scope register is DCE as well AND we OSR exit in the middle of the outer function to a path where we create the inner function, there won't be a valid scope needed to create the inner function and we crash. Since the DFG determined that the scope is dead, the inner function must have been proven dead. Therefore, we can fix this in the baseline generated code by checking the scope before creating the function. If the scope is undefined, return undefined instead of creating a new function. Created attachment 243874 [details]
Patch
Comment on attachment 243874 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243874&action=review > Source/JavaScriptCore/ChangeLog:11 > + exit to a path where that funciton gets created, check the scope register value function > Source/JavaScriptCore/jit/JITOpcodes.cpp:1071 > + notUndefinedScope = branch64(NotEqual, regT0, TrustedImm64(JSValue::encode(jsUndefined()))); > + store64(TrustedImm64(JSValue::encode(jsUndefined())), Address(callFrameRegister, sizeof(Register) * dst)); > #else > emitLoadPayload(currentInstruction[2].u.operand, regT0); > + notUndefinedScope = branch32(NotEqual, tagFor(dst), TrustedImm32(JSValue::UndefinedTag)); > + emitStore(dst, jsUndefined()); > #endif i thought we have macro functions for checking and set undefined Created attachment 243879 [details] Updated patch. > Comment on attachment 243874 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=243874&action=review > > > Source/JavaScriptCore/ChangeLog:11 > > + exit to a path where that funciton gets created, check the scope register value > > function Fixed. > > Source/JavaScriptCore/jit/JITOpcodes.cpp:1071 > > + notUndefinedScope = branch64(NotEqual, regT0, TrustedImm64(JSValue::encode(jsUndefined()))); > > + store64(TrustedImm64(JSValue::encode(jsUndefined())), Address(callFrameRegister, sizeof(Register) * dst)); > > #else > > emitLoadPayload(currentInstruction[2].u.operand, regT0); > > + notUndefinedScope = branch32(NotEqual, tagFor(dst), TrustedImm32(JSValue::UndefinedTag)); > > + emitStore(dst, jsUndefined()); > > #endif > > i thought we have macro functions for checking and set undefined I did find is / is not integer, but I couldn't find any "is undefined" macros. I made a fix to the 32 bit case after running JSC tests. Committed r177871: <http://trac.webkit.org/changeset/177871> Comment on attachment 243879 [details]
Updated patch.
This approach is pretty crazy, and likely wrong. Our engine is architected to produce the correct values upon OSR exit -- not to recover from incorrect values after OSR exit.
You should rethink your solution, and figure out why the OSR exit machinery did not know that the scope register would be used at this point in the bytecode.
|