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

fix: make raiseIgnoringPanicHandler ignore the panic handler #12578

Merged
merged 6 commits into from
Jul 27, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .vscode/launch.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 3 additions & 18 deletions src/Global.zig
Original file line number Diff line number Diff line change
Expand Up @@ -119,27 +119,12 @@ pub fn exit(code: u32) noreturn {
bun.C.quick_exit(@bitCast(code));
}

pub fn raiseIgnoringPanicHandler(sig: anytype) noreturn {
if (comptime @TypeOf(sig) == bun.SignalCode) {
return raiseIgnoringPanicHandler(@intFromEnum(sig));
}

pub fn raiseIgnoringPanicHandler(sig: bun.SignalCode) noreturn {
Output.flush();

if (!Environment.isWindows) {
if (sig >= 1 and sig != std.posix.SIG.STOP and sig != std.posix.SIG.KILL) {
const act = std.posix.Sigaction{
.handler = .{ .sigaction = @ptrCast(@alignCast(std.posix.SIG.DFL)) },
.mask = std.posix.empty_sigset,
.flags = 0,
};
std.posix.sigaction(@intCast(sig), &act, null) catch {};
}
}

Output.Source.Stdio.restore();

_ = std.c.raise(sig);
bun.crash_handler.resetSegfaultHandler();
_ = std.c.raise(@intFromEnum(sig));
std.c.abort();
}

Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/WebKit
Submodule WebKit updated 1 files
+1 −0 mac-release.bash
6 changes: 2 additions & 4 deletions src/cli/run_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -578,8 +578,7 @@ pub const RunCommand = struct {
});
}

Output.flush();
Global.raiseIgnoringPanicHandler(@intFromEnum(signal));
Global.raiseIgnoringPanicHandler(signal);
},

.exited => |exit_code| {
Expand All @@ -592,8 +591,7 @@ pub const RunCommand = struct {
});
}

Output.flush();
Global.raiseIgnoringPanicHandler(@intFromEnum(exit_code.signal));
Global.raiseIgnoringPanicHandler(exit_code.signal);
}

const code = exit_code.code;
Expand Down
8 changes: 8 additions & 0 deletions src/crash_handler.zig
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ pub fn crashHandler(
break :check_flag false;
}
}
// Act like release build when explicitly enabling reporting
if (isReportingEnabled()) break :check_flag false;

break :check_flag true;
};

Expand Down Expand Up @@ -1562,6 +1565,7 @@ pub const js_bindings = struct {
.{ "panic", jsPanic },
.{ "rootError", jsRootError },
.{ "outOfMemory", jsOutOfMemory },
.{ "raiseIgnoringPanicHandler", jsRaiseIgnoringPanicHandler },
}) |tuple| {
const name = JSC.ZigString.static(tuple[0]);
obj.put(global, name, JSC.createCallback(global, name, 1, tuple[1]));
Expand Down Expand Up @@ -1599,6 +1603,10 @@ pub const js_bindings = struct {
bun.outOfMemory();
}

pub fn jsRaiseIgnoringPanicHandler(_: *JSC.JSGlobalObject, _: *JSC.CallFrame) JSC.JSValue {
bun.Global.raiseIgnoringPanicHandler(.SIGSEGV);
}

pub fn jsGetFeaturesAsVLQ(global: *JSC.JSGlobalObject, _: *JSC.CallFrame) JSC.JSValue {
const bits = bun.Analytics.packedFeatures();
var buf = std.BoundedArray(u8, 16){};
Expand Down
1 change: 0 additions & 1 deletion src/fd.zig
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ pub const FDImpl = packed struct {
if (env.isDebug) {
if (result) |err| {
if (err.errno == @intFromEnum(posix.E.BADF)) {
// TODO(@paperdave): Zig Compiler Bug, if you remove `this` from the log. An error is correctly printed, but with the wrong reference trace
bun.Output.debugWarn("close({s}) = EBADF. This is an indication of a file descriptor UAF", .{this_fmt});
} else {
log("close({s}) = {}", .{ this_fmt, err });
Expand Down
9 changes: 4 additions & 5 deletions src/install/lifecycle_script_runner.zig
Original file line number Diff line number Diff line change
Expand Up @@ -353,15 +353,15 @@ pub const LifecycleScriptSubprocess = struct {
},
.signaled => |signal| {
this.printOutput();
const signal_code = bun.SignalCode.from(signal);

Output.prettyErrorln("<r><red>error<r><d>:<r> <b>{s}<r> script from \"<b>{s}<r>\" terminated by {}<r>", .{
this.scriptName(),
this.package_name,

bun.SignalCode.from(signal).fmt(Output.enable_ansi_colors_stderr),
signal_code.fmt(Output.enable_ansi_colors_stderr),
});
Global.raiseIgnoringPanicHandler(@intFromEnum(signal));

return;
Global.raiseIgnoringPanicHandler(signal);
},
.err => |err| {
Output.prettyErrorln("<r><red>error<r>: Failed to run <b>{s}<r> script from \"<b>{s}<r>\" due to\n{}", .{
Expand All @@ -372,7 +372,6 @@ pub const LifecycleScriptSubprocess = struct {
this.deinit();
Output.flush();
Global.exit(1);
return;
},
else => {
Output.panic("<r><red>error<r>: Failed to run <b>{s}<r> script from \"<b>{s}<r>\" due to unexpected status\n{any}", .{
Expand Down
1 change: 1 addition & 0 deletions src/js/internal-for-testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const crash_handler = $zig("crash_handler.zig", "js_bindings.generate") a
panic: () => void;
rootError: () => void;
outOfMemory: () => void;
raiseIgnoringPanicHandler: () => void;
};

export const upgrade_test_helpers = $zig("upgrade_command.zig", "upgrade_js_bindings.generate") as {
Expand Down
2 changes: 1 addition & 1 deletion test/cli/run/fixture-crash.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ const approach = process.argv[2];
if (approach in crash_handler) {
crash_handler[approach]();
} else {
console.error("usage: bun fixture-crash.js <segfault|panic|rootError|outOfMemory>");
console.error("usage: bun fixture-crash.js <segfault|panic|rootError|outOfMemory|raiseIgnoringPanicHandler>");
}
110 changes: 68 additions & 42 deletions test/cli/run/run-crash-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,58 +11,84 @@ test.if(process.platform === "darwin")("macOS has the assumed image offset", ()
expect(getMachOImageZeroOffset()).toBe(0x100000000);
});

test("raise ignoring panic handler does not trigger the panic handler", async () => {
let sent = false;
let onresolve = Promise.withResolvers();

using server = Bun.serve({
port: 0,
fetch(request, server) {
sent = true;
onresolve.resolve();
return new Response("OK");
},
});

const proc = Bun.spawn({
cmd: [bunExe(), path.join(import.meta.dir, "fixture-crash.js"), "raiseIgnoringPanicHandler"],
env: mergeWindowEnvs([
bunEnv,
{
BUN_CRASH_REPORT_URL: server.url.toString(),
BUN_ENABLE_CRASH_REPORTING: "1",
},
]),
});
await proc.exited;

await Promise.race([onresolve.promise, Bun.sleep(1000)]);

expect(proc.exitCode).not.toBe(0);
expect(sent).toBe(false);
});

describe("automatic crash reporter", () => {
const has_reporting = process.platform !== "linux";
for (const approach of ["panic", "segfault", "outOfMemory"]) {
test(`${approach} should report`, async () => {
let sent = false;
let onresolve = Promise.withResolvers();

for (const should_report of has_reporting ? [true, false] : [false]) {
for (const approach of ["panic", "segfault"]) {
// TODO: this dependency injection no worky. fix later
test.todo(`${approach} ${should_report ? "should" : "should not"} report`, async () => {
const temp = tempDirWithFiles("crash-handler-path", {
"curl": ({ root }) => `#!/usr/bin/env bash
echo $@ > ${root}/request.out
`,
"powershell.cmd": ({ root }) => `echo true > ${root}\\request.out
`,
});
// Self host the crash report backend.
using server = Bun.serve({
port: 0,
fetch(request, server) {
expect(request.url).toEndWith("/ack");
sent = true;
onresolve.resolve();
return new Response("OK");
},
});

const env: any = mergeWindowEnvs([
const proc = Bun.spawn({
cmd: [bunExe(), path.join(import.meta.dir, "fixture-crash.js"), approach],
env: mergeWindowEnvs([
bunEnv,
{
...bunEnv,
BUN_CRASH_REPORT_URL: server.url.toString(),
BUN_ENABLE_CRASH_REPORTING: "1",
GITHUB_ACTIONS: undefined,
CI: undefined,
},
{
PATH: temp + path.delimiter + process.env.PATH,
},
]);

if (!should_report) {
env.DO_NOT_TRACK = "1";
}
]),
stdio: ["ignore", "pipe", "pipe"],
});
await proc.exited;

const result = Bun.spawnSync(
[
bunExe(),
path.join(import.meta.dir, "fixture-crash.js"),
approach,
"--debug-crash-handler-use-trace-string",
],
{ env },
);
await Promise.race([onresolve.promise, Bun.sleep(1000)]);

console.log(result.stderr.toString("utf-8"));
try {
expect(result.stderr.toString("utf-8")).toInclude("https://bun.report/");
} catch (e) {
throw e;
}
const stderr = await Bun.readableStreamToText(proc.stderr);

await Bun.sleep(1000);
console.log(stderr);

const did_report = existsSync(path.join(temp, "request.out"));
expect(did_report).toBe(should_report);
});
}
expect(proc.exitCode).not.toBe(0);
expect(stderr).toContain(server.url.toString());
if (approach !== "outOfMemory") {
expect(stderr).toContain("oh no: Bun has crashed. This indicates a bug in Bun, not your code");
} else {
expect(stderr.toLowerCase()).toContain("out of memory");
expect(stderr.toLowerCase()).not.toContain("panic");
}
expect(sent).toBe(true);
});
}
});
Loading