commit 310a64d8d88232c185f2ba07b89450fbbd230769
parent 48c7b2c166a4209ef64fe57f9fa97cba6aee0ce0
Author: agoloman <agoloman@mozilla.com>
Date: Wed, 12 Nov 2025 21:01:00 +0200
Revert "Bug 1966196 - Expand EnvironmentCoordinate hops bitwidth to 16 r=nbp" for causing spidermonkey bustages.
This reverts commit f0c591856b1421d38a70231f3912c494309689e4.
Diffstat:
15 files changed, 55 insertions(+), 64 deletions(-)
diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp
@@ -1435,12 +1435,12 @@ bool BytecodeEmitter::emitThisEnvironmentCallee() {
size_t numHops = countThisEnvironmentHops();
static_assert(
- ENVCOORD_HOPS_LIMIT - 1 <= UINT16_MAX,
+ ENVCOORD_HOPS_LIMIT - 1 <= UINT8_MAX,
"JSOp::EnvCallee operand size should match ENVCOORD_HOPS_LIMIT");
MOZ_ASSERT(numHops < ENVCOORD_HOPS_LIMIT - 1);
- return emitUint16Operand(JSOp::EnvCallee, numHops);
+ return emit2(JSOp::EnvCallee, numHops);
}
bool BytecodeEmitter::emitSuperBase() {
@@ -2206,7 +2206,7 @@ bool BytecodeEmitter::emitSetThis(BinaryNode* setThisNode) {
lexicalLoc = NameLocation::FrameSlot(BindingKind::Let, loc.frameSlot());
} else if (loc.kind() == NameLocation::Kind::EnvironmentCoordinate) {
EnvironmentCoordinate coord = loc.environmentCoordinate();
- uint16_t hops = AssertedCast<uint16_t>(coord.hops());
+ uint8_t hops = AssertedCast<uint8_t>(coord.hops());
lexicalLoc = NameLocation::EnvironmentCoordinate(BindingKind::Let, hops,
coord.slot());
} else {
diff --git a/js/src/frontend/EmitterScope.cpp b/js/src/frontend/EmitterScope.cpp
@@ -69,7 +69,7 @@ bool EmitterScope::checkEnvironmentChainLength(BytecodeEmitter* bce) {
return false;
}
- environmentChainLength_ = mozilla::AssertedCast<uint16_t>(hops + 1);
+ environmentChainLength_ = mozilla::AssertedCast<uint8_t>(hops + 1);
return true;
}
@@ -147,7 +147,7 @@ bool EmitterScope::nameCanBeFree(BytecodeEmitter* bce,
NameLocation EmitterScope::searchAndCache(BytecodeEmitter* bce,
TaggedParserAtomIndex name) {
Maybe<NameLocation> loc;
- uint16_t hops = hasEnvironment() ? 1 : 0;
+ uint8_t hops = hasEnvironment() ? 1 : 0;
DebugOnly<bool> inCurrentScript = enclosingInFrame();
// Start searching in the current compilation.
@@ -1169,7 +1169,7 @@ Maybe<NameLocation> EmitterScope::locationBoundInScope(
TaggedParserAtomIndex name, EmitterScope* target) {
// The target scope must be an intra-frame enclosing scope of this
// one. Count the number of extra hops to reach it.
- uint16_t extraHops = 0;
+ uint8_t extraHops = 0;
for (EmitterScope* es = this; es != target; es = es->enclosingInFrame()) {
if (es->hasEnvironment()) {
extraHops++;
diff --git a/js/src/frontend/EmitterScope.h b/js/src/frontend/EmitterScope.h
@@ -57,7 +57,7 @@ class MOZ_STACK_CLASS EmitterScope : public Nestable<EmitterScope> {
#endif
// The number of enclosing environments. Used for error checking.
- uint16_t environmentChainLength_;
+ uint8_t environmentChainLength_;
// The next usable slot on the frame for not-closed over bindings.
//
diff --git a/js/src/frontend/NameAnalysisTypes.h b/js/src/frontend/NameAnalysisTypes.h
@@ -280,8 +280,8 @@ class NameLocation {
// If the name is closed over and accessed via EnvironmentCoordinate, the
// number of dynamic environments to skip.
//
- // Otherwise UINT16_MAX.
- uint16_t hops_;
+ // Otherwise UINT8_MAX.
+ uint8_t hops_;
// If the name lives on the frame, the slot frame.
//
@@ -294,7 +294,7 @@ class NameLocation {
static_assert(LOCALNO_BITS == ENVCOORD_SLOT_BITS,
"Frame and environment slots must be same sized.");
- NameLocation(Kind kind, BindingKind bindingKind, uint16_t hops = UINT16_MAX,
+ NameLocation(Kind kind, BindingKind bindingKind, uint8_t hops = UINT8_MAX,
uint32_t slot = 0)
: kind_(kind), bindingKind_(bindingKind), hops_(hops), slot_(slot) {}
@@ -330,13 +330,13 @@ class NameLocation {
return NameLocation(Kind::FrameSlot, bindKind, 0, slot);
}
- static NameLocation EnvironmentCoordinate(BindingKind bindKind, uint16_t hops,
+ static NameLocation EnvironmentCoordinate(BindingKind bindKind, uint8_t hops,
uint32_t slot) {
MOZ_ASSERT(slot < ENVCOORD_SLOT_LIMIT);
return NameLocation(Kind::EnvironmentCoordinate, bindKind, hops, slot);
}
static NameLocation DebugEnvironmentCoordinate(BindingKind bindKind,
- uint16_t hops, uint32_t slot) {
+ uint8_t hops, uint32_t slot) {
MOZ_ASSERT(slot < ENVCOORD_SLOT_LIMIT);
return NameLocation(Kind::DebugEnvironmentCoordinate, bindKind, hops, slot);
}
@@ -368,7 +368,7 @@ class NameLocation {
return slot_;
}
- NameLocation addHops(uint16_t more) {
+ NameLocation addHops(uint8_t more) {
MOZ_ASSERT(hops_ < ENVCOORD_HOPS_LIMIT - more);
MOZ_ASSERT(kind_ == Kind::EnvironmentCoordinate);
return NameLocation(kind_, bindingKind_, hops_ + more, slot_);
diff --git a/js/src/frontend/Stencil.cpp b/js/src/frontend/Stencil.cpp
@@ -1151,7 +1151,7 @@ NameLocation ScopeContext::searchInEnclosingScopeWithCache(
mozilla::Maybe<NameLocation> found;
// Number of enclosing scope we walked over.
- uint16_t hops = 0;
+ uint8_t hops = 0;
for (InputScopeIter si(input.enclosingScope); si; si++) {
MOZ_ASSERT(NameIsOnEnvironment(fc, parserAtoms, input.atomCache, si.scope(),
@@ -1220,8 +1220,8 @@ NameLocation ScopeContext::searchInEnclosingScopeNoCache(
// NameLocation which contains relative locations to access `name`.
mozilla::Maybe<NameLocation> result;
- // Number of enclosing scope we walked over.
- uint16_t hops = 0;
+ // Number of enclosing scoep we walked over.
+ uint8_t hops = 0;
for (InputScopeIter si(input.enclosingScope); si; si++) {
MOZ_ASSERT(NameIsOnEnvironment(fc, parserAtoms, input.atomCache, si.scope(),
diff --git a/js/src/jit-test/tests/class/super-in-nested-eval.js b/js/src/jit-test/tests/class/super-in-nested-eval.js
@@ -15,18 +15,19 @@ class D extends C {
}
}
+const MAX_DEPTH = 250;
+
// These values should work.
-var depths = [0, 1, 10, 200, 300];
+var depths = [0, 1, 10, 200, MAX_DEPTH];
for (var d of depths) {
var o = new D(d);
assertEq(o.x, d + 1);
}
-// When we use a large enough depth that we run out of stack, we throw instead
-// of crashing
+// This should fail.
var ex;
try {
- new D(2000);
+ new D(MAX_DEPTH + 1);
} catch(e) {
ex = e;
}
diff --git a/js/src/jit-test/tests/debug/breakpoint-dot-generator.js b/js/src/jit-test/tests/debug/breakpoint-dot-generator.js
@@ -12,7 +12,7 @@ const script = dg.makeDebuggeeValue(g.func).script;
//
// 00000: Generator # GENERATOR
// 00001: SetAliasedVar ".generator" # GENERATOR
-// 00007: InitialYield 0 # RVAL GENERATOR RESUMEKIND
+// 00006: InitialYield 0 # RVAL GENERATOR RESUMEKIND
// Setting a breakpoint at `SetAliasedVar ".generator"` should be disallow.
let caught = false;
@@ -27,7 +27,7 @@ assertEq(caught, true);
// Setting breakpoints to other opcodes should be allowed.
script.setBreakpoint(0, {});
-script.setBreakpoint(7, {});
+script.setBreakpoint(6, {});
// Offset 1 shouldn't be exposed.
assertEq(script.getPossibleBreakpoints().some(p => p.offset == 1), false);
diff --git a/js/src/jit-test/tests/debug/bug1368736.js b/js/src/jit-test/tests/debug/bug1368736.js
@@ -2,7 +2,7 @@ g = newGlobal({newCompartment: true});
hits = 0;
Debugger(g).onDebuggerStatement = function(frame) {
// Set a breakpoint at the JSOP_AFTERYIELD op.
- frame.script.setBreakpoint(128, {hit: function() { hits++; }});
+ frame.script.setBreakpoint(120, {hit: function() { hits++; }});
}
g.eval(`
function* range() {
diff --git a/js/src/jit-test/tests/environments/bug1966196.js b/js/src/jit-test/tests/environments/bug1966196.js
@@ -1,10 +0,0 @@
-// |jit-test|
-
-let REPEAT_COUNT = 300;
-let someCondition = Math.random() > 0.5;
-function generateJS() {
- let str = "if (someCondition) {let foo;".repeat(REPEAT_COUNT) +
- "}".repeat(REPEAT_COUNT);
- return str;
-}
-eval(generateJS());
diff --git a/js/src/jit/BaselineCodeGen.cpp b/js/src/jit/BaselineCodeGen.cpp
@@ -3907,9 +3907,9 @@ Address BaselineCodeGen<Handler>::getEnvironmentCoordinateAddress(
// number of environment objects.
static void LoadAliasedVarEnv(MacroAssembler& masm, Register env,
Register scratch) {
- static_assert(ENVCOORD_HOPS_LEN == 2,
- "Code assumes number of hops is stored in uint16 operand");
- LoadUint16Operand(masm, scratch);
+ static_assert(ENVCOORD_HOPS_LEN == 1,
+ "Code assumes number of hops is stored in uint8 operand");
+ LoadUint8Operand(masm, scratch);
Label top, done;
masm.branchTest32(Assembler::Zero, scratch, scratch, &done);
@@ -5918,7 +5918,7 @@ bool BaselineCodeGen<Handler>::emit_Callee() {
template <>
bool BaselineCompilerCodeGen::emit_EnvCallee() {
frame.syncStack(0);
- uint16_t numHops = GET_ENVCOORD_HOPS(handler.pc());
+ uint8_t numHops = GET_UINT8(handler.pc());
Register scratch = R0.scratchReg();
masm.loadPtr(frame.addressOfEnvironmentChain(), scratch);
@@ -5939,7 +5939,7 @@ bool BaselineInterpreterCodeGen::emit_EnvCallee() {
Register env = R1.scratchReg();
static_assert(JSOpLength_EnvCallee - sizeof(jsbytecode) == ENVCOORD_HOPS_LEN,
- "op must have uint16 operand for LoadAliasedVarEnv");
+ "op must have uint8 operand for LoadAliasedVarEnv");
// Load the right environment object.
masm.loadPtr(frame.addressOfEnvironmentChain(), env);
diff --git a/js/src/vm/BytecodeLocation.h b/js/src/vm/BytecodeLocation.h
@@ -10,7 +10,7 @@
#include "frontend/NameAnalysisTypes.h"
#include "js/TypeDecls.h"
#include "vm/BuiltinObjectKind.h"
-#include "vm/BytecodeUtil.h" // GET_ENVCOORD_HOPS
+#include "vm/BytecodeUtil.h"
#include "vm/CheckIsObjectKind.h" // CheckIsObjectKind
#include "vm/CompletionKind.h" // CompletionKind
#include "vm/ConstantCompareOperand.h" // ConstantCompareOperand
@@ -261,7 +261,7 @@ class BytecodeLocation {
uint32_t getEnvCalleeNumHops() const {
MOZ_ASSERT(is(JSOp::EnvCallee));
- return GET_ENVCOORD_HOPS(rawBytecode_);
+ return GET_UINT8(rawBytecode_);
}
EnvironmentCoordinate getEnvironmentCoordinate() const {
diff --git a/js/src/vm/BytecodeUtil.h b/js/src/vm/BytecodeUtil.h
@@ -235,23 +235,23 @@ static inline bool IsBackedgeForLoopHead(jsbytecode* pc, jsbytecode* loopHead) {
/*
* Describes the 'hops' component of a JOF_ENVCOORD opcode.
*
- * Note: this component is only 16 bits wide, limiting the maximum number of
- * scopes between a use and def to roughly 65535. This is a pretty small limit
- * but note that SpiderMonkey's recursive descent parser can't parse too many
- * functions before hitting the C-stack recursion limit so this shouldn't
+ * Note: this component is only 8 bits wide, limiting the maximum number of
+ * scopes between a use and def to roughly 255. This is a pretty small limit but
+ * note that SpiderMonkey's recursive descent parser can only parse about this
+ * many functions before hitting the C-stack recursion limit so this shouldn't
* be a significant limitation in practice.
*/
-static inline uint16_t GET_ENVCOORD_HOPS(jsbytecode* pc) {
- return GET_UINT16(pc);
+static inline uint8_t GET_ENVCOORD_HOPS(jsbytecode* pc) {
+ return GET_UINT8(pc);
}
-static inline void SET_ENVCOORD_HOPS(jsbytecode* pc, uint16_t hops) {
- SET_UINT16(pc, hops);
+static inline void SET_ENVCOORD_HOPS(jsbytecode* pc, uint8_t hops) {
+ SET_UINT8(pc, hops);
}
-static const unsigned ENVCOORD_HOPS_LEN = 2;
-static const unsigned ENVCOORD_HOPS_BITS = 16;
+static const unsigned ENVCOORD_HOPS_LEN = 1;
+static const unsigned ENVCOORD_HOPS_BITS = 8;
static const unsigned ENVCOORD_HOPS_LIMIT = 1 << ENVCOORD_HOPS_BITS;
/* Describes the 'slot' component of a JOF_ENVCOORD opcode. */
diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp
@@ -4390,7 +4390,7 @@ bool MOZ_NEVER_INLINE JS_HAZ_JSNATIVE_CALLER js::Interpret(JSContext* cx,
END_CASE(DynamicImport)
CASE(EnvCallee) {
- uint16_t numHops = GET_ENVCOORD_HOPS(REGS.pc);
+ uint8_t numHops = GET_UINT8(REGS.pc);
JSObject* env = ®S.fp()->environmentChain()->as<EnvironmentObject>();
for (unsigned i = 0; i < numHops; i++) {
env = &env->as<EnvironmentObject>().enclosingEnvironment();
diff --git a/js/src/vm/Opcodes.h b/js/src/vm/Opcodes.h
@@ -2726,10 +2726,10 @@
*
* Category: Variables and scopes
* Type: Initialization
- * Operands: uint16_t hops, uint24_t slot
+ * Operands: uint8_t hops, uint24_t slot
* Stack: v => v
*/ \
- MACRO(InitAliasedLexical, init_aliased_lexical, NULL, 6, 1, 1, JOF_ENVCOORD|JOF_PROPINIT) \
+ MACRO(InitAliasedLexical, init_aliased_lexical, NULL, 5, 1, 1, JOF_ENVCOORD|JOF_PROPINIT) \
/*
* Throw a ReferenceError if the value on top of the stack is uninitialized.
*
@@ -2758,10 +2758,10 @@
*
* Category: Variables and scopes
* Type: Initialization
- * Operands: uint16_t hops, uint24_t slot
+ * Operands: uint8_t hops, uint24_t slot
* Stack: v => v
*/ \
- MACRO(CheckAliasedLexical, check_aliased_lexical, NULL, 6, 1, 1, JOF_ENVCOORD) \
+ MACRO(CheckAliasedLexical, check_aliased_lexical, NULL, 5, 1, 1, JOF_ENVCOORD) \
/*
* Throw a ReferenceError if the value on top of the stack is
* `MagicValue(JS_UNINITIALIZED_LEXICAL)`. Used in derived class
@@ -2937,20 +2937,20 @@
*
* Category: Variables and scopes
* Type: Getting binding values
- * Operands: uint16_t hops, uint24_t slot
+ * Operands: uint8_t hops, uint24_t slot
* Stack: => aliasedVar
*/ \
- MACRO(GetAliasedVar, get_aliased_var, NULL, 6, 0, 1, JOF_ENVCOORD|JOF_USES_ENV) \
+ MACRO(GetAliasedVar, get_aliased_var, NULL, 5, 0, 1, JOF_ENVCOORD|JOF_USES_ENV) \
/*
* Push the value of an aliased binding, which may have to bypass a DebugEnvironmentProxy
* on the environment chain.
*
* Category: Variables and scopes
* Type: Getting binding values
- * Operands: uint16_t hops, uint24_t slot
+ * Operands: uint8_t hops, uint24_t slot
* Stack: => aliasedVar
*/ \
- MACRO(GetAliasedDebugVar, get_aliased_debug_var, NULL, 6, 0, 1, JOF_DEBUGCOORD) \
+ MACRO(GetAliasedDebugVar, get_aliased_debug_var, NULL, 5, 0, 1, JOF_DEBUGCOORD) \
/*
* Get the value of a module import by name and pushes it onto the stack.
*
@@ -3029,10 +3029,10 @@
*
* Category: Variables and scopes
* Type: Getting binding values
- * Operands: uint16_t numHops
+ * Operands: uint8_t numHops
* Stack: => callee
*/ \
- MACRO(EnvCallee, env_callee, NULL, 3, 0, 1, JOF_UINT16) \
+ MACRO(EnvCallee, env_callee, NULL, 2, 0, 1, JOF_UINT8) \
/*
* Assign `val` to the binding in `env` with the name given by `nameIndex`.
* Throw a ReferenceError if the binding is an uninitialized lexical.
@@ -3126,10 +3126,10 @@
*
* Category: Variables and scopes
* Type: Setting binding values
- * Operands: uint16_t hops, uint24_t slot
+ * Operands: uint8_t hops, uint24_t slot
* Stack: val => val
*/ \
- MACRO(SetAliasedVar, set_aliased_var, NULL, 6, 1, 1, JOF_ENVCOORD|JOF_PROPSET|JOF_USES_ENV) \
+ MACRO(SetAliasedVar, set_aliased_var, NULL, 5, 1, 1, JOF_ENVCOORD|JOF_PROPSET|JOF_USES_ENV) \
/*
* Assign to an intrinsic.
*
diff --git a/js/src/vm/PortableBaselineInterpret.cpp b/js/src/vm/PortableBaselineInterpret.cpp
@@ -8677,7 +8677,7 @@ PBIResult PortableBaselineInterpret(
}
CASE(EnvCallee) {
- uint16_t numHops = GET_ENVCOORD_HOPS(pc);
+ uint8_t numHops = GET_UINT8(pc);
JSObject* env = &frame->environmentChain()->as<EnvironmentObject>();
for (unsigned i = 0; i < numHops; i++) {
env = &env->as<EnvironmentObject>().enclosingEnvironment();
