feat: overhaul the crash handler (oven-sh#10203)

* some work * linux things * linux things * feat: tracestrings on Windows * bwaa * more work on the crash handler * okay * adgadsgbcxcv * ya * dsafds * a * wuh * a * bru h * ok * yay * window * alright * oops * yeah * a * a * OOM handling * fix on window
anonrig · Apr 17, 2024 · c99d7ed · c99d7ed
1 parent f764c12
commit c99d7ed
Show file tree

Hide file tree

Showing 48 changed files with 1,574 additions and 1,019 deletions.
diff --git a/.github/ISSUE_TEMPLATE/6-crash-report.yml b/.github/ISSUE_TEMPLATE/6-crash-report.yml
@@ -0,0 +1,20 @@
+name: Crash
+description: Crash report
+labels: [bug, crash]
+body:
+  - type: markdown
+    attributes:
+      value: |
+        Thank you for submitting a crash report. It helps make Bun better.
+
+  - type: textarea
+    id: remapped_trace
+    attributes:
+      label: Stack Trace
+    validations:
+      required: true
+
+  - type: textarea
+    attributes:
+      label: Any extra info?
+      description: If this is a reproducible bug, please provide a code snippet or list of steps that can reproduce it.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -738,7 +738,7 @@ if(NOT NO_CODEGEN)
         OUTPUT ${BUN_IDENTIFIER_CACHE_OUT}
         MAIN_DEPENDENCY "${BUN_SRC}/js_lexer/identifier_data.zig"
         DEPENDS "${BUN_SRC}/js_lexer/identifier_cache.zig"
-        COMMAND ${ZIG_COMPILER} run "${BUN_SRC}/js_lexer/identifier_data.zig"
+        COMMAND ${ZIG_COMPILER} run "--zig-lib-dir" "${ZIG_LIB_DIR}" "${BUN_SRC}/js_lexer/identifier_data.zig"
         VERBATIM
         COMMENT "Building Identifier Cache"
     )
@@ -763,6 +763,7 @@ if(NOT NO_CODEGEN)
         "${BUN_SRC}/js/node/*.ts"
         "${BUN_SRC}/js/thirdparty/*.js"
         "${BUN_SRC}/js/thirdparty/*.ts"
+        "${BUN_SRC}/js/internal-for-testing.ts"
     )
 
     file(GLOB CODEGEN_FILES ${CONFIGURE_DEPENDS} "${BUN_CODEGEN_SRC}/*.ts")
@@ -1014,7 +1015,7 @@ endif()
 # --- clang and linker flags ---
 if(CMAKE_BUILD_TYPE STREQUAL "Debug")
     if(NOT WIN32)
-        target_compile_options(${bun} PUBLIC -g3 -O0 -gdwarf-4
+        target_compile_options(${bun} PUBLIC -O0 -g -g3 -ggdb -gdwarf-4
             -Werror=return-type
             -Werror=return-stack-address
             -Werror=implicit-function-declaration
@@ -1061,8 +1062,8 @@ elseif(CMAKE_BUILD_TYPE STREQUAL "Release")
             list(APPEND LTO_LINK_FLAG "/LTCG")
         endif()
 
-        target_compile_options(${bun} PUBLIC /O2 ${LTO_FLAG} /DEBUG /Z7)
-        target_link_options(${bun} PUBLIC ${LTO_LINK_FLAG} /DEBUG)
+        target_compile_options(${bun} PUBLIC /O2 ${LTO_FLAG} /DEBUG:FULL)
+        target_link_options(${bun} PUBLIC ${LTO_LINK_FLAG} /DEBUG:FULL)
     endif()
 endif()
 

diff --git a/build.zig b/build.zig
@@ -146,7 +146,7 @@ const fs = std.fs;
 pub fn build(b: *Build) !void {
     build_(b) catch |err| {
         if (@errorReturnTrace()) |trace| {
-            std.debug.dumpStackTrace(trace.*);
+            (std.debug).dumpStackTrace(trace.*);
         }
 
         return err;
@@ -394,7 +394,7 @@ pub fn build_(b: *Build) !void {
         obj.linkLibC();
         obj.dll_export_fns = true;
         obj.strip = false;
-        obj.omit_frame_pointer = optimize != .Debug;
+        obj.omit_frame_pointer = false;
         obj.subsystem = .Console;
 
         // Disable stack probing on x86 so we don't need to include compiler_rt

diff --git a/packages/bun-internal-test/src/banned.json b/packages/bun-internal-test/src/banned.json
@@ -1,11 +1,12 @@
 {
-  "std.debug.assert": "Use bun.assert instead",
+  " != undefined": "This is by definition Undefined Behavior.",
+  " == undefined": "This is by definition Undefined Behavior.",
   "@import(\"root\").bun.": "Only import 'bun' once",
-  "std.mem.indexOfAny": "Use bun.strings.indexAny or bun.strings.indexAnyComptime",
+  "std.debug.assert": "Use bun.assert instead",
+  "std.debug.dumpStackTrace": "Use bun.handleErrorReturnTrace or bun.crash_handler.dumpStackTrace instead",
   "std.debug.print": "Don't let this be committed",
-  " == undefined": "This is by definition Undefined Behavior.",
-  " != undefined": "This is by definition Undefined Behavior.",
-  "undefined == ": "This is by definition Undefined Behavior.",
+  "std.mem.indexOfAny": "Use bun.strings.indexAny or bun.strings.indexAnyComptime",
   "undefined != ": "This is by definition Undefined Behavior.",
+  "undefined == ": "This is by definition Undefined Behavior.",
   "": ""
 }
diff --git a/src/__global.zig → src/Global.zig b/src/__global.zig → src/Global.zig
@@ -64,13 +64,17 @@ pub inline fn getStartTime() i128 {
     return bun.start_time;
 }
 
-pub fn setThreadName(name: StringTypes.stringZ) void {
+extern "kernel32" fn SetThreadDescription(thread: std.os.windows.HANDLE, name: [*:0]const u16) callconv(std.os.windows.WINAPI) std.os.windows.HRESULT;
+
+pub fn setThreadName(name: [:0]const u8) void {
     if (Environment.isLinux) {
-        _ = std.os.prctl(.SET_NAME, .{@intFromPtr(name.ptr)}) catch 0;
+        _ = std.os.prctl(.SET_NAME, .{@intFromPtr(name.ptr)}) catch {};
     } else if (Environment.isMac) {
         _ = std.c.pthread_setname_np(name);
     } else if (Environment.isWindows) {
-        // _ = std.os.SetThreadDescription(std.os.GetCurrentThread(), name);
+        var name_buf: [1024]u16 = undefined;
+        const name_wide = bun.strings.convertUTF8toUTF16InBufferZ(&name_buf, name);
+        _ = SetThreadDescription(std.os.windows.kernel32.GetCurrentThread(), name_wide);
     }
 }
 
@@ -105,7 +109,7 @@ pub fn raiseIgnoringPanicHandler(sig: anytype) noreturn {
     }
 
     Output.flush();
-    @import("./crash_reporter.zig").on_error = null;
+
     if (!Environment.isWindows) {
         if (sig >= 1 and sig != std.os.SIG.STOP and sig != std.os.SIG.KILL) {
             const act = std.os.Sigaction{
@@ -119,9 +123,7 @@ pub fn raiseIgnoringPanicHandler(sig: anytype) noreturn {
 
     Output.Source.Stdio.restore();
 
-    // TODO(@paperdave): report a bug that this intcast shouldnt be needed. signals are i32 not u32
-    // after that is fixed we can make this function take i32
-    _ = std.c.raise(@intCast(sig));
+    _ = std.c.raise(sig);
     std.c.abort();
 }
 

diff --git a/src/StandaloneModuleGraph.zig b/src/StandaloneModuleGraph.zig
@@ -694,7 +694,7 @@ pub const StandaloneModuleGraph = struct {
 
     fn openSelf() std.fs.OpenSelfExeError!bun.FileDescriptor {
         if (!Environment.isWindows) {
-            const argv = bun.argv();
+            const argv = bun.argv;
             if (argv.len > 0) {
                 if (isBuiltInExe(u8, argv[0])) {
                     return error.FileNotFound;
@@ -707,14 +707,14 @@ pub const StandaloneModuleGraph = struct {
                 if (std.fs.openFileAbsoluteZ("/proc/self/exe", .{})) |easymode| {
                     return bun.toFD(easymode.handle);
                 } else |_| {
-                    if (bun.argv().len > 0) {
+                    if (bun.argv.len > 0) {
                         // The user doesn't have /proc/ mounted, so now we just guess and hope for the best.
                         var whichbuf: [bun.MAX_PATH_BYTES]u8 = undefined;
                         if (bun.which(
                             &whichbuf,
                             bun.getenvZ("PATH") orelse return error.FileNotFound,
                             "",
-                            bun.argv()[0],
+                            bun.argv[0],
                         )) |path| {
                             return bun.toFD((try std.fs.cwd().openFileZ(path, .{})).handle);
                         }

diff --git a/src/analytics.zig b/src/analytics.zig
diff --git a/src/analytics/analytics_thread.zig b/src/analytics/analytics_thread.zig
@@ -72,6 +72,7 @@ pub const Features = struct {
     pub fn formatter() Formatter {
         return Formatter{};
     }
+
     pub const Formatter = struct {
         pub fn format(_: Formatter, comptime _: []const u8, _: std.fmt.FormatOptions, writer: anytype) !void {
             const fields = comptime brk: {
@@ -91,9 +92,14 @@ pub const Features = struct {
                 break :brk buffer[0..count];
             };
 
+            var is_first_feature = true;
             inline for (fields) |field| {
                 const count = @field(Features, field);
                 if (count > 0) {
+                    if (is_first_feature) {
+                        try writer.writeAll("Features: ");
+                        is_first_feature = false;
+                    }
                     try writer.writeAll(field);
                     if (count > 1) {
                         try writer.print("({d}) ", .{count});
@@ -102,10 +108,13 @@ pub const Features = struct {
                     }
                 }
             }
+            if (!is_first_feature) {
+                try writer.writeAll("\n");
+            }
 
             var builtins = builtin_modules.iterator();
             if (builtins.next()) |first| {
-                try writer.writeAll("\nBuiltins: \"");
+                try writer.writeAll("Builtins: \"");
                 try writer.writeAll(@tagName(first));
                 try writer.writeAll("\" ");
 
@@ -115,8 +124,6 @@ pub const Features = struct {
                     try writer.writeAll("\" ");
                 }
 
-                try writer.writeAll("\n");
-            } else {
                 try writer.writeAll("\n");
             }
         }

diff --git a/src/bun.js/api/BunObject.zig b/src/bun.js/api/BunObject.zig
@@ -3325,7 +3325,6 @@ const UnsafeObject = struct {
         const object = JSValue.createEmptyObject(globalThis, 3);
         const fields = comptime .{
             .gcAggressionLevel = &gcAggressionLevel,
-            .segfault = &__debug__doSegfault,
             .arrayBufferToString = &arrayBufferToString,
             .mimallocDump = &dump_mimalloc,
         };
@@ -3357,15 +3356,6 @@ const UnsafeObject = struct {
         return ret;
     }
 
-    // For testing the segfault handler
-    pub fn __debug__doSegfault(
-        _: *JSC.JSGlobalObject,
-        _: *JSC.CallFrame,
-    ) callconv(.C) JSC.JSValue {
-        const Reporter = @import("../../report.zig");
-        Reporter.globalError(error.SegfaultTest, null);
-    }
-
     pub fn arrayBufferToString(
         globalThis: *JSC.JSGlobalObject,
         callframe: *JSC.CallFrame,

diff --git a/src/bun.js/api/bun/process.zig b/src/bun.js/api/bun/process.zig
@@ -1065,7 +1065,7 @@ pub const PosixSpawnResult = struct {
     }
 
     fn pidfdFlagsForLinux() u32 {
-        const kernel = @import("../../../analytics.zig").GenerateHeader.GeneratePlatform.kernelVersion();
+        const kernel = bun.analytics.GenerateHeader.GeneratePlatform.kernelVersion();
 
         // pidfd_nonblock only supported in 5.10+
         return if (kernel.orderWithoutTag(.{ .major = 5, .minor = 10, .patch = 0 }).compare(.gte))

diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig
@@ -1897,8 +1897,8 @@ pub fn NewGlobalObject(comptime Type: type) type {
             }
 
             Output.flush();
-            const Reporter = @import("../../report.zig");
-            Reporter.fatal(null, "A C++ exception occurred");
+
+            @panic("A C++ exception occurred");
         }
     };
 }

diff --git a/src/bun.js/bindings/exports.zig b/src/bun.js/bindings/exports.zig
@@ -25,7 +25,6 @@ const Exception = JSC.Exception;
 const JSModuleLoader = JSC.JSModuleLoader;
 const Microtask = JSC.Microtask;
 
-const Backtrace = @import("../../crash_reporter.zig");
 const JSPrinter = bun.js_printer;
 const JSLexer = bun.js_lexer;
 const typeBaseName = @import("../../meta.zig").typeBaseName;
@@ -49,7 +48,6 @@ pub const ZigGlobalObject = extern struct {
         worker_ptr: ?*anyopaque,
     ) *JSGlobalObject {
         const global = shim.cppFn("create", .{ console, context_id, mini_mode, eval_mode, worker_ptr });
-        Backtrace.reloadHandlers() catch unreachable;
         return global;
     }
 

diff --git a/src/bun.js/bindings/wtf-bindings.cpp b/src/bun.js/bindings/wtf-bindings.cpp
@@ -178,38 +178,6 @@ extern "C" void WTF__copyLCharsFromUCharSource(LChar* destination, const UChar*
     WTF::StringImpl::copyCharacters(destination, source, length);
 }
 
-extern "C" void Bun__crashReportWrite(void* ctx, const char* message, size_t length);
-extern "C" void Bun__crashReportDumpStackTrace(void* ctx)
-{
-    static constexpr int framesToShow = 32;
-    static constexpr int framesToSkip = 2;
-    void* stack[framesToShow + framesToSkip];
-    int frames = framesToShow + framesToSkip;
-    WTFGetBacktrace(stack, &frames);
-    int size = frames - framesToSkip;
-    bool isFirst = true;
-    for (int frameNumber = 0; frameNumber < size; ++frameNumber) {
-        auto demangled = WTF::StackTraceSymbolResolver::demangle(stack[frameNumber]);
-
-        StringPrintStream out;
-        if (isFirst) {
-            isFirst = false;
-            if (demangled)
-                out.printf("\n%-3d %p %s", frameNumber, stack[frameNumber], demangled->demangledName() ? demangled->demangledName() : demangled->mangledName());
-            else
-                out.printf("\n%-3d %p", frameNumber, stack[frameNumber]);
-        } else {
-            if (demangled)
-                out.printf("%-3d ??? %s", frameNumber, demangled->demangledName() ? demangled->demangledName() : demangled->mangledName());
-            else
-                out.printf("%-3d ???", frameNumber);
-        }
-
-        auto str = out.toCString();
-        Bun__crashReportWrite(ctx, str.data(), str.length());
-    }
-}
-
 // For whatever reason
 // Doing this in C++/C is 2x faster than doing it in Zig.
 // However, it's still slower than it should be.

diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig
@@ -850,7 +850,11 @@ pub const VirtualMachine = struct {
         Output.debug("Reloading...", .{});
         if (this.hot_reload == .watch) {
             Output.flush();
-            bun.reloadProcess(bun.default_allocator, !strings.eqlComptime(this.bundler.env.get("BUN_CONFIG_NO_CLEAR_TERMINAL_ON_RELOAD") orelse "0", "true"));
+            bun.reloadProcess(
+                bun.default_allocator,
+                !strings.eqlComptime(this.bundler.env.get("BUN_CONFIG_NO_CLEAR_TERMINAL_ON_RELOAD") orelse "0", "true"),
+                false,
+            );
         }
 
         if (!strings.eqlComptime(this.bundler.env.get("BUN_CONFIG_NO_CLEAR_TERMINAL_ON_RELOAD") orelse "0", "true")) {
@@ -3462,7 +3466,7 @@ pub fn NewHotReloader(comptime Ctx: type, comptime EventLoopType: type, comptime
                     if (comptime Ctx == ImportWatcher) {
                         this.reloader.ctx.rareData().closeAllListenSocketsForWatchMode();
                     }
-                    bun.reloadProcess(bun.default_allocator, clear_screen);
+                    bun.reloadProcess(bun.default_allocator, clear_screen, false);
                     unreachable;
                 }
 

diff --git a/src/bun.js/module_loader.zig b/src/bun.js/module_loader.zig
@@ -2354,13 +2354,6 @@ pub const ModuleLoader = struct {
                     if (!Environment.isDebug) {
                         if (!is_allowed_to_use_internal_testing_apis)
                             return null;
-                        const is_outside_our_ci = brk: {
-                            const repo = jsc_vm.bundler.env.get("GITHUB_REPOSITORY") orelse break :brk true;
-                            break :brk !strings.endsWithComptime(repo, "/bun");
-                        };
-                        if (is_outside_our_ci) {
-                            return null;
-                        }
                     }
 
                     return jsSyntheticModule(.InternalForTesting, specifier);

diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig
@@ -4843,7 +4843,7 @@ pub const Path = struct {
 
 pub const Process = struct {
     pub fn getArgv0(globalObject: *JSC.JSGlobalObject) callconv(.C) JSC.JSValue {
-        return JSC.ZigString.fromUTF8(bun.argv()[0]).toValueGC(globalObject);
+        return JSC.ZigString.fromUTF8(bun.argv[0]).toValueGC(globalObject);
     }
 
     pub fn getExecPath(globalObject: *JSC.JSGlobalObject) callconv(.C) JSC.JSValue {
@@ -4874,13 +4874,13 @@ pub const Process = struct {
             JSC.ZigString,
             // argv omits "bun" because it could be "bun run" or "bun" and it's kind of ambiguous
             // argv also omits the script name
-            bun.argv().len -| 1,
+            bun.argv.len -| 1,
         ) catch bun.outOfMemory();
         defer allocator.free(args);
         var used: usize = 0;
         const offset = 1;
 
-        for (bun.argv()[@min(bun.argv().len, offset)..]) |arg| {
+        for (bun.argv[@min(bun.argv.len, offset)..]) |arg| {
             if (arg.len == 0)
                 continue;