...
<rdar://problem/63827506>
Created attachment 402056 [details] Patch
Comment on attachment 402056 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402056&action=review r=me > Source/JavaScriptCore/ChangeLog:26 > + we replace the calling target with special function which throws an error. In DFG / FTL, we emit `CheckNotJSCast` DFT node which typo: /DFT node/DFG node/ > Source/JavaScriptCore/bytecode/AccessCase.cpp:1606 > + // Note that GetterSetter alwyas has cells for both. If it is not set (like, getter exits, but setter is not set), Null{Getter,Setter}Function is stored. typo: /alwyas/always/ > Source/JavaScriptCore/bytecode/AccessCase.cpp:1614 > + // We replace getter with this AccessCase's JSGlobalObject::nullSetterStrictFunction, which will throw an error with the right JSGlobalObject. You're replacing the "setter" here, not the ""getter", right? > Source/JavaScriptCore/dfg/DFGStructureAbstractValue.cpp:358 > + // Note taht this function returns true if the structure set is empty. typo: /taht/that/. Same for StructureAbstractValue::isSubClassOf above. > Source/JavaScriptCore/runtime/NullSetterFunction.cpp:93 > + constexpr bool useCurrentFrame = false; At first I thought this is wrong, but then, I realized that this function is an InternalFunction, and therefore should be skipped. Can you write a short test to verify that error.stack's top frame entry is that of the JS function exercising the IC? Or augment one of your existing tests?
Comment on attachment 402056 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402056&action=review >> Source/JavaScriptCore/bytecode/AccessCase.cpp:1606 >> + // Note that GetterSetter alwyas has cells for both. If it is not set (like, getter exits, but setter is not set), Null{Getter,Setter}Function is stored. > > typo: /alwyas/always/ Fixed. >> Source/JavaScriptCore/bytecode/AccessCase.cpp:1614 >> + // We replace getter with this AccessCase's JSGlobalObject::nullSetterStrictFunction, which will throw an error with the right JSGlobalObject. > > You're replacing the "setter" here, not the ""getter", right? Fixed. >> Source/JavaScriptCore/dfg/DFGStructureAbstractValue.cpp:358 >> + // Note taht this function returns true if the structure set is empty. > > typo: /taht/that/. Same for StructureAbstractValue::isSubClassOf above. Fixed. >> Source/JavaScriptCore/runtime/NullSetterFunction.cpp:93 >> + constexpr bool useCurrentFrame = false; > > At first I thought this is wrong, but then, I realized that this function is an InternalFunction, and therefore should be skipped. Can you write a short test to verify that error.stack's top frame entry is that of the JS function exercising the IC? Or augment one of your existing tests? Added.
Committed r263134: <https://trac.webkit.org/changeset/263134>
Committed r263165: <https://trac.webkit.org/changeset/263165>