From 6402ad29a6861d2bc0e0737c68eaf6b5ee3192ec Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Fri, 23 Feb 2024 12:30:39 +0100
Subject: [PATCH] LibJS/Bytecode: Don't clobber dst when assigning from object
 expression

When compiling code like this:

    x = { foo: x }

We don't want to put a new JS::Object in `x` until *after* we've
evaluated `x` for the `foo` field.

This fixes an issue when loading https://puter.com/ :^)
---
 Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp          | 4 ++--
 .../Libraries/LibJS/Tests/builtins/Array/array-basic.js   | 8 ++++++++
 Userland/Libraries/LibJS/Tests/object-basic.js            | 8 ++++++++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
index 6781a0756c..4f2e9c82a8 100644
--- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
@@ -948,11 +948,11 @@ Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> ForStatement::gener
     return body_result;
 }
 
-Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> ObjectExpression::generate_bytecode(Bytecode::Generator& generator, Optional<Bytecode::Operand> preferred_dst) const
+Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> ObjectExpression::generate_bytecode(Bytecode::Generator& generator, [[maybe_unused]] Optional<Bytecode::Operand> preferred_dst) const
 {
     Bytecode::Generator::SourceLocationScope scope(generator, *this);
 
-    auto object = choose_dst(generator, preferred_dst);
+    auto object = Bytecode::Operand(generator.allocate_register());
 
     generator.emit<Bytecode::Op::NewObject>(object);
     if (m_properties.is_empty())
diff --git a/Userland/Libraries/LibJS/Tests/builtins/Array/array-basic.js b/Userland/Libraries/LibJS/Tests/builtins/Array/array-basic.js
index d6e6976421..d232d1ecbe 100644
--- a/Userland/Libraries/LibJS/Tests/builtins/Array/array-basic.js
+++ b/Userland/Libraries/LibJS/Tests/builtins/Array/array-basic.js
@@ -55,3 +55,11 @@ test("basic functionality", () => {
     expect(a.toString()).toBe("1,20,2,,,3");
     expect(a.getterSetterValue).toBe(20);
 });
+
+test("assigning array expression with destination referenced in array expression", () => {
+    function go(i) {
+        var i = [i];
+        return i;
+    }
+    expect(go("foo")).toEqual(["foo"]);
+});
diff --git a/Userland/Libraries/LibJS/Tests/object-basic.js b/Userland/Libraries/LibJS/Tests/object-basic.js
index 879a0a114a..a81908a82f 100644
--- a/Userland/Libraries/LibJS/Tests/object-basic.js
+++ b/Userland/Libraries/LibJS/Tests/object-basic.js
@@ -122,6 +122,14 @@ describe("correct behavior", () => {
         Object.setPrototypeOf(derived, base);
         expect(derived.getNumber()).toBe(30);
     });
+
+    test("assigning object expression with destination referenced in object expression", () => {
+        function go(i) {
+            var i = { f: i };
+            return i;
+        }
+        expect(go("foo")).toEqual({ f: "foo" });
+    });
 });
 
 describe("side effects", () => {