From 0f8c6dc9adac476eb0bc6c41da7e5fd43796605c Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Tue, 5 Mar 2024 09:48:13 +0100
Subject: [PATCH] LibJS/Bytecode: Always evaluate LHS first in assignment
 expressions

This fixes an issue where expressions like `a[i] = a[++i]` could
evaluate `++i` before `a[i]`.
---
 .../Libraries/LibJS/Bytecode/ASTCodegen.cpp   |  4 +++-
 .../Libraries/LibJS/Bytecode/Generator.cpp    |  4 +++-
 .../Tests/assignment-evaluation-order.js      | 21 +++++++++++++++++++
 3 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 Userland/Libraries/LibJS/Tests/assignment-evaluation-order.js

diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
index 96f69677e0..112d75d6d8 100644
--- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
@@ -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()) {
diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp
index 67bdf5a1fe..388607084f 100644
--- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp
@@ -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,
         };
diff --git a/Userland/Libraries/LibJS/Tests/assignment-evaluation-order.js b/Userland/Libraries/LibJS/Tests/assignment-evaluation-order.js
new file mode 100644
index 0000000000..6e1e156d37
--- /dev/null
+++ b/Userland/Libraries/LibJS/Tests/assignment-evaluation-order.js
@@ -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]);
+});