Skip to content

Commit

Permalink
Cherry-pick 2e29306. rdar://problem/110634913
Browse files Browse the repository at this point in the history
    Prevent duplication of WASM throw and rethrow patchpoints in B3
    https://bugs.webkit.org/show_bug.cgi?id=258408
    rdar://110634913

    Reviewed by Yusuke Suzuki.

    Adds a cloningForbidden property to B3Kind, used to prevent a B3 value
    from being cloned during optimizations, and applies it to the patchpoints
    generated for the WASM throw and rethrow opcodes in WasmB3IRGenerator.
    This prevents a problem where these patchpoints could be duplicated, still
    share a stackmap/callsite index, but have conflicting live value
    locations.

    * JSTests/wasm/stress/phi-live-across-rethrow.js: Added.
    (async test):
    * JSTests/wasm/stress/phi-live-across-throw.js: Added.
    (async test):
    * Source/JavaScriptCore/b3/B3DuplicateTails.cpp:
    * Source/JavaScriptCore/b3/B3Kind.cpp:
    (JSC::B3::Kind::dump const):
    * Source/JavaScriptCore/b3/B3Kind.h:
    (JSC::B3::Kind::hasCloningForbidden):
    (JSC::B3::Kind::hasCloningForbidden const):
    (JSC::B3::Kind::isCloningForbidden const):
    (JSC::B3::Kind::setIsCloningForbidden):
    (JSC::B3::Kind::operator== const):
    (JSC::B3::Kind::hash const):
    (JSC::B3::cloningForbidden):
    * Source/JavaScriptCore/b3/B3PatchpointValue.cpp:
    (JSC::B3::PatchpointValue::PatchpointValue):
    * Source/JavaScriptCore/b3/B3PatchpointValue.h:
    * Source/JavaScriptCore/b3/B3ValueInlines.h:
    (JSC::B3::Value::cloneImpl const):
    * Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:
    (JSC::Wasm::B3IRGenerator::addThrow):
    (JSC::Wasm::B3IRGenerator::addRethrow):

    Canonical link: https://commits.webkit.org/259548.844@safari-7615-branch
Identifier: 259548.843@safari-7615.3.9.11-branch
  • Loading branch information
ddegazio authored and MyahCobbs committed Jun 22, 2023
1 parent eec7917 commit 4863705
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 9 deletions.
37 changes: 37 additions & 0 deletions JSTests/wasm/stress/phi-live-across-rethrow.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { instantiate } from "../wabt-wrapper.js";
import * as assert from "../assert.js";

let wat = `
(module
(global i32 i32.const 43)
(global i32 i32.const 1)
(func (export "test") (result i32) (local i32)
try
try
throw $exc
catch $exc
i32.const 42
local.set 0
block
global.get 1
br_if 0
global.get 0
local.set 0
end
rethrow 0
end
catch $exc
end
local.get 0
)
(tag $exc)
)
`;

async function test() {
const instance = await instantiate(wat, {}, { exceptions: true });
for (let i = 0; i < 20000; i ++)
assert.eq(instance.exports.test(), 42);
}

assert.asyncTest(test());
33 changes: 33 additions & 0 deletions JSTests/wasm/stress/phi-live-across-throw.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { instantiate } from "../wabt-wrapper.js";
import * as assert from "../assert.js";

let wat = `
(module
(global i32 i32.const 43)
(global i32 i32.const 1)
(func (export "test") (result i32) (local i32)
i32.const 42
local.set 0
block
global.get 1
br_if 0
global.get 0
local.set 0
end
try
throw $exc
catch $exc
end
local.get 0
)
(tag $exc)
)
`;

async function test() {
const instance = await instantiate(wat, {}, { exceptions: true });
for (let i = 0; i < 20000; i ++)
assert.eq(instance.exports.test(), 42);
}

assert.asyncTest(test());
11 changes: 10 additions & 1 deletion Source/JavaScriptCore/b3/B3DuplicateTails.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,16 @@ class DuplicateTails {
if (block->last()->type() != Void) // Demoting doesn't handle terminals with values.
continue;

candidates.add(block);
bool canCopyBlock = true;
for (Value* value : *block) {
if (value->kind().isCloningForbidden()) {
canCopyBlock = false;
break;
}
}

if (canCopyBlock)
candidates.add(block);
}

// Collect the set of values that must be de-SSA'd.
Expand Down
2 changes: 2 additions & 0 deletions Source/JavaScriptCore/b3/B3Kind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ void Kind::dump(PrintStream& out) const
out.print(comma, "Traps");
if (isSensitiveToNaN())
out.print(comma, "SensitiveToNaN");
if (isCloningForbidden())
out.print(comma, "CloningForbidden");
if (comma.didPrint())
out.print(">");
}
Expand Down
35 changes: 33 additions & 2 deletions Source/JavaScriptCore/b3/B3Kind.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,29 @@ class Kind {
ASSERT(hasIsSensitiveToNaN());
m_isSensitiveToNaN = isSensitiveToNaN;
}

static constexpr bool hasCloningForbidden(Opcode opcode)
{
switch (opcode) {
case Patchpoint:
return true;
default:
return false;
}
}
bool hasCloningForbidden() const
{
return hasCloningForbidden(m_opcode);
}
bool isCloningForbidden() const
{
return m_cloningForbidden;
}
void setIsCloningForbidden(bool isCloningForbidden)
{
ASSERT(hasCloningForbidden());
m_cloningForbidden = isCloningForbidden;
}

// Rules for adding new properties:
// - Put the accessors here.
Expand All @@ -189,7 +212,8 @@ class Kind {
return m_opcode == other.m_opcode
&& m_isChill == other.m_isChill
&& m_traps == other.m_traps
&& m_isSensitiveToNaN == other.m_isSensitiveToNaN;
&& m_isSensitiveToNaN == other.m_isSensitiveToNaN
&& m_cloningForbidden == other.m_cloningForbidden;
}

bool operator!=(const Kind& other) const
Expand All @@ -203,7 +227,7 @@ class Kind {
{
// It's almost certainly more important that this hash function is cheap to compute than
// anything else. We can live with some kind hash collisions.
return m_opcode + (static_cast<unsigned>(m_isChill) << 16) + (static_cast<unsigned>(m_traps) << 7) + (static_cast<unsigned>(m_isSensitiveToNaN) << 24);
return m_opcode + (static_cast<unsigned>(m_isChill) << 16) + (static_cast<unsigned>(m_traps) << 7) + (static_cast<unsigned>(m_isSensitiveToNaN) << 24) + (static_cast<unsigned>(m_cloningForbidden) << 13);
}

Kind(WTF::HashTableDeletedValueType)
Expand All @@ -222,6 +246,7 @@ class Kind {
bool m_isChill : 1 { false };
bool m_traps : 1 { false };
bool m_isSensitiveToNaN : 1 { false };
bool m_cloningForbidden : 1 { false };
};

// For every flag 'foo' you add, it's customary to create a Kind B3::foo(Kind) function that makes
Expand Down Expand Up @@ -250,6 +275,12 @@ inline Kind sensitiveToNaN(Kind kind)
return kind;
}

inline Kind cloningForbidden(Kind kind)
{
kind.setIsCloningForbidden(true);
return kind;
}

struct KindHash {
static unsigned hash(const Kind& key) { return key.hash(); }
static bool equal(const Kind& a, const Kind& b) { return a == b; }
Expand Down
5 changes: 3 additions & 2 deletions Source/JavaScriptCore/b3/B3PatchpointValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ void PatchpointValue::dumpMeta(CommaPrinter& comma, PrintStream& out) const
out.print(comma, "numFPScratchRegisters = ", numFPScratchRegisters);
}

PatchpointValue::PatchpointValue(Type type, Origin origin)
: Base(CheckedOpcode, Patchpoint, type, origin)
PatchpointValue::PatchpointValue(Type type, Origin origin, Kind kind)
: Base(CheckedOpcode, kind, type, origin)
, effects(Effects::forCall())
{
ASSERT(accepts(kind));
if (!type.isTuple())
resultConstraints.append(type == Void ? ValueRep::WarmAny : ValueRep::SomeRegister);
}
Expand Down
5 changes: 3 additions & 2 deletions Source/JavaScriptCore/b3/B3PatchpointValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class PatchpointValue final : public StackmapValue {
public:
typedef StackmapValue Base;

static bool accepts(Kind kind) { return kind == Patchpoint; }
static bool accepts(Kind kind) { return kind.opcode() == Patchpoint; }

~PatchpointValue() final;

Expand Down Expand Up @@ -71,7 +71,8 @@ class PatchpointValue final : public StackmapValue {
friend class Value;

static Opcode opcodeFromConstructor(Type, Origin) { return Patchpoint; }
JS_EXPORT_PRIVATE PatchpointValue(Type, Origin);
static Opcode opcodeFromConstructor(Type, Origin, Kind) { return Patchpoint; }
JS_EXPORT_PRIVATE PatchpointValue(Type, Origin, Kind = Patchpoint);
};

} } // namespace JSC::B3
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/b3/B3ValueInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ ALWAYS_INLINE size_t Value::adjacencyListOffset() const

ALWAYS_INLINE Value* Value::cloneImpl() const
{
ASSERT(!kind().isCloningForbidden());
#define VALUE_TYPE_CLONE(ValueType) allocate<ValueType>(*static_cast<const ValueType*>(this))
DISPATCH_ON_KIND(VALUE_TYPE_CLONE);
#undef VALUE_TYPE_CLONE
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3676,7 +3676,7 @@ auto B3IRGenerator::addDelegateToUnreachable(ControlType& target, ControlType& d

auto B3IRGenerator::addThrow(unsigned exceptionIndex, Vector<ExpressionType>& args, Stack&) -> PartialResult
{
PatchpointValue* patch = m_proc.add<PatchpointValue>(B3::Void, origin());
PatchpointValue* patch = m_proc.add<PatchpointValue>(B3::Void, origin(), cloningForbidden(Patchpoint));
patch->effects.terminal = true;
patch->append(instanceValue(), ValueRep::reg(GPRInfo::argumentGPR0));
for (unsigned i = 0; i < args.size(); ++i) {
Expand All @@ -3701,7 +3701,7 @@ auto B3IRGenerator::addThrow(unsigned exceptionIndex, Vector<ExpressionType>& ar

auto B3IRGenerator::addRethrow(unsigned, ControlType& data) -> PartialResult
{
PatchpointValue* patch = m_proc.add<PatchpointValue>(B3::Void, origin());
PatchpointValue* patch = m_proc.add<PatchpointValue>(B3::Void, origin(), cloningForbidden(Patchpoint));
patch->clobber(RegisterSetBuilder::registersToSaveForJSCall(m_proc.usesSIMD() ? RegisterSetBuilder::allRegisters() : RegisterSetBuilder::allScalarRegisters()));
patch->effects.terminal = true;
patch->append(instanceValue(), ValueRep::reg(GPRInfo::argumentGPR0));
Expand Down

0 comments on commit 4863705

Please sign in to comment.