Skip to content

Commit

Permalink
Cherry-pick 272448.653@safari-7618-branch (f6e2c3b). https://bugs.web…
Browse files Browse the repository at this point in the history
…kit.org/show_bug.cgi?id=270259

    GetWebAssemblyInstanceExports nodes should not be blindly hoisted
    https://bugs.webkit.org/show_bug.cgi?id=270259
    rdar://123617167

    Reviewed by Alexey Shvayka.

    GetWebAssemblyInstanceExports nodes should not be blindly hoisted above
    their structure check.

    ```
    case WebAssemblyInstanceExportsIntrinsic:
        ...
        addToGraph(CheckStructure, OpInfo(m_graph.addStructureSet(variant.structureSet())), thisNode);
        set(result, addToGraph(GetWebAssemblyInstanceExports, Edge(thisNode, KnownCellUse)));
    ```

    Similar to GetByOffset, we should only hoist this node if we have proven
    that the child has the structure of a WebAssembly Instance.

    * JSTests/stress/hoist-get-wasm-exports.js: Added.
    (opt):
    (main):
    * Source/JavaScriptCore/dfg/DFGSafeToExecute.h:
    (JSC::DFG::safeToExecute):

    Canonical link: https://commits.webkit.org/272448.653@safari-7618-branch

Canonical link: https://commits.webkit.org/274313.223@webkitglib/2.44
  • Loading branch information
Justin Michaud authored and aperezdc committed May 13, 2024
1 parent 9655eab commit 56df757
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
30 changes: 30 additions & 0 deletions JSTests/stress/hoist-get-wasm-exports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
function opt(access, instance, transition) {
let result;
instance.x;

for (let i = 0; i < 2; i++) {
transition.y = 1;

if (access) {
result = instance.exports;
}
}

return result;
}

function main() {
const emptyModule = new WebAssembly.Module(new Uint8Array([0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00]));
const instance = new WebAssembly.Instance(emptyModule, {});
instance.x = 1;

const object = {x: 1, p1: 1, p2: 1, p3: 1, p4: 0x1234};

for (let i = 0; i < 100000; i++) {
opt(/* access */ true, instance, {});
opt(/* access */ false, object, {});
}
}

if (typeof WebAssembly === "object")
main();
17 changes: 16 additions & 1 deletion Source/JavaScriptCore/dfg/DFGSafeToExecute.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node, bool igno
case DataViewGetInt:
case DataViewGetFloat:
case ResolveRope:
case GetWebAssemblyInstanceExports:
case NumberIsNaN:
case StringIndexOf:
return true;
Expand Down Expand Up @@ -439,6 +438,22 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node, bool igno
return true;
}

case GetWebAssemblyInstanceExports: {
if (!state.forNode(node->child1()).isType(SpecCell))
return false;

StructureAbstractValue& value = state.forNode(node->child1()).m_structure;
if (value.isInfinite())
return false;
for (unsigned i = value.size(); i--;) {
Structure* structure = value[i].get();
if (structure->typeInfo().type() != WebAssemblyInstanceType)
return false;
}

return true;
}

case GetByOffset:
case PutByOffset: {
// If it's an inline property, we need to make sure it's a cell before trusting what the structure set tells us.
Expand Down

0 comments on commit 56df757

Please sign in to comment.