Fix environment record panics

boa/src/environment/declarative_environment_record.rs

@@ -34,44 +34,51 @@ pub struct DeclarativeEnvironmentRecordBinding {
 /// declarations contained within its scope.
 #[derive(Debug, Trace, Finalize, Clone)]
 pub struct DeclarativeEnvironmentRecord {
-    pub env_rec: FxHashMap<Box<str>, DeclarativeEnvironmentRecordBinding>,
+    pub env_rec: GcCell<FxHashMap<Box<str>, DeclarativeEnvironmentRecordBinding>>,
     pub outer_env: Option<Environment>,
 }
 
 impl DeclarativeEnvironmentRecord {
+    pub fn new_declarative_environment_record(
+        env: Option<Environment>,
+    ) -> DeclarativeEnvironmentRecord {
+        DeclarativeEnvironmentRecord {
+            env_rec: GcCell::new(FxHashMap::default()),
+            outer_env: env,
+        }
+    }
+
     #[allow(clippy::new_ret_no_self)]
     pub fn new(env: Option<Environment>) -> Environment {
         let _timer = BoaProfiler::global().start_event("new_declarative_environment", "env");
-        let boxed_env = Box::new(DeclarativeEnvironmentRecord {
-            env_rec: FxHashMap::default(),
-            outer_env: env,
-        });
 
-        Gc::new(GcCell::new(boxed_env))
+        Gc::new(GcCell::new(Box::new(
+            Self::new_declarative_environment_record(env),
+        )))
     }
 }
 
 impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
     fn has_binding(&self, name: &str) -> bool {
-        self.env_rec.contains_key(name)
+        self.env_rec.borrow().contains_key(name)
     }
 
     fn create_mutable_binding(
-        &mut self,
+        &self,
         name: String,
         deletion: bool,
         allow_name_reuse: bool,
         _context: &mut Context,
     ) -> Result<()> {
         if !allow_name_reuse {
             assert!(
-                !self.env_rec.contains_key(name.as_str()),
+                !self.env_rec.borrow().contains_key(name.as_str()),
                 "Identifier {} has already been declared",
                 name
             );
         }
 
-        self.env_rec.insert(
+        self.env_rec.borrow_mut().insert(
             name.into_boxed_str(),
             DeclarativeEnvironmentRecordBinding {
                 value: None,
@@ -84,18 +91,18 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
     }
 
     fn create_immutable_binding(
-        &mut self,
+        &self,
         name: String,
         strict: bool,
         _context: &mut Context,
     ) -> Result<()> {
         assert!(
-            !self.env_rec.contains_key(name.as_str()),
+            !self.env_rec.borrow().contains_key(name.as_str()),
             "Identifier {} has already been declared",
             name
         );
 
-        self.env_rec.insert(
+        self.env_rec.borrow_mut().insert(
             name.into_boxed_str(),
             DeclarativeEnvironmentRecordBinding {
                 value: None,
@@ -107,13 +114,8 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
         Ok(())
     }
 
-    fn initialize_binding(
-        &mut self,
-        name: &str,
-        value: Value,
-        _context: &mut Context,
-    ) -> Result<()> {
-        if let Some(ref mut record) = self.env_rec.get_mut(name) {
+    fn initialize_binding(&self, name: &str, value: Value, _context: &mut Context) -> Result<()> {
+        if let Some(ref mut record) = self.env_rec.borrow_mut().get_mut(name) {
             if record.value.is_none() {
                 record.value = Some(value);
                 return Ok(());
@@ -124,13 +126,13 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
 
     #[allow(clippy::else_if_without_else)]
     fn set_mutable_binding(
-        &mut self,
+        &self,
         name: &str,
         value: Value,
         mut strict: bool,
         context: &mut Context,
     ) -> Result<()> {
-        if self.env_rec.get(name).is_none() {
+        if self.env_rec.borrow().get(name).is_none() {
             if strict {
                 return Err(context.construct_reference_error(format!("{} not found", name)));
             }
@@ -140,16 +142,22 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
             return Ok(());
         }
 
-        let record: &mut DeclarativeEnvironmentRecordBinding = self.env_rec.get_mut(name).unwrap();
-        if record.strict {
+        let (record_strict, record_has_no_value, record_mutable) = {
+            let env_rec = self.env_rec.borrow();
+            let record = env_rec.get(name).unwrap();
+            (record.strict, record.value.is_none(), record.mutable)
+        };
+        if record_strict {
             strict = true
         }
-        if record.value.is_none() {
+        if record_has_no_value {
             return Err(
                 context.construct_reference_error(format!("{} has not been initialized", name))
             );
         }
-        if record.mutable {
+        if record_mutable {
+            let mut env_rec = self.env_rec.borrow_mut();
+            let record = env_rec.get_mut(name).unwrap();
             record.value = Some(value);
         } else if strict {
             return Err(context.construct_reference_error(format!(
@@ -162,7 +170,7 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
     }
 
     fn get_binding_value(&self, name: &str, _strict: bool, context: &mut Context) -> Result<Value> {
-        if let Some(binding) = self.env_rec.get(name) {
+        if let Some(binding) = self.env_rec.borrow().get(name) {
             if let Some(ref val) = binding.value {
                 Ok(val.clone())
             } else {
@@ -173,11 +181,11 @@ impl EnvironmentRecordTrait for DeclarativeEnvironmentRecord {
         }
     }
 
-    fn delete_binding(&mut self, name: &str) -> bool {
-        match self.env_rec.get(name) {
+    fn delete_binding(&self, name: &str) -> bool {
+        match self.env_rec.borrow().get(name) {
             Some(binding) => {
                 if binding.can_delete {
-                    self.env_rec.remove(name);
+                    self.env_rec.borrow_mut().remove(name);
                     true
                 } else {
                     false

boa/src/environment/environment_record_trait.rs

@@ -34,7 +34,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
     /// Most variable names cannot be reused, but functions in JavaScript are allowed to have multiple
     /// paraments with the same name.
     fn create_mutable_binding(
-        &mut self,
+        &self,
         name: String,
         deletion: bool,
         allow_name_reuse: bool,
@@ -46,7 +46,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
     /// If strict is true then attempts to set it after it has been initialized will always throw an exception,
     /// regardless of the strict mode setting of operations that reference that binding.
     fn create_immutable_binding(
-        &mut self,
+        &self,
         name: String,
         strict: bool,
         context: &mut Context,
@@ -55,15 +55,14 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
     /// Set the value of an already existing but uninitialized binding in an Environment Record.
     /// The String value N is the text of the bound name.
     /// V is the value for the binding and is a value of any ECMAScript language type.
-    fn initialize_binding(&mut self, name: &str, value: Value, context: &mut Context)
-        -> Result<()>;
+    fn initialize_binding(&self, name: &str, value: Value, context: &mut Context) -> Result<()>;
 
     /// Set the value of an already existing mutable binding in an Environment Record.
     /// The String value `name` is the text of the bound name.
     /// value is the `value` for the binding and may be a value of any ECMAScript language type. S is a Boolean flag.
     /// If `strict` is true and the binding cannot be set throw a TypeError exception.
     fn set_mutable_binding(
-        &mut self,
+        &self,
         name: &str,
         value: Value,
         strict: bool,
@@ -80,7 +79,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
     /// The String value name is the text of the bound name.
     /// If a binding for name exists, remove the binding and return true.
     /// If the binding exists but cannot be removed return false. If the binding does not exist return true.
-    fn delete_binding(&mut self, name: &str) -> bool;
+    fn delete_binding(&self, name: &str) -> bool;
 
     /// Determine if an Environment Record establishes a this binding.
     /// Return true if it does and false if it does not.
@@ -123,7 +122,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
 
     /// Create mutable binding while handling outer environments
     fn recursive_create_mutable_binding(
-        &mut self,
+        &self,
         name: String,
         deletion: bool,
         scope: VariableScope,
@@ -134,14 +133,14 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
             VariableScope::Function => self
                 .get_outer_environment_ref()
                 .expect("No function or global environment")
-                .borrow_mut()
+                .borrow()
                 .recursive_create_mutable_binding(name, deletion, scope, context),
         }
     }
 
     /// Create immutable binding while handling outer environments
     fn recursive_create_immutable_binding(
-        &mut self,
+        &self,
         name: String,
         deletion: bool,
         scope: VariableScope,
@@ -152,14 +151,14 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
             VariableScope::Function => self
                 .get_outer_environment_ref()
                 .expect("No function or global environment")
-                .borrow_mut()
+                .borrow()
                 .recursive_create_immutable_binding(name, deletion, scope, context),
         }
     }
 
     /// Set mutable binding while handling outer environments
     fn recursive_set_mutable_binding(
-        &mut self,
+        &self,
         name: &str,
         value: Value,
         strict: bool,
@@ -170,14 +169,14 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
         } else {
             self.get_outer_environment_ref()
                 .expect("Environment stack underflow")
-                .borrow_mut()
+                .borrow()
                 .recursive_set_mutable_binding(name, value, strict, context)
         }
     }
 
     /// Initialize binding while handling outer environments
     fn recursive_initialize_binding(
-        &mut self,
+        &self,
         name: &str,
         value: Value,
         context: &mut Context,
@@ -187,7 +186,7 @@ pub trait EnvironmentRecordTrait: Debug + Trace + Finalize {
         } else {
             self.get_outer_environment_ref()
                 .expect("Environment stack underflow")
-                .borrow_mut()
+                .borrow()
                 .recursive_initialize_binding(name, value, context)
         }
     }

boa/src/environment/function_environment_record.rs

@@ -9,7 +9,6 @@
 //! More info: <https://tc39.es/ecma262/#sec-function-environment-records>
 
 use gc::{Gc, GcCell};
-use rustc_hash::FxHashMap;
 
 use crate::{
     environment::{
@@ -68,10 +67,9 @@ impl FunctionEnvironmentRecord {
         new_target: Value,
     ) -> Environment {
         let mut func_env = FunctionEnvironmentRecord {
-            declarative_record: DeclarativeEnvironmentRecord {
-                env_rec: FxHashMap::default(),
-                outer_env: outer, // this will come from Environment set as a private property of F - https://tc39.es/ecma262/#sec-ecmascript-function-objects
-            },
+            declarative_record: DeclarativeEnvironmentRecord::new_declarative_environment_record(
+                outer,
+            ), // the outer environment will come from Environment set as a private property of F - https://tc39.es/ecma262/#sec-ecmascript-function-objects
             function: f,
             this_binding_status: binding_status,
             home_object: Value::undefined(),
@@ -123,7 +121,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord {
     }
 
     fn create_mutable_binding(
-        &mut self,
+        &self,
         name: String,
         deletion: bool,
         allow_name_reuse: bool,
@@ -134,7 +132,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord {
     }
 
     fn create_immutable_binding(
-        &mut self,
+        &self,
         name: String,
         strict: bool,
         context: &mut Context,
@@ -143,18 +141,13 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord {
             .create_immutable_binding(name, strict, context)
     }
 
-    fn initialize_binding(
-        &mut self,
-        name: &str,
-        value: Value,
-        context: &mut Context,
-    ) -> Result<()> {
+    fn initialize_binding(&self, name: &str, value: Value, context: &mut Context) -> Result<()> {
         self.declarative_record
             .initialize_binding(name, value, context)
     }
 
     fn set_mutable_binding(
-        &mut self,
+        &self,
         name: &str,
         value: Value,
         strict: bool,
@@ -169,7 +162,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord {
             .get_binding_value(name, strict, context)
     }
 
-    fn delete_binding(&mut self, name: &str) -> bool {
+    fn delete_binding(&self, name: &str) -> bool {
         self.declarative_record.delete_binding(name)
     }
 
@@ -214,7 +207,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord {
     }
 
     fn recursive_create_mutable_binding(
-        &mut self,
+        &self,
         name: String,
         deletion: bool,
         _scope: VariableScope,
@@ -224,7 +217,7 @@ impl EnvironmentRecordTrait for FunctionEnvironmentRecord {
     }
 
     fn recursive_create_immutable_binding(
-        &mut self,
+        &self,
         name: String,
         deletion: bool,
         _scope: VariableScope,