Bug 232019 – We should watch isHavingABadTime if we read from the structureCache

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Justin Michaud]

rdar://84338462

We should watch isHavingABadTime if we read from the structureCache off the mutator thread

[Justin Michaud]

Created attachment 441889 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 441889 [details]
Patch

r=me

[Yusuke Suzuki]

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]

Created attachment 441931 [details]
Patch

[EWS]

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 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 "."