Skip to content

Fix register forwarding when the right operand mutates the left local #5457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dev-willbird1936 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix register forwarding when the right operand mutates the left local #5457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e04ec1a
fix(engine): preserve binary operand values
dev-willbird1936 Jul 22, 2026
aecd0d7
Update bytecode snapshots for operand preservation
codex Jul 26, 2026
5a469a6
test(engine): cover immutable binary operand path
codex Jul 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/engine/src/bytecompiler/expression/binary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ impl ByteCompiler<'_> {
pub(crate) fn compile_binary(&mut self, binary: &Binary, dst: &Register) {
match binary.op() {
BinaryOp::Arithmetic(op) => {
self.compile_expr_operand(binary.lhs(), |self_, lhs| {
self.compile_expr_operand_snapshot(binary.lhs(), |self_, lhs| {
self_.compile_binary_arithmetic(op, binary.rhs(), dst, lhs);
});
}
BinaryOp::Bitwise(op) => {
self.compile_expr_operand(binary.lhs(), |self_, lhs| {
self.compile_expr_operand_snapshot(binary.lhs(), |self_, lhs| {
self_.compile_binary_bitwise(op, binary.rhs(), dst, lhs);
});
}
BinaryOp::Relational(op) => {
self.compile_expr_operand(binary.lhs(), |self_, lhs| {
self.compile_expr_operand_snapshot(binary.lhs(), |self_, lhs| {
self_.compile_binary_relational(op, binary.rhs(), dst, lhs);
});
}
Expand Down
36 changes: 35 additions & 1 deletion core/engine/src/bytecompiler/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1351,7 +1351,7 @@ impl<'ctx> ByteCompiler<'ctx> {
});
}
None => {
self.compile_expr_operand(binary.lhs(), |compiler, lhs| {
self.compile_expr_operand_snapshot(binary.lhs(), |compiler, lhs| {
compiler.compile_expr_operand(binary.rhs(), |compiler, rhs| {
label_index = compiler.next_opcode_location();
emit_fn(&mut compiler.bytecode, Self::DUMMY_ADDRESS, lhs, rhs);
Expand Down Expand Up @@ -1825,6 +1825,40 @@ impl<'ctx> ByteCompiler<'ctx> {
self.register_allocator.dealloc(reg);
}

/// Compile an expression operand while preserving the value of mutable local identifiers.
///
/// Mutable locals live in persistent registers. If compiling a later operand can mutate the
/// local, using that persistent register directly would observe the later value instead of the
/// value produced by the earlier expression. Snapshot mutable locals into a temporary register
/// before invoking the callback; immutable locals and cached constants remain on the fast path.
pub(crate) fn compile_expr_operand_snapshot(
&mut self,
expr: &Expression,
inner_fn: impl FnOnce(&mut Self, RegisterOperand),
) {
if let Expression::Identifier(name) = expr {
let name = self.resolve_identifier_expect(*name);
let binding = self.lexical_scope.get_identifier_reference(name.clone());
let index = self.get_binding(&binding);
if let BindingKind::Local(Some(local_reg)) = &index {
if !matches!(self.lexical_scope.is_binding_mutable(&name), Some(false)) {
let snapshot = self.register_allocator.alloc();
self.bytecode
.emit_move(snapshot.variable(), (*local_reg).into());
let op = snapshot.variable();
inner_fn(self, op);
self.register_allocator.dealloc(snapshot);
return;
}

inner_fn(self, (*local_reg).into());
return;
}
}

self.compile_expr_operand(expr, inner_fn);
}

/// Compile a property access expression, prepending `this` to the property value in the stack.
///
/// This compiles the access in a way that the state of the stack after executing the property
Expand Down
17 changes: 17 additions & 0 deletions core/engine/src/vm/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,23 @@ fn super_construction_with_parameter_expression() {
]);
}

#[test]
fn binary_operands_preserve_left_to_right_evaluation() {
run_test_actions([
TestAction::assert_eq("(function() { let x = 1; return x + (x = 2); })()", 3),
TestAction::assert_eq("(function() { let x = 1; return x | (x = 2); })()", 3),
TestAction::assert_eq("(function() { let x = 1; return x === (x = 2); })()", false),
TestAction::assert_eq(
"(function() { let x = 1; return x < (x = 2) ? 1 : 0; })()",
1,
),
TestAction::assert_eq(
"(function() { const x = 1; let y = 0; return x + (y = 2); })()",
3,
),
]);
}

#[test]
fn cross_context_function_call() {
let context1 = &mut Context::default();
Expand Down
47 changes: 24 additions & 23 deletions tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@loop-hoisting.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,27 @@ Location Handler Opcode Operands
000049 Jump address:000058
00004e IncrementLoopIteration
00004f Inc src:r05, dst:r05
000058 JumpIfNotLessThan lhs:r05, rhs:r04, address:00006a
000065 Jump address:00004e
00006a StoreInt8 value:100, dst:r06
000070 PutLexicalValue src:r06, binding_index:1
000079 Move src:r06, dst:r07
000082 PushFromRegister src:r00
000087 GetNameGlobal dst:r06, binding_index:2, ic_index:0
000094 PushFromRegister src:r06
000099 Call argument_count:0
00009e Pop
00009f StoreInt8 value:100, dst:r06
0000a5 PutLexicalValue src:r06, binding_index:3
0000ae PushFromRegister src:r00
0000b3 GetNameGlobal dst:r08, binding_index:4, ic_index:1
0000c0 PushFromRegister src:r08
0000c5 Call argument_count:0
0000ca PopIntoRegister dst:r06
0000cf SetAccumulator src:r06
0000d4 CheckReturn
0000d5 Return
000058 Move src:r05, dst:r06
000061 JumpIfNotLessThan lhs:r06, rhs:r04, address:000073
00006e Jump address:00004e
000073 StoreInt8 value:100, dst:r06
000079 PutLexicalValue src:r06, binding_index:1
000082 Move src:r06, dst:r07
00008b PushFromRegister src:r00
000090 GetNameGlobal dst:r06, binding_index:2, ic_index:0
00009d PushFromRegister src:r06
0000a2 Call argument_count:0
0000a7 Pop
0000a8 StoreInt8 value:100, dst:r06
0000ae PutLexicalValue src:r06, binding_index:3
0000b7 PushFromRegister src:r00
0000bc GetNameGlobal dst:r08, binding_index:4, ic_index:1
0000c9 PushFromRegister src:r08
0000ce Call argument_count:0
0000d3 PopIntoRegister dst:r06
0000d8 SetAccumulator src:r06
0000dd CheckReturn
0000de Return

Register Count: 9, Flags: CodeBlockFlags(HAS_PROTOTYPE_PROPERTY)
Constants:
Expand All @@ -58,6 +59,6 @@ Bindings:
Handlers: <empty>
Source Map:
0000: 17..79: (2, 26)
0001: 79..153: (5, 24)
0002: 153..197: (14, 4)
0003: 197..207: (23, 4)
0001: 79..162: (5, 24)
0002: 162..206: (14, 4)
0003: 206..216: (23, 4)
94 changes: 48 additions & 46 deletions tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,59 +12,61 @@ Location Handler Opcode Operands
000019 Jump address:000028
00001e IncrementLoopIteration
00001f Inc src:r02, dst:r02
000028 JumpIfNotLessThan lhs:r02, rhs:r03, address:000147
000028 JumpIfNotLessThan lhs:r02, rhs:r03, address:000159
000035 StoreTrue dst:r04
00003a StoreZero dst:r05
00003f > 0: 0000aa StoreInt8 value:2, dst:r07
000045 0: 0000aa StrictEq lhs:r02, rhs:r07, dst:r06
000052 0: 0000aa JumpIfFalse value:r06, address:00006f
00005b 0: 0000aa StoreFalse dst:r04
000060 0: 0000aa StoreOne dst:r05
000065 0: 0000aa Jump address:0000b9
00006a 0: 0000aa Jump address:00006f
00006f 0: 0000aa StoreInt8 value:4, dst:r07
000075 0: 0000aa StrictEq lhs:r02, rhs:r07, dst:r06
000082 0: 0000aa JumpIfFalse value:r06, address:0000a0
00008b 0: 0000aa StoreFalse dst:r04
000090 0: 0000aa StoreInt8 value:2, dst:r05
000096 0: 0000aa Jump address:0000b9
00009b 0: 0000aa Jump address:0000a0
0000a0 0: 0000aa StoreFalse dst:r04
0000a5 < 0: 0000aa Jump address:0000b9
0000aa > 1: 0000b9 Exception dst:r06
0000af 1: 0000b9 StoreTrue dst:r04
0000b4 < 1: 0000b9 Jump address:0000b9
0000b9 SetRegisterFromAccumulator dst:r07
0000be GetName dst:r08, binding_index:0
0000c7 Move src:r02, dst:r09
0000d0 Add lhs:r08, rhs:r09, dst:r08
0000dd SetName src:r08, binding_index:0
0000e6 SetAccumulator src:r07
0000eb JumpIfFalse value:r04, address:0000f9
0000f4 Throw src:r06
0000f9 JumpTable index:5, jump_table:(00011d, 000113, 000118)
00010e Jump address:00011d
000113 Jump address:00001e
000118 Jump address:000147
00011d GetName dst:r04, binding_index:0
000126 StoreInt8 value:100, dst:r05
00012c Add lhs:r04, rhs:r05, dst:r04
000139 SetName src:r04, binding_index:0
000142 Jump address:00001e
000147 GetName dst:r03, binding_index:0
000150 SetAccumulator src:r03
000155 CheckReturn
000156 Return
00003f > 0: 0000bc Move src:r02, dst:r07
000048 0: 0000bc StoreInt8 value:2, dst:r08
00004e 0: 0000bc StrictEq lhs:r07, rhs:r08, dst:r06
00005b 0: 0000bc JumpIfFalse value:r06, address:000078
000064 0: 0000bc StoreFalse dst:r04
000069 0: 0000bc StoreOne dst:r05
00006e 0: 0000bc Jump address:0000cb
000073 0: 0000bc Jump address:000078
000078 0: 0000bc Move src:r02, dst:r07
000081 0: 0000bc StoreInt8 value:4, dst:r08
000087 0: 0000bc StrictEq lhs:r07, rhs:r08, dst:r06
000094 0: 0000bc JumpIfFalse value:r06, address:0000b2
00009d 0: 0000bc StoreFalse dst:r04
0000a2 0: 0000bc StoreInt8 value:2, dst:r05
0000a8 0: 0000bc Jump address:0000cb
0000ad 0: 0000bc Jump address:0000b2
0000b2 0: 0000bc StoreFalse dst:r04
0000b7 < 0: 0000bc Jump address:0000cb
0000bc > 1: 0000cb Exception dst:r06
0000c1 1: 0000cb StoreTrue dst:r04
0000c6 < 1: 0000cb Jump address:0000cb
0000cb SetRegisterFromAccumulator dst:r07
0000d0 GetName dst:r08, binding_index:0
0000d9 Move src:r02, dst:r09
0000e2 Add lhs:r08, rhs:r09, dst:r08
0000ef SetName src:r08, binding_index:0
0000f8 SetAccumulator src:r07
0000fd JumpIfFalse value:r04, address:00010b
000106 Throw src:r06
00010b JumpTable index:5, jump_table:(00012f, 000125, 00012a)
000120 Jump address:00012f
000125 Jump address:00001e
00012a Jump address:000159
00012f GetName dst:r04, binding_index:0
000138 StoreInt8 value:100, dst:r05
00013e Add lhs:r04, rhs:r05, dst:r04
00014b SetName src:r04, binding_index:0
000154 Jump address:00001e
000159 GetName dst:r03, binding_index:0
000162 SetAccumulator src:r03
000167 CheckReturn
000168 Return

Register Count: 10, Flags: CodeBlockFlags(HAS_PROTOTYPE_PROPERTY)
Constants:
0000: [STRING] "total"
Bindings:
0000: total, scope: GlobalDeclarative
Handlers:
0000: Range: [00003f, 0000aa): Handler: 0000aa, Environment: 00
0001: Range: [0000aa, 0000b9): Handler: 0000b9, Environment: 00
0000: Range: [00003f, 0000bc): Handler: 0000bc, Environment: 00
0001: Range: [0000bc, 0000cb): Handler: 0000cb, Environment: 00
Source Map:
0000: 31..190: (7, 24)
0001: 190..285: (12, 5)
0002: 285..322: (14, 3)
0000: 31..208: (7, 24)
0001: 208..303: (12, 5)
0002: 303..340: (14, 3)
Loading