Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: overhaul the crash handler #10203

Merged
merged 35 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2a633fa
some work
paperdave Apr 10, 2024
3b33c49
linux things
paperdave Apr 10, 2024
0b6b865
linux things
paperdave Apr 10, 2024
fa34944
feat: tracestrings on Windows
paperdave Apr 11, 2024
79af8d2
bwaa
paperdave Apr 10, 2024
eb03a28
Merge remote-tracking branch 'origin/main' into dave/crash-handler
paperdave Apr 12, 2024
a9912a5
more work on the crash handler
paperdave Apr 12, 2024
c021476
Merge branch 'dave/crash-handler' of github.com:oven-sh/bun into dave…
paperdave Apr 12, 2024
455ff79
okay
paperdave Apr 12, 2024
3e5c927
adgadsgbcxcv
paperdave Apr 12, 2024
f906a08
ya
paperdave Apr 12, 2024
3e53dbb
dsafds
paperdave Apr 12, 2024
256f434
Merge remote-tracking branch 'origin/main' into dave/crash-handler
paperdave Apr 12, 2024
fb77fee
a
paperdave Apr 12, 2024
f9d1ee6
wuh
paperdave Apr 13, 2024
82e3894
Merge remote-tracking branch 'origin/main' into dave/crash-handler
paperdave Apr 13, 2024
cbd5103
a
paperdave Apr 13, 2024
2d1c830
bru h
paperdave Apr 13, 2024
2f666c6
ok
paperdave Apr 15, 2024
a5c9979
yay
paperdave Apr 16, 2024
7cc66c8
Merge remote-tracking branch 'origin/main' into dave/crash-handler
paperdave Apr 16, 2024
1b48b1d
window
paperdave Apr 15, 2024
f7dcade
Merge remote-tracking branch 'origin/main' into dave/crash-handler
paperdave Apr 16, 2024
a3c06e0
alright
paperdave Apr 16, 2024
b730add
oops
paperdave Apr 17, 2024
eb43700
yeah
paperdave Apr 17, 2024
cab8ebe
a
paperdave Apr 17, 2024
304fb98
Merge remote-tracking branch 'origin/main' into dave/crash-handler
paperdave Apr 17, 2024
fe2055a
a
paperdave Apr 17, 2024
cc5aa4d
OOM handling
paperdave Apr 17, 2024
cc4fe81
hi
paperdave Apr 17, 2024
907b900
review comments
paperdave Apr 17, 2024
739dbd6
fix on window
paperdave Apr 18, 2024
141127f
Merge branch 'dave/crash-handler' of github.com:oven-sh/bun into dave…
paperdave Apr 18, 2024
c9748fa
Merge remote-tracking branch 'origin/main' into dave/crash-handler
paperdave Apr 17, 2024
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
20 changes: 20 additions & 0 deletions .github/ISSUE_TEMPLATE/6-crash-report.yml
Original file line number Diff line number Diff line change
@@ -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.
9 changes: 5 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down
4 changes: 2 additions & 2 deletions build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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.*);
dylan-conway marked this conversation as resolved.
Show resolved Hide resolved
}

return err;
Expand Down Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions packages/bun-internal-test/src/banned.json
Original file line number Diff line number Diff line change
@@ -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.",
"": ""
}
16 changes: 9 additions & 7 deletions src/__global.zig → src/Global.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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{
Expand All @@ -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();
}

Expand Down
6 changes: 3 additions & 3 deletions src/StandaloneModuleGraph.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
1 change: 0 additions & 1 deletion src/analytics.zig

This file was deleted.

13 changes: 10 additions & 3 deletions src/analytics/analytics_thread.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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});
Expand All @@ -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("\" ");

Expand All @@ -115,8 +124,6 @@ pub const Features = struct {
try writer.writeAll("\" ");
}

try writer.writeAll("\n");
} else {
try writer.writeAll("\n");
}
}
Expand Down
10 changes: 0 additions & 10 deletions src/bun.js/api/BunObject.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/api/bun/process.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions src/bun.js/bindings/bindings.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
};
}
Expand Down
2 changes: 0 additions & 2 deletions src/bun.js/bindings/exports.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down
32 changes: 0 additions & 32 deletions src/bun.js/bindings/wtf-bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 6 additions & 2 deletions src/bun.js/javascript.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Expand Down Expand Up @@ -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;
}

Expand Down
7 changes: 0 additions & 7 deletions src/bun.js/module_loader.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/bun.js/node/types.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;

Expand Down
Loading
Loading