From 13a6adc918a62eb5a3c97f40f12be1b8e3e572ae Mon Sep 17 00:00:00 2001 From: Keith Miller Date: Mon, 26 Apr 2021 20:20:13 +0000 Subject: [PATCH] numCalleeLocals, numParameters, and numVars should be unsigned https://bugs.webkit.org/show_bug.cgi?id=224995 Reviewed by Mark Lam. All of the various CodeBlock classes currently have the numCalleeLocals and numVars marked as ints. I believe this is just a historical artifact or because VirtualRegister's offset is an int to make handling constants easier. Regardless, it's a bit strange to not handle the sign conversion at the point of comparison between a VirtualRegister offset and the local/var count. This doesn't completely fix every place we use ints for these values but starts on the right track. Lastly, I also added some Checks to the wasm parser for sanity checking. * bytecode/CodeBlock.cpp: (JSC::CodeBlock::setNumParameters): (JSC::CodeBlock::ensureCatchLivenessIsComputedForBytecodeIndexSlow): * bytecode/CodeBlock.h: (JSC::CodeBlock::numParameters const): (JSC::CodeBlock::numberOfArgumentsToSkip const): (JSC::CodeBlock::numCalleeLocals const): (JSC::CodeBlock::numVars const): (JSC::CodeBlock::numTmps const): (JSC::CodeBlock::addressOfNumParameters): (JSC::CodeBlock::isTemporaryRegister): * bytecode/UnlinkedCodeBlock.h: (JSC::UnlinkedCodeBlock::numCalleeLocals const): (JSC::UnlinkedCodeBlock::numVars const): * bytecode/UnlinkedCodeBlockGenerator.h: (JSC::UnlinkedCodeBlockGenerator::numCalleeLocals const): (JSC::UnlinkedCodeBlockGenerator::numVars const): (JSC::UnlinkedCodeBlockGenerator::setNumCalleeLocals): (JSC::UnlinkedCodeBlockGenerator::setNumVars): (JSC::UnlinkedCodeBlockGenerator::setNumParameters): * bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::generate): (JSC::BytecodeGenerator::emitPushFunctionNameScope): * bytecompiler/BytecodeGeneratorBaseInlines.h: (JSC::BytecodeGeneratorBase::newRegister): * dfg/DFGByteCodeParser.cpp: (JSC::DFG::ByteCodeParser::handleRecursiveTailCall): (JSC::DFG::ByteCodeParser::inliningCost): (JSC::DFG::ByteCodeParser::parseBlock): * dfg/DFGOSREntrypointCreationPhase.cpp: (JSC::DFG::OSREntrypointCreationPhase::run): * dfg/DFGSpeculativeJIT.cpp: (JSC::DFG::SpeculativeJIT::checkArgumentTypes): * ftl/FTLLowerDFGToB3.cpp: (JSC::FTL::DFG::LowerDFGToB3::lower): * ftl/FTLOSREntry.cpp: (JSC::FTL::prepareOSREntry): * interpreter/CallFrameClosure.h: * interpreter/ProtoCallFrameInlines.h: (JSC::ProtoCallFrame::init): * jit/JIT.cpp: (JSC::JIT::compileWithoutLinking): * runtime/CommonSlowPaths.h: (JSC::CommonSlowPaths::numberOfStackPaddingSlots): (JSC::CommonSlowPaths::numberOfStackPaddingSlotsWithExtraSlots): * wasm/WasmFunctionCodeBlock.h: (JSC::Wasm::FunctionCodeBlock::numVars const): (JSC::Wasm::FunctionCodeBlock::numCalleeLocals const): (JSC::Wasm::FunctionCodeBlock::setNumVars): (JSC::Wasm::FunctionCodeBlock::setNumCalleeLocals): * wasm/WasmLLIntGenerator.cpp: (JSC::Wasm::LLIntGenerator::push): (JSC::Wasm::LLIntGenerator::getDropKeepCount): (JSC::Wasm::LLIntGenerator::walkExpressionStack): (JSC::Wasm::LLIntGenerator::checkConsistency): (JSC::Wasm::LLIntGenerator::materializeConstantsAndLocals): (JSC::Wasm::LLIntGenerator::splitStack): (JSC::Wasm::LLIntGenerator::finalize): (JSC::Wasm::LLIntGenerator::callInformationForCaller): (JSC::Wasm::LLIntGenerator::addLoop): (JSC::Wasm::LLIntGenerator::addTopLevel): (JSC::Wasm::LLIntGenerator::addBlock): (JSC::Wasm::LLIntGenerator::addIf): (JSC::Wasm::LLIntGenerator::addElseToUnreachable): Canonical link: https://commits.webkit.org/237038@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276609 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/JavaScriptCore/ChangeLog | 82 +++++++++++++++++++ Source/JavaScriptCore/bytecode/CodeBlock.cpp | 4 +- Source/JavaScriptCore/bytecode/CodeBlock.h | 24 +++--- .../bytecode/UnlinkedCodeBlock.h | 10 +-- .../bytecode/UnlinkedCodeBlockGenerator.h | 10 +-- .../bytecompiler/BytecodeGenerator.cpp | 4 +- .../BytecodeGeneratorBaseInlines.h | 5 +- .../JavaScriptCore/dfg/DFGByteCodeParser.cpp | 14 ++-- .../dfg/DFGOSREntrypointCreationPhase.cpp | 6 +- .../JavaScriptCore/dfg/DFGSpeculativeJIT.cpp | 2 +- Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp | 2 +- Source/JavaScriptCore/ftl/FTLOSREntry.cpp | 3 +- .../interpreter/CallFrameClosure.h | 2 +- .../interpreter/ProtoCallFrameInlines.h | 2 +- Source/JavaScriptCore/jit/JIT.cpp | 2 +- .../JavaScriptCore/runtime/CommonSlowPaths.h | 4 +- Source/JavaScriptCore/tools/VMInspector.cpp | 2 +- .../wasm/WasmFunctionCodeBlock.h | 12 +-- .../wasm/WasmLLIntGenerator.cpp | 36 ++++---- 19 files changed, 155 insertions(+), 71 deletions(-) diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index cb7a2cf0a097..7541c08bb79c 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,85 @@ +2021-04-26 Keith Miller + + numCalleeLocals, numParameters, and numVars should be unsigned + https://bugs.webkit.org/show_bug.cgi?id=224995 + + Reviewed by Mark Lam. + + All of the various CodeBlock classes currently have the + numCalleeLocals and numVars marked as ints. I believe this is just + a historical artifact or because VirtualRegister's offset is an + int to make handling constants easier. Regardless, it's a bit + strange to not handle the sign conversion at the point of + comparison between a VirtualRegister offset and the local/var + count. This doesn't completely fix every place we use ints for + these values but starts on the right track. Lastly, I also added + some Checks to the wasm parser for sanity checking. + + * bytecode/CodeBlock.cpp: + (JSC::CodeBlock::setNumParameters): + (JSC::CodeBlock::ensureCatchLivenessIsComputedForBytecodeIndexSlow): + * bytecode/CodeBlock.h: + (JSC::CodeBlock::numParameters const): + (JSC::CodeBlock::numberOfArgumentsToSkip const): + (JSC::CodeBlock::numCalleeLocals const): + (JSC::CodeBlock::numVars const): + (JSC::CodeBlock::numTmps const): + (JSC::CodeBlock::addressOfNumParameters): + (JSC::CodeBlock::isTemporaryRegister): + * bytecode/UnlinkedCodeBlock.h: + (JSC::UnlinkedCodeBlock::numCalleeLocals const): + (JSC::UnlinkedCodeBlock::numVars const): + * bytecode/UnlinkedCodeBlockGenerator.h: + (JSC::UnlinkedCodeBlockGenerator::numCalleeLocals const): + (JSC::UnlinkedCodeBlockGenerator::numVars const): + (JSC::UnlinkedCodeBlockGenerator::setNumCalleeLocals): + (JSC::UnlinkedCodeBlockGenerator::setNumVars): + (JSC::UnlinkedCodeBlockGenerator::setNumParameters): + * bytecompiler/BytecodeGenerator.cpp: + (JSC::BytecodeGenerator::generate): + (JSC::BytecodeGenerator::emitPushFunctionNameScope): + * bytecompiler/BytecodeGeneratorBaseInlines.h: + (JSC::BytecodeGeneratorBase::newRegister): + * dfg/DFGByteCodeParser.cpp: + (JSC::DFG::ByteCodeParser::handleRecursiveTailCall): + (JSC::DFG::ByteCodeParser::inliningCost): + (JSC::DFG::ByteCodeParser::parseBlock): + * dfg/DFGOSREntrypointCreationPhase.cpp: + (JSC::DFG::OSREntrypointCreationPhase::run): + * dfg/DFGSpeculativeJIT.cpp: + (JSC::DFG::SpeculativeJIT::checkArgumentTypes): + * ftl/FTLLowerDFGToB3.cpp: + (JSC::FTL::DFG::LowerDFGToB3::lower): + * ftl/FTLOSREntry.cpp: + (JSC::FTL::prepareOSREntry): + * interpreter/CallFrameClosure.h: + * interpreter/ProtoCallFrameInlines.h: + (JSC::ProtoCallFrame::init): + * jit/JIT.cpp: + (JSC::JIT::compileWithoutLinking): + * runtime/CommonSlowPaths.h: + (JSC::CommonSlowPaths::numberOfStackPaddingSlots): + (JSC::CommonSlowPaths::numberOfStackPaddingSlotsWithExtraSlots): + * wasm/WasmFunctionCodeBlock.h: + (JSC::Wasm::FunctionCodeBlock::numVars const): + (JSC::Wasm::FunctionCodeBlock::numCalleeLocals const): + (JSC::Wasm::FunctionCodeBlock::setNumVars): + (JSC::Wasm::FunctionCodeBlock::setNumCalleeLocals): + * wasm/WasmLLIntGenerator.cpp: + (JSC::Wasm::LLIntGenerator::push): + (JSC::Wasm::LLIntGenerator::getDropKeepCount): + (JSC::Wasm::LLIntGenerator::walkExpressionStack): + (JSC::Wasm::LLIntGenerator::checkConsistency): + (JSC::Wasm::LLIntGenerator::materializeConstantsAndLocals): + (JSC::Wasm::LLIntGenerator::splitStack): + (JSC::Wasm::LLIntGenerator::finalize): + (JSC::Wasm::LLIntGenerator::callInformationForCaller): + (JSC::Wasm::LLIntGenerator::addLoop): + (JSC::Wasm::LLIntGenerator::addTopLevel): + (JSC::Wasm::LLIntGenerator::addBlock): + (JSC::Wasm::LLIntGenerator::addIf): + (JSC::Wasm::LLIntGenerator::addElseToUnreachable): + 2021-04-26 Alexey Shvayka [JSC] OrdinarySet should invoke custom [[Set]] methods diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp index de9cb5e1c392..fc5afafc431d 100644 --- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp +++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp @@ -924,7 +924,7 @@ void CodeBlock::setAlternative(VM& vm, CodeBlock* alternative) m_alternative.set(vm, this, alternative); } -void CodeBlock::setNumParameters(int newValue) +void CodeBlock::setNumParameters(unsigned newValue) { m_numParameters = newValue; @@ -2015,7 +2015,7 @@ void CodeBlock::ensureCatchLivenessIsComputedForBytecodeIndexSlow(const OpCatch& liveOperands.append(virtualRegisterForLocal(liveLocal)); }); - for (int i = 0; i < numParameters(); ++i) + for (unsigned i = 0; i < numParameters(); ++i) liveOperands.append(virtualRegisterForArgumentIncludingThis(i)); auto* profiles = ValueProfileAndVirtualRegisterBuffer::create(liveOperands.size()); diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.h b/Source/JavaScriptCore/bytecode/CodeBlock.h index 16e8d51e327c..5748d139d9cb 100644 --- a/Source/JavaScriptCore/bytecode/CodeBlock.h +++ b/Source/JavaScriptCore/bytecode/CodeBlock.h @@ -156,17 +156,17 @@ class CodeBlock : public JSCell { MetadataTable* metadataTable() const { return m_metadata.get(); } - int numParameters() const { return m_numParameters; } - void setNumParameters(int newValue); + unsigned numParameters() const { return m_numParameters; } + void setNumParameters(unsigned newValue); - int numberOfArgumentsToSkip() const { return m_numberOfArgumentsToSkip; } + unsigned numberOfArgumentsToSkip() const { return m_numberOfArgumentsToSkip; } - int numCalleeLocals() const { return m_numCalleeLocals; } + unsigned numCalleeLocals() const { return m_numCalleeLocals; } - int numVars() const { return m_numVars; } - int numTmps() const { return m_unlinkedCode->hasCheckpoints() * maxNumCheckpointTmps; } + unsigned numVars() const { return m_numVars; } + unsigned numTmps() const { return m_unlinkedCode->hasCheckpoints() * maxNumCheckpointTmps; } - int* addressOfNumParameters() { return &m_numParameters; } + unsigned* addressOfNumParameters() { return &m_numParameters; } static ptrdiff_t offsetOfNumParameters() { return OBJECT_OFFSETOF(CodeBlock, m_numParameters); } CodeBlock* alternative() const { return static_cast(m_alternative.get()); } @@ -243,7 +243,7 @@ class CodeBlock : public JSCell { ALWAYS_INLINE bool isTemporaryRegister(VirtualRegister reg) { - return reg.offset() >= m_numVars; + return reg.offset() >= static_cast(m_numVars); } HandlerInfo* handlerForBytecodeIndex(BytecodeIndex, RequiredHandler = RequiredHandler::AnyHandler); @@ -989,10 +989,10 @@ class CodeBlock : public JSCell { void insertBasicBlockBoundariesForControlFlowProfiler(); void ensureCatchLivenessIsComputedForBytecodeIndexSlow(const OpCatch&, BytecodeIndex); - int m_numCalleeLocals; - int m_numVars; - int m_numParameters; - int m_numberOfArgumentsToSkip { 0 }; + unsigned m_numCalleeLocals; + unsigned m_numVars; + unsigned m_numParameters; + unsigned m_numberOfArgumentsToSkip { 0 }; unsigned m_numberOfNonArgumentValueProfiles { 0 }; union { unsigned m_debuggerRequests; diff --git a/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h b/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h index b4210467ce2d..316ebca842e3 100644 --- a/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h +++ b/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h @@ -209,8 +209,8 @@ class UnlinkedCodeBlock : public JSCell { const InstructionStream& instructions() const; - int numCalleeLocals() const { return m_numCalleeLocals; } - int numVars() const { return m_numVars; } + unsigned numCalleeLocals() const { return m_numCalleeLocals; } + unsigned numVars() const { return m_numVars; } // Jump Tables @@ -396,9 +396,9 @@ class UnlinkedCodeBlock : public JSCell { unsigned m_lineCount { 0 }; unsigned m_endColumn { UINT_MAX }; - int m_numVars { 0 }; - int m_numCalleeLocals { 0 }; - int m_numParameters { 0 }; + unsigned m_numVars { 0 }; + unsigned m_numCalleeLocals { 0 }; + unsigned m_numParameters { 0 }; PackedRefPtr m_sourceURLDirective; PackedRefPtr m_sourceMappingURLDirective; diff --git a/Source/JavaScriptCore/bytecode/UnlinkedCodeBlockGenerator.h b/Source/JavaScriptCore/bytecode/UnlinkedCodeBlockGenerator.h index 49abbaa1c73b..ce330d6a259c 100644 --- a/Source/JavaScriptCore/bytecode/UnlinkedCodeBlockGenerator.h +++ b/Source/JavaScriptCore/bytecode/UnlinkedCodeBlockGenerator.h @@ -59,8 +59,8 @@ class UnlinkedCodeBlockGenerator { EvalContextType evalContextType() const { return m_codeBlock->evalContextType(); } bool isArrowFunctionContext() const { return m_codeBlock->isArrowFunctionContext(); } bool isClassContext() const { return m_codeBlock->isClassContext(); } - int numCalleeLocals() const { return m_codeBlock->m_numCalleeLocals; } - int numVars() const { return m_codeBlock->m_numVars; } + unsigned numCalleeLocals() const { return m_codeBlock->m_numCalleeLocals; } + unsigned numVars() const { return m_codeBlock->m_numVars; } unsigned numParameters() const { return m_codeBlock->numParameters(); } VirtualRegister thisRegister() const { return m_codeBlock->thisRegister(); } VirtualRegister scopeRegister() const { return m_codeBlock->scopeRegister(); } @@ -71,11 +71,11 @@ class UnlinkedCodeBlockGenerator { // Updating UnlinkedCodeBlock. void setHasCheckpoints() { m_codeBlock->setHasCheckpoints(); } void setHasTailCalls() { m_codeBlock->setHasTailCalls(); } - void setNumCalleeLocals(int numCalleeLocals) { m_codeBlock->m_numCalleeLocals = numCalleeLocals; } - void setNumVars(int numVars) { m_codeBlock->m_numVars = numVars; } + void setNumCalleeLocals(unsigned numCalleeLocals) { m_codeBlock->m_numCalleeLocals = numCalleeLocals; } + void setNumVars(unsigned numVars) { m_codeBlock->m_numVars = numVars; } void setThisRegister(VirtualRegister thisRegister) { m_codeBlock->setThisRegister(thisRegister); } void setScopeRegister(VirtualRegister thisRegister) { m_codeBlock->setScopeRegister(thisRegister); } - void setNumParameters(int newValue) { m_codeBlock->setNumParameters(newValue); } + void setNumParameters(unsigned newValue) { m_codeBlock->setNumParameters(newValue); } UnlinkedMetadataTable& metadata() { return m_codeBlock->metadata(); } void addExpressionInfo(unsigned instructionOffset, int divot, int startOffset, int endOffset, unsigned line, unsigned column); diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp index 426093f11768..f9b0fbf3cec2 100644 --- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp +++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp @@ -282,7 +282,7 @@ ParserError BytecodeGenerator::generate() if (m_isAsync) performGeneratorification(*this, m_codeBlock.get(), m_writer, m_generatorFrameSymbolTable.get(), m_generatorFrameSymbolTableIndex); - RELEASE_ASSERT(static_cast(m_codeBlock->numCalleeLocals()) < static_cast(FirstConstantRegisterIndex)); + RELEASE_ASSERT(m_codeBlock->numCalleeLocals() < static_cast(FirstConstantRegisterIndex)); m_codeBlock->finalize(m_writer.finalize()); if (m_expressionTooDeep) return ParserError(ParserError::OutOfMemory); @@ -3987,7 +3987,7 @@ void BytecodeGenerator::emitPushFunctionNameScope(const Identifier& property, Re addResult.iterator->value.setIsConst(); // The function name scope name acts like a const variable. unsigned numVars = m_codeBlock->numVars(); pushLexicalScopeInternal(nameScopeEnvironment, TDZCheckOptimization::Optimize, NestedScopeType::IsNotNested, nullptr, TDZRequirement::NotUnderTDZ, ScopeType::FunctionNameScope, ScopeRegisterType::Var); - ASSERT_UNUSED(numVars, m_codeBlock->numVars() == static_cast(numVars + 1)); // Should have only created one new "var" for the function name scope. + ASSERT_UNUSED(numVars, m_codeBlock->numVars() == numVars + 1); // Should have only created one new "var" for the function name scope. bool shouldTreatAsLexicalVariable = ecmaMode().isStrict(); Variable functionVar = variableForLocalEntry(property, m_lexicalScopeStack.last().m_symbolTable->get(NoLockingNecessary, property.impl()), m_lexicalScopeStack.last().m_symbolTableConstantIndex, shouldTreatAsLexicalVariable); emitPutToScope(m_lexicalScopeStack.last().m_scope, functionVar, callee, ThrowIfNotFound, InitializationMode::NotInitialization); diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGeneratorBaseInlines.h b/Source/JavaScriptCore/bytecompiler/BytecodeGeneratorBaseInlines.h index 1bd924fcb001..0f4e407579f9 100644 --- a/Source/JavaScriptCore/bytecompiler/BytecodeGeneratorBaseInlines.h +++ b/Source/JavaScriptCore/bytecompiler/BytecodeGeneratorBaseInlines.h @@ -161,9 +161,10 @@ template RegisterID* BytecodeGeneratorBase::newRegister() { m_calleeLocals.append(virtualRegisterForLocal(m_calleeLocals.size())); - int numCalleeLocals = std::max(m_codeBlock->numCalleeLocals(), m_calleeLocals.size()); + size_t numCalleeLocals = std::max(m_codeBlock->numCalleeLocals(), m_calleeLocals.size()); numCalleeLocals = WTF::roundUpToMultipleOf(stackAlignmentRegisters(), numCalleeLocals); - m_codeBlock->setNumCalleeLocals(numCalleeLocals); + m_codeBlock->setNumCalleeLocals(static_cast(numCalleeLocals)); + RELEASE_ASSERT(numCalleeLocals == m_codeBlock->numCalleeLocals()); return &m_calleeLocals.last(); } diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index 408dc08c1430..baeac6cb705c 100644 --- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp +++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp @@ -1494,7 +1494,7 @@ bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant c // Some code may statically use the argument count from the InlineCallFrame, so it would be invalid to loop back if it does not match. // We "continue" instead of returning false in case another stack entry further on the stack has the right number of arguments. - if (argumentCountIncludingThis != static_cast(callFrame->argumentCountIncludingThis)) + if (argumentCountIncludingThis != callFrame->argumentCountIncludingThis) continue; // If the target InlineCallFrame is Varargs, we do not know how many arguments are actually filled by LoadVarargs. Varargs InlineCallFrame's // argumentCountIncludingThis is maximum number of potentially filled arguments by xkLoadVarargs. We "continue" to the upper frame which may be @@ -1504,7 +1504,7 @@ bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant c } else { // We are in the machine code entry (i.e. the original caller). // If we have more arguments than the number of parameters to the function, it is not clear where we could put them on the stack. - if (argumentCountIncludingThis > m_codeBlock->numParameters()) + if (static_cast(argumentCountIncludingThis) > m_codeBlock->numParameters()) return false; } @@ -1530,8 +1530,8 @@ bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant c // We must set the arguments to the right values if (!stackEntry->m_inlineCallFrame) addToGraph(SetArgumentCountIncludingThis, OpInfo(argumentCountIncludingThis)); - int argIndex = 0; - for (; argIndex < argumentCountIncludingThis; ++argIndex) { + unsigned argIndex = 0; + for (; argIndex < static_cast(argumentCountIncludingThis); ++argIndex) { Node* value = get(virtualRegisterForArgumentIncludingThis(argIndex, registerOffset)); setDirect(stackEntry->remapOperand(virtualRegisterForArgumentIncludingThis(argIndex)), value, NormalSet); } @@ -1541,7 +1541,7 @@ bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant c // We must repeat the work of op_enter here as we will jump right after it. // We jump right after it and not before it, because of some invariant saying that a CFG root cannot have predecessors in the IR. - for (int i = 0; i < stackEntry->m_codeBlock->numVars(); ++i) + for (unsigned i = 0; i < stackEntry->m_codeBlock->numVars(); ++i) setDirect(stackEntry->remapOperand(virtualRegisterForLocal(i)), undefined, NormalSet); // We want to emit the SetLocals with an exit origin that points to the place we are jumping to. @@ -1597,7 +1597,7 @@ unsigned ByteCodeParser::inliningCost(CallVariant callee, int argumentCountInclu } if (!Options::useArityFixupInlining()) { - if (codeBlock->numParameters() > argumentCountIncludingThis) { + if (codeBlock->numParameters() > static_cast(argumentCountIncludingThis)) { VERBOSE_LOG(" Failing because of arity mismatch.\n"); return UINT_MAX; } @@ -5418,7 +5418,7 @@ void ByteCodeParser::parseBlock(unsigned limit) case op_enter: { Node* undefined = addToGraph(JSConstant, OpInfo(m_constantUndefined)); // Initialize all locals to undefined. - for (int i = 0; i < m_inlineStackTop->m_codeBlock->numVars(); ++i) + for (unsigned i = 0; i < m_inlineStackTop->m_codeBlock->numVars(); ++i) set(virtualRegisterForLocal(i), undefined, ImmediateNakedSet); NEXT_OPCODE(op_enter); diff --git a/Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp b/Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp index 727d58276741..8eae81f495ad 100644 --- a/Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp +++ b/Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp @@ -94,7 +94,7 @@ class OSREntrypointCreationPhase : public Phase { NodeOrigin origin = NodeOrigin(CodeOrigin(BytecodeIndex(0)), CodeOrigin(BytecodeIndex(0)), false); Vector locals(baseline->numCalleeLocals()); - for (int local = 0; local < baseline->numCalleeLocals(); ++local) { + for (unsigned local = 0; local < baseline->numCalleeLocals(); ++local) { Node* previousHead = target->variablesAtHead.local(local); if (!previousHead) continue; @@ -113,7 +113,7 @@ class OSREntrypointCreationPhase : public Phase { origin = target->at(0)->origin; ArgumentsVector newArguments = m_graph.m_rootToArguments.find(m_graph.block(0))->value; - for (int argument = 0; argument < baseline->numParameters(); ++argument) { + for (unsigned argument = 0; argument < baseline->numParameters(); ++argument) { Node* oldNode = target->variablesAtHead.argument(argument); if (!oldNode) { // Just for sanity, always have a SetArgumentDefinitely even if it's not needed. @@ -125,7 +125,7 @@ class OSREntrypointCreationPhase : public Phase { newArguments[argument] = node; } - for (int local = 0; local < baseline->numCalleeLocals(); ++local) { + for (unsigned local = 0; local < baseline->numCalleeLocals(); ++local) { Node* previousHead = target->variablesAtHead.local(local); if (!previousHead) continue; diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp index b2bf4d0b327c..907a4c2aaf45 100644 --- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp +++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp @@ -2079,7 +2079,7 @@ void SpeculativeJIT::checkArgumentTypes() m_origin = NodeOrigin(CodeOrigin(BytecodeIndex(0)), CodeOrigin(BytecodeIndex(0)), true); auto& arguments = m_jit.graph().m_rootToArguments.find(m_jit.graph().block(0))->value; - for (int i = 0; i < m_jit.codeBlock()->numParameters(); ++i) { + for (unsigned i = 0; i < m_jit.codeBlock()->numParameters(); ++i) { Node* node = arguments[i]; if (!node) { // The argument is dead. We don't do any checks for such arguments. diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp index 6210b9601b60..2078691b104a 100644 --- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp +++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp @@ -362,7 +362,7 @@ class LowerDFGToB3 { if (m_graph.m_plan.mode() == FTLForOSREntryMode) { auto* jitCode = m_ftlState.jitCode->ftlForOSREntry(); FixedVector argumentFlushFormats(codeBlock()->numParameters()); - for (int i = 0; i < codeBlock()->numParameters(); ++i) + for (unsigned i = 0; i < codeBlock()->numParameters(); ++i) argumentFlushFormats[i] = m_graph.m_argumentFormats[0][i]; jitCode->setArgumentFlushFormats(WTFMove(argumentFlushFormats)); } else { diff --git a/Source/JavaScriptCore/ftl/FTLOSREntry.cpp b/Source/JavaScriptCore/ftl/FTLOSREntry.cpp index df0850540635..041acdff7419 100644 --- a/Source/JavaScriptCore/ftl/FTLOSREntry.cpp +++ b/Source/JavaScriptCore/ftl/FTLOSREntry.cpp @@ -113,8 +113,7 @@ void* prepareOSREntry( RELEASE_ASSERT_NOT_REACHED(); } - RELEASE_ASSERT( - static_cast(values.numberOfLocals()) == baseline->numCalleeLocals()); + RELEASE_ASSERT(values.numberOfLocals() == baseline->numCalleeLocals()); EncodedJSValue* scratch = static_cast( entryCode->entryBuffer()->dataBuffer()); diff --git a/Source/JavaScriptCore/interpreter/CallFrameClosure.h b/Source/JavaScriptCore/interpreter/CallFrameClosure.h index a625caf9b628..ae7a680e5d4c 100644 --- a/Source/JavaScriptCore/interpreter/CallFrameClosure.h +++ b/Source/JavaScriptCore/interpreter/CallFrameClosure.h @@ -36,7 +36,7 @@ struct CallFrameClosure { FunctionExecutable* functionExecutable; VM* vm; JSScope* scope; - int parameterCountIncludingThis; + unsigned parameterCountIncludingThis; int argumentCountIncludingThis; void setThis(JSValue value) diff --git a/Source/JavaScriptCore/interpreter/ProtoCallFrameInlines.h b/Source/JavaScriptCore/interpreter/ProtoCallFrameInlines.h index e73556e355f4..92e3ff6480e2 100644 --- a/Source/JavaScriptCore/interpreter/ProtoCallFrameInlines.h +++ b/Source/JavaScriptCore/interpreter/ProtoCallFrameInlines.h @@ -37,7 +37,7 @@ inline void ProtoCallFrame::init(CodeBlock* codeBlock, JSGlobalObject* globalObj this->setCallee(callee); this->setGlobalObject(globalObject); this->setArgumentCountIncludingThis(argCountIncludingThis); - if (codeBlock && argCountIncludingThis < codeBlock->numParameters()) + if (codeBlock && static_cast(argCountIncludingThis) < codeBlock->numParameters()) this->hasArityMismatch = true; else this->hasArityMismatch = false; diff --git a/Source/JavaScriptCore/jit/JIT.cpp b/Source/JavaScriptCore/jit/JIT.cpp index 66ad84b0d620..76d1a517d9c8 100644 --- a/Source/JavaScriptCore/jit/JIT.cpp +++ b/Source/JavaScriptCore/jit/JIT.cpp @@ -769,7 +769,7 @@ void JIT::compileWithoutLinking(JITCompilationEffort effort) if (m_codeBlock->codeType() == FunctionCode) { ASSERT(!m_bytecodeIndex); if (shouldEmitProfiling()) { - for (int argument = 0; argument < m_codeBlock->numParameters(); ++argument) { + for (unsigned argument = 0; argument < m_codeBlock->numParameters(); ++argument) { // If this is a constructor, then we want to put in a dummy profiling site (to // keep things consistent) but we don't actually want to record the dummy value. if (m_codeBlock->isConstructor() && !argument) diff --git a/Source/JavaScriptCore/runtime/CommonSlowPaths.h b/Source/JavaScriptCore/runtime/CommonSlowPaths.h index c76b1610f997..84e2da5f7ab6 100644 --- a/Source/JavaScriptCore/runtime/CommonSlowPaths.h +++ b/Source/JavaScriptCore/runtime/CommonSlowPaths.h @@ -57,7 +57,7 @@ ALWAYS_INLINE int numberOfExtraSlots(int argumentCountIncludingThis) ALWAYS_INLINE int numberOfStackPaddingSlots(CodeBlock* codeBlock, int argumentCountIncludingThis) { - if (argumentCountIncludingThis >= codeBlock->numParameters()) + if (static_cast(argumentCountIncludingThis) >= codeBlock->numParameters()) return 0; int alignedFrameSize = WTF::roundUpToMultipleOf(stackAlignmentRegisters(), argumentCountIncludingThis + CallFrame::headerSizeInRegisters); int alignedFrameSizeForParameters = WTF::roundUpToMultipleOf(stackAlignmentRegisters(), codeBlock->numParameters() + CallFrame::headerSizeInRegisters); @@ -66,7 +66,7 @@ ALWAYS_INLINE int numberOfStackPaddingSlots(CodeBlock* codeBlock, int argumentCo ALWAYS_INLINE int numberOfStackPaddingSlotsWithExtraSlots(CodeBlock* codeBlock, int argumentCountIncludingThis) { - if (argumentCountIncludingThis >= codeBlock->numParameters()) + if (static_cast(argumentCountIncludingThis) >= codeBlock->numParameters()) return 0; return numberOfStackPaddingSlots(codeBlock, argumentCountIncludingThis) + numberOfExtraSlots(argumentCountIncludingThis); } diff --git a/Source/JavaScriptCore/tools/VMInspector.cpp b/Source/JavaScriptCore/tools/VMInspector.cpp index 1d09ca432ef3..bbbc236d217a 100644 --- a/Source/JavaScriptCore/tools/VMInspector.cpp +++ b/Source/JavaScriptCore/tools/VMInspector.cpp @@ -467,7 +467,7 @@ void VMInspector::dumpRegisters(CallFrame* callFrame) end = it; // Stop the dump } else { end = bitwise_cast(nextCallFrame); - RELEASE_ASSERT(it - end < codeBlock->numCalleeLocals() - codeBlock->numVars()); + RELEASE_ASSERT(static_cast(it - end) < codeBlock->numCalleeLocals() - codeBlock->numVars()); } if (it != end) { diff --git a/Source/JavaScriptCore/wasm/WasmFunctionCodeBlock.h b/Source/JavaScriptCore/wasm/WasmFunctionCodeBlock.h index 7d67ce19c357..ea75fe2f6b0f 100644 --- a/Source/JavaScriptCore/wasm/WasmFunctionCodeBlock.h +++ b/Source/JavaScriptCore/wasm/WasmFunctionCodeBlock.h @@ -65,15 +65,15 @@ class FunctionCodeBlock { } uint32_t functionIndex() const { return m_functionIndex; } - int numVars() const { return m_numVars; } - int numCalleeLocals() const { return m_numCalleeLocals; } + unsigned numVars() const { return m_numVars; } + unsigned numCalleeLocals() const { return m_numCalleeLocals; } uint32_t numArguments() const { return m_numArguments; } const Vector& constantTypes() const { return m_constantTypes; } const Vector& constants() const { return m_constants; } const InstructionStream& instructions() const { return *m_instructions; } - void setNumVars(int numVars) { m_numVars = numVars; } - void setNumCalleeLocals(int numCalleeLocals) { m_numCalleeLocals = numCalleeLocals; } + void setNumVars(unsigned numVars) { m_numVars = numVars; } + void setNumCalleeLocals(unsigned numCalleeLocals) { m_numCalleeLocals = numCalleeLocals; } ALWAYS_INLINE uint64_t getConstant(VirtualRegister reg) const { return m_constants[reg.toConstantIndex()]; } ALWAYS_INLINE Type getConstantType(VirtualRegister reg) const @@ -126,9 +126,9 @@ class FunctionCodeBlock { uint32_t m_functionIndex; // Used for the number of WebAssembly locals, as in https://webassembly.github.io/spec/core/syntax/modules.html#syntax-local - int m_numVars { 0 }; + unsigned m_numVars { 0 }; // Number of VirtualRegister. The naming is unfortunate, but has to match UnlinkedCodeBlock - int m_numCalleeLocals { 0 }; + unsigned m_numCalleeLocals { 0 }; uint32_t m_numArguments { 0 }; Vector m_constantTypes; Vector m_constants; diff --git a/Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp b/Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp index 054e31ed6ba4..2a1811168a06 100644 --- a/Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp +++ b/Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp @@ -169,7 +169,7 @@ class LLIntGenerator : public BytecodeGeneratorBase { ExpressionType push(NoConsistencyCheckTag) { m_maxStackSize = std::max(m_maxStackSize, ++m_stackSize); - return virtualRegisterForLocal(m_stackSize - 1); + return virtualRegisterForLocal((m_stackSize - 1).unsafeGet()); } ExpressionType push() @@ -315,7 +315,7 @@ class LLIntGenerator : public BytecodeGeneratorBase { { startOffset = target.stackSize() + 1; keep = target.branchTargetArity(); - drop = m_stackSize - target.stackSize() - target.branchTargetArity(); + drop = (m_stackSize - target.stackSize() - target.branchTargetArity()).unsafeGet(); } void dropKeep(Stack& values, const ControlType& target, bool dropValues) @@ -348,7 +348,7 @@ class LLIntGenerator : public BytecodeGeneratorBase { template void walkExpressionStack(Stack& expressionStack, const Functor& functor) { - walkExpressionStack(expressionStack, m_stackSize, functor); + walkExpressionStack(expressionStack, m_stackSize.unsafeGet(), functor); } template @@ -373,7 +373,7 @@ class LLIntGenerator : public BytecodeGeneratorBase { }); } walkExpressionStack(m_parser->expressionStack(), [&](VirtualRegister expression, VirtualRegister slot) { - ASSERT(expression == slot || expression.isConstant() || expression.isArgument() || expression.toLocal() < m_codeBlock->m_numVars); + ASSERT(expression == slot || expression.isConstant() || expression.isArgument() || static_cast(expression.toLocal()) < m_codeBlock->m_numVars); }); #endif // ASSERT_ENABLED } @@ -385,7 +385,7 @@ class LLIntGenerator : public BytecodeGeneratorBase { checkConsistency(); walkExpressionStack(expressionStack, [&](TypedExpression& expression, VirtualRegister slot) { - ASSERT(expression.value() == slot || expression.value().isConstant() || expression.value().isArgument() || expression.value().toLocal() < m_codeBlock->m_numVars); + ASSERT(expression.value() == slot || expression.value().isConstant() || expression.value().isArgument() || static_cast(expression.value().toLocal()) < m_codeBlock->m_numVars); if (expression.value() == slot) return; WasmMov::emit(this, slot, expression); @@ -401,7 +401,7 @@ class LLIntGenerator : public BytecodeGeneratorBase { m_stackSize -= newStack.size(); checkConsistency(); walkExpressionStack(enclosingStack, [&](TypedExpression& expression, VirtualRegister slot) { - ASSERT(expression.value() == slot || expression.value().isConstant() || expression.value().isArgument() || expression.value().toLocal() < m_codeBlock->m_numVars); + ASSERT(expression.value() == slot || expression.value().isConstant() || expression.value().isArgument() || static_cast(expression.value().toLocal()) < m_codeBlock->m_numVars); if (expression.value() == slot || expression.value().isConstant()) return; WasmMov::emit(this, slot, expression); @@ -432,8 +432,8 @@ class LLIntGenerator : public BytecodeGeneratorBase { ResultList m_unitializedLocals; HashMap, ConstantMapHashTraits> m_constantMap; Vector m_results; - unsigned m_stackSize { 0 }; - unsigned m_maxStackSize { 0 }; + Checked m_stackSize { 0 }; + Checked m_maxStackSize { 0 }; }; Expected, String> parseAndCompileBytecode(const uint8_t* functionStart, size_t functionLength, const Signature& signature, const ModuleInformation& info, uint32_t functionIndex) @@ -482,7 +482,9 @@ LLIntGenerator::LLIntGenerator(const ModuleInformation& info, unsigned functionI std::unique_ptr LLIntGenerator::finalize() { RELEASE_ASSERT(m_codeBlock); - m_codeBlock->m_numCalleeLocals = WTF::roundUpToMultipleOf(stackAlignmentRegisters(), m_maxStackSize); + size_t numCalleeLocals = WTF::roundUpToMultipleOf(stackAlignmentRegisters(), m_maxStackSize.unsafeGet()); + m_codeBlock->m_numCalleeLocals = numCalleeLocals; + RELEASE_ASSERT(numCalleeLocals == m_codeBlock->m_numCalleeLocals); auto& threadSpecific = threadSpecificBuffer(); Buffer usedBuffer; @@ -571,7 +573,7 @@ auto LLIntGenerator::callInformationForCaller(const Signature& signature) -> LLI // FIXME: we are allocating the extra space for the argument/return count in order to avoid interference, but we could do better // NOTE: We increase arg count by 1 for the case of indirect calls m_stackSize += std::max(signature.argumentCount() + 1, signature.returnCount()) + gprCount + fprCount + stackCount + CallFrame::headerSizeInRegisters; - if (m_stackSize % stackAlignmentRegisters()) + if (m_stackSize.unsafeGet() % stackAlignmentRegisters()) ++m_stackSize; if (m_maxStackSize < m_stackSize) m_maxStackSize = m_stackSize; @@ -580,7 +582,7 @@ auto LLIntGenerator::callInformationForCaller(const Signature& signature) -> LLI ResultList arguments(signature.argumentCount()); ResultList temporaryResults(signature.returnCount()); - const unsigned stackOffset = m_stackSize; + const unsigned stackOffset = m_stackSize.unsafeGet(); const unsigned base = stackOffset - CallFrame::headerSizeInRegisters; const uint32_t gprLimit = base - stackCount - gprCount; @@ -873,7 +875,7 @@ auto LLIntGenerator::addLoop(BlockSignature signature, Stack& enclosingStack, Co Ref