commit cfade7054a4ae0a80d3c39f71224b5738ad3bf2a
parent f9dd4b533df712f6ad101ee3f70d9042c86b8b66
Author: Jan de Mooij <jdemooij@mozilla.com>
Date: Sat, 13 Dec 2025 08:11:45 +0000
Bug 2005479 part 10 - Ensure LIRGeneratorShared::add is only called for instructions with no outputs. r=iain
We use template arguments to ensure at compile-time that `define*` is only called for
LIR instructions with the expected number of outputs, but we didn't have a similar
check for `add`. This patch fixes that.
This caught a number of LIR instructions with unused output `LDefinition`s and it would
also have caught the issue fixed in the previous patch.
Differential Revision: https://phabricator.services.mozilla.com/D276185
Diffstat:
7 files changed, 97 insertions(+), 91 deletions(-)
diff --git a/js/src/jit/LIROps.yaml b/js/src/jit/LIROps.yaml
@@ -3127,7 +3127,6 @@
mir_op: true
- name: AsmJSStoreHeap
- result_type: WordSized
operands:
ptr: WordSized
value: WordSized
@@ -3683,7 +3682,6 @@
mir_op: true
- name: WasmStoreLaneSimd128
- result_type: WordSized
operands:
ptr: WordSized
src: WordSized
diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp
@@ -4227,7 +4227,7 @@ void LIRGenerator::visitHomeObjectSuperBase(MHomeObjectSuperBase* ins) {
}
void LIRGenerator::visitInterruptCheck(MInterruptCheck* ins) {
- LInstruction* lir = new (alloc()) LInterruptCheck();
+ auto* lir = new (alloc()) LInterruptCheck();
add(lir, ins);
assignSafepoint(lir, ins);
}
@@ -4264,13 +4264,11 @@ void LIRGenerator::visitReinterpretCast(MReinterpretCast* ins) {
}
void LIRGenerator::visitStoreDynamicSlot(MStoreDynamicSlot* ins) {
- LInstruction* lir;
-
switch (ins->value()->type()) {
case MIRType::Value:
- lir = new (alloc())
- LStoreDynamicSlotV(useRegister(ins->slots()), useBox(ins->value()));
- add(lir, ins);
+ add(new (alloc()) LStoreDynamicSlotV(useRegister(ins->slots()),
+ useBox(ins->value())),
+ ins);
break;
case MIRType::Double:
@@ -4736,17 +4734,19 @@ void LIRGenerator::visitBoundsCheck(MBoundsCheck* ins) {
return;
}
- LInstruction* check;
if (ins->minimum() || ins->maximum()) {
- check = new (alloc())
+ auto* check = new (alloc())
LBoundsCheckRange(useRegisterOrInt32Constant(ins->index()),
useAny(ins->length()), temp());
+ assignSnapshot(check, ins->bailoutKind());
+ add(check, ins);
} else {
- check = new (alloc()) LBoundsCheck(useRegisterOrInt32Constant(ins->index()),
- useAnyOrInt32Constant(ins->length()));
+ auto* check =
+ new (alloc()) LBoundsCheck(useRegisterOrInt32Constant(ins->index()),
+ useAnyOrInt32Constant(ins->length()));
+ assignSnapshot(check, ins->bailoutKind());
+ add(check, ins);
}
- assignSnapshot(check, ins->bailoutKind());
- add(check, ins);
}
void LIRGenerator::visitSpectreMaskIndex(MSpectreMaskIndex* ins) {
@@ -4766,8 +4766,7 @@ void LIRGenerator::visitBoundsCheckLower(MBoundsCheckLower* ins) {
return;
}
- LInstruction* check =
- new (alloc()) LBoundsCheckLower(useRegister(ins->index()));
+ auto* check = new (alloc()) LBoundsCheckLower(useRegister(ins->index()));
assignSnapshot(check, ins->bailoutKind());
add(check, ins);
}
@@ -4835,7 +4834,7 @@ void LIRGenerator::visitStoreElement(MStoreElement* ins) {
switch (ins->value()->type()) {
case MIRType::Value: {
- LInstruction* lir =
+ auto* lir =
new (alloc()) LStoreElementV(elements, index, useBox(ins->value()));
if (ins->fallible()) {
assignSnapshot(lir, ins->bailoutKind());
@@ -4846,7 +4845,7 @@ void LIRGenerator::visitStoreElement(MStoreElement* ins) {
default: {
const LAllocation value = useRegisterOrNonDoubleConstant(ins->value());
- LInstruction* lir = new (alloc()) LStoreElementT(elements, index, value);
+ auto* lir = new (alloc()) LStoreElementT(elements, index, value);
if (ins->fallible()) {
assignSnapshot(lir, ins->bailoutKind());
}
@@ -4883,24 +4882,25 @@ void LIRGenerator::visitStoreElementHole(MStoreElementHole* ins) {
const LUse elements = useRegister(ins->elements());
const LAllocation index = useRegister(ins->index());
- LInstruction* lir;
switch (ins->value()->type()) {
- case MIRType::Value:
- lir = new (alloc()) LStoreElementHoleV(object, elements, index,
- useBox(ins->value()), temp());
+ case MIRType::Value: {
+ auto* lir = new (alloc()) LStoreElementHoleV(
+ object, elements, index, useBox(ins->value()), temp());
+ assignSnapshot(lir, ins->bailoutKind());
+ add(lir, ins);
+ assignSafepoint(lir, ins);
break;
-
+ }
default: {
const LAllocation value = useRegisterOrNonDoubleConstant(ins->value());
- lir = new (alloc())
+ auto* lir = new (alloc())
LStoreElementHoleT(object, elements, index, value, temp());
+ assignSnapshot(lir, ins->bailoutKind());
+ add(lir, ins);
+ assignSafepoint(lir, ins);
break;
}
}
-
- assignSnapshot(lir, ins->bailoutKind());
- add(lir, ins);
- assignSafepoint(lir, ins);
}
void LIRGenerator::visitEffectiveAddress3(MEffectiveAddress3* ins) {
@@ -5768,7 +5768,7 @@ void LIRGenerator::visitGuardShape(MGuardShape* ins) {
auto* lir = new (alloc())
LGuardShape(useRegister(ins->object()), LDefinition::BogusTemp());
assignSnapshot(lir, ins->bailoutKind());
- add(lir, ins);
+ addUnchecked(lir, ins);
redefine(ins, ins->object());
}
}
@@ -5787,7 +5787,7 @@ void LIRGenerator::visitGuardMultipleShapes(MGuardMultipleShapes* ins) {
useRegister(ins->object()), useRegister(ins->shapeList()), temp(),
temp(), temp(), LDefinition::BogusTemp());
assignSnapshot(lir, ins->bailoutKind());
- add(lir, ins);
+ addUnchecked(lir, ins);
redefine(ins, ins->object());
}
}
@@ -5804,7 +5804,7 @@ void LIRGenerator::visitGuardShapeList(MGuardShapeList* ins) {
auto* lir = new (alloc()) LGuardShapeList(useRegister(ins->object()),
temp(), LDefinition::BogusTemp());
assignSnapshot(lir, ins->bailoutKind());
- add(lir, ins);
+ addUnchecked(lir, ins);
redefine(ins, ins->object());
}
}
@@ -6185,36 +6185,33 @@ void LIRGenerator::visitGuardFunctionScript(MGuardFunctionScript* ins) {
void LIRGenerator::visitAssertRange(MAssertRange* ins) {
MDefinition* input = ins->input();
- LInstruction* lir = nullptr;
-
switch (input->type()) {
case MIRType::Boolean:
case MIRType::Int32:
case MIRType::IntPtr:
- lir = new (alloc()) LAssertRangeI(useRegisterAtStart(input));
+ add(new (alloc()) LAssertRangeI(useRegisterAtStart(input)), ins);
break;
case MIRType::Double:
- lir = new (alloc()) LAssertRangeD(useRegister(input), tempDouble());
+ add(new (alloc()) LAssertRangeD(useRegister(input), tempDouble()), ins);
break;
case MIRType::Float32:
- lir = new (alloc())
- LAssertRangeF(useRegister(input), tempDouble(), tempDouble());
+ add(new (alloc())
+ LAssertRangeF(useRegister(input), tempDouble(), tempDouble()),
+ ins);
break;
case MIRType::Value:
- lir = new (alloc()) LAssertRangeV(useBox(input), tempToUnbox(),
- tempDouble(), tempDouble());
+ add(new (alloc()) LAssertRangeV(useBox(input), tempToUnbox(),
+ tempDouble(), tempDouble()),
+ ins);
break;
default:
MOZ_CRASH("Unexpected Range for MIRType");
break;
}
-
- lir->setMir(ins);
- add(lir);
}
void LIRGenerator::visitAssertClass(MAssertClass* ins) {
@@ -6317,7 +6314,7 @@ void LIRGenerator::visitSetPropertyCache(MSetPropertyCache* ins) {
// We need a double temp register for TypedArray stubs.
LDefinition tempD = tempFixed(FloatReg0);
- LInstruction* lir = new (alloc()) LSetPropertyCache(
+ auto* lir = new (alloc()) LSetPropertyCache(
useRegister(ins->object()), useBoxOrTypedOrConstant(id, useConstId),
useBoxOrTypedOrConstant(ins->value(), useConstValue), temp(), tempD);
add(lir, ins);
@@ -6830,7 +6827,7 @@ void LIRGenerator::visitWasmBoundsCheck(MWasmBoundsCheck* ins) {
auto* lir = new (alloc()) LWasmBoundsCheckInstanceField64(
useRegisterAtStart(lengthFromInstance->instance()),
useInt64RegisterAtStart(index), lengthFromInstance->offset());
- add(lir, ins);
+ addUnchecked(lir, ins);
}
} else {
MOZ_ASSERT(index->type() == MIRType::Int32);
@@ -6844,7 +6841,7 @@ void LIRGenerator::visitWasmBoundsCheck(MWasmBoundsCheck* ins) {
auto* lir = new (alloc()) LWasmBoundsCheckInstanceField(
useRegisterAtStart(lengthFromInstance->instance()),
useRegisterAtStart(index), lengthFromInstance->offset());
- add(lir, ins);
+ addUnchecked(lir, ins);
}
}
@@ -6861,7 +6858,7 @@ void LIRGenerator::visitWasmBoundsCheck(MWasmBoundsCheck* ins) {
auto* lir = new (alloc()) LWasmBoundsCheck64(
useInt64RegisterAtStart(index),
useInt64RegisterOrConstantAtStart(boundsCheckLimit));
- add(lir, ins);
+ addUnchecked(lir, ins);
}
} else {
MOZ_ASSERT(index->type() == MIRType::Int32);
@@ -6874,7 +6871,7 @@ void LIRGenerator::visitWasmBoundsCheck(MWasmBoundsCheck* ins) {
auto* lir = new (alloc())
LWasmBoundsCheck(useRegisterAtStart(index),
useRegisterOrConstantAtStart(boundsCheckLimit));
- add(lir, ins);
+ addUnchecked(lir, ins);
}
}
}
@@ -7004,15 +7001,16 @@ void LIRGenerator::visitWasmStoreStackResult(MWasmStoreStackResult* ins) {
MDefinition* stackResultArea = ins->stackResultArea();
MDefinition* value = ins->value();
size_t offs = ins->offset();
- LInstruction* lir;
if (value->type() == MIRType::Int64) {
- lir = new (alloc()) LWasmStoreStackResultI64(
- useInt64Register(value), useRegister(stackResultArea), offs);
+ add(new (alloc()) LWasmStoreStackResultI64(
+ useInt64Register(value), useRegister(stackResultArea), offs),
+ ins);
} else {
- lir = new (alloc()) LWasmStoreStackResult(
- useRegister(value), useRegister(stackResultArea), offs, value->type());
+ add(new (alloc()) LWasmStoreStackResult(useRegister(value),
+ useRegister(stackResultArea), offs,
+ value->type()),
+ ins);
}
- add(lir, ins);
}
void LIRGenerator::visitWasmDerivedPointer(MWasmDerivedPointer* ins) {
@@ -7166,7 +7164,7 @@ void LIRGenerator::visitWasmRegisterResult(MWasmRegisterResult* ins) {
auto type = LDefinition::TypeFrom(ins->type());
lir->setDef(0, LDefinition(vreg, type, LGeneralReg(ins->loc())));
ins->setVirtualRegister(vreg);
- add(lir, ins);
+ addUnchecked(lir, ins);
}
void LIRGenerator::visitWasmFloatRegisterResult(MWasmFloatRegisterResult* ins) {
@@ -7175,7 +7173,7 @@ void LIRGenerator::visitWasmFloatRegisterResult(MWasmFloatRegisterResult* ins) {
auto type = LDefinition::TypeFrom(ins->type());
lir->setDef(0, LDefinition(vreg, type, LFloatReg(ins->loc())));
ins->setVirtualRegister(vreg);
- add(lir, ins);
+ addUnchecked(lir, ins);
}
void LIRGenerator::visitWasmBuiltinFloatRegisterResult(
@@ -7185,7 +7183,7 @@ void LIRGenerator::visitWasmBuiltinFloatRegisterResult(
auto type = LDefinition::TypeFrom(ins->type());
lir->setDef(0, LDefinition(vreg, type, LFloatReg(ins->loc())));
ins->setVirtualRegister(vreg);
- add(lir, ins);
+ addUnchecked(lir, ins);
}
void LIRGenerator::visitWasmRegister64Result(MWasmRegister64Result* ins) {
@@ -7210,7 +7208,7 @@ void LIRGenerator::visitWasmRegister64Result(MWasmRegister64Result* ins) {
#endif
ins->setVirtualRegister(vreg);
- add(lir, ins);
+ addUnchecked(lir, ins);
}
void LIRGenerator::visitWasmStackResultArea(MWasmStackResultArea* ins) {
@@ -7220,7 +7218,7 @@ void LIRGenerator::visitWasmStackResultArea(MWasmStackResultArea* ins) {
lir->setDef(0,
LDefinition(vreg, LDefinition::STACKRESULTS, LDefinition::STACK));
ins->setVirtualRegister(vreg);
- add(lir, ins);
+ addUnchecked(lir, ins);
}
void LIRGenerator::visitWasmStackResult(MWasmStackResult* ins) {
@@ -7240,7 +7238,7 @@ void LIRGenerator::visitWasmStackResult(MWasmStackResult* ins) {
lir->setDef(0, LDefinition(vreg, typ, pol));
#endif
ins->setVirtualRegister(vreg);
- add(lir, ins);
+ addUnchecked(lir, ins);
return;
}
@@ -7250,7 +7248,7 @@ void LIRGenerator::visitWasmStackResult(MWasmStackResult* ins) {
LDefinition::Type typ = LDefinition::TypeFrom(ins->type());
lir->setDef(0, LDefinition(vreg, typ, pol));
ins->setVirtualRegister(vreg);
- add(lir, ins);
+ addUnchecked(lir, ins);
}
void LIRGenerator::visitWasmStackSwitchToSuspendable(
@@ -8683,17 +8681,17 @@ void LIRGenerator::visitWasmStoreField(MWasmStoreField* ins) {
uint32_t offs = ins->offset();
MNarrowingOp narrowingOp = ins->narrowingOp();
LAllocation base = useRegister(ins->base());
- LInstruction* lir;
if (value->type() == MIRType::Int64) {
MOZ_RELEASE_ASSERT(narrowingOp == MNarrowingOp::None);
- lir = new (alloc()) LWasmStoreSlotI64(useInt64Register(value), base, offs,
- ins->maybeTrap());
+ add(new (alloc()) LWasmStoreSlotI64(useInt64Register(value), base, offs,
+ ins->maybeTrap()),
+ ins);
} else {
- lir = new (alloc())
- LWasmStoreSlot(useRegister(value), base, offs, value->type(),
- narrowingOp, ins->maybeTrap());
+ add(new (alloc())
+ LWasmStoreSlot(useRegister(value), base, offs, value->type(),
+ narrowingOp, ins->maybeTrap()),
+ ins);
}
- add(lir, ins);
if (ins->keepAlive() != ins->base()) {
add(new (alloc()) LKeepAliveObject(useKeepalive(ins->keepAlive())), ins);
}
@@ -8718,19 +8716,19 @@ void LIRGenerator::visitWasmStoreElement(MWasmStoreElement* ins) {
MDefinition* value = ins->value();
MNarrowingOp narrowingOp = ins->narrowingOp();
Scale scale = ins->scale();
- LInstruction* lir;
if (value->type() == MIRType::Int64) {
MOZ_RELEASE_ASSERT(narrowingOp == MNarrowingOp::None);
- lir = new (alloc()) LWasmStoreElementI64(
- base, index, useInt64Register(value), ins->maybeTrap());
+ add(new (alloc()) LWasmStoreElementI64(base, index, useInt64Register(value),
+ ins->maybeTrap()),
+ ins);
} else {
LDefinition tmp =
value->type() == MIRType::Simd128 ? temp() : LDefinition::BogusTemp();
- lir = new (alloc())
- LWasmStoreElement(base, index, useRegister(value), tmp, value->type(),
- narrowingOp, scale, ins->maybeTrap());
+ add(new (alloc()) LWasmStoreElement(base, index, useRegister(value), tmp,
+ value->type(), narrowingOp, scale,
+ ins->maybeTrap()),
+ ins);
}
- add(lir, ins);
if (ins->keepAlive() != ins->base()) {
add(new (alloc()) LKeepAliveObject(useKeepalive(ins->keepAlive())), ins);
}
diff --git a/js/src/jit/MIROps.yaml b/js/src/jit/MIROps.yaml
@@ -1964,6 +1964,7 @@
congruent_to: if_operands_equal
alias_set: none
generate_lir: true
+ lir_result_type: none
lir_temps: 1
# Inlined TypedArray.prototype.subarray
@@ -3543,6 +3544,7 @@
result_type: Object
alias_set: custom
generate_lir: true
+ lir_result_type: none
# Return true if the object is definitely a TypedArray constructor, but not
# necessarily from the currently active realm. Return false if the object is
@@ -3645,6 +3647,7 @@
folds_to: custom
alias_set: none
generate_lir: true
+ lir_result_type: none
- name: GuardInt32Range
operands:
@@ -4179,6 +4182,7 @@
congruent_to: if_operands_equal
type_policy: none
generate_lir: true
+ lir_result_type: none
lir_temps: 1
alias_set: none
guard: true
diff --git a/js/src/jit/arm/Lowering-arm.cpp b/js/src/jit/arm/Lowering-arm.cpp
@@ -80,7 +80,7 @@ void LIRGenerator::visitBox(MBox* box) {
lir->setDef(0, LDefinition(vreg, LDefinition::GENERAL));
lir->setDef(1, LDefinition::BogusTemp());
box->setVirtualRegister(vreg);
- add(lir);
+ addUnchecked(lir);
}
void LIRGenerator::visitUnbox(MUnbox* unbox) {
diff --git a/js/src/jit/shared/Lowering-shared-inl.h b/js/src/jit/shared/Lowering-shared-inl.h
@@ -54,7 +54,7 @@ void LIRGeneratorShared::define(
lir->getDef(0)->setVirtualRegister(vreg);
lir->setMir(mir);
mir->setVirtualRegister(vreg);
- add(lir);
+ addUnchecked(lir);
}
template <size_t X, size_t Y>
@@ -95,7 +95,7 @@ void LIRGeneratorShared::defineInt64Fixed(
lir->setMir(mir);
mir->setVirtualRegister(vreg);
- add(lir);
+ addUnchecked(lir);
}
template <size_t Ops, size_t Temps>
@@ -149,7 +149,7 @@ void LIRGeneratorShared::defineInt64ReuseInput(
lir->setMir(mir);
mir->setVirtualRegister(vreg);
- add(lir);
+ addUnchecked(lir);
}
template <size_t Ops, size_t Temps>
@@ -191,7 +191,7 @@ void LIRGeneratorShared::defineBoxReuseInput(
lir->setMir(mir);
mir->setVirtualRegister(vreg);
- add(lir);
+ addUnchecked(lir);
}
template <size_t Temps>
@@ -216,7 +216,7 @@ void LIRGeneratorShared::defineBox(
lir->setMir(mir);
mir->setVirtualRegister(vreg);
- add(lir);
+ addUnchecked(lir);
}
template <size_t Ops, size_t Temps>
@@ -246,7 +246,7 @@ void LIRGeneratorShared::defineInt64(
lir->setMir(mir);
mir->setVirtualRegister(vreg);
- add(lir);
+ addUnchecked(lir);
}
void LIRGeneratorShared::defineReturn(LInstruction* lir, MDefinition* mir) {
@@ -323,7 +323,7 @@ void LIRGeneratorShared::defineReturn(LInstruction* lir, MDefinition* mir) {
}
mir->setVirtualRegister(vreg);
- add(lir);
+ addUnchecked(lir);
}
#ifdef DEBUG
@@ -660,13 +660,11 @@ LDefinition LIRGeneratorShared::tempCopy(MDefinition* input,
return t;
}
-template <typename T>
-void LIRGeneratorShared::annotate(T* ins) {
+void LIRGeneratorShared::annotate(LNode* ins) {
ins->setId(lirGraph_.getInstructionId());
}
-template <typename T>
-void LIRGeneratorShared::add(T* ins, MInstruction* mir) {
+void LIRGeneratorShared::addUnchecked(LInstruction* ins, MInstruction* mir) {
MOZ_ASSERT(!ins->isPhi());
current->add(ins);
if (mir) {
diff --git a/js/src/jit/shared/Lowering-shared.h b/js/src/jit/shared/Lowering-shared.h
@@ -305,10 +305,18 @@ class LIRGeneratorShared {
return vreg;
}
- template <typename T>
- void annotate(T* ins);
- template <typename T>
- void add(T* ins, MInstruction* mir = nullptr);
+ inline void annotate(LNode* ins);
+ inline void addUnchecked(LInstruction* ins, MInstruction* mir = nullptr);
+
+ // The template parameter ensures this can only be called for LIR instructions
+ // with no outputs. Call addUnchecked directly to ignore this check for code
+ // that sets the output manually with setDef or for LIR instructions with an
+ // optional output register.
+ template <size_t Temps>
+ void add(details::LInstructionFixedDefsTempsHelper<0, Temps>* ins,
+ MInstruction* mir = nullptr) {
+ addUnchecked(ins, mir);
+ }
void lowerTypedPhiInput(MPhi* phi, uint32_t inputPosition, LBlock* block,
size_t lirIndex);
diff --git a/js/src/jit/x86/Lowering-x86.cpp b/js/src/jit/x86/Lowering-x86.cpp
@@ -80,7 +80,7 @@ void LIRGenerator::visitBox(MBox* box) {
lir->setDef(0, LDefinition(vreg, LDefinition::GENERAL));
lir->setDef(1, LDefinition::BogusTemp());
box->setVirtualRegister(vreg);
- add(lir);
+ addUnchecked(lir);
}
void LIRGenerator::visitUnbox(MUnbox* unbox) {
