From ea7ba34a31b3ef9f3ef49bb77068ffdb0167497c Mon Sep 17 00:00:00 2001
From: Ali Mohammad Pur <ali.mpfard@gmail.com>
Date: Wed, 2 Jun 2021 18:11:48 +0430
Subject: [PATCH] AK+LibWasm+LibJS: Disallow Variant.has() on types that aren't
 contained

Checking for this (and get()'ing it) is always invalid, so let's just
disallow it.
This also finds two bugs where the code is checking for types that can
never actually be in the variant (which was actually a refactor
artifact).
---
 AK/Variant.h                                  | 26 +++++++++----------
 Userland/Libraries/LibJS/Parser.cpp           |  2 +-
 .../LibWasm/AbstractMachine/Interpreter.cpp   |  4 +--
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/AK/Variant.h b/AK/Variant.h
index 3475143793..af6b649105 100644
--- a/AK/Variant.h
+++ b/AK/Variant.h
@@ -201,6 +201,12 @@ private:
     static constexpr IndexType index_of() { return Detail::index_of<T, IndexType, Ts...>(); }
 
 public:
+    template<typename T>
+    static constexpr bool can_contain()
+    {
+        return index_of<T>() != invalid_index;
+    }
+
     template<typename... NewTs>
     friend struct Variant;
 
@@ -246,7 +252,7 @@ public:
     using Detail::MergeAndDeduplicatePacks<Detail::VariantConstructors<Ts, Variant<Ts...>>...>::MergeAndDeduplicatePacks;
 
     template<typename T, typename StrippedT = RemoveCV<RemoveReference<T>>>
-    void set(T&& t) requires(index_of<StrippedT>() != invalid_index)
+    void set(T&& t) requires(can_contain<StrippedT>())
     {
         constexpr auto new_index = index_of<StrippedT>();
         Helper::delete_(m_index, m_data);
@@ -255,7 +261,7 @@ public:
     }
 
     template<typename T, typename StrippedT = RemoveCV<RemoveReference<T>>>
-    void set(T&& t, Detail::VariantNoClearTag) requires(index_of<StrippedT>() != invalid_index)
+    void set(T&& t, Detail::VariantNoClearTag) requires(can_contain<StrippedT>())
     {
         constexpr auto new_index = index_of<StrippedT>();
         new (m_data) StrippedT(forward<T>(t));
@@ -263,7 +269,7 @@ public:
     }
 
     template<typename T>
-    T* get_pointer()
+    T* get_pointer() requires(can_contain<T>())
     {
         if (index_of<T>() == m_index)
             return bit_cast<T*>(&m_data);
@@ -271,14 +277,14 @@ public:
     }
 
     template<typename T>
-    T& get()
+    T& get() requires(can_contain<T>())
     {
         VERIFY(has<T>());
         return *bit_cast<T*>(&m_data);
     }
 
     template<typename T>
-    const T* get_pointer() const
+    const T* get_pointer() const requires(can_contain<T>())
     {
         if (index_of<T>() == m_index)
             return bit_cast<const T*>(&m_data);
@@ -286,14 +292,14 @@ public:
     }
 
     template<typename T>
-    const T& get() const
+    const T& get() const requires(can_contain<T>())
     {
         VERIFY(has<T>());
         return *bit_cast<const T*>(&m_data);
     }
 
     template<typename T>
-    [[nodiscard]] bool has() const
+    [[nodiscard]] bool has() const requires(can_contain<T>())
     {
         return index_of<T>() == m_index;
     }
@@ -336,12 +342,6 @@ public:
         return instance;
     }
 
-    template<typename T>
-    static constexpr bool can_contain()
-    {
-        return index_of<T>() != invalid_index;
-    }
-
 private:
     static constexpr auto data_size = integer_sequence_generate_array<size_t>(0, IntegerSequence<size_t, sizeof(Ts)...>()).max();
     static constexpr auto data_alignment = integer_sequence_generate_array<size_t>(0, IntegerSequence<size_t, alignof(Ts)...>()).max();
diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp
index 33b7ae6c4c..c1a8051f94 100644
--- a/Userland/Libraries/LibJS/Parser.cpp
+++ b/Userland/Libraries/LibJS/Parser.cpp
@@ -1609,7 +1609,7 @@ NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration(bool for_l
             init = parse_expression(2);
         } else if (!for_loop_variable_declaration && declaration_kind == DeclarationKind::Const) {
             syntax_error("Missing initializer in 'const' variable declaration");
-        } else if (target.has<BindingPattern>()) {
+        } else if (target.has<NonnullRefPtr<BindingPattern>>()) {
             syntax_error("Missing initializer in destructuring assignment");
         }
 
diff --git a/Userland/Libraries/LibWasm/AbstractMachine/Interpreter.cpp b/Userland/Libraries/LibWasm/AbstractMachine/Interpreter.cpp
index 120d2fb0f7..905159266e 100644
--- a/Userland/Libraries/LibWasm/AbstractMachine/Interpreter.cpp
+++ b/Userland/Libraries/LibWasm/AbstractMachine/Interpreter.cpp
@@ -482,12 +482,12 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi
             return;
         }
         auto element = table_instance->elements()[index.value()];
-        if (!element.has_value() || !element->ref().has<FunctionAddress>()) {
+        if (!element.has_value() || !element->ref().has<Reference::Func>()) {
             dbgln("LibWasm: call_indirect attempted with invalid address element (not a function)");
             m_do_trap = true;
             return;
         }
-        auto address = element->ref().get<FunctionAddress>();
+        auto address = element->ref().get<Reference::Func>().address;
         dbgln_if(WASM_TRACE_DEBUG, "call_indirect({} -> {})", index.value(), address.value());
         call_address(configuration, address);
         return;