Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 83 additions & 24 deletions src/ast/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,22 +782,51 @@ impl Expr {
// `ConstParamTy` (Op.rs owns the enum); pass at runtime here. Revisit once
// `Code` gains `ConstParamTy` — call sites are a handful of literal ops.
pub fn join_with_left_associative_op(op: Op::Code, a: Expr, b: Expr) -> Expr {
// "(a, b) op c" => "a, b op c"
if let Data::EBinary(mut comma) = a.data {
if comma.op == crate::OpCode::BinComma {
comma.right = Self::join_with_left_associative_op(op, comma.right, b);
// Re-associating recurses once per level of same-op nesting in `b`;
// such chains are built iteratively and can be deeper than the call
// stack allows, so the recursion carries a stack check and falls back
// to the unflattened `a op b` shape when it is nearly exhausted.
Self::join_with_left_associative_op_with_check(op, a, b, bun_core::StackCheck::init())
}

fn join_with_left_associative_op_with_check(
op: Op::Code,
a: Expr,
b: Expr,
stack_check: bun_core::StackCheck,
) -> Expr {
if stack_check.is_safe_to_recurse() {
// "(a, b) op c" => "a, b op c"
if let Data::EBinary(mut comma) = a.data {
if comma.op == crate::OpCode::BinComma {
comma.right = Self::join_with_left_associative_op_with_check(
op,
comma.right,
b,
stack_check,
);
// `a` now ends with `… op b`; falling through would build
// `a op b` on top of it and duplicate `b`'s side effects.
return a;
}
}
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}

// "a op (b op c)" => "(a op b) op c"
// "a op (b op (c op d))" => "((a op b) op c) op d"
if let Data::EBinary(binary) = b.data {
if binary.op == op {
return Self::join_with_left_associative_op(
op,
Self::join_with_left_associative_op(op, a, binary.left),
binary.right,
);
// "a op (b op c)" => "(a op b) op c"
// "a op (b op (c op d))" => "((a op b) op c) op d"
if let Data::EBinary(binary) = b.data {
if binary.op == op {
return Self::join_with_left_associative_op_with_check(
op,
Self::join_with_left_associative_op_with_check(
op,
a,
binary.left,
stack_check,
),
binary.right,
stack_check,
);
}
}
}

Expand Down Expand Up @@ -2845,6 +2874,17 @@ impl Data {
}

pub fn known_primitive(&self) -> PrimitiveType {
// Recurses through `??`/`&&`/`||`/`+` operands, ternaries, and unary
// values. Those chains are built and visited iteratively, so they can
// be deeper than the call stack allows; answer `Unknown` (skip the
// optimization) instead of overflowing.
self.known_primitive_with_check(bun_core::StackCheck::init())
}

fn known_primitive_with_check(&self, stack_check: bun_core::StackCheck) -> PrimitiveType {
if !stack_check.is_safe_to_recurse() {
return PrimitiveType::Unknown;
}
match self {
Data::EBigInt(_) => PrimitiveType::Bigint,
Data::EBoolean(_) | Data::EBranchBoolean(_) => PrimitiveType::Boolean,
Expand All @@ -2859,7 +2899,10 @@ impl Data {
PrimitiveType::Unknown
}
}
Data::EIf(e_if) => e_if.yes.data.merge_known_primitive(&e_if.no.data),
Data::EIf(e_if) => e_if
.yes
.data
.merge_known_primitive_with_check(&e_if.no.data, stack_check),
Data::EBinary(binary) => 'brk: {
match binary.op {
crate::OpCode::BinStrictEq
Expand All @@ -2873,12 +2916,15 @@ impl Data {
| crate::OpCode::BinInstanceof
| crate::OpCode::BinIn => break 'brk PrimitiveType::Boolean,
crate::OpCode::BinLogicalOr | crate::OpCode::BinLogicalAnd => {
break 'brk binary.left.data.merge_known_primitive(&binary.right.data);
break 'brk binary
.left
.data
.merge_known_primitive_with_check(&binary.right.data, stack_check);
}

crate::OpCode::BinNullishCoalescing => {
let left = binary.left.data.known_primitive();
let right = binary.right.data.known_primitive();
let left = binary.left.data.known_primitive_with_check(stack_check);
let right = binary.right.data.known_primitive_with_check(stack_check);
if left == PrimitiveType::Null || left == PrimitiveType::Undefined {
break 'brk right;
}
Expand All @@ -2895,8 +2941,8 @@ impl Data {
}

crate::OpCode::BinAdd => {
let left = binary.left.data.known_primitive();
let right = binary.right.data.known_primitive();
let left = binary.left.data.known_primitive_with_check(stack_check);
let right = binary.right.data.known_primitive_with_check(stack_check);

if left == PrimitiveType::String || right == PrimitiveType::String {
break 'brk PrimitiveType::String;
Expand Down Expand Up @@ -2945,7 +2991,7 @@ impl Data {
| crate::OpCode::BinUShrAssign => break 'brk PrimitiveType::Mixed, // Can be number or bigint (or an exception)

crate::OpCode::BinAssign | crate::OpCode::BinComma => {
break 'brk binary.right.data.known_primitive();
break 'brk binary.right.data.known_primitive_with_check(stack_check);
}

_ => {}
Expand All @@ -2960,7 +3006,7 @@ impl Data {
crate::OpCode::UnNot | crate::OpCode::UnDelete => PrimitiveType::Boolean,
crate::OpCode::UnPos => PrimitiveType::Number, // Cannot be bigint because that throws an exception
crate::OpCode::UnNeg | crate::OpCode::UnCpl => {
match unary.value.data.known_primitive() {
match unary.value.data.known_primitive_with_check(stack_check) {
PrimitiveType::Bigint => PrimitiveType::Bigint,
PrimitiveType::Unknown | PrimitiveType::Mixed => PrimitiveType::Mixed,
_ => PrimitiveType::Number, // Can be number or bigint
Expand All @@ -2974,14 +3020,27 @@ impl Data {
_ => PrimitiveType::Unknown,
},

Data::EInlinedEnum(inlined) => inlined.value.data.known_primitive(),
Data::EInlinedEnum(inlined) => {
inlined.value.data.known_primitive_with_check(stack_check)
}

_ => PrimitiveType::Unknown,
}
}

pub fn merge_known_primitive(&self, rhs: &Data) -> PrimitiveType {
PrimitiveType::merge(self.known_primitive(), rhs.known_primitive())
self.merge_known_primitive_with_check(rhs, bun_core::StackCheck::init())
}

fn merge_known_primitive_with_check(
&self,
rhs: &Data,
stack_check: bun_core::StackCheck,
) -> PrimitiveType {
PrimitiveType::merge(
self.known_primitive_with_check(stack_check),
rhs.known_primitive_with_check(stack_check),
)
}

/// Returns true if the result of the "typeof" operator on this expression is
Expand Down
50 changes: 50 additions & 0 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,12 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> {
/// hard-overflow. Same `MAX_STMT_DEPTH` rationale as `interchange/json.rs`.
pub parse_stmt_depth: u32,

/// Set once `report_stack_overflow` has logged "Maximum call stack size
/// exceeded" so sibling subtrees that hit the same stack limit don't each
/// log a duplicate error. `Cell` because some reporters (`SideEffects::
/// to_boolean`) only hold `&P`.
pub reported_stack_overflow: core::cell::Cell<bool>,

/// When this flag is enabled, we attempt to fold all expressions that
/// TypeScript would consider to be "constant expressions". This flag is
/// enabled inside each enum body block since TypeScript requires numeric
Expand Down Expand Up @@ -776,6 +782,19 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
unsafe { &mut *self.log.as_ptr() }
}

/// Logs "Maximum call stack size exceeded" (once per parse) when a pass
/// that recurses over the AST runs out of stack. `_parse` halts with a
/// SyntaxError as soon as the log has errors, so callers only need to stop
/// recursing after calling this.
pub fn report_stack_overflow(&self, loc: bun_ast::Loc) {
if self.reported_stack_overflow.get() {
return;
}
self.reported_stack_overflow.set(true);
self.log()
.add_error(Some(self.source), loc, b"Maximum call stack size exceeded");
}

/// Safe mutable projection of `nearest_stmt_list`.
///
/// The pointer targets a `ListManaged` living on a parent
Expand Down Expand Up @@ -2518,6 +2537,13 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
replacement: Expr,
replacement_can_be_removed: bool,
) -> Substitution {
// Recurses into the left side of every binary operator (among others),
// and those chains are built iteratively, so they can be deeper than
// the call stack allows; report the stack overflow instead of crashing.
if !self.stack_check.is_safe_to_recurse() {
self.report_stack_overflow(expr.loc);
return Substitution::Failure(expr);
}
Comment thread
robobun marked this conversation as resolved.
Outdated
// Zig matched on `expr.data` (a tagged union of `*E.*`) and mutated through
// the captured pointer. `ExprData` is `Copy`; matching by value yields owned
// `StoreRef<E::*>` copies whose `DerefMut` writes to the same arena slot,
Expand Down Expand Up @@ -3582,6 +3608,20 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
kind: js_ast::scope::Kind,
loc: bun_ast::Loc,
) -> Result<(), bun_core::Error> {
// After a stack-overflow bail the visit pass skips subtrees, so the
// scope entries the parse pass recorded for them are never consumed.
// Entries are in source order; re-sync by skipping ahead to the entry
// this push is asking for so the sanity check below doesn't panic on
// an already-failing parse.
if self.reported_stack_overflow.get() {
while let Some((head, rest)) = self.scope_order_to_visit.split_first() {
if head.loc.start == loc.start && head.scope_ref().kind == kind {
break;
}
self.scope_order_to_visit = rest;
}
}

let order = self.next_scope_in_order_for_visit_pass();

// Sanity-check that the scopes generated by the first and second passes match
Expand Down Expand Up @@ -5845,6 +5885,15 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
}

fn expr_can_be_removed_if_unused_without_dce_check(&mut self, expr: &Expr) -> bool {
// This walks arbitrarily deep ASTs; report the stack overflow instead
// of crashing. The parse fails with that error, so the answer is moot.
// Once the visit pass has bailed out, parts of the AST were never
// visited (identifier refs are still parse-time slices), so skip the
// analysis entirely rather than reading them.
if !self.stack_check.is_safe_to_recurse() || self.reported_stack_overflow.get() {
self.report_stack_overflow(expr.loc);
return false;
}
match &expr.data {
js_ast::ExprData::ENull(_)
| js_ast::ExprData::EUndefined(_)
Expand Down Expand Up @@ -9203,6 +9252,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
log,
stack_check: bun_core::StackCheck::init(),
parse_stmt_depth: 0,
reported_stack_overflow: core::cell::Cell::new(false),
arena,
then_catch_chain: ThenCatchChain {
next_target: null_expr_data(),
Expand Down
5 changes: 5 additions & 0 deletions src/js_parser/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O

pub fn parse_binding(&mut self, opts: ParseBindingOptions) -> Result<Binding, Error> {
let p = self;
// Destructuring patterns nest (`[[[x]]]`, `{a:{b:{…}}}`) without going
// through `parse_expr_common`, so they need the same stack check.
if !p.stack_check.is_safe_to_recurse() {
return Err(err!("StackOverflow"));
}
let loc = p.lexer.loc();

match p.lexer.token {
Expand Down
49 changes: 41 additions & 8 deletions src/js_parser/scan/scan_side_effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ impl SideEffects {
if !p.options.features.dead_code_elimination {
return Some(expr);
}
// This walks arbitrarily deep ASTs; report the stack overflow instead
// of crashing. The parse fails with that error, so the value returned
// here never reaches the printer. Once the visit pass has bailed out,
// parts of the AST were never visited (identifier refs are still
// parse-time slices), so skip the analysis entirely rather than
// reading them.
if !p.stack_check.is_safe_to_recurse() || p.reported_stack_overflow.get() {
p.report_stack_overflow(expr.loc);
return Some(expr);
Comment thread
robobun marked this conversation as resolved.
}
// PORT NOTE: `Expr`/`ExprData`/`StoreRef<_>` are all `Copy`. We match on
// `expr.data` *by value* so `expr` itself is never borrowed across a
// recursive `simplify_unused_expr(p, ..)` call. Mutations to boxed
Expand Down Expand Up @@ -304,8 +314,12 @@ impl SideEffects {
| Op::Code::BinGt
| Op::Code::BinLe
| Op::Code::BinGe => {
if Self::is_primitive_with_side_effects(&bin.left.data)
&& Self::is_primitive_with_side_effects(&bin.right.data)
if Self::is_primitive_with_side_effects(p, bin.left.loc, &bin.left.data)
&& Self::is_primitive_with_side_effects(
p,
bin.right.loc,
&bin.right.data,
)
{
let left = bin.left;
let right = bin.right;
Expand Down Expand Up @@ -728,7 +742,18 @@ impl SideEffects {
}
}

pub fn is_primitive_with_side_effects(data: &ExprData) -> bool {
pub fn is_primitive_with_side_effects<'a, const TS: bool, const SCAN: bool>(
p: &P<'a, TS, SCAN>,
loc: bun_ast::Loc,
data: &ExprData,
) -> bool {
// Recurses through `&&`/`||`/`??` operands and ternary branches, which
// are built and visited iteratively and so can be deeper than the call
// stack allows; report the stack overflow instead of crashing.
if !p.stack_check.is_safe_to_recurse() {
p.report_stack_overflow(loc);
return false;
}
Comment thread
robobun marked this conversation as resolved.
match data {
ExprData::ENull(_)
| ExprData::EUndefined(_)
Expand Down Expand Up @@ -773,17 +798,17 @@ impl SideEffects {
Op::Code::BinLogicalAnd | Op::Code::BinLogicalOr | Op::Code::BinNullishCoalescing
| Op::Code::BinLogicalAndAssign | Op::Code::BinLogicalOrAssign
| Op::Code::BinNullishCoalescingAssign => {
Self::is_primitive_with_side_effects(&e.left.data)
&& Self::is_primitive_with_side_effects(&e.right.data)
Self::is_primitive_with_side_effects(p, e.left.loc, &e.left.data)
&& Self::is_primitive_with_side_effects(p, e.right.loc, &e.right.data)
}
Op::Code::BinComma => {
Self::is_primitive_with_side_effects(&e.right.data)
Self::is_primitive_with_side_effects(p, e.right.loc, &e.right.data)
}
_ => false,
},
ExprData::EIf(e) => {
Self::is_primitive_with_side_effects(&e.yes.data)
&& Self::is_primitive_with_side_effects(&e.no.data)
Self::is_primitive_with_side_effects(p, e.yes.loc, &e.yes.data)
&& Self::is_primitive_with_side_effects(p, e.no.loc, &e.no.data)
}
_ => false,
}
Expand Down Expand Up @@ -883,6 +908,14 @@ impl SideEffects {
if !p.options.features.dead_code_elimination {
return Result::default();
}
// Recurses through nested unary/binary operands; report the stack
// overflow instead of crashing. The parse fails with that error, so
// the "unknown" result returned here never affects emitted code.
// (`ExprData` carries no location, so report with an empty one.)
if !p.stack_check.is_safe_to_recurse() {
p.report_stack_overflow(bun_ast::Loc::EMPTY);
return Result::default();
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}
match exp {
ExprData::ENull(_) | ExprData::EUndefined(_) => Result {
value: false,
Expand Down
6 changes: 6 additions & 0 deletions src/js_parser/visit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,12 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
binding: BindingNodeIndex,
mut duplicate_arg_check: Option<&mut StringVoidMap>,
) {
// Nested destructuring patterns recurse here with larger frames than
// `parse_binding`'s, so re-check the stack like `visit_expr_in_out`.
if !self.stack_check.is_safe_to_recurse() {
self.report_stack_overflow(binding.loc);
return;
}
match binding.data {
BData::BMissing(_) => {}
BData::BIdentifier(bind) => {
Expand Down
Loading
Loading