Skip to content

Commit

Permalink
LibJS/Bytecode: Always evaluate LHS first in assignment expressions
Browse files Browse the repository at this point in the history
This fixes an issue where expressions like `a[i] = a[++i]` could
evaluate `++i` before `a[i]`.
  • Loading branch information
awesomekling committed Mar 5, 2024
1 parent 1986693 commit 0f8c6dc
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 2 deletions.
4 changes: 3 additions & 1 deletion Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,9 @@ Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> AssignmentExpressio
}

if (expression.is_computed()) {
computed_property = TRY(expression.property().generate_bytecode(generator)).value();
auto property = TRY(expression.property().generate_bytecode(generator)).value();
computed_property = Bytecode::Operand(generator.allocate_register());
generator.emit<Bytecode::Op::Mov>(*computed_property, property);

// To be continued later with PutByValue.
} else if (expression.property().is_identifier()) {
Expand Down
4 changes: 3 additions & 1 deletion Userland/Libraries/LibJS/Bytecode/Generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,13 @@ CodeGenerationErrorOr<Generator::ReferenceOperands> Generator::emit_load_from_re
auto base = TRY(expression.object().generate_bytecode(*this)).value();
if (expression.is_computed()) {
auto property = TRY(expression.property().generate_bytecode(*this)).value();
auto saved_property = Operand(allocate_register());
emit<Bytecode::Op::Mov>(saved_property, property);
auto dst = preferred_dst.has_value() ? preferred_dst.value() : Operand(allocate_register());
emit<Bytecode::Op::GetByValue>(dst, base, property);
return ReferenceOperands {
.base = base,
.referenced_name = property,
.referenced_name = saved_property,
.this_value = base,
.loaded_value = dst,
};
Expand Down
21 changes: 21 additions & 0 deletions Userland/Libraries/LibJS/Tests/assignment-evaluation-order.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
test("Assignment should always evaluate LHS first", () => {
function go(a) {
let i = 0;
a[i] = a[++i];
}

let a = [1, 2, 3];
go(a);
expect(a).toEqual([2, 2, 3]);
});

test("Binary assignment should always evaluate LHS first", () => {
function go(a) {
let i = 0;
a[i] |= a[++i];
}

let a = [1, 2];
go(a);
expect(a).toEqual([3, 2]);
});

0 comments on commit 0f8c6dc

Please sign in to comment.