From bc21ed151e4f7b50d5a3e92d6b3879d1f438302e Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Fri, 1 Mar 2024 12:54:50 +0100
Subject: [PATCH] LibJS/Bytecode: Handle awkward initialization case for
 duplicate `var`

`var` declarations can have duplicates, but duplicate `let` or `const`
bindings are a syntax error.

Because of this, we can sink `let` and `const` directly into the
preferred_dst if available. This is not safe for `var` since the
preferred_dst may be used in the initializer.

This patch fixes the issue by simply skipping the preferred_dst
optimization for `var` declarations.
---
 Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp | 11 ++++++++---
 Userland/Libraries/LibJS/Tests/var-scoping.js    | 15 +++++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
index ca37047d75..b4dfd9e6ea 100644
--- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
@@ -1452,10 +1452,15 @@ Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> VariableDeclaration
     Bytecode::Generator::SourceLocationScope scope(generator, *this);
 
     for (auto& declarator : m_declarations) {
+        // NOTE: `var` declarations can have duplicates, but duplicate `let` or `const` bindings are a syntax error.
+        //       Because of this, we can sink `let` and `const` directly into the preferred_dst if available.
+        //       This is not safe for `var` since the preferred_dst may be used in the initializer.
         Optional<Bytecode::Operand> init_dst;
-        if (auto const* identifier = declarator->target().get_pointer<NonnullRefPtr<Identifier const>>()) {
-            if ((*identifier)->is_local()) {
-                init_dst = Bytecode::Operand(Bytecode::Operand::Type::Local, (*identifier)->local_variable_index());
+        if (declaration_kind() != DeclarationKind::Var) {
+            if (auto const* identifier = declarator->target().get_pointer<NonnullRefPtr<Identifier const>>()) {
+                if ((*identifier)->is_local()) {
+                    init_dst = Bytecode::Operand(Bytecode::Operand::Type::Local, (*identifier)->local_variable_index());
+                }
             }
         }
 
diff --git a/Userland/Libraries/LibJS/Tests/var-scoping.js b/Userland/Libraries/LibJS/Tests/var-scoping.js
index 4d3cfd3a2e..8b82a5d887 100644
--- a/Userland/Libraries/LibJS/Tests/var-scoping.js
+++ b/Userland/Libraries/LibJS/Tests/var-scoping.js
@@ -33,3 +33,18 @@ test("Issue #8198 arrow function escapes function scope", () => {
     f();
     expect(b).toBe(3);
 });
+
+test("Referencing the declared var in the initializer of a duplicate var declaration", () => {
+    function c(e) {
+        e.foo;
+    }
+    function h() {}
+    function go() {
+        var p = true;
+        var p = h() || c(p);
+        return 0;
+    }
+
+    // It's all good as long as go() doesn't throw.
+    expect(go()).toBe(0);
+});