From 9d7cb62cba775f039c24b6c34572b36a00ed6559 Mon Sep 17 00:00:00 2001 From: Dunqing Date: Tue, 24 Mar 2026 23:09:10 +0800 Subject: [PATCH 1/4] fix(mangler): prevent slot reuse for block-scoped functions in sloppy mode In sloppy mode JavaScript, function declarations inside blocks (if, try, switch, {}) have Annex B.3.2.1 semantics: they create an implicit var-like assignment in the enclosing function scope at block exit. The mangler was treating these as purely block-scoped (like let/const), allowing slot reuse with outer var bindings. When both got the same mangled name, the Annex B assignment would overwrite the outer variable at runtime, changing program behavior. The fix has two parts: 1. Annex B function declarations in sloppy block scopes are partitioned to the end of bindings and always get fresh slots (never reuse existing ones) 2. Their slot liveness is extended upward to the enclosing var-scope, preventing sibling scopes from reusing their slots either Strict mode and ES modules are unaffected (no Annex B, no size regression). Fixes #20610 Fixes #14316 Co-Authored-By: Claude Opus 4.6 --- crates/oxc_mangler/src/lib.rs | 43 +++++++++- crates/oxc_minifier/tests/mangler/mod.rs | 52 +++++++++++- .../annex_b_block_scoped_function.snap | 84 +++++++++++++++++++ 3 files changed, 175 insertions(+), 4 deletions(-) create mode 100644 crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs index b27b26c9840c0..fcbdde24a8a6a 100644 --- a/crates/oxc_mangler/src/lib.rs +++ b/crates/oxc_mangler/src/lib.rs @@ -347,7 +347,8 @@ impl<'t> Mangler<'t> { } // Scopes with direct eval: collect binding names as reserved (they can be // accessed by eval at runtime) and skip slot assignment (keep original names). - if scoping.scope_flags(scope_id).contains_direct_eval() { + let scope_flags = scoping.scope_flags(scope_id); + if scope_flags.contains_direct_eval() { for (name, _) in bindings { eval_reserved_names.insert(name.as_str()); } @@ -366,6 +367,31 @@ impl<'t> Mangler<'t> { } tmp_bindings.sort_unstable(); + // Annex B.3.2.1: In sloppy mode, function declarations inside block scopes create + // an implicit `var`-like assignment in the enclosing function scope at block exit. + // If such a function gets the same mangled name as an outer `var`, the Annex B + // assignment overwrites it. Prevent this by giving them fresh slots (no reuse). + let is_sloppy_block = !scope_flags.is_var() && !scope_flags.is_strict_mode(); + let regular_count = if is_sloppy_block { + // Partition `tmp_bindings` into two regions: + // [regular bindings ... | function declarations ...] + // ^ regular_end (returned) + // For each non-function binding, swap it to the front of the array. + // Function declarations are left behind and end up at the tail. + // Only the first `regular_end` bindings will be eligible for slot reuse; + // the function declarations after that boundary always get fresh slots. + let mut regular_end = 0; + for i in 0..tmp_bindings.len() { + if !scoping.symbol_flags(tmp_bindings[i]).is_function() { + tmp_bindings.swap(regular_end, i); + regular_end += 1; + } + } + regular_end + } else { + tmp_bindings.len() + }; + let mut slot = slot_liveness.len(); reusable_slots.clear(); @@ -380,7 +406,7 @@ impl<'t> Mangler<'t> { #[expect(clippy::cast_possible_truncation)] |(slot, _)| slot as Slot, ) - .take(tmp_bindings.len()), + .take(regular_count), // Annex B functions get fresh slots, not reused ones ); // The number of new slots that needs to be allocated. @@ -453,6 +479,19 @@ impl<'t> Mangler<'t> { slot_liveness_bitset.set_bit(ancestor_index); } } + + // Annex B: extend liveness to enclosing var-scope so sibling + // block scopes also cannot reuse this slot. + if regular_count < tmp_bindings.len() + && scoping.symbol_flags(symbol_id).is_function() + { + for ancestor_id in scoping.scope_ancestors(scope_id).skip(1) { + slot_liveness_bitset.set_bit(ancestor_id.index()); + if scoping.scope_flags(ancestor_id).is_var() { + break; + } + } + } } } diff --git a/crates/oxc_minifier/tests/mangler/mod.rs b/crates/oxc_minifier/tests/mangler/mod.rs index a0f2d0acb8d9e..4804d587e7308 100644 --- a/crates/oxc_minifier/tests/mangler/mod.rs +++ b/crates/oxc_minifier/tests/mangler/mod.rs @@ -6,9 +6,12 @@ use oxc_mangler::{MangleOptions, MangleOptionsKeepNames, Mangler}; use oxc_parser::Parser; use oxc_span::SourceType; -fn mangle(source_text: &str, options: MangleOptions) -> String { +fn mangle_with_source_type( + source_text: &str, + options: MangleOptions, + source_type: SourceType, +) -> String { let allocator = Allocator::default(); - let source_type = SourceType::mjs().with_unambiguous(true); let ret = Parser::new(&allocator, source_text, source_type).parse(); assert!(ret.errors.is_empty(), "Parser errors: {:?}", ret.errors); let program = ret.program; @@ -20,6 +23,14 @@ fn mangle(source_text: &str, options: MangleOptions) -> String { .code } +fn mangle(source_text: &str, options: MangleOptions) -> String { + mangle_with_source_type(source_text, options, SourceType::mjs().with_unambiguous(true)) +} + +fn mangle_script(source_text: &str, options: MangleOptions) -> String { + mangle_with_source_type(source_text, options, SourceType::script()) +} + fn test(source_text: &str, expected: &str, options: MangleOptions) { let mangled = mangle(source_text, options); let expected = { @@ -207,3 +218,40 @@ fn private_member_mangling() { insta::assert_snapshot!("private_member_mangling", snapshot); }); } + +/// In sloppy mode (script), function declarations inside blocks have Annex B.3.2.1 semantics: +/// they create an implicit `var`-like assignment in the enclosing function scope at block exit. +/// The mangler must not assign the same name to such a function and an outer `var` binding, +/// or the Annex B assignment would overwrite the outer variable. +#[test] +fn annex_b_block_scoped_function() { + let cases = [ + // Core bug: var + block function in if statement (vitejs/vite#22009) + "function _() { var x = 1; if (true) { function y() {} } use(x); }", + // var + block function in try block (oxc-project/oxc#14316) + "function _() { var x = 1; try { function y() {} } finally {} use(x); }", + // var + block function in plain block + "function _() { var x = 1; { function y() {} } use(x); }", + // Parameter + block function (Annex B doesn't overwrite params, but still safe) + "function _(x) { if (true) { function y() {} } use(x); }", + // Deeply nested blocks + "function _() { var x = 1; { { if (true) { function y() {} } } } use(x); }", + // Arrow callback + block function (oxc-project/oxc#20610) + "function outer(input) { let data = input; Object.keys(data).forEach((key) => { let value = data[key]; if (value) { function appendStyle() {} console.log(appendStyle); } console.log(value); }); }", + // Multiple block functions in same scope + "function _() { var x = 1; if (true) { function y() {} function z() {} } use(x); }", + // Block function referencing outer var (liveness already covers this, but verify) + "function _() { var x = 1; if (true) { function y() { return x; } } use(x); }", + ]; + + let mut snapshot = String::new(); + cases.into_iter().fold(&mut snapshot, |w, case| { + let options = MangleOptions::default(); + write!(w, "{case}\n{}\n", mangle_script(case, options)).unwrap(); + w + }); + + insta::with_settings!({ prepend_module_to_snapshot => false, omit_expression => true }, { + insta::assert_snapshot!("annex_b_block_scoped_function", snapshot); + }); +} diff --git a/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap b/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap new file mode 100644 index 0000000000000..3e42c4fc06278 --- /dev/null +++ b/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap @@ -0,0 +1,84 @@ +--- +source: crates/oxc_minifier/tests/mangler/mod.rs +--- +function _() { var x = 1; if (true) { function y() {} } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + } + use(e); +} + +function _() { var x = 1; try { function y() {} } finally {} use(x); } +function _() { + var e = 1; + try { + function t() {} + } finally {} + use(e); +} + +function _() { var x = 1; { function y() {} } use(x); } +function _() { + var e = 1; + { + function t() {} + } + use(e); +} + +function _(x) { if (true) { function y() {} } use(x); } +function _(e) { + if (true) { + function t() {} + } + use(e); +} + +function _() { var x = 1; { { if (true) { function y() {} } } } use(x); } +function _() { + var e = 1; + { + { + if (true) { + function t() {} + } + } + } + use(e); +} + +function outer(input) { let data = input; Object.keys(data).forEach((key) => { let value = data[key]; if (value) { function appendStyle() {} console.log(appendStyle); } console.log(value); }); } +function outer(e) { + let t = e; + Object.keys(t).forEach((e) => { + let n = t[e]; + if (n) { + function r() {} + console.log(r); + } + console.log(n); + }); +} + +function _() { var x = 1; if (true) { function y() {} function z() {} } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + function n() {} + } + use(e); +} + +function _() { var x = 1; if (true) { function y() { return x; } } use(x); } +function _() { + var e = 1; + if (true) { + function t() { + return e; + } + } + use(e); +} From 16f150cce38d7a165d59df5db8bf30d22ba66a5e Mon Sep 17 00:00:00 2001 From: Dunqing Date: Wed, 25 Mar 2026 13:37:16 +0800 Subject: [PATCH 2/4] remove part2 --- crates/oxc_mangler/src/lib.rs | 13 ------- crates/oxc_minifier/tests/mangler/mod.rs | 6 +++ .../annex_b_block_scoped_function.snap | 39 +++++++++++++++++++ 3 files changed, 45 insertions(+), 13 deletions(-) diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs index fcbdde24a8a6a..ad64ad138c7f6 100644 --- a/crates/oxc_mangler/src/lib.rs +++ b/crates/oxc_mangler/src/lib.rs @@ -479,19 +479,6 @@ impl<'t> Mangler<'t> { slot_liveness_bitset.set_bit(ancestor_index); } } - - // Annex B: extend liveness to enclosing var-scope so sibling - // block scopes also cannot reuse this slot. - if regular_count < tmp_bindings.len() - && scoping.symbol_flags(symbol_id).is_function() - { - for ancestor_id in scoping.scope_ancestors(scope_id).skip(1) { - slot_liveness_bitset.set_bit(ancestor_id.index()); - if scoping.scope_flags(ancestor_id).is_var() { - break; - } - } - } } } diff --git a/crates/oxc_minifier/tests/mangler/mod.rs b/crates/oxc_minifier/tests/mangler/mod.rs index 4804d587e7308..e3e08c6cd54f1 100644 --- a/crates/oxc_minifier/tests/mangler/mod.rs +++ b/crates/oxc_minifier/tests/mangler/mod.rs @@ -242,6 +242,12 @@ fn annex_b_block_scoped_function() { "function _() { var x = 1; if (true) { function y() {} function z() {} } use(x); }", // Block function referencing outer var (liveness already covers this, but verify) "function _() { var x = 1; if (true) { function y() { return x; } } use(x); }", + // Sibling block with let reusing function's slot is safe (let is truly block-scoped) + "function _() { var x = 1; if (true) { function y() {} } { let z = 2; use(z); } use(x); }", + // Sibling Annex B functions both get fresh slots + "function _() { var x = 1; if (true) { function y() {} } if (true) { function z() {} } use(x); }", + // Catch parameter in sibling scope + "function _() { var x = 1; if (true) { function y() {} } try { throw 0; } catch (z) { use(z); } use(x); }", ]; let mut snapshot = String::new(); diff --git a/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap b/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap index 3e42c4fc06278..c8c494752dba1 100644 --- a/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap +++ b/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap @@ -82,3 +82,42 @@ function _() { } use(e); } + +function _() { var x = 1; if (true) { function y() {} } { let z = 2; use(z); } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + } + { + let e = 2; + use(e); + } + use(e); +} + +function _() { var x = 1; if (true) { function y() {} } if (true) { function z() {} } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + } + if (true) { + function n() {} + } + use(e); +} + +function _() { var x = 1; if (true) { function y() {} } try { throw 0; } catch (z) { use(z); } use(x); } +function _() { + var e = 1; + if (true) { + function t() {} + } + try { + throw 0; + } catch (e) { + use(e); + } + use(e); +} From 414623288dcc302fab863f65268945d0cbe0fb09 Mon Sep 17 00:00:00 2001 From: Dunqing Date: Wed, 25 Mar 2026 13:44:55 +0800 Subject: [PATCH 3/4] add sibling `var` test --- crates/oxc_minifier/tests/mangler/mod.rs | 2 ++ .../snapshots/annex_b_block_scoped_function.snap | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/crates/oxc_minifier/tests/mangler/mod.rs b/crates/oxc_minifier/tests/mangler/mod.rs index e3e08c6cd54f1..e4c52bddcd7aa 100644 --- a/crates/oxc_minifier/tests/mangler/mod.rs +++ b/crates/oxc_minifier/tests/mangler/mod.rs @@ -248,6 +248,8 @@ fn annex_b_block_scoped_function() { "function _() { var x = 1; if (true) { function y() {} } if (true) { function z() {} } use(x); }", // Catch parameter in sibling scope "function _() { var x = 1; if (true) { function y() {} } try { throw 0; } catch (z) { use(z); } use(x); }", + // Sibling var is hoisted to function scope, so no conflict + "function _() { var x = 1; if (true) { function y() {} } { var z = 2; use(z); } use(x); }", ]; let mut snapshot = String::new(); diff --git a/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap b/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap index c8c494752dba1..8faf5c34a34c3 100644 --- a/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap +++ b/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap @@ -121,3 +121,16 @@ function _() { } use(e); } + +function _() { var x = 1; if (true) { function y() {} } { var z = 2; use(z); } use(x); } +function _() { + var e = 1; + if (true) { + function n() {} + } + { + var t = 2; + use(t); + } + use(e); +} From 14c83d6781db9d206258f6c5cab0702a3412950d Mon Sep 17 00:00:00 2001 From: Dunqing Date: Wed, 25 Mar 2026 16:32:57 +0800 Subject: [PATCH 4/4] hoisted function approach --- crates/oxc_mangler/src/lib.rs | 92 ++++++++++++------- crates/oxc_minifier/tests/mangler/mod.rs | 2 + .../annex_b_block_scoped_function.snap | 20 +++- 3 files changed, 76 insertions(+), 38 deletions(-) diff --git a/crates/oxc_mangler/src/lib.rs b/crates/oxc_mangler/src/lib.rs index ad64ad138c7f6..81df3a8d7fd27 100644 --- a/crates/oxc_mangler/src/lib.rs +++ b/crates/oxc_mangler/src/lib.rs @@ -338,60 +338,82 @@ impl<'t> Mangler<'t> { // // TODO: This is conservative, ideally, we would reserve names per-slot. let mut eval_reserved_names: FxHashSet<&str> = FxHashSet::default(); + + // Annex B.3.2.1: In sloppy mode, function declarations inside block scopes create + // an implicit `var`-like assignment in the enclosing function scope at block exit. + // If such a function gets the same mangled name as an outer `var`, the Annex B + // assignment overwrites it. To prevent this, hoist these functions to the enclosing + // var scope for slot assignment — the same way `var` is hoisted by oxc_semantic. + let mut annex_b_hoisted: FxHashMap> = FxHashMap::default(); + let mut annex_b_skip = BitSet::new_in(scoping.symbols_len(), temp_allocator); + for (scope_id, bindings) in scoping.iter_bindings() { + let flags = scoping.scope_flags(scope_id); + if flags.is_var() || flags.is_strict_mode() { + continue; + } + for &symbol_id in bindings.values() { + if !scoping.symbol_flags(symbol_id).is_function() { + continue; + } + for ancestor_id in scoping.scope_ancestors(scope_id).skip(1) { + if scoping.scope_flags(ancestor_id).is_var() { + annex_b_hoisted + .entry(ancestor_id.index()) + .or_insert_with(|| Vec::new_in(temp_allocator)) + .push(symbol_id); + annex_b_skip.set_bit(symbol_id.index()); + break; + } + } + } + } + // Walk down the scope tree and assign a slot number for each symbol. // It is possible to do this in a loop over the symbol list, // but walking down the scope tree seems to generate a better code. for (scope_id, bindings) in scoping.iter_bindings() { - if bindings.is_empty() { + if bindings.is_empty() && !annex_b_hoisted.contains_key(&scope_id.index()) { continue; } // Scopes with direct eval: collect binding names as reserved (they can be // accessed by eval at runtime) and skip slot assignment (keep original names). - let scope_flags = scoping.scope_flags(scope_id); - if scope_flags.contains_direct_eval() { + if scoping.scope_flags(scope_id).contains_direct_eval() { for (name, _) in bindings { eval_reserved_names.insert(name.as_str()); } + if let Some(hoisted) = annex_b_hoisted.get(&scope_id.index()) { + for &symbol_id in hoisted { + eval_reserved_names.insert(scoping.symbol_name(symbol_id)); + } + } continue; } - // Sort `bindings` in declaration order. + // Collect bindings in declaration order. + // Annex B functions are skipped here — they're processed with their var scope. tmp_bindings.clear(); - tmp_bindings.extend(bindings.values().copied().filter(|binding| { - !keep_name_symbols - .as_ref() - .is_some_and(|keep_name_symbols| keep_name_symbols.has_bit(binding.index())) - })); + tmp_bindings.extend( + bindings + .values() + .copied() + .filter(|id| !annex_b_skip.has_bit(id.index())) + .chain( + annex_b_hoisted + .get(&scope_id.index()) + .into_iter() + .flat_map(|v| v.iter().copied()), + ) + .filter(|binding| { + !keep_name_symbols.as_ref().is_some_and(|keep_name_symbols| { + keep_name_symbols.has_bit(binding.index()) + }) + }), + ); if tmp_bindings.is_empty() { continue; } tmp_bindings.sort_unstable(); - // Annex B.3.2.1: In sloppy mode, function declarations inside block scopes create - // an implicit `var`-like assignment in the enclosing function scope at block exit. - // If such a function gets the same mangled name as an outer `var`, the Annex B - // assignment overwrites it. Prevent this by giving them fresh slots (no reuse). - let is_sloppy_block = !scope_flags.is_var() && !scope_flags.is_strict_mode(); - let regular_count = if is_sloppy_block { - // Partition `tmp_bindings` into two regions: - // [regular bindings ... | function declarations ...] - // ^ regular_end (returned) - // For each non-function binding, swap it to the front of the array. - // Function declarations are left behind and end up at the tail. - // Only the first `regular_end` bindings will be eligible for slot reuse; - // the function declarations after that boundary always get fresh slots. - let mut regular_end = 0; - for i in 0..tmp_bindings.len() { - if !scoping.symbol_flags(tmp_bindings[i]).is_function() { - tmp_bindings.swap(regular_end, i); - regular_end += 1; - } - } - regular_end - } else { - tmp_bindings.len() - }; - let mut slot = slot_liveness.len(); reusable_slots.clear(); @@ -406,7 +428,7 @@ impl<'t> Mangler<'t> { #[expect(clippy::cast_possible_truncation)] |(slot, _)| slot as Slot, ) - .take(regular_count), // Annex B functions get fresh slots, not reused ones + .take(tmp_bindings.len()), ); // The number of new slots that needs to be allocated. diff --git a/crates/oxc_minifier/tests/mangler/mod.rs b/crates/oxc_minifier/tests/mangler/mod.rs index e4c52bddcd7aa..ae609bc40405f 100644 --- a/crates/oxc_minifier/tests/mangler/mod.rs +++ b/crates/oxc_minifier/tests/mangler/mod.rs @@ -250,6 +250,8 @@ fn annex_b_block_scoped_function() { "function _() { var x = 1; if (true) { function y() {} } try { throw 0; } catch (z) { use(z); } use(x); }", // Sibling var is hoisted to function scope, so no conflict "function _() { var x = 1; if (true) { function y() {} } { var z = 2; use(z); } use(x); }", + // Annex B function reuses name from sibling function scope (hoisting enables this) + "function _() { function foo() { var x; use(x); } function bar() { if (true) { function baz() {} use(baz); } } }", ]; let mut snapshot = String::new(); diff --git a/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap b/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap index 8faf5c34a34c3..b7a63a67c79f8 100644 --- a/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap +++ b/crates/oxc_minifier/tests/mangler/snapshots/annex_b_block_scoped_function.snap @@ -126,11 +126,25 @@ function _() { var x = 1; if (true) { function y() {} } { var z = 2; use(z); } u function _() { var e = 1; if (true) { - function n() {} + function t() {} } { - var t = 2; - use(t); + var n = 2; + use(n); } use(e); } + +function _() { function foo() { var x; use(x); } function bar() { if (true) { function baz() {} use(baz); } } } +function _() { + function e() { + var e; + use(e); + } + function t() { + if (true) { + function e() {} + use(e); + } + } +}