From a0e52537a5a9a0a488878d8ecf465e6ab2267cd4 Mon Sep 17 00:00:00 2001 From: Adrian Eddy Date: Fri, 9 Oct 2026 19:41:52 +0200 Subject: [PATCH] fix(engine): initialize module var bindings at link time InitializeEnvironment creates each `var` binding of a module and initializes it to `undefined` while the module is linked. Boa compiled that step into the module's body as `DefInitVar`, so the bindings stayed uninitialized until the body started. In an import cycle, a module whose body runs first threw a `ReferenceError` when it read such a binding or wrote it through a hoisted function. Initialize the bindings during the link, next to the function declarations. Every binding of the module scope escapes, so all of them live in the module environment. Drop the `DefInitVar` from the body; the body's `var` declarations without an initializer compile to `DefVar`, which keeps a value written before the body runs. --- core/engine/src/module/source.rs | 30 +++++--- core/engine/tests/module.rs | 113 +++++++++++++++++++++++++++++-- 2 files changed, 129 insertions(+), 14 deletions(-) diff --git a/core/engine/src/module/source.rs b/core/engine/src/module/source.rs index c904a9be70a..8ca5aee8f87 100644 --- a/core/engine/src/module/source.rs +++ b/core/engine/src/module/source.rs @@ -29,7 +29,7 @@ use crate::{ Context, JsArgs, JsError, JsExpect, JsNativeError, JsObject, JsResult, JsString, JsValue, NativeFunction, SpannedSourceText, builtins::{Promise, promise::PromiseCapability}, - bytecompiler::{BindingAccessOpcode, ByteCompiler, FunctionSpec, ToJsString}, + bytecompiler::{ByteCompiler, FunctionSpec, ToJsString}, environments::{DeclarativeEnvironment, EnvironmentStack}, job::NativeAsyncJob, js_string, @@ -1662,6 +1662,7 @@ impl SourceTextModule { compiler.async_handler = self.code.has_tla.then(|| compiler.push_handler()); let mut imports = Vec::new(); + let mut vars = Vec::new(); let (codeblock, functions) = { // 7. For each ImportEntry Record in of module.[[ImportEntries]], do @@ -1747,15 +1748,9 @@ impl SourceTextModule { if !declared_var_names.contains(&name) { // 1. Perform ! env.CreateMutableBinding(dn, false). // 2. Perform ! env.InitializeBinding(dn, undefined). - let binding = env - .get_binding_reference(&name) - .js_expect("binding must exist")?; - let index = compiler.insert_binding(binding); - compiler.emit_binding_access( - BindingAccessOpcode::DefInitVar, - &index, - &CallFrame::undefined_register(), - ); + // deferred to initialization below + let locator = env.get_binding(&name).js_expect("binding must exist")?; + vars.push(locator); // 3. Append dn to declaredVarNames. declared_var_names.push(name); @@ -1901,6 +1896,21 @@ impl SourceTextModule { } } + // deferred initialization of var bindings (module-scope bindings always escape in + // `BindingEscapeAnalyzer::visit_module_mut`, so these locators address environment slots) + { + let frame = context.vm.frame_mut(); + let global = frame.realm.environment(); + for locator in vars { + frame.environments.put_lexical_value( + locator.scope(), + locator.binding_index(), + JsValue::undefined(), + global, + ); + } + } + // deferred initialization of function exports for (index, locator) in functions { let code = codeblock.constant_function(index as usize); diff --git a/core/engine/tests/module.rs b/core/engine/tests/module.rs index 9c49ce569e7..80564913e4f 100644 --- a/core/engine/tests/module.rs +++ b/core/engine/tests/module.rs @@ -5,8 +5,11 @@ use std::future; use std::rc::Rc; use boa_engine::builtins::promise::PromiseState; -use boa_engine::module::{ModuleLoader, Referrer}; -use boa_engine::{Context, JsResult, JsString, Module, Source, js_string}; +use boa_engine::module::{MapModuleLoader, ModuleLoader, Referrer}; +use boa_engine::{ + Context, JsNativeError, JsNativeErrorKind, JsResult, JsString, JsValue, Module, Source, + js_string, +}; #[test] fn test_json_module_from_str() { @@ -192,7 +195,7 @@ fn test_json_module_static_import_with_attributes() { assert_eq!( promise.state(), - PromiseState::Fulfilled(boa_engine::JsValue::undefined()) + PromiseState::Fulfilled(JsValue::undefined()) ); let value = module @@ -248,7 +251,7 @@ fn test_json_module_reexport_with_attributes() { assert_eq!( promise.state(), - PromiseState::Fulfilled(boa_engine::JsValue::undefined()) + PromiseState::Fulfilled(JsValue::undefined()) ); let json = module @@ -452,3 +455,105 @@ fn test_dynamic_import_symbol_key() { PromiseState::Pending => panic!("Dynamic import is still pending"), } } + +/// Linking a module initializes its `var` bindings to `undefined`, while its lexical bindings stay +/// uninitialized until its body runs. +#[test] +fn test_module_var_bindings_are_initialized_at_link_time() { + let mut context = Context::default(); + let module = Module::parse( + Source::from_bytes("export var x = 1; export let y = 2;"), + None, + &mut context, + ) + .unwrap(); + + let promise = module.load(&mut context); + context.run_jobs().unwrap(); + assert!(promise.state().as_fulfilled().is_some()); + module.link(&mut context).unwrap(); + + let namespace = module.namespace(&mut context); + assert_eq!( + namespace.get(js_string!("x"), &mut context).unwrap(), + JsValue::undefined() + ); + let error = namespace.get(js_string!("y"), &mut context).unwrap_err(); + assert_eq!( + error.as_native().map(JsNativeError::kind), + Some(&JsNativeErrorKind::Reference) + ); +} + +/// Links and evaluates the module `a`, which imports the module `b`, which imports `a` back, and +/// returns `a`'s export named `result`. +/// +/// `b`'s body runs before `a`'s, because `b`'s import of `a` finds `a` already evaluating. +fn evaluate_cycle(a: &str, b: &str) -> JsValue { + let loader = Rc::new(MapModuleLoader::new()); + let mut context = Context::builder() + .module_loader(loader.clone()) + .build() + .unwrap(); + + let a = Module::parse(Source::from_bytes(a), None, &mut context).unwrap(); + let b = Module::parse(Source::from_bytes(b), None, &mut context).unwrap(); + loader.insert("a", a.clone()); + loader.insert("b", b); + + let promise = a.load_link_evaluate(&mut context); + context.run_jobs().unwrap(); + if let PromiseState::Rejected(reason) = promise.state() { + panic!("module evaluation failed: {}", reason.display()); + } + assert_eq!( + promise.state(), + PromiseState::Fulfilled(JsValue::undefined()) + ); + + a.namespace(&mut context) + .get(js_string!("result"), &mut context) + .unwrap() +} + +/// A module's `var` bindings are initialized to `undefined` when the module is linked, so code that +/// runs before the module's body reads them as `undefined`. +#[test] +fn test_module_var_bindings_read_before_evaluation_are_undefined() { + let result = evaluate_cycle( + r#" + import { observed } from "b"; + export var exported = 1; + var local = 2; + export function readLocal() { return local; } + export const result = observed; + "#, + r#" + import { exported, readLocal } from "a"; + import * as a from "a"; + export const observed = [exported, a.exported, readLocal()].map(String).join(); + "#, + ); + + assert_eq!(result, js_string!("undefined,undefined,undefined").into()); +} + +/// A module's body does not reinitialize its `var` bindings, so a value written to one before the +/// body runs is kept. +#[test] +fn test_module_var_binding_written_before_evaluation_keeps_its_value() { + let result = evaluate_cycle( + r#" + import "b"; + export var value; + export function setValue(v) { value = v; } + export const result = value; + "#, + r#" + import { setValue } from "a"; + setValue(5); + "#, + ); + + assert_eq!(result, JsValue::from(5)); +}