From 014ebe8ae2e0deaa545f2fcfaf933bc7577a888a Mon Sep 17 00:00:00 2001 From: Martok Date: Sun, 23 Apr 2023 02:11:42 +0200 Subject: [PATCH] Issue #2142 - Fold BytecodeEmitter::checkTypeSet into BytecodeEmitter::emitCheck The bytecode emitter used to call checkTypeSet for each JOF_TYPESET op. Despite correctness asserts in the TypeScript code, this was pretty error prone. The solution is to move this check to BytecodeEmitter::emitCheck (called for each opcode we emit), so we don't have to worry about this anymore. Based-on: m-c 1521491/3 --- js/src/frontend/BytecodeEmitter.cpp | 53 +++++++++------------------- js/src/frontend/BytecodeEmitter.h | 6 +--- js/src/frontend/CallOrNewEmitter.cpp | 1 - 3 files changed, 17 insertions(+), 43 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 78b134cf58..51ebd81969 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -245,7 +245,7 @@ BytecodeEmitter::locationOfNameBoundInFunctionScope(JSAtom* name, EmitterScope* } bool -BytecodeEmitter::emitCheck(ptrdiff_t delta, ptrdiff_t* offset) +BytecodeEmitter::emitCheck(JSOp op, ptrdiff_t delta, ptrdiff_t* offset) { size_t oldLength = code().length(); *offset = ptrdiff_t(oldLength); @@ -260,6 +260,13 @@ BytecodeEmitter::emitCheck(ptrdiff_t delta, ptrdiff_t* offset) ReportOutOfMemory(cx); return false; } + + // If op is JOF_TYPESET (see the type barriers comment in TypeInference.h), + // reserve a type set to store its result. + if (CodeSpec[op].format & JOF_TYPESET) { + if (typesetCount < UINT16_MAX) + typesetCount++; + } return true; } @@ -297,7 +304,7 @@ BytecodeEmitter::emit1(JSOp op) MOZ_ASSERT(checkStrictOrSloppy(op)); ptrdiff_t offset; - if (!emitCheck(1, &offset)) + if (!emitCheck(op, 1, &offset)) return false; jsbytecode* code = this->code(offset); @@ -312,7 +319,7 @@ BytecodeEmitter::emit2(JSOp op, uint8_t op1) MOZ_ASSERT(checkStrictOrSloppy(op)); ptrdiff_t offset; - if (!emitCheck(2, &offset)) + if (!emitCheck(op, 2, &offset)) return false; jsbytecode* code = this->code(offset); @@ -332,7 +339,7 @@ BytecodeEmitter::emit3(JSOp op, jsbytecode op1, jsbytecode op2) MOZ_ASSERT(!IsLocalOp(op)); ptrdiff_t offset; - if (!emitCheck(3, &offset)) + if (!emitCheck(op, 3, &offset)) return false; jsbytecode* code = this->code(offset); @@ -350,7 +357,7 @@ BytecodeEmitter::emitN(JSOp op, size_t extra, ptrdiff_t* offset) ptrdiff_t length = 1 + ptrdiff_t(extra); ptrdiff_t off; - if (!emitCheck(length, &off)) + if (!emitCheck(op, length, &off)) return false; jsbytecode* code = this->code(off); @@ -391,7 +398,7 @@ bool BytecodeEmitter::emitJumpNoFallthrough(JSOp op, JumpList* jump) { ptrdiff_t offset; - if (!emitCheck(5, &offset)) + if (!emitCheck(op, 5, &offset)) return false; jsbytecode* code = this->code(offset); @@ -634,22 +641,12 @@ BytecodeEmitter::emitLoopEntry(ParseNode* nextpn, JumpList entryJump) return emit2(JSOP_LOOPENTRY, loopDepthAndFlags); } -void -BytecodeEmitter::checkTypeSet(JSOp op) -{ - if (CodeSpec[op].format & JOF_TYPESET) { - if (typesetCount < UINT16_MAX) - typesetCount++; - } -} - bool BytecodeEmitter::emitUint16Operand(JSOp op, uint32_t operand) { MOZ_ASSERT(operand <= UINT16_MAX); if (!emit3(op, UINT16_HI(operand), UINT16_LO(operand))) return false; - checkTypeSet(op); return true; } @@ -660,7 +657,6 @@ BytecodeEmitter::emitUint32Operand(JSOp op, uint32_t operand) if (!emitN(op, 4, &off)) return false; SET_UINT32(code(off), operand); - checkTypeSet(op); return true; } @@ -881,13 +877,12 @@ BytecodeEmitter::emitIndex32(JSOp op, uint32_t index) MOZ_ASSERT(len == size_t(CodeSpec[op].length)); ptrdiff_t offset; - if (!emitCheck(len, &offset)) + if (!emitCheck(op, len, &offset)) return false; jsbytecode* code = this->code(offset); code[0] = jsbytecode(op); SET_UINT32_INDEX(code, index); - checkTypeSet(op); updateDepth(offset); return true; } @@ -901,13 +896,12 @@ BytecodeEmitter::emitIndexOp(JSOp op, uint32_t index) MOZ_ASSERT(len >= 1 + UINT32_INDEX_LEN); ptrdiff_t offset; - if (!emitCheck(len, &offset)) + if (!emitCheck(op, len, &offset)) return false; jsbytecode* code = this->code(offset); code[0] = jsbytecode(op); SET_UINT32_INDEX(code, index); - checkTypeSet(op); updateDepth(offset); return true; } @@ -1023,7 +1017,6 @@ BytecodeEmitter::emitEnvCoordOp(JSOp op, EnvironmentCoordinate ec) pc += ENVCOORD_HOPS_LEN; SET_ENVCOORD_SLOT(pc, ec.slot()); pc += ENVCOORD_SLOT_LEN; - checkTypeSet(op); return true; } @@ -1713,7 +1706,7 @@ BytecodeEmitter::emitNewInit(JSProtoKey key) { const size_t len = 1 + UINT32_INDEX_LEN; ptrdiff_t offset; - if (!emitCheck(len, &offset)) + if (!emitCheck(JSOP_NEWINIT, len, &offset)) return false; jsbytecode* code = this->code(offset); @@ -1722,7 +1715,6 @@ BytecodeEmitter::emitNewInit(JSProtoKey key) code[2] = 0; code[3] = 0; code[4] = 0; - checkTypeSet(JSOP_NEWINIT); updateDepth(offset); return true; } @@ -1953,7 +1945,6 @@ BytecodeEmitter::emitElemOpBase(JSOp op) if (!emit1(op)) return false; - checkTypeSet(op); return true; } @@ -2770,7 +2761,6 @@ BytecodeEmitter::emitIteratorNext(ParseNode* pn, IteratorKind iterKind /* = Iter if (!emitCheckIsObj(CheckIsObjectKind::IteratorNext)) // ... RESULT return false; - checkTypeSet(JSOP_CALL); return true; } @@ -2860,7 +2850,6 @@ BytecodeEmitter::emitIteratorCloseInScope(EmitterScope& currentScope, if (!emitCall(JSOP_CALL, 0)) // ... ... RESULT return false; - checkTypeSet(JSOP_CALL); if (iterKind == IteratorKind::Async) { if (completionKind != CompletionKind::Throw) { @@ -4496,7 +4485,6 @@ BytecodeEmitter::emitRequireObjectCoercible() return false; if (!emitCall(JSOP_CALL_IGNORES_RV, 1))// VAL IGNORED return false; - checkTypeSet(JSOP_CALL_IGNORES_RV); if (!emit1(JSOP_POP)) // VAL return false; @@ -4543,7 +4531,6 @@ BytecodeEmitter::emitCopyDataProperties(CopyOption option) } if (!emitCall(JSOP_CALL_IGNORES_RV, argc)) // IGNORED return false; - checkTypeSet(JSOP_CALL_IGNORES_RV); if (!emit1(JSOP_POP)) // - return false; @@ -4566,7 +4553,6 @@ BytecodeEmitter::emitIterator() return false; if (!emitCall(JSOP_CALLITER, 0)) // ITER return false; - checkTypeSet(JSOP_CALLITER); if (!emitCheckIsObj(CheckIsObjectKind::GetIterator)) // ITER return false; return true; @@ -4605,7 +4591,6 @@ BytecodeEmitter::emitAsyncIterator() return false; if (!emitCall(JSOP_CALLITER, 0)) // ITER return false; - checkTypeSet(JSOP_CALLITER); if (!emitCheckIsObj(CheckIsObjectKind::GetIterator)) // ITER return false; @@ -4619,7 +4604,6 @@ BytecodeEmitter::emitAsyncIterator() return false; if (!emitCall(JSOP_CALLITER, 0)) // ITER return false; - checkTypeSet(JSOP_CALLITER); if (!emitCheckIsObj(CheckIsObjectKind::GetIterator)) // ITER return false; @@ -6425,7 +6409,6 @@ BytecodeEmitter::emitYieldStar(ParseNode* iter) return false; if (!emitCall(JSOP_CALL, 1, iter)) // ITER OLDRESULT RESULT return false; - checkTypeSet(JSOP_CALL); if (isAsyncGenerator) { if (!emitAwaitInInnermostScope()) // NEXT ITER OLDRESULT RESULT @@ -6496,7 +6479,6 @@ BytecodeEmitter::emitYieldStar(ParseNode* iter) return false; if (!emitCall(JSOP_CALL, 1)) // ITER OLDRESULT FTYPE FVALUE RESULT return false; - checkTypeSet(JSOP_CALL); if (iterKind == IteratorKind::Async) { if (!emitAwaitInInnermostScope()) // ... FTYPE FVALUE RESULT @@ -6577,7 +6559,6 @@ BytecodeEmitter::emitYieldStar(ParseNode* iter) return false; if (!emitCall(JSOP_CALL, 1, iter)) // ITER RESULT return false; - checkTypeSet(JSOP_CALL); if (isAsyncGenerator) { if (!emitAwaitInInnermostScope()) // NEXT ITER RESULT RESULT @@ -7039,7 +7020,6 @@ BytecodeEmitter::emitSelfHostedCallFunction(BinaryNode* callNode) if (!emitCall(callOp, argc)) return false; - checkTypeSet(callOp); return true; } @@ -8329,7 +8309,6 @@ BytecodeEmitter::emitFunctionFormalParameters(ListNode* paramsBody) } else if (isRest) { if (!emit1(JSOP_REST)) return false; - checkTypeSet(JSOP_REST); } // Initialize the parameter name. diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 7a59cf0825..9cbf6ee38b 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -418,17 +418,13 @@ struct MOZ_STACK_CLASS BytecodeEmitter // Emit function code for the tree rooted at body. MOZ_MUST_USE bool emitFunctionScript(FunctionNode* funNode); - // If op is JOF_TYPESET (see the type barriers comment in TypeInference.h), - // reserve a type set to store its result. - void checkTypeSet(JSOp op); - void updateDepth(ptrdiff_t target); MOZ_MUST_USE bool updateLineNumberNotes(uint32_t offset); MOZ_MUST_USE bool updateSourceCoordNotes(uint32_t offset); JSOp strictifySetNameOp(JSOp op); - MOZ_MUST_USE bool emitCheck(ptrdiff_t delta, ptrdiff_t* offset); + MOZ_MUST_USE bool emitCheck(JSOp op, ptrdiff_t delta, ptrdiff_t* offset); // Emit one bytecode. MOZ_MUST_USE bool emit1(JSOp op); diff --git a/js/src/frontend/CallOrNewEmitter.cpp b/js/src/frontend/CallOrNewEmitter.cpp index bd1e32ab02..81e1956c25 100644 --- a/js/src/frontend/CallOrNewEmitter.cpp +++ b/js/src/frontend/CallOrNewEmitter.cpp @@ -305,7 +305,6 @@ CallOrNewEmitter::emitEnd(uint32_t argc, const Maybe& beginPos) return false; } } - bce_->checkTypeSet(op_); if (isEval() && beginPos) { uint32_t lineNum = bce_->parser->tokenStream.srcCoords.lineNum(*beginPos);