From b2bf3f0dcf03897e131c9cc46c9b52a923bd4c09 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 2 Dec 2023 12:46:25 +0100 Subject: [PATCH] Handle stack overflow in binary expressions in JS Parser (#7414) * Fix stack overflow in large files * Add test for stack overflow * wip * Disable cache in debug build * Remove our extra `captureStackTrace` call * Update RuntimeTranspilerCache.zig * Update RuntimeTranspilerCache.zig * Fix issues with integer environment variables * Add missing ref * Add missing null check * Update bindings.cpp * Update transpiler-cache.test.ts * Add version check --------- Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> --- .github/workflows/bun-linux-build.yml | 11 +- .github/workflows/bun-mac-aarch64.yml | 6 +- .github/workflows/bun-mac-x64-baseline.yml | 6 +- .github/workflows/bun-mac-x64.yml | 6 +- src/bun.js/RuntimeTranspilerCache.zig | 46 +- src/bun.js/bindings/CommonJSModuleRecord.cpp | 7 +- .../bindings/JSEnvironmentVariableMap.cpp | 21 +- src/bun.js/bindings/ZigGlobalObject.cpp | 13 +- src/bun.js/bindings/bindings.cpp | 4 +- src/bun.js/node/types.zig | 2 +- src/js_parser.zig | 799 ++++++++++-------- src/js_printer.zig | 348 +++++--- test/cli/run/transpiler-cache.test.ts | 5 +- .../default-pages-dir/test/dev-server.test.ts | 1 + test/js/node/v8/capture-stack-trace.test.js | 6 +- .../transpiler-stack-overflow.test.ts | 25 + 16 files changed, 815 insertions(+), 491 deletions(-) create mode 100644 test/transpiler/transpiler-stack-overflow.test.ts diff --git a/.github/workflows/bun-linux-build.yml b/.github/workflows/bun-linux-build.yml index 53803ed5f9..9fa7cfc670 100644 --- a/.github/workflows/bun-linux-build.yml +++ b/.github/workflows/bun-linux-build.yml @@ -121,6 +121,11 @@ jobs: platforms: linux/${{matrix.build_arch}} target: ${{matrix.target}} outputs: type=local,dest=${{runner.temp}}/release + - id: bun-version-check + name: Bun version check + run: | + # If this hangs, it means something is seriously wrong with the build + ${{runner.temp}}/release/bun-profile --version - name: Zip run: | # if zip is not found @@ -237,7 +242,11 @@ jobs: cd bun-${{matrix.tag}} chmod +x bun pwd >> $GITHUB_PATH - ./bun --version + - id: bun-version-check + name: Bun version check + run: | + # If this hangs, it means something is seriously wrong with the build + bun --version - id: install-dependnecies name: Install dependencies run: | diff --git a/.github/workflows/bun-mac-aarch64.yml b/.github/workflows/bun-mac-aarch64.yml index 52ceb4f532..ded636328c 100644 --- a/.github/workflows/bun-mac-aarch64.yml +++ b/.github/workflows/bun-mac-aarch64.yml @@ -393,7 +393,11 @@ jobs: cd ${{matrix.tag}} chmod +x bun pwd >> $GITHUB_PATH - ./bun --version + - id: bun-version-check + name: Bun version check + run: | + # If this hangs, it means something is seriously wrong with the build + bun --version - id: install name: Install dependencies run: | diff --git a/.github/workflows/bun-mac-x64-baseline.yml b/.github/workflows/bun-mac-x64-baseline.yml index 61e6ea7f86..53f5be8eef 100644 --- a/.github/workflows/bun-mac-x64-baseline.yml +++ b/.github/workflows/bun-mac-x64-baseline.yml @@ -408,7 +408,11 @@ jobs: cd ${{matrix.tag}} chmod +x bun pwd >> $GITHUB_PATH - ./bun --version + - id: bun-version-check + name: Bun version check + run: | + # If this hangs, it means something is seriously wrong with the build + bun --version - id: install name: Install dependencies run: | diff --git a/.github/workflows/bun-mac-x64.yml b/.github/workflows/bun-mac-x64.yml index 7210b9596c..b48ed3b524 100644 --- a/.github/workflows/bun-mac-x64.yml +++ b/.github/workflows/bun-mac-x64.yml @@ -407,7 +407,11 @@ jobs: cd ${{matrix.tag}} chmod +x bun pwd >> $GITHUB_PATH - ./bun --version + - id: bun-version-check + name: Bun version check + run: | + # If this hangs, it means something is seriously wrong with the build + bun --version - id: install name: Install dependencies run: | diff --git a/src/bun.js/RuntimeTranspilerCache.zig b/src/bun.js/RuntimeTranspilerCache.zig index cee2ec7c33..d796ad583c 100644 --- a/src/bun.js/RuntimeTranspilerCache.zig +++ b/src/bun.js/RuntimeTranspilerCache.zig @@ -1,12 +1,17 @@ +// ** Update the version number when any breaking changes are made to the cache format or to the JS parser ** +const expected_version = 1; + const bun = @import("root").bun; const std = @import("std"); const Output = bun.Output; const JSC = bun.JSC; -const expected_version = 1; const debug = Output.scoped(.cache, false); const MINIMUM_CACHE_SIZE = 50 * 1024; +// When making parser changes, it gets extremely confusing. +var bun_debug_restore_from_cache = false; + pub const RuntimeTranspilerCache = struct { input_hash: ?u64 = null, input_byte_length: ?u64 = null, @@ -111,6 +116,15 @@ pub const RuntimeTranspilerCache = struct { utf8: []const u8, string: bun.String, + pub fn deinit(this: *OutputCode, allocator: std.mem.Allocator) void { + switch (this.*) { + .utf8 => { + allocator.free(this.utf8); + }, + .string => this.string.deref(), + } + } + pub fn byteSlice(this: *const OutputCode) []const u8 { switch (this.*) { .utf8 => return this.utf8, @@ -119,6 +133,13 @@ pub const RuntimeTranspilerCache = struct { } }; + pub fn deinit(this: *Entry, sourcemap_allocator: std.mem.Allocator, output_code_allocator: std.mem.Allocator) void { + this.output_code.deinit(output_code_allocator); + if (this.sourcemap.len > 0) { + sourcemap_allocator.free(this.sourcemap); + } + } + pub fn save( destination_dir: bun.FileDescriptor, destination_path: bun.PathString, @@ -352,6 +373,10 @@ pub const RuntimeTranspilerCache = struct { fn reallyGetCacheDir( buf: *[bun.MAX_PATH_BYTES]u8, ) [:0]const u8 { + if (comptime bun.Environment.allow_assert) { + bun_debug_restore_from_cache = bun.getenvZ("BUN_DEBUG_ENABLE_RESTORE_FROM_TRANSPILER_CACHE") != null; + } + if (bun.getenvZ("BUN_RUNTIME_TRANSPILER_CACHE_PATH")) |dir| { if (dir.len == 0 or (dir.len == 1 and dir[0] == '0')) { return ""; @@ -573,11 +598,24 @@ pub const RuntimeTranspilerCache = struct { debug("get(\"{s}\") = {s}", .{ source.path.text, @errorName(err) }); return false; }; - if (comptime bun.Environment.allow_assert) - debug("get(\"{s}\") = {d} bytes", .{ source.path.text, this.entry.?.output_code.byteSlice().len }); - + if (comptime bun.Environment.allow_assert) { + if (bun_debug_restore_from_cache) { + debug("get(\"{s}\") = {d} bytes, restored", .{ source.path.text, this.entry.?.output_code.byteSlice().len }); + } else { + debug("get(\"{s}\") = {d} bytes, ignored for debug build", .{ source.path.text, this.entry.?.output_code.byteSlice().len }); + } + } bun.Analytics.Features.transpiler_cache = true; + if (comptime bun.Environment.allow_assert) { + if (!bun_debug_restore_from_cache) { + if (this.entry) |*entry| { + entry.deinit(this.sourcemap_allocator, this.output_code_allocator); + this.entry = null; + } + } + } + return this.entry != null; } diff --git a/src/bun.js/bindings/CommonJSModuleRecord.cpp b/src/bun.js/bindings/CommonJSModuleRecord.cpp index bc89a06eb5..39356e4519 100644 --- a/src/bun.js/bindings/CommonJSModuleRecord.cpp +++ b/src/bun.js/bindings/CommonJSModuleRecord.cpp @@ -117,13 +117,16 @@ static bool evaluateCommonJSModuleOnce(JSC::VM& vm, Zig::GlobalObject* globalObj moduleObject->hasEvaluated = true; - JSFunction* fn = jsCast(JSC::evaluate(globalObject, code->sourceCode(), jsUndefined(), exception)); + // This will return 0 if there was a syntax error or an allocation failure + JSValue fnValue = JSC::evaluate(globalObject, code->sourceCode(), jsUndefined(), exception); - if (exception.get()) { + if (UNLIKELY(exception.get() || fnValue.isEmpty())) { moduleObject->sourceCode.clear(); return false; } + JSFunction* fn = jsCast(fnValue); + JSC::CallData callData = JSC::getCallData(fn); MarkedArgumentBuffer args; args.append(moduleObject->exportsObject()); // exports diff --git a/src/bun.js/bindings/JSEnvironmentVariableMap.cpp b/src/bun.js/bindings/JSEnvironmentVariableMap.cpp index abf794786c..b23eac87d0 100644 --- a/src/bun.js/bindings/JSEnvironmentVariableMap.cpp +++ b/src/bun.js/bindings/JSEnvironmentVariableMap.cpp @@ -135,7 +135,26 @@ JSValue createEnvironmentVariablesMap(Zig::GlobalObject* globalObject) hasTZ = true; continue; } - object->putDirectCustomAccessor(vm, Identifier::fromString(vm, name), JSC::CustomGetterSetter::create(vm, jsGetterEnvironmentVariable, jsSetterEnvironmentVariable), JSC::PropertyAttribute::CustomAccessor | 0); + ASSERT(len > 0); + + Identifier identifier = Identifier::fromString(vm, name); + + // CustomGetterSetter doesn't support indexed properties yet. + // This causes strange issues when the environment variable name is an integer. + if (UNLIKELY(chars[0] >= '0' && chars[0] <= '9')) { + if (auto index = parseIndex(identifier)) { + ZigString valueString = { nullptr, 0 }; + ZigString nameStr = toZigString(name); + JSValue value = jsUndefined(); + if (Bun__getEnvValue(globalObject, &nameStr, &valueString)) { + value = jsString(vm, Zig::toStringCopy(valueString)); + } + object->putDirectIndex(globalObject, *index, value, 0, PutDirectIndexLikePutDirect); + continue; + } + } + + object->putDirectCustomAccessor(vm, identifier, JSC::CustomGetterSetter::create(vm, jsGetterEnvironmentVariable, jsSetterEnvironmentVariable), JSC::PropertyAttribute::CustomAccessor | 0); } unsigned int TZAttrs = JSC::PropertyAttribute::CustomAccessor | 0; diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 9323583577..511d7ea6a8 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -341,7 +341,7 @@ WTF::String Bun::formatStackTrace(JSC::VM& vm, JSC::JSGlobalObject* globalObject bool hasSet = false; - if(errorInstance) { + if (errorInstance) { if (JSC::ErrorInstance* err = jsDynamicCast(errorInstance)) { if (err->errorType() == ErrorType::SyntaxError && (stackTrace.isEmpty() || stackTrace.at(0).sourceURL(vm) != err->sourceURL())) { // There appears to be an off-by-one error. @@ -2848,17 +2848,6 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb globalObject->formatStackTrace(vm, lexicalGlobalObject, errorObject, callSites, JSC::JSValue()); RETURN_IF_EXCEPTION(scope, JSC::JSValue::encode({})); - if (auto* instance = jsDynamicCast(errorObject)) { - // we make a separate copy of the StackTrace unfortunately so that we - // can later console.log it without losing the info - // - // This is not good. We should remove this in the future as it strictly makes this function - // already slower than necessary. - instance->captureStackTrace(vm, globalObject, 1, false); - } - - RETURN_IF_EXCEPTION(scope, JSC::JSValue::encode(JSValue {})); - return JSC::JSValue::encode(JSC::jsUndefined()); } diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp index 7fffdce237..25bd7671cb 100644 --- a/src/bun.js/bindings/bindings.cpp +++ b/src/bun.js/bindings/bindings.cpp @@ -1502,7 +1502,7 @@ BunString WebCore__DOMURL__fileSystemPath(WebCore__DOMURL* arg0) { const WTF::URL& url = arg0->href(); if (url.protocolIsFile()) { - return Bun::toString(url.fileSystemPath()); + return Bun::toStringRef(url.fileSystemPath()); } return BunStringEmpty; @@ -1812,6 +1812,8 @@ bool JSC__JSFunction__getSourceCode(JSC__JSValue JSValue0, ZigString* outSourceC } return false; } + + return false; } void JSC__JSValue__jsonStringify(JSC__JSValue JSValue0, JSC__JSGlobalObject* arg1, uint32_t arg2, diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig index c5138e0945..1bdb74c878 100644 --- a/src/bun.js/node/types.zig +++ b/src/bun.js/node/types.zig @@ -869,6 +869,7 @@ pub const PathLike = union(Tag) { else => { if (arg.as(JSC.DOMURL)) |domurl| { const path_str: bun.String = domurl.fileSystemPath(); + defer path_str.deref(); if (path_str.isEmpty()) { JSC.throwInvalidArguments("URL must be a non-empty \"file:\" path", .{}, ctx, exception); return null; @@ -876,7 +877,6 @@ pub const PathLike = union(Tag) { arguments.eat(); if (!Valid.pathStringLength(path_str.length(), ctx, exception)) { - defer path_str.deref(); return null; } diff --git a/src/js_parser.zig b/src/js_parser.zig index 9744be7dc4..57e93e5d59 100644 --- a/src/js_parser.zig +++ b/src/js_parser.zig @@ -1,3 +1,7 @@ +/// ** IMPORTANT ** +/// ** When making changes to the JavaScript Parser that impact runtime behavior or fix bugs ** +/// ** you must also increment the `expected_version` in RuntimeTranspilerCache.zig ** +/// ** IMPORTANT ** pub const std = @import("std"); pub const logger = @import("root").bun.logger; pub const js_lexer = bun.js_lexer; @@ -3076,8 +3080,10 @@ pub const Parser = struct { try ParserType.init(self.allocator, self.log, self.source, self.define, self.lexer, self.options, &p); p.should_fold_typescript_constant_expressions = self.options.features.should_fold_typescript_constant_expressions; defer p.lexer.deinit(); - var result: js_ast.Result = undefined; - _ = result; + + var binary_expression_stack_heap = std.heap.stackFallback(1024, bun.default_allocator); + p.binary_expression_stack = std.ArrayList(ParserType.BinaryExpressionVisitor).init(binary_expression_stack_heap.get()); + defer p.binary_expression_stack.clearAndFree(); // defer { // if (p.allocated_names_pool) |pool| { @@ -4854,6 +4860,8 @@ fn NewParser_( const_values: js_ast.Ast.ConstValuesMap = .{}, + binary_expression_stack: std.ArrayList(BinaryExpressionVisitor) = undefined, + pub fn transposeImport(p: *P, arg: Expr, state: anytype) Expr { // The argument must be a string if (@as(Expr.Tag, arg.data) == .e_string) { @@ -6879,6 +6887,393 @@ fn NewParser_( return ExprBindingTuple{ .binding = bind, .expr = initializer }; } + const BinaryExpressionVisitor = struct { + e: *E.Binary, + loc: logger.Loc, + in: ExprIn, + + /// Input for visiting the left child + left_in: ExprIn, + + /// "Local variables" passed from "checkAndPrepare" to "visitRightAndFinish" + is_stmt_expr: bool = false, + + pub fn visitRightAndFinish( + v: *BinaryExpressionVisitor, + p: *P, + ) Expr { + var e_ = v.e; + const is_call_target = @as(Expr.Tag, p.call_target) == .e_binary and e_ == p.call_target.e_binary; + // const is_stmt_expr = @as(Expr.Tag, p.stmt_expr_value) == .e_binary and expr.data.e_binary == p.stmt_expr_value.e_binary; + const was_anonymous_named_expr = e_.right.isAnonymousNamed(); + + // Mark the control flow as dead if the branch is never taken + switch (e_.op) { + .bin_logical_or => { + const side_effects = SideEffects.toBoolean(p, e_.left.data); + if (side_effects.ok and side_effects.value) { + // "true || dead" + const old = p.is_control_flow_dead; + p.is_control_flow_dead = true; + e_.right = p.visitExpr(e_.right); + p.is_control_flow_dead = old; + } else { + e_.right = p.visitExpr(e_.right); + } + }, + .bin_logical_and => { + const side_effects = SideEffects.toBoolean(p, e_.left.data); + if (side_effects.ok and !side_effects.value) { + // "false && dead" + const old = p.is_control_flow_dead; + p.is_control_flow_dead = true; + e_.right = p.visitExpr(e_.right); + p.is_control_flow_dead = old; + } else { + e_.right = p.visitExpr(e_.right); + } + }, + .bin_nullish_coalescing => { + const side_effects = SideEffects.toNullOrUndefined(p, e_.left.data); + if (side_effects.ok and !side_effects.value) { + // "notNullOrUndefined ?? dead" + const old = p.is_control_flow_dead; + p.is_control_flow_dead = true; + e_.right = p.visitExpr(e_.right); + p.is_control_flow_dead = old; + } else { + e_.right = p.visitExpr(e_.right); + } + }, + else => { + e_.right = p.visitExpr(e_.right); + }, + } + + // Always put constants on the right for equality comparisons to help + // reduce the number of cases we have to check during pattern matching. We + // can only reorder expressions that do not have any side effects. + switch (e_.op) { + .bin_loose_eq, .bin_loose_ne, .bin_strict_eq, .bin_strict_ne => { + if (SideEffects.isPrimitiveToReorder(e_.left.data) and !SideEffects.isPrimitiveToReorder(e_.right.data)) { + const _left = e_.left; + const _right = e_.right; + e_.left = _right; + e_.right = _left; + } + }, + else => {}, + } + + switch (e_.op) { + .bin_comma => { + // "(1, 2)" => "2" + // "(sideEffects(), 2)" => "(sideEffects(), 2)" + if (p.options.features.minify_syntax) { + e_.left = SideEffects.simpifyUnusedExpr(p, e_.left) orelse return e_.right; + } + }, + .bin_loose_eq => { + const equality = e_.left.data.eql(e_.right.data, p.allocator, .loose); + if (equality.ok) { + return p.newExpr( + E.Boolean{ .value = equality.equal }, + v.loc, + ); + } + + if (p.options.features.minify_syntax) { + // "x == void 0" => "x == null" + if (e_.left.data == .e_undefined) { + e_.left.data = .{ .e_null = E.Null{} }; + } else if (e_.right.data == .e_undefined) { + e_.right.data = .{ .e_null = E.Null{} }; + } + } + + // const after_op_loc = locAfterOp(e_.); + // TODO: warn about equality check + // TODO: warn about typeof string + + }, + .bin_strict_eq => { + const equality = e_.left.data.eql(e_.right.data, p.allocator, .strict); + if (equality.ok) { + return p.newExpr(E.Boolean{ .value = equality.equal }, v.loc); + } + + // const after_op_loc = locAfterOp(e_.); + // TODO: warn about equality check + // TODO: warn about typeof string + }, + .bin_loose_ne => { + const equality = e_.left.data.eql(e_.right.data, p.allocator, .loose); + if (equality.ok) { + return p.newExpr(E.Boolean{ .value = !equality.equal }, v.loc); + } + // const after_op_loc = locAfterOp(e_.); + // TODO: warn about equality check + // TODO: warn about typeof string + + // "x != void 0" => "x != null" + if (@as(Expr.Tag, e_.right.data) == .e_undefined) { + e_.right = p.newExpr(E.Null{}, e_.right.loc); + } + }, + .bin_strict_ne => { + const equality = e_.left.data.eql(e_.right.data, p.allocator, .strict); + if (equality.ok) { + return p.newExpr(E.Boolean{ .value = !equality.equal }, v.loc); + } + }, + .bin_nullish_coalescing => { + const nullorUndefined = SideEffects.toNullOrUndefined(p, e_.left.data); + if (nullorUndefined.ok) { + if (!nullorUndefined.value) { + return e_.left; + } else if (nullorUndefined.side_effects == .no_side_effects) { + // "(null ?? fn)()" => "fn()" + // "(null ?? this.fn)" => "this.fn" + // "(null ?? this.fn)()" => "(0, this.fn)()" + if (is_call_target and e_.right.hasValueForThisInCall()) { + return Expr.joinWithComma(Expr{ .data = .{ .e_number = .{ .value = 0.0 } }, .loc = e_.left.loc }, e_.right, p.allocator); + } + + return e_.right; + } + } + }, + .bin_logical_or => { + const side_effects = SideEffects.toBoolean(p, e_.left.data); + if (side_effects.ok and side_effects.value) { + return e_.left; + } else if (side_effects.ok and side_effects.side_effects == .no_side_effects) { + // "(0 || fn)()" => "fn()" + // "(0 || this.fn)" => "this.fn" + // "(0 || this.fn)()" => "(0, this.fn)()" + if (is_call_target and e_.right.hasValueForThisInCall()) { + return Expr.joinWithComma(Expr{ .data = Prefill.Data.Zero, .loc = e_.left.loc }, e_.right, p.allocator); + } + + return e_.right; + } + }, + .bin_logical_and => { + const side_effects = SideEffects.toBoolean(p, e_.left.data); + if (side_effects.ok) { + if (!side_effects.value) { + return e_.left; + } else if (side_effects.side_effects == .no_side_effects) { + // "(1 && fn)()" => "fn()" + // "(1 && this.fn)" => "this.fn" + // "(1 && this.fn)()" => "(0, this.fn)()" + if (is_call_target and e_.right.hasValueForThisInCall()) { + return Expr.joinWithComma(Expr{ .data = Prefill.Data.Zero, .loc = e_.left.loc }, e_.right, p.allocator); + } + + return e_.right; + } + } + }, + .bin_add => { + if (p.should_fold_typescript_constant_expressions) { + if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + return p.newExpr(E.Number{ .value = vals[0] + vals[1] }, v.loc); + } + } + + if (foldStringAddition(e_.left, e_.right)) |res| { + return res; + } + }, + .bin_sub => { + if (p.should_fold_typescript_constant_expressions) { + if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + return p.newExpr(E.Number{ .value = vals[0] - vals[1] }, v.loc); + } + } + }, + .bin_mul => { + if (p.should_fold_typescript_constant_expressions) { + if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + return p.newExpr(E.Number{ .value = vals[0] * vals[1] }, v.loc); + } + } + }, + .bin_div => { + if (p.should_fold_typescript_constant_expressions) { + if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + return p.newExpr(E.Number{ .value = vals[0] / vals[1] }, v.loc); + } + } + }, + .bin_rem => { + if (p.should_fold_typescript_constant_expressions) { + if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + return p.newExpr( + // Use libc fmod here to be consistent with what JavaScriptCore does + // https://github.com/oven-sh/WebKit/blob/7a0b13626e5db69aa5a32d037431d381df5dfb61/Source/JavaScriptCore/runtime/MathCommon.cpp#L574-L597 + E.Number{ .value = if (comptime Environment.isNative) bun.C.fmod(vals[0], vals[1]) else std.math.mod(f64, vals[0], vals[1]) catch 0 }, + v.loc, + ); + } + } + }, + .bin_pow => { + if (p.should_fold_typescript_constant_expressions) { + if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + return p.newExpr(E.Number{ .value = std.math.pow(f64, vals[0], vals[1]) }, v.loc); + } + } + }, + .bin_shl => { + // TODO: + // if (p.should_fold_typescript_constant_expressions) { + // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) << @intFromFloat(u32, vals[1])) & 31) }, expr.loc); + // } + // } + }, + .bin_shr => { + // TODO: + // if (p.should_fold_typescript_constant_expressions) { + // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) >> @intFromFloat(u32, vals[1])) & 31) }, expr.loc); + // } + // } + }, + .bin_u_shr => { + // TODO: + // if (p.should_fold_typescript_constant_expressions) { + // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) >> @intFromFloat(u32, vals[1])) & 31) }, expr.loc); + // } + // } + }, + .bin_bitwise_and => { + // TODO: + // if (p.should_fold_typescript_constant_expressions) { + // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) >> @intFromFloat(u32, vals[1])) & 31) }, expr.loc); + // } + // } + }, + .bin_bitwise_or => { + // TODO: + // if (p.should_fold_typescript_constant_expressions) { + // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) >> @intFromFloat(u32, vals[1])) & 31) }, expr.loc); + // } + // } + }, + .bin_bitwise_xor => { + // TODO: + // if (p.should_fold_typescript_constant_expressions) { + // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { + // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) >> @intFromFloat(u32, vals[1])) & 31) }, expr.loc); + // } + // } + }, + // --------------------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------------- + // --------------------------------------------------------------------------------------------------- + .bin_assign => { + + // Optionally preserve the name + if (@as(Expr.Tag, e_.left.data) == .e_identifier) { + e_.right = p.maybeKeepExprSymbolName(e_.right, p.symbols.items[e_.left.data.e_identifier.ref.innerIndex()].original_name, was_anonymous_named_expr); + } + }, + .bin_add_assign => { + // notimpl(); + }, + .bin_sub_assign => { + // notimpl(); + }, + .bin_mul_assign => { + // notimpl(); + }, + .bin_div_assign => { + // notimpl(); + }, + .bin_rem_assign => { + // notimpl(); + }, + .bin_pow_assign => { + // notimpl(); + }, + .bin_shl_assign => { + // notimpl(); + }, + .bin_shr_assign => { + // notimpl(); + }, + .bin_u_shr_assign => { + // notimpl(); + }, + .bin_bitwise_or_assign => { + // notimpl(); + }, + .bin_bitwise_and_assign => { + // notimpl(); + }, + .bin_bitwise_xor_assign => { + // notimpl(); + }, + .bin_nullish_coalescing_assign => { + // notimpl(); + }, + .bin_logical_and_assign => { + // notimpl(); + }, + .bin_logical_or_assign => { + // notimpl(); + }, + else => {}, + } + + return Expr{ .loc = v.loc, .data = .{ .e_binary = e_ } }; + } + + pub fn checkAndPrepare(v: *BinaryExpressionVisitor, p: *P) ?Expr { + var e_ = v.e; + switch (e_.left.data) { + // Special-case private identifiers + .e_private_identifier => |_private| { + if (e_.op == .bin_in) { + var private = _private; + const name = p.loadNameFromRef(private.ref); + const result = p.findSymbol(e_.left.loc, name) catch unreachable; + private.ref = result.ref; + + // Unlike regular identifiers, there are no unbound private identifiers + const kind: Symbol.Kind = p.symbols.items[result.ref.innerIndex()].kind; + if (!Symbol.isKindPrivate(kind)) { + const r = logger.Range{ .loc = e_.left.loc, .len = @as(i32, @intCast(name.len)) }; + p.log.addRangeErrorFmt(p.source, r, p.allocator, "Private name \"{s}\" must be declared in an enclosing class", .{name}) catch unreachable; + } + + e_.right = p.visitExpr(e_.right); + e_.left = .{ .data = .{ .e_private_identifier = private }, .loc = e_.left.loc }; + + // privateSymbolNeedsToBeLowered + return Expr{ .loc = v.loc, .data = .{ .e_binary = e_ } }; + } + }, + else => {}, + } + + v.is_stmt_expr = p.stmt_expr_value == .e_binary and p.stmt_expr_value.e_binary == e_; + + v.left_in = ExprIn{ + .assign_target = e_.op.binaryAssignTarget(), + }; + + return null; + } + }; + fn forbidLexicalDecl(p: *P, loc: logger.Loc) anyerror!void { try p.log.addError(p.source, loc, "Cannot use a declaration in a single-statement context"); } @@ -15505,352 +15900,74 @@ fn NewParser_( }, .e_binary => |e_| { - switch (e_.left.data) { - // Special-case private identifiers - .e_private_identifier => |_private| { - if (e_.op == .bin_in) { - var private = _private; - const name = p.loadNameFromRef(private.ref); - const result = p.findSymbol(e_.left.loc, name) catch unreachable; - private.ref = result.ref; - // Unlike regular identifiers, there are no unbound private identifiers - const kind: Symbol.Kind = p.symbols.items[result.ref.innerIndex()].kind; - if (!Symbol.isKindPrivate(kind)) { - const r = logger.Range{ .loc = e_.left.loc, .len = @as(i32, @intCast(name.len)) }; - p.log.addRangeErrorFmt(p.source, r, p.allocator, "Private name \"{s}\" must be declared in an enclosing class", .{name}) catch unreachable; - } + // The handling of binary expressions is convoluted because we're using + // iteration on the heap instead of recursion on the call stack to avoid + // stack overflow for deeply-nested ASTs. + var v = BinaryExpressionVisitor{ + .e = e_, + .loc = expr.loc, + .in = in, + .left_in = ExprIn{}, + }; - e_.right = p.visitExpr(e_.right); - e_.left = .{ .data = .{ .e_private_identifier = private }, .loc = e_.left.loc }; + // Everything uses a single stack to reduce allocation overhead. This stack + // should almost always be very small, and almost all visits should reuse + // existing memory without allocating anything. + const stack_bottom = p.binary_expression_stack.items.len; - // privateSymbolNeedsToBeLowered - return expr; - } - }, - else => {}, + var current = Expr{ .data = .{ .e_binary = e_ }, .loc = v.loc }; + + // Iterate down into the AST along the left node of the binary operation. + // Continue iterating until we encounter something that's not a binary node. + + while (true) { + if (v.checkAndPrepare(p)) |out| { + current = out; + break; + } + + // Grab the arguments to our nested "visitExprInOut" call for the left + // node. We only care about deeply-nested left nodes because most binary + // operators in JavaScript are left-associative and the problematic edge + // cases we're trying to avoid crashing on have lots of left-associative + // binary operators chained together without parentheses (e.g. "1+2+..."). + var left = v.e.left; + const left_in = v.left_in; + + var left_binary: ?*E.Binary = if (left.data == .e_binary) left.data.e_binary else null; + + // Stop iterating if iteration doesn't apply to the left node. This checks + // the assignment target because "visitExprInOut" has additional behavior + // in that case that we don't want to miss (before the top-level "switch" + // statement). + if (left_binary == null or left_in.assign_target != .none) { + v.e.left = p.visitExprInOut(left, left_in); + current = v.visitRightAndFinish(p); + break; + } + + // Note that we only append to the stack (and therefore allocate memory + // on the heap) when there are nested binary expressions. A single binary + // expression doesn't add anything to the stack. + p.binary_expression_stack.append(v) catch bun.outOfMemory(); + v = BinaryExpressionVisitor{ + .e = left_binary.?, + .loc = left.loc, + .in = left_in, + .left_in = .{}, + }; } - const is_call_target = @as(Expr.Tag, p.call_target) == .e_binary and expr.data.e_binary == p.call_target.e_binary; - // const is_stmt_expr = @as(Expr.Tag, p.stmt_expr_value) == .e_binary and expr.data.e_binary == p.stmt_expr_value.e_binary; - const was_anonymous_named_expr = e_.right.isAnonymousNamed(); - - e_.left = p.visitExprInOut(e_.left, ExprIn{ - .assign_target = e_.op.binaryAssignTarget(), - }); - - // Mark the control flow as dead if the branch is never taken - switch (e_.op) { - .bin_logical_or => { - const side_effects = SideEffects.toBoolean(p, e_.left.data); - if (side_effects.ok and side_effects.value) { - // "true || dead" - const old = p.is_control_flow_dead; - p.is_control_flow_dead = true; - e_.right = p.visitExpr(e_.right); - p.is_control_flow_dead = old; - } else { - e_.right = p.visitExpr(e_.right); - } - }, - .bin_logical_and => { - const side_effects = SideEffects.toBoolean(p, e_.left.data); - if (side_effects.ok and !side_effects.value) { - // "false && dead" - const old = p.is_control_flow_dead; - p.is_control_flow_dead = true; - e_.right = p.visitExpr(e_.right); - p.is_control_flow_dead = old; - } else { - e_.right = p.visitExpr(e_.right); - } - }, - .bin_nullish_coalescing => { - const side_effects = SideEffects.toNullOrUndefined(p, e_.left.data); - if (side_effects.ok and !side_effects.value) { - // "notNullOrUndefined ?? dead" - const old = p.is_control_flow_dead; - p.is_control_flow_dead = true; - e_.right = p.visitExpr(e_.right); - p.is_control_flow_dead = old; - } else { - e_.right = p.visitExpr(e_.right); - } - }, - else => { - e_.right = p.visitExpr(e_.right); - }, + // Process all binary operations from the deepest-visited node back toward + // our original top-level binary operation. + while (p.binary_expression_stack.items.len > stack_bottom) { + v = p.binary_expression_stack.pop(); + v.e.left = current; + current = v.visitRightAndFinish(p); } - // Always put constants on the right for equality comparisons to help - // reduce the number of cases we have to check during pattern matching. We - // can only reorder expressions that do not have any side effects. - switch (e_.op) { - .bin_loose_eq, .bin_loose_ne, .bin_strict_eq, .bin_strict_ne => { - if (SideEffects.isPrimitiveToReorder(e_.left.data) and !SideEffects.isPrimitiveToReorder(e_.right.data)) { - const _left = e_.left; - const _right = e_.right; - e_.left = _right; - e_.right = _left; - } - }, - else => {}, - } - - switch (e_.op) { - .bin_comma => { - // notimpl(); - }, - .bin_loose_eq => { - const equality = e_.left.data.eql(e_.right.data, p.allocator, .loose); - if (equality.ok) { - return p.newExpr( - E.Boolean{ .value = equality.equal }, - expr.loc, - ); - } - - // const after_op_loc = locAfterOp(e_.); - // TODO: warn about equality check - // TODO: warn about typeof string - - }, - .bin_strict_eq => { - const equality = e_.left.data.eql(e_.right.data, p.allocator, .strict); - if (equality.ok) { - return p.newExpr(E.Boolean{ .value = equality.equal }, expr.loc); - } - - // const after_op_loc = locAfterOp(e_.); - // TODO: warn about equality check - // TODO: warn about typeof string - }, - .bin_loose_ne => { - const equality = e_.left.data.eql(e_.right.data, p.allocator, .loose); - if (equality.ok) { - return p.newExpr(E.Boolean{ .value = !equality.equal }, expr.loc); - } - // const after_op_loc = locAfterOp(e_.); - // TODO: warn about equality check - // TODO: warn about typeof string - - // "x != void 0" => "x != null" - if (@as(Expr.Tag, e_.right.data) == .e_undefined) { - e_.right = p.newExpr(E.Null{}, e_.right.loc); - } - }, - .bin_strict_ne => { - const equality = e_.left.data.eql(e_.right.data, p.allocator, .strict); - if (equality.ok) { - return p.newExpr(E.Boolean{ .value = !equality.equal }, expr.loc); - } - }, - .bin_nullish_coalescing => { - const nullorUndefined = SideEffects.toNullOrUndefined(p, e_.left.data); - if (nullorUndefined.ok) { - if (!nullorUndefined.value) { - return e_.left; - } else if (nullorUndefined.side_effects == .no_side_effects) { - // "(null ?? fn)()" => "fn()" - // "(null ?? this.fn)" => "this.fn" - // "(null ?? this.fn)()" => "(0, this.fn)()" - if (is_call_target and e_.right.hasValueForThisInCall()) { - return Expr.joinWithComma(Expr{ .data = .{ .e_number = .{ .value = 0.0 } }, .loc = e_.left.loc }, e_.right, p.allocator); - } - - return e_.right; - } - } - }, - .bin_logical_or => { - const side_effects = SideEffects.toBoolean(p, e_.left.data); - if (side_effects.ok and side_effects.value) { - return e_.left; - } else if (side_effects.ok and side_effects.side_effects == .no_side_effects) { - // "(0 || fn)()" => "fn()" - // "(0 || this.fn)" => "this.fn" - // "(0 || this.fn)()" => "(0, this.fn)()" - if (is_call_target and e_.right.hasValueForThisInCall()) { - return Expr.joinWithComma(Expr{ .data = Prefill.Data.Zero, .loc = e_.left.loc }, e_.right, p.allocator); - } - - return e_.right; - } - }, - .bin_logical_and => { - const side_effects = SideEffects.toBoolean(p, e_.left.data); - if (side_effects.ok) { - if (!side_effects.value) { - return e_.left; - } else if (side_effects.side_effects == .no_side_effects) { - // "(1 && fn)()" => "fn()" - // "(1 && this.fn)" => "this.fn" - // "(1 && this.fn)()" => "(0, this.fn)()" - if (is_call_target and e_.right.hasValueForThisInCall()) { - return Expr.joinWithComma(Expr{ .data = Prefill.Data.Zero, .loc = e_.left.loc }, e_.right, p.allocator); - } - - return e_.right; - } - } - }, - .bin_add => { - if (p.should_fold_typescript_constant_expressions) { - if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - return p.newExpr(E.Number{ .value = vals[0] + vals[1] }, expr.loc); - } - } - - if (foldStringAddition(e_.left, e_.right)) |res| { - return res; - } - }, - .bin_sub => { - if (p.should_fold_typescript_constant_expressions) { - if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - return p.newExpr(E.Number{ .value = vals[0] - vals[1] }, expr.loc); - } - } - }, - .bin_mul => { - if (p.should_fold_typescript_constant_expressions) { - if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - return p.newExpr(E.Number{ .value = vals[0] * vals[1] }, expr.loc); - } - } - }, - .bin_div => { - if (p.should_fold_typescript_constant_expressions) { - if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - return p.newExpr(E.Number{ .value = vals[0] / vals[1] }, expr.loc); - } - } - }, - .bin_rem => { - if (p.should_fold_typescript_constant_expressions) { - if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - return p.newExpr( - // Use libc fmod here to be consistent with what JavaScriptCore does - // https://github.com/oven-sh/WebKit/blob/7a0b13626e5db69aa5a32d037431d381df5dfb61/Source/JavaScriptCore/runtime/MathCommon.cpp#L574-L597 - E.Number{ .value = if (comptime Environment.isNative) bun.C.fmod(vals[0], vals[1]) else std.math.mod(f64, vals[0], vals[1]) catch 0 }, - expr.loc, - ); - } - } - }, - .bin_pow => { - if (p.should_fold_typescript_constant_expressions) { - if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - return p.newExpr(E.Number{ .value = std.math.pow(f64, vals[0], vals[1]) }, expr.loc); - } - } - }, - .bin_shl => { - // TODO: - // if (p.should_fold_typescript_constant_expressions) { - // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) << @intFromFloat(u32, vals[1])) & 31) }, expr.loc); - // } - // } - }, - .bin_shr => { - // TODO: - // if (p.should_fold_typescript_constant_expressions) { - // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) >> @intFromFloat(u32, vals[1])) & 31) }, expr.loc); - // } - // } - }, - .bin_u_shr => { - // TODO: - // if (p.should_fold_typescript_constant_expressions) { - // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) >> @intFromFloat(u32, vals[1])) & 31) }, expr.loc); - // } - // } - }, - .bin_bitwise_and => { - // TODO: - // if (p.should_fold_typescript_constant_expressions) { - // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) >> @intFromFloat(u32, vals[1])) & 31) }, expr.loc); - // } - // } - }, - .bin_bitwise_or => { - // TODO: - // if (p.should_fold_typescript_constant_expressions) { - // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) >> @intFromFloat(u32, vals[1])) & 31) }, expr.loc); - // } - // } - }, - .bin_bitwise_xor => { - // TODO: - // if (p.should_fold_typescript_constant_expressions) { - // if (Expr.extractNumericValues(e_.left.data, e_.right.data)) |vals| { - // return p.newExpr(E.Number{ .value = ((@intFromFloat(i32, vals[0]) >> @intFromFloat(u32, vals[1])) & 31) }, expr.loc); - // } - // } - }, - // --------------------------------------------------------------------------------------------------- - // --------------------------------------------------------------------------------------------------- - // --------------------------------------------------------------------------------------------------- - // --------------------------------------------------------------------------------------------------- - .bin_assign => { - - // Optionally preserve the name - if (@as(Expr.Tag, e_.left.data) == .e_identifier) { - e_.right = p.maybeKeepExprSymbolName(e_.right, p.symbols.items[e_.left.data.e_identifier.ref.innerIndex()].original_name, was_anonymous_named_expr); - } - }, - .bin_add_assign => { - // notimpl(); - }, - .bin_sub_assign => { - // notimpl(); - }, - .bin_mul_assign => { - // notimpl(); - }, - .bin_div_assign => { - // notimpl(); - }, - .bin_rem_assign => { - // notimpl(); - }, - .bin_pow_assign => { - // notimpl(); - }, - .bin_shl_assign => { - // notimpl(); - }, - .bin_shr_assign => { - // notimpl(); - }, - .bin_u_shr_assign => { - // notimpl(); - }, - .bin_bitwise_or_assign => { - // notimpl(); - }, - .bin_bitwise_and_assign => { - // notimpl(); - }, - .bin_bitwise_xor_assign => { - // notimpl(); - }, - .bin_nullish_coalescing_assign => { - // notimpl(); - }, - .bin_logical_and_assign => { - // notimpl(); - }, - .bin_logical_or_assign => { - // notimpl(); - }, - else => {}, - } + return current; }, .e_index => |e_| { const is_call_target = std.meta.activeTag(p.call_target) == .e_index and expr.data.e_index == p.call_target.e_index; diff --git a/src/js_printer.zig b/src/js_printer.zig index 5cc8126883..56d644d8f9 100644 --- a/src/js_printer.zig +++ b/src/js_printer.zig @@ -736,8 +736,178 @@ fn NewPrinter( temporary_bindings: std.ArrayListUnmanaged(B.Property) = .{}, + binary_expression_stack: std.ArrayList(BinaryExpressionVisitor) = undefined, + const Printer = @This(); + /// The handling of binary expressions is convoluted because we're using + /// iteration on the heap instead of recursion on the call stack to avoid + /// stack overflow for deeply-nested ASTs. See the comments for the similar + /// code in the JavaScript parser for details. + pub const BinaryExpressionVisitor = struct { + // Inputs + e: *E.Binary, + level: Level = .lowest, + flags: ExprFlag.Set = ExprFlag.None(), + + // Input for visiting the left child + left_level: Level = .lowest, + left_flags: ExprFlag.Set = ExprFlag.None(), + + // "Local variables" passed from "checkAndPrepare" to "visitRightAndFinish" + entry: *const Op = undefined, + wrap: bool = false, + right_level: Level = .lowest, + + pub fn checkAndPrepare(v: *BinaryExpressionVisitor, p: *Printer) bool { + var e = v.e; + + const entry: *const Op = Op.Table.getPtrConst(e.op); + const e_level = entry.level; + v.entry = entry; + v.wrap = v.level.gte(e_level) or (e.op == Op.Code.bin_in and v.flags.contains(.forbid_in)); + + // Destructuring assignments must be parenthesized + const n = p.writer.written; + if (n == p.stmt_start or n == p.arrow_expr_start) { + switch (e.left.data) { + .e_object => { + v.wrap = true; + }, + else => {}, + } + } + + if (v.wrap) { + p.print("("); + v.flags.insert(.forbid_in); + } + + v.left_level = e_level.sub(1); + v.right_level = e_level.sub(1); + var left_level = &v.left_level; + var right_level = &v.right_level; + + if (e.op.isRightAssociative()) { + left_level.* = e_level; + } + + if (e.op.isLeftAssociative()) { + right_level.* = e_level; + } + + switch (e.op) { + // "??" can't directly contain "||" or "&&" without being wrapped in parentheses + .bin_nullish_coalescing => { + switch (e.left.data) { + .e_binary => { + const left = e.left.data.e_binary; + switch (left.op) { + .bin_logical_and, .bin_logical_or => { + left_level.* = .prefix; + }, + else => {}, + } + }, + else => {}, + } + + switch (e.right.data) { + .e_binary => { + const right = e.right.data.e_binary; + switch (right.op) { + .bin_logical_and, .bin_logical_or => { + right_level.* = .prefix; + }, + else => {}, + } + }, + else => {}, + } + }, + // "**" can't contain certain unary expressions + .bin_pow => { + switch (e.left.data) { + .e_unary => { + const left = e.left.data.e_unary; + if (left.op.unaryAssignTarget() == .none) { + left_level.* = .call; + } + }, + .e_await, .e_undefined, .e_number => { + left_level.* = .call; + }, + .e_boolean => { + // When minifying, booleans are printed as "!0 and "!1" + if (p.options.minify_syntax) { + left_level.* = .call; + } + }, + else => {}, + } + }, + else => {}, + } + + // Special-case "#foo in bar" + if (e.left.data == .e_private_identifier and e.op == .bin_in) { + const private = e.left.data.e_private_identifier; + const name = p.renamer.nameForSymbol(private.ref); + p.addSourceMappingForName(e.left.loc, name, private.ref); + p.printIdentifier(name); + v.visitRightAndFinish(p); + return false; + } + + v.left_flags = ExprFlag.Set{}; + + if (v.flags.contains(.forbid_in)) { + v.left_flags.insert(.forbid_in); + } + + if (e.op == .bin_comma) + v.left_flags.insert(.expr_result_is_unused); + + return true; + } + pub fn visitRightAndFinish(v: *BinaryExpressionVisitor, p: *Printer) void { + var e = v.e; + const entry = v.entry; + var flags = ExprFlag.Set{}; + + if (e.op != .bin_comma) { + p.printSpace(); + } + + if (entry.is_keyword) { + p.printSpaceBeforeIdentifier(); + p.print(entry.text); + } else { + p.printSpaceBeforeOperator(e.op); + p.print(entry.text); + p.prev_op = e.op; + p.prev_op_end = p.writer.written; + } + + p.printSpace(); + + // The result of the right operand of the comma operator is unused if the caller doesn't use it + if (e.op == .bin_comma and v.flags.contains(.expr_result_is_unused)) { + flags.insert(.expr_result_is_unused); + } + + if (v.flags.contains(.forbid_in)) { + flags.insert(.forbid_in); + } + + p.printExpr(e.right, v.right_level, flags); + + if (v.wrap) { + p.print(")"); + } + } + }; + pub fn writeAll(p: *Printer, bytes: anytype) anyerror!void { p.print(bytes); return; @@ -1197,18 +1367,14 @@ fn NewPrinter( printer.source_map_builder.addSourceMapping(location, printer.writer.slice()); } - // pub inline fn addSourceMappingForName(printer: *Printer, location: logger.Loc, name: string, ref: Ref) void { - // _ = location; - // if (comptime !generate_source_map) { - // return; - // } + pub inline fn addSourceMappingForName(printer: *Printer, location: logger.Loc, _: string, _: Ref) void { + if (comptime !generate_source_map) { + return; + } - // if (printer.symbols().get(printer.symbols().follow(ref))) |symbol| { - // if (!strings.eqlLong(symbol.original_name, name)) { - // printer.source_map_builder.addSourceMapping() - // } - // } - // } + // TODO: + printer.addSourceMapping(location); + } pub fn printSymbol(p: *Printer, ref: Ref) void { std.debug.assert(!ref.isNull()); @@ -2921,120 +3087,49 @@ fn NewPrinter( } }, .e_binary => |e| { - // 4.00 ms enums.EnumIndexer(src.js_ast.Op.Code).indexOf - const entry: *const Op = Op.Table.getPtrConst(e.op); - const e_level = entry.level; + // The handling of binary expressions is convoluted because we're using + // iteration on the heap instead of recursion on the call stack to avoid + // stack overflow for deeply-nested ASTs. See the comments for the similar + // code in the JavaScript parser for details. + var v = BinaryExpressionVisitor{ + .e = e, + .level = level, + .flags = flags, + .entry = Op.Table.getPtrConst(e.op), + }; - var wrap = level.gte(e_level) or (e.op == Op.Code.bin_in and flags.contains(.forbid_in)); + // Use a single stack to reduce allocation overhead + const stack_bottom = p.binary_expression_stack.items.len; - // Destructuring assignments must be parenthesized - const n = p.writer.written; - if (n == p.stmt_start or n == p.arrow_expr_start) { - switch (e.left.data) { - .e_object => { - wrap = true; - }, - else => {}, + while (true) { + if (!v.checkAndPrepare(p)) { + break; } + + const left = v.e.left; + var left_binary: ?*E.Binary = if (left.data == .e_binary) left.data.e_binary else null; + + // Stop iterating if iteration doesn't apply to the left node + if (left_binary == null) { + p.printExpr(left, v.left_level, v.left_flags); + v.visitRightAndFinish(p); + break; + } + + // Only allocate heap memory on the stack for nested binary expressions + p.binary_expression_stack.append(v) catch bun.outOfMemory(); + v = BinaryExpressionVisitor{ + .e = left_binary.?, + .level = v.left_level, + .flags = v.left_flags, + }; } - if (wrap) { - p.print("("); - flags.insert(.forbid_in); - } - - var left_level = e_level.sub(1); - var right_level = e_level.sub(1); - - if (e.op.isRightAssociative()) { - left_level = e_level; - } - - if (e.op.isLeftAssociative()) { - right_level = e_level; - } - - switch (e.op) { - // "??" can't directly contain "||" or "&&" without being wrapped in parentheses - .bin_nullish_coalescing => { - switch (e.left.data) { - .e_binary => { - const left = e.left.data.e_binary; - switch (left.op) { - .bin_logical_and, .bin_logical_or => { - left_level = .prefix; - }, - else => {}, - } - }, - else => {}, - } - - switch (e.right.data) { - .e_binary => { - const right = e.right.data.e_binary; - switch (right.op) { - .bin_logical_and, .bin_logical_or => { - right_level = .prefix; - }, - else => {}, - } - }, - else => {}, - } - }, - // "**" can't contain certain unary expressions - .bin_pow => { - switch (e.left.data) { - .e_unary => { - const left = e.left.data.e_unary; - if (left.op.unaryAssignTarget() == .none) { - left_level = .call; - } - }, - .e_await, .e_undefined, .e_number => { - left_level = .call; - }, - else => {}, - } - }, - else => {}, - } - - // Special-case "#foo in bar" - if (e.op == .bin_in and @as(Expr.Tag, e.left.data) == .e_private_identifier) { - p.printSymbol(e.left.data.e_private_identifier.ref); - } else { - flags.insert(.forbid_in); - p.printExpr(e.left, left_level, flags); - } - - if (e.op != .bin_comma) { - p.printSpace(); - } - - if (entry.is_keyword) { - p.printSpaceBeforeIdentifier(); - p.print(entry.text); - } else { - p.printSpaceBeforeOperator(e.op); - p.print(entry.text); - p.prev_op = e.op; - p.prev_op_end = p.writer.written; - } - - p.printSpace(); - flags.insert(.forbid_in); - - // this feels like a hack? I think something is wrong here. - if (e.op == .bin_assign) { - flags.remove(.expr_result_is_unused); - } - - p.printExpr(e.right, right_level, flags); - - if (wrap) { - p.print(")"); + // Process all binary operations from the deepest-visited node back toward + // our original top-level binary operation + while (p.binary_expression_stack.items.len > stack_bottom) { + var last = p.binary_expression_stack.pop(); + last.visitRightAndFinish(p); } }, else => { @@ -5769,6 +5864,9 @@ pub fn printAst( renamer, getSourceMapBuilder(if (generate_source_map) .lazy else .disable, ascii_only, opts, source, &tree), ); + var bin_stack_heap = std.heap.stackFallback(1024, bun.default_allocator); + printer.binary_expression_stack = std.ArrayList(PrinterType.BinaryExpressionVisitor).init(bin_stack_heap.get()); + defer printer.binary_expression_stack.clearAndFree(); defer { imported_module_ids_list = printer.imported_module_ids; } @@ -5842,6 +5940,9 @@ pub fn printJSON( renamer.toRenamer(), undefined, ); + var bin_stack_heap = std.heap.stackFallback(1024, bun.default_allocator); + printer.binary_expression_stack = std.ArrayList(PrinterType.BinaryExpressionVisitor).init(bin_stack_heap.get()); + defer printer.binary_expression_stack.clearAndFree(); printer.printExpr(expr, Level.lowest, ExprFlag.Set{}); if (printer.writer.getError()) {} else |err| { @@ -5940,6 +6041,10 @@ pub fn printWithWriterAndPlatform( renamer, getSourceMapBuilder(if (generate_source_maps) .eager else .disable, is_bun_platform, opts, source, &ast), ); + var bin_stack_heap = std.heap.stackFallback(1024, bun.default_allocator); + printer.binary_expression_stack = std.ArrayList(PrinterType.BinaryExpressionVisitor).init(bin_stack_heap.get()); + defer printer.binary_expression_stack.clearAndFree(); + defer printer.temporary_bindings.deinit(bun.default_allocator); defer _writer.* = printer.writer.*; defer { @@ -5997,6 +6102,9 @@ pub fn printCommonJS( renamer.toRenamer(), getSourceMapBuilder(if (generate_source_map) .lazy else .disable, false, opts, source, &tree), ); + var bin_stack_heap = std.heap.stackFallback(1024, bun.default_allocator); + printer.binary_expression_stack = std.ArrayList(PrinterType.BinaryExpressionVisitor).init(bin_stack_heap.get()); + defer printer.binary_expression_stack.clearAndFree(); defer { imported_module_ids_list = printer.imported_module_ids; } diff --git a/test/cli/run/transpiler-cache.test.ts b/test/cli/run/transpiler-cache.test.ts index 27bd80bbad..5845c9a162 100644 --- a/test/cli/run/transpiler-cache.test.ts +++ b/test/cli/run/transpiler-cache.test.ts @@ -21,6 +21,7 @@ let cache_dir = ""; const env = { ...bunEnv, BUN_RUNTIME_TRANSPILER_CACHE_PATH: cache_dir, + BUN_DEBUG_ENABLE_RESTORE_FROM_TRANSPILER_CACHE: "1", }; let prev_cache_count = 0; @@ -90,7 +91,7 @@ describe("transpiler cache", () => { expect(b.stdout == "b"); expect(newCacheCount()).toBe(0); }); - test("doing 500 buns at once does not crash", async () => { + test("doing 50 buns at once does not crash", async () => { writeFileSync(join(temp_dir, "a.js"), dummyFile(50 * 1024, "1", "b")); writeFileSync(join(temp_dir, "b.js"), dummyFile(50 * 1024, "2", "b")); @@ -102,7 +103,7 @@ describe("transpiler cache", () => { let processes: Subprocess<"ignore", "pipe", "inherit">[] = []; let killing = false; - for (let i = 0; i < 500; i++) { + for (let i = 0; i < 50; i++) { processes.push( Bun.spawn({ cmd: [bunExe(), i % 2 == 0 ? "a.js" : "b.js"], diff --git a/test/integration/next/default-pages-dir/test/dev-server.test.ts b/test/integration/next/default-pages-dir/test/dev-server.test.ts index 62e97a0498..4288638920 100644 --- a/test/integration/next/default-pages-dir/test/dev-server.test.ts +++ b/test/integration/next/default-pages-dir/test/dev-server.test.ts @@ -55,6 +55,7 @@ test("ssr works for 100 requests", async () => { }); expect(x.status).toBe(200); const text = await x.text(); + console.count("Completed request"); expect(text).toContain(`>${Bun.version}`); })(), ); diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index 3598dd62e9..f8ee599013 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -469,7 +469,7 @@ test("CallFrame.p.toString", () => { expect(e.stack[0].toString().includes("")).toBe(true); }); -test.todo("err.stack should invoke prepareStackTrace", () => { +test("err.stack should invoke prepareStackTrace", () => { var lineNumber = -1; var functionName = ""; var parentLineNumber = -1; @@ -492,9 +492,9 @@ test.todo("err.stack should invoke prepareStackTrace", () => { functionWithAName(); expect(functionName).toBe("functionWithAName"); - expect(lineNumber).toBe(488); + expect(lineNumber).toBe(491); // TODO: this is wrong - expect(parentLineNumber).toBe(497); + expect(parentLineNumber).toBe(499); }); test("Error.prepareStackTrace inside a node:vm works", () => { diff --git a/test/transpiler/transpiler-stack-overflow.test.ts b/test/transpiler/transpiler-stack-overflow.test.ts new file mode 100644 index 0000000000..1501292b2f --- /dev/null +++ b/test/transpiler/transpiler-stack-overflow.test.ts @@ -0,0 +1,25 @@ +import { test, expect } from "bun:test"; +import { writeFileSync, mkdirSync } from "node:fs"; +import { join } from "path"; +import { tmpdir } from "node:os"; +import { bunEnv, bunExe } from "harness"; + +test("long chain of expressions does not cause stack overflow", () => { + const chain = `globalThis.a = {};` + "\n" + `globalThis.a + globalThis.a +`.repeat(1000000) + `globalThis.a` + "\n"; + const temp_dir = join( + tmpdir(), + `bun-test-transpiler-cache-${Date.now()}-` + (Math.random() * 81023).toString(36).slice(2), + ); + mkdirSync(temp_dir, { recursive: true }); + writeFileSync(join(temp_dir, "index.js"), chain, "utf-8"); + const { exitCode } = Bun.spawnSync({ + cmd: [bunExe(), "build", "--no-bundle", join(temp_dir, "index.js")], + cwd: import.meta.dir, + env: bunEnv, + stderr: "inherit", + stdout: Bun.file("/dev/null"), + stdin: Bun.file("/dev/null"), + }); + + expect(exitCode).toBe(0); +}, 1000000);