Bug 232019

Summary: We should watch isHavingABadTime if we read from the structureCache
Product: WebKit Reporter: Justin Michaud <justin_michaud>
Component: JavaScriptCoreAssignee: Justin Michaud <justin_michaud>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, ews-feeder, justin_michaud, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Justin Michaud
Reported 2021-10-20 09:30:44 PDT
rdar://84338462 We should watch isHavingABadTime if we read from the structureCache off the mutator thread
Attachments
Patch (7.44 KB, patch)
2021-10-20 09:45 PDT, Justin Michaud 		no flags
DetailsFormatted DiffDiff
Patch (7.64 KB, patch)
2021-10-20 14:08 PDT, Justin Michaud 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Justin Michaud
Comment 1 2021-10-20 09:45:46 PDT
Created attachment 441889 [details] Patch
Yusuke Suzuki
Comment 2 2021-10-20 11:19:47 PDT
Comment on attachment 441889 [details] Patch r=me
Yusuke Suzuki
Comment 3 2021-10-20 11:44:49 PDT
Comment on attachment 441889 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441889&action=review > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3131 > + if (!globalObject->isHavingABadTime()) I think you need to insert load-load-fence just after this. So, bool isHavingABadTime = globalObject->isHavingABadTime(); WTF::loadLoadFence(); if (!isHavingABadTime) m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint()); > Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:846 > + if (!globalObject->isHavingABadTime()) Ditto
Justin Michaud
Comment 4 2021-10-20 14:08:50 PDT
Created attachment 441931 [details] Patch
EWS
Comment 5 2021-10-20 15:08:52 PDT
Committed r284576 (243316@main): <https://commits.webkit.org/243316@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441931 [details].
Saam Barati
Comment 6 2021-10-22 11:19:00 PDT
Comment on attachment 441931 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441931&action=review > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3138 > + WTF::loadLoadFence(); Why do we have 2 load load fences? > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3140 > + structure = m_vm.structureCache > + .emptyObjectStructureConcurrently(globalObject, base.getObject(), JSFinalObject::defaultInlineCapacity()); this should not be on multiple lines. > Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:855 > + structure = globalObject->vm().structureCache > + .emptyObjectStructureConcurrently(globalObject, base.getObject(), JSFinalObject::defaultInlineCapacity()); ditto, not 2 lines. If you really like putting things on 2 lines, you should do it for the arguments, not the "."
Note You need to log in before you can comment on or make changes to this bug.