Bug 1744520: Fix OOM handling for variadic instructions r=jandem

Variadic instructions contain a FixedList of operands which is initialized fallibly in MVariadicT::init. This means MFoo::New is fallible for variadic instructions. A fuzz bug found one unhandled OOM in scalar replacement. I did a quick survey of places where we create new variadic nodes, and found a few more latent bugs that I introduced in my patches to scalar-replace arguments. I fixed those bugs and added a comment on MVariadicInstruction in the hopes of avoiding the same mistake in the future.

I'm not adding the fuzz testcase, because OOM tests of Ion internals are incredibly fragile and will stop working as soon as we add or remove one more allocation somewhere.

Differential Revision: https://phabricator.services.mozilla.com/D133017
This commit is contained in:
Iain Ireland 2021-12-07 17:47:08 +00:00
Родитель 94504d59a5
Коммит c9b8775be5
3 изменённых файлов: 33 добавлений и 0 удалений

Просмотреть файл

@ -1201,6 +1201,9 @@ class MVariadicT : public T {
}
};
// An instruction with a variable number of operands. Note that the
// MFoo::New constructor for variadic instructions fallibly
// initializes the operands_ array and must be checked for OOM.
using MVariadicInstruction = MVariadicT<MInstruction>;
MIR_OPCODE_CLASS_GENERATED

Просмотреть файл

@ -1451,6 +1451,10 @@ void ArrayMemoryView::visitApplyArray(MApplyArray* ins) {
bool isDOMCall = false;
auto* call = MakeCall(alloc_, addUndefined, callInfo, needsThisCheck,
ins->getSingleTarget(), isDOMCall);
if (!call) {
oom_ = true;
return;
}
if (!ins->maybeCrossRealm()) {
call->setNotCrossRealm();
}
@ -1493,6 +1497,10 @@ void ArrayMemoryView::visitConstructArray(MConstructArray* ins) {
bool isDOMCall = false;
auto* call = MakeCall(alloc_, addUndefined, callInfo, needsThisCheck,
ins->getSingleTarget(), isDOMCall);
if (!call) {
oom_ = true;
return;
}
if (!ins->maybeCrossRealm()) {
call->setNotCrossRealm();
}
@ -1517,6 +1525,8 @@ class ArgumentsReplacer : public MDefinitionVisitorDefaultNoop {
MIRGraph& graph_;
MInstruction* args_;
bool oom_ = false;
TempAllocator& alloc() { return graph_.alloc(); }
bool isInlinedArguments() const {
@ -1536,6 +1546,8 @@ class ArgumentsReplacer : public MDefinitionVisitorDefaultNoop {
void visitArrayFromArgumentsObject(MArrayFromArgumentsObject* ins);
void visitLoadFixedSlot(MLoadFixedSlot* ins);
bool oom() const { return oom_; }
public:
ArgumentsReplacer(MIRGenerator* mir, MIRGraph& graph, MInstruction* args)
: mir_(mir), graph_(graph), args_(args) {
@ -1693,6 +1705,9 @@ bool ArgumentsReplacer::run() {
MIR_OPCODE_LIST(MIR_OP)
#undef MIR_OP
}
if (oom()) {
return false;
}
}
}
@ -1864,6 +1879,10 @@ void ArgumentsReplacer::visitLoadArgumentsObjectArg(
}
loadArg = MGetInlinedArgument::New(alloc(), check, actualArgs);
if (!loadArg) {
oom_ = true;
return;
}
} else {
// Insert bounds check.
auto* length = MArgumentsLength::New(alloc());
@ -1905,6 +1924,10 @@ void ArgumentsReplacer::visitLoadArgumentsObjectArgHole(
auto* actualArgs = args_->toCreateInlinedArgumentsObject();
loadArg = MGetInlinedArgumentHole::New(alloc(), index, actualArgs);
if (!loadArg) {
oom_ = true;
return;
}
} else {
auto* length = MArgumentsLength::New(alloc());
ins->block()->insertBefore(ins, length);
@ -1999,6 +2022,10 @@ void ArgumentsReplacer::visitApplyArgsObj(MApplyArgsObj* ins) {
bool isDOMCall = false;
auto* call = MakeCall(alloc(), addUndefined, callInfo, needsThisCheck,
ins->getSingleTarget(), isDOMCall);
if (!call) {
oom_ = true;
return;
}
if (!ins->maybeCrossRealm()) {
call->setNotCrossRealm();
}

Просмотреть файл

@ -1658,6 +1658,9 @@ bool WarpBuilder::build_Arguments(BytecodeLocation loc) {
if (inlineCallInfo()) {
argsObj = MCreateInlinedArgumentsObject::New(alloc(), env, getCallee(),
inlineCallInfo()->argv());
if (!argsObj) {
return false;
}
} else {
argsObj = MCreateArgumentsObject::New(alloc(), env, templateObj);
}