Revert "LibJS: Add bytecode instruction handles"

This reverts commit a01bd35c67e35e9f0e33f46bb3a4431e49af1b60.

This broke simple programs like:

function sum(a, b) { return a + b; }
console.log(sum(1, 2));

8 files changed, 79 insertions, 81 deletions

diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
index ad24e32e50..dd92c4a196 100644
--- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
@@ -119,16 +119,16 @@ void LogicalExpression::generate_bytecode(Bytecode::Generator& generator) const
 {
     m_lhs->generate_bytecode(generator);
 
-    Bytecode::InstructionHandle<Bytecode::Op::Jump> test_instr;
+    Bytecode::Op::Jump* test_instr;
     switch (m_op) {
     case LogicalOp::And:
-        test_instr = generator.emit<Bytecode::Op::JumpIfFalse>();
+        test_instr = &generator.emit<Bytecode::Op::JumpIfFalse>();
         break;
     case LogicalOp::Or:
-        test_instr = generator.emit<Bytecode::Op::JumpIfTrue>();
+        test_instr = &generator.emit<Bytecode::Op::JumpIfTrue>();
         break;
     case LogicalOp::NullishCoalescing:
-        test_instr = generator.emit<Bytecode::Op::JumpIfNotNullish>();
+        test_instr = &generator.emit<Bytecode::Op::JumpIfNotNullish>();
         break;
     default:
         VERIFY_NOT_REACHED();
@@ -285,10 +285,10 @@ void WhileStatement::generate_bytecode(Bytecode::Generator& generator) const
     auto result_reg = generator.allocate_register();
     generator.emit<Bytecode::Op::Store>(result_reg);
     m_test->generate_bytecode(generator);
-    auto test_jump = generator.emit<Bytecode::Op::JumpIfFalse>();
+    auto& test_jump = generator.emit<Bytecode::Op::JumpIfFalse>();
     m_body->generate_bytecode(generator);
     generator.emit<Bytecode::Op::Jump>(test_label);
-    test_jump->set_target(generator.make_label());
+    test_jump.set_target(generator.make_label());
     generator.end_continuable_scope();
     generator.emit<Bytecode::Op::Load>(result_reg);
 }
@@ -309,7 +309,7 @@ void DoWhileStatement::generate_bytecode(Bytecode::Generator& generator) const
 
 void ForStatement::generate_bytecode(Bytecode::Generator& generator) const
 {
-    Bytecode::InstructionHandle<Bytecode::Op::Jump> test_jump;
+    Bytecode::Op::Jump* test_jump { nullptr };
 
     if (m_init)
         m_init->generate_bytecode(generator);
@@ -321,7 +321,7 @@ void ForStatement::generate_bytecode(Bytecode::Generator& generator) const
     generator.emit<Bytecode::Op::Store>(result_reg);
     if (m_test) {
         m_test->generate_bytecode(generator);
-        test_jump = generator.emit<Bytecode::Op::JumpIfFalse>();
+        test_jump = &generator.emit<Bytecode::Op::JumpIfFalse>();
     }
 
     m_body->generate_bytecode(generator);
@@ -387,16 +387,16 @@ void ReturnStatement::generate_bytecode(Bytecode::Generator& generator) const
 void IfStatement::generate_bytecode(Bytecode::Generator& generator) const
 {
     m_predicate->generate_bytecode(generator);
-    auto else_jump = generator.emit<Bytecode::Op::JumpIfFalse>();
+    auto& else_jump = generator.emit<Bytecode::Op::JumpIfFalse>();
 
     m_consequent->generate_bytecode(generator);
     if (m_alternate) {
-        auto if_jump = generator.emit<Bytecode::Op::Jump>();
-        else_jump->set_target(generator.make_label());
+        auto& if_jump = generator.emit<Bytecode::Op::Jump>();
+        else_jump.set_target(generator.make_label());
         m_alternate->generate_bytecode(generator);
-        if_jump->set_target(generator.make_label());
+        if_jump.set_target(generator.make_label());
     } else {
-        else_jump->set_target(generator.make_label());
+        else_jump.set_target(generator.make_label());
     }
 }
 
@@ -412,15 +412,15 @@ void DebuggerStatement::generate_bytecode(Bytecode::Generator&) const
 void ConditionalExpression::generate_bytecode(Bytecode::Generator& generator) const
 {
     m_test->generate_bytecode(generator);
-    auto alternate_jump = generator.emit<Bytecode::Op::JumpIfFalse>();
+    auto& alternate_jump = generator.emit<Bytecode::Op::JumpIfFalse>();
 
     m_consequent->generate_bytecode(generator);
-    auto end_jump = generator.emit<Bytecode::Op::Jump>();
+    auto& end_jump = generator.emit<Bytecode::Op::Jump>();
 
-    alternate_jump->set_target(generator.make_label());
+    alternate_jump.set_target(generator.make_label());
     m_alternate->generate_bytecode(generator);
 
-    end_jump->set_target(generator.make_label());
+    end_jump.set_target(generator.make_label());
 }
 
 void SequenceExpression::generate_bytecode(Bytecode::Generator& generator) const
diff --git a/Userland/Libraries/LibJS/Bytecode/Block.cpp b/Userland/Libraries/LibJS/Bytecode/Block.cpp
index 7ca1a43ec0..ef0add31a1 100644
--- a/Userland/Libraries/LibJS/Bytecode/Block.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/Block.cpp
@@ -4,8 +4,10 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <AK/String.h>
 #include <LibJS/Bytecode/Block.h>
 #include <LibJS/Bytecode/Op.h>
+#include <sys/mman.h>
 
 namespace JS::Bytecode {
 
@@ -14,6 +16,16 @@ NonnullOwnPtr<Block> Block::create()
     return adopt_own(*new Block);
 }
 
+Block::Block()
+{
+    // FIXME: This is not the smartest solution ever. Find something cleverer!
+    // The main issue we're working around here is that we don't want pointers into the bytecode stream to become invalidated
+    // during code generation due to dynamic buffer resizing. Otherwise we could just use a Vector.
+    m_buffer_capacity = 64 * KiB;
+    m_buffer = (u8*)mmap(nullptr, m_buffer_capacity, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+    VERIFY(m_buffer != MAP_FAILED);
+}
+
 Block::~Block()
 {
     Bytecode::InstructionStreamIterator it(instruction_stream());
@@ -22,6 +34,16 @@ Block::~Block()
         ++it;
         Instruction::destroy(const_cast<Instruction&>(to_destroy));
     }
+
+    munmap(m_buffer, m_buffer_capacity);
+}
+
+void Block::seal()
+{
+    // FIXME: mprotect the instruction stream as PROT_READ
+    // This is currently not possible because instructions can have destructors (that clean up strings)
+    // Instructions should instead be destructor-less and refer to strings in a string table on the Bytecode::Block.
+    // It also doesn't work because instructions that have String members use RefPtr internally which must be in writable memory.
 }
 
 void Block::dump() const
@@ -33,6 +55,12 @@ void Block::dump() const
     }
 }
 
+void Block::grow(size_t additional_size)
+{
+    m_buffer_size += additional_size;
+    VERIFY(m_buffer_size <= m_buffer_capacity);
+}
+
 void InstructionStreamIterator::operator++()
 {
     m_offset += dereference().length();
diff --git a/Userland/Libraries/LibJS/Bytecode/Block.h b/Userland/Libraries/LibJS/Bytecode/Block.h
index 09dca9461a..4e81d2b1d3 100644
--- a/Userland/Libraries/LibJS/Bytecode/Block.h
+++ b/Userland/Libraries/LibJS/Bytecode/Block.h
@@ -8,7 +8,6 @@
 
 #include <AK/Badge.h>
 #include <AK/NonnullOwnPtrVector.h>
-#include <LibJS/Bytecode/Register.h>
 #include <LibJS/Forward.h>
 
 namespace JS::Bytecode {
@@ -43,20 +42,25 @@ public:
     static NonnullOwnPtr<Block> create();
     ~Block();
 
+    void seal();
+
     void dump() const;
-    ReadonlyBytes instruction_stream() const { return m_buffer.span(); }
+    ReadonlyBytes instruction_stream() const { return ReadonlyBytes { m_buffer, m_buffer_size }; }
 
     size_t register_count() const { return m_register_count; }
 
     void set_register_count(Badge<Bytecode::Generator>, size_t count) { m_register_count = count; }
 
-    Vector<u8>& buffer() { return m_buffer; }
+    void* next_slot() { return m_buffer + m_buffer_size; }
+    void grow(size_t additional_size);
 
 private:
-    Block() = default;
+    Block();
 
     size_t m_register_count { 0 };
-    Vector<u8> m_buffer;
+    u8* m_buffer { nullptr };
+    size_t m_buffer_capacity { 0 };
+    size_t m_buffer_size { 0 };
 };
 
 }
diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp
index 822da0ebfa..2d2ad73520 100644
--- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp
@@ -10,6 +10,7 @@
 #include <LibJS/Bytecode/Generator.h>
 #include <LibJS/Bytecode/Instruction.h>
 #include <LibJS/Bytecode/Register.h>
+#include <LibJS/Forward.h>
 
 namespace JS::Bytecode {
 
@@ -27,9 +28,20 @@ OwnPtr<Block> Generator::generate(ASTNode const& node)
     Generator generator;
     node.generate_bytecode(generator);
     generator.m_block->set_register_count({}, generator.m_next_register);
+    generator.m_block->seal();
     return move(generator.m_block);
 }
 
+void Generator::grow(size_t additional_size)
+{
+    m_block->grow(additional_size);
+}
+
+void* Generator::next_slot()
+{
+    return m_block->next_slot();
+}
+
 Register Generator::allocate_register()
 {
     VERIFY(m_next_register != NumericLimits<u32>::max());
diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.h b/Userland/Libraries/LibJS/Bytecode/Generator.h
index 110ad25ac5..88bac76ebe 100644
--- a/Userland/Libraries/LibJS/Bytecode/Generator.h
+++ b/Userland/Libraries/LibJS/Bytecode/Generator.h
@@ -7,8 +7,8 @@
 #pragma once
 
 #include <AK/OwnPtr.h>
-#include <LibJS/Bytecode/Block.h>
 #include <LibJS/Bytecode/Label.h>
+#include <LibJS/Bytecode/Register.h>
 #include <LibJS/Forward.h>
 
 namespace JS::Bytecode {
@@ -20,15 +20,21 @@ public:
     Register allocate_register();
 
     template<typename OpType, typename... Args>
-    InstructionHandle<OpType> emit(Args&&... args)
+    OpType& emit(Args&&... args)
     {
-        return make_instruction<OpType>(0, forward<Args>(args)...);
+        void* slot = next_slot();
+        grow(sizeof(OpType));
+        new (slot) OpType(forward<Args>(args)...);
+        return *static_cast<OpType*>(slot);
     }
 
     template<typename OpType, typename... Args>
-    InstructionHandle<OpType> emit_with_extra_register_slots(size_t extra_register_slots, Args&&... args)
+    OpType& emit_with_extra_register_slots(size_t extra_register_slots, Args&&... args)
     {
-        return make_instruction<OpType>(extra_register_slots, forward<Args>(args)...);
+        void* slot = next_slot();
+        grow(sizeof(OpType) + extra_register_slots * sizeof(Register));
+        new (slot) OpType(forward<Args>(args)...);
+        return *static_cast<OpType*>(slot);
     }
 
     Label make_label() const;
@@ -42,15 +48,8 @@ private:
     Generator();
     ~Generator();
 
-    template<typename OpType, typename... Args>
-    InstructionHandle<OpType> make_instruction(size_t extra_register_slots, Args&&... args)
-    {
-        auto& buffer = m_block->buffer();
-        auto offset = buffer.size();
-        buffer.resize(buffer.size() + sizeof(OpType) + extra_register_slots * sizeof(Register));
-        new (buffer.data() + offset) OpType(forward<Args>(args)...);
-        return InstructionHandle<OpType>(offset, m_block);
-    }
+    void grow(size_t);
+    void* next_slot();
 
     OwnPtr<Block> m_block;
     u32 m_next_register { 1 };
diff --git a/Userland/Libraries/LibJS/Bytecode/Instruction.cpp b/Userland/Libraries/LibJS/Bytecode/Instruction.cpp
index 7c85efec48..899229607d 100644
--- a/Userland/Libraries/LibJS/Bytecode/Instruction.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/Instruction.cpp
@@ -4,7 +4,6 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <LibJS/Bytecode/Block.h>
 #include <LibJS/Bytecode/Instruction.h>
 #include <LibJS/Bytecode/Op.h>
 
diff --git a/Userland/Libraries/LibJS/Bytecode/Instruction.h b/Userland/Libraries/LibJS/Bytecode/Instruction.h
index b38ed0b804..1b7286eea8 100644
--- a/Userland/Libraries/LibJS/Bytecode/Instruction.h
+++ b/Userland/Libraries/LibJS/Bytecode/Instruction.h
@@ -7,7 +7,6 @@
 #pragma once
 
 #include <AK/Forward.h>
-#include <LibJS/Bytecode/Block.h>
 #include <LibJS/Forward.h>
 
 #define ENUMERATE_BYTECODE_OPS(O) \
@@ -84,45 +83,4 @@ private:
     Type m_type {};
 };
 
-template<typename OpType>
-class InstructionHandle {
-public:
-    InstructionHandle() = default;
-
-    InstructionHandle(size_t offset, Block* block)
-        : m_offset(offset)
-        , m_block(block)
-    {
-    }
-
-    OpType* operator->() const
-    {
-        VERIFY(m_block);
-        return reinterpret_cast<OpType*>(m_block->buffer().data() + m_offset);
-    }
-
-    OpType& operator*() const
-    {
-        VERIFY(m_block);
-        return *reinterpret_cast<OpType*>(m_block->buffer().data() + m_offset);
-    }
-
-    template<typename T>
-    InstructionHandle<OpType>& operator=(InstructionHandle<T> const& other) requires(IsBaseOf<OpType, T>)
-    {
-        m_offset = other.offset();
-        m_block = other.block();
-        return *this;
-    }
-
-    size_t offset() const { return m_offset; }
-    Block* block() const { return m_block; }
-
-private:
-    friend class Block;
-
-    size_t m_offset { 0 };
-    Block* m_block { nullptr };
-};
-
 }
diff --git a/Userland/Libraries/LibJS/Forward.h b/Userland/Libraries/LibJS/Forward.h
index a4fd02fb08..0a2665cc42 100644
--- a/Userland/Libraries/LibJS/Forward.h
+++ b/Userland/Libraries/LibJS/Forward.h
@@ -166,8 +166,6 @@ namespace Bytecode {
 class Block;
 class Generator;
 class Instruction;
-template<typename OpType>
-class InstructionHandle;
 class Interpreter;
 class Register;
 }