From 946581456dc08262056e69145d53e4068aaad6e6 Mon Sep 17 00:00:00 2001
From: Iban Eguia Moraza <razican@protonmail.ch>
Date: Sun, 10 May 2020 11:59:25 +0200
Subject: [PATCH] Trying to fix unsoundness in `InternalState`

---
 boa/src/builtins/console/mod.rs           |  3 ++-
 boa/src/builtins/function/mod.rs          |  6 +++++-
 boa/src/builtins/object/internal_state.rs | 17 +++++++++--------
 boa/src/builtins/regexp/mod.rs            | 12 +++++++++---
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/boa/src/builtins/console/mod.rs b/boa/src/builtins/console/mod.rs
index bd5cf26cea0..4b8a14f5eda 100644
--- a/boa/src/builtins/console/mod.rs
+++ b/boa/src/builtins/console/mod.rs
@@ -23,11 +23,12 @@ use crate::{
     },
     exec::Interpreter,
 };
+use gc::{Finalize, Trace};
 use rustc_hash::FxHashMap;
 use std::time::SystemTime;
 
 /// This is the internal console object state.
-#[derive(Debug, Default)]
+#[derive(Debug, Default, Trace, Finalize)]
 pub struct ConsoleState {
     count_map: FxHashMap<String, u32>,
     timer_map: FxHashMap<String, u128>,
diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs
index e0c7a346bc4..d7fbad405f4 100644
--- a/boa/src/builtins/function/mod.rs
+++ b/boa/src/builtins/function/mod.rs
@@ -39,12 +39,16 @@ pub enum ConstructorKind {
 /// Defines how this references are interpreted within the formal parameters and code body of the function.
 ///
 /// Arrow functions don't define a `this` and thus are lexical, `function`s do define a this and thus are NonLexical
-#[derive(Trace, Finalize, Debug, Clone)]
+#[derive(Finalize, Copy, Debug, Clone)]
 pub enum ThisMode {
     Lexical,
     NonLexical,
 }
 
+unsafe impl Trace for ThisMode {
+    unsafe_empty_trace!();
+}
+
 /// FunctionBody is specific to this interpreter, it will either be Rust code or JavaScript code (AST Node)
 #[derive(Clone, Finalize)]
 pub enum FunctionBody {
diff --git a/boa/src/builtins/object/internal_state.rs b/boa/src/builtins/object/internal_state.rs
index 06955258cc5..8c417684412 100644
--- a/boa/src/builtins/object/internal_state.rs
+++ b/boa/src/builtins/object/internal_state.rs
@@ -1,5 +1,6 @@
 //! Implementations for storing normal rust structs inside any object as internal state.
 
+use gc::{unsafe_empty_trace, Finalize, Trace};
 use std::{
     any::Any,
     fmt::{self, Debug},
@@ -7,13 +8,11 @@ use std::{
     rc::Rc,
 };
 
-use gc::{unsafe_empty_trace, Finalize, Trace};
-
 /// Wrapper around `Rc` to implement `Trace` and `Finalize`.
 #[derive(Clone)]
 pub struct InternalStateCell {
     /// The internal state.
-    state: Rc<dyn Any>,
+    state: Rc<dyn InternalState + 'static>,
 }
 
 impl Finalize for InternalStateCell {}
@@ -23,7 +22,7 @@ unsafe impl Trace for InternalStateCell {
 }
 
 impl Deref for InternalStateCell {
-    type Target = dyn Any;
+    type Target = dyn InternalState;
     fn deref(&self) -> &Self::Target {
         Deref::deref(&self.state)
     }
@@ -45,20 +44,22 @@ impl Debug for InternalStateCell {
 
 impl InternalStateCell {
     /// Create new `InternalStateCell` from a value.
-    pub fn new<T: Any + InternalState>(value: T) -> Self {
+    pub fn new<T: 'static + InternalState>(value: T) -> Self {
         Self {
             state: Rc::new(value),
         }
     }
+
     /// Get a reference to the stored value and cast it to `T`.
     pub fn downcast_ref<T: Any + InternalState>(&self) -> Option<&T> {
         self.deref().downcast_ref::<T>()
     }
+
     /// Get a mutable reference to the stored value and cast it to `T`.
-    pub fn downcast_mut<T: Any + InternalState>(&mut self) -> Option<&mut T> {
-        self.deref_mut().downcast_mut::<T>()
+    pub fn downcast_mut<T: InternalState>(&mut self) -> Option<&mut T> {
+        self.state.downcast_mut::<T>()
     }
 }
 
 /// This trait must be implemented by all structs used for internal state.
-pub trait InternalState: Debug {}
+pub trait InternalState: Debug + Trace + Any {}
diff --git a/boa/src/builtins/regexp/mod.rs b/boa/src/builtins/regexp/mod.rs
index b2cbecc2610..168453bc69f 100644
--- a/boa/src/builtins/regexp/mod.rs
+++ b/boa/src/builtins/regexp/mod.rs
@@ -9,9 +9,9 @@
 //! [spec]: https://tc39.es/ecma262/#sec-regexp-constructor
 //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp
 
-use std::ops::Deref;
-
+use gc::{unsafe_empty_trace, Finalize, Gc, Trace};
 use regex::Regex;
+use std::ops::Deref;
 
 use crate::{
     builtins::{
@@ -26,7 +26,7 @@ use crate::{
 mod tests;
 
 /// The internal representation on a `RegExp` object.
-#[derive(Debug)]
+#[derive(Debug, Finalize)]
 struct RegExp {
     /// Regex matcher.
     matcher: Regex,
@@ -56,6 +56,12 @@ struct RegExp {
     unicode: bool,
 }
 
+// FIXME: Maybe `Regex` could at some point implement `Trace`, and we need to take that into
+// account.
+unsafe impl Trace for RegExp {
+    unsafe_empty_trace!();
+}
+
 impl InternalState for RegExp {}
 
 /// Create a new `RegExp`