Skip to content

Commit

Permalink
std.posix: Consider invalid signal numbers to sigaction() to be progr…
Browse files Browse the repository at this point in the history
…ammer error.

The set of signals that cannot have their action changed is documented in POSIX,
and any additional, non-standard signals are documented by the specific OS. I
see no valid reason why EINVAL should be considered an unpredictable error here.
  • Loading branch information
alexrp authored and Igor Stojkovic committed Aug 11, 2024
1 parent 1294f7b commit 9394db8
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 29 deletions.
4 changes: 1 addition & 3 deletions lib/std/Progress.zig
Original file line number Diff line number Diff line change
Expand Up @@ -414,9 +414,7 @@ pub fn start(options: Options) Node {
.mask = posix.empty_sigset,
.flags = (posix.SA.SIGINFO | posix.SA.RESTART),
};
posix.sigaction(posix.SIG.WINCH, &act, null) catch |err| {
std.log.warn("failed to install SIGWINCH signal handler for noticing terminal resizes: {s}", .{@errorName(err)});
};
posix.sigaction(posix.SIG.WINCH, &act, null);
}

if (switch (global_progress.terminal_mode) {
Expand Down
17 changes: 7 additions & 10 deletions lib/std/debug.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2601,11 +2601,11 @@ pub fn maybeEnableSegfaultHandler() void {

var windows_segfault_handle: ?windows.HANDLE = null;

pub fn updateSegfaultHandler(act: ?*const posix.Sigaction) error{OperationNotSupported}!void {
try posix.sigaction(posix.SIG.SEGV, act, null);
try posix.sigaction(posix.SIG.ILL, act, null);
try posix.sigaction(posix.SIG.BUS, act, null);
try posix.sigaction(posix.SIG.FPE, act, null);
pub fn updateSegfaultHandler(act: ?*const posix.Sigaction) void {
posix.sigaction(posix.SIG.SEGV, act, null);
posix.sigaction(posix.SIG.ILL, act, null);
posix.sigaction(posix.SIG.BUS, act, null);
posix.sigaction(posix.SIG.FPE, act, null);
}

/// Attaches a global SIGSEGV handler which calls `@panic("segmentation fault");`
Expand All @@ -2623,9 +2623,7 @@ pub fn attachSegfaultHandler() void {
.flags = (posix.SA.SIGINFO | posix.SA.RESTART | posix.SA.RESETHAND),
};

updateSegfaultHandler(&act) catch {
@panic("unable to install segfault handler, maybe adjust have_segfault_handling_support in std/debug.zig");
};
updateSegfaultHandler(&act);
}

fn resetSegfaultHandler() void {
Expand All @@ -2641,8 +2639,7 @@ fn resetSegfaultHandler() void {
.mask = posix.empty_sigset,
.flags = 0,
};
// To avoid a double-panic, do nothing if an error happens here.
updateSegfaultHandler(&act) catch {};
updateSegfaultHandler(&act);
}

fn handleSegfaultPosix(sig: i32, info: *const posix.siginfo_t, ctx_ptr: ?*anyopaque) callconv(.C) noreturn {
Expand Down
11 changes: 6 additions & 5 deletions lib/std/posix.zig
Original file line number Diff line number Diff line change
Expand Up @@ -685,9 +685,7 @@ pub fn abort() noreturn {
.mask = empty_sigset,
.flags = 0,
};
sigaction(SIG.ABRT, &sigact, null) catch |err| switch (err) {
error.OperationNotSupported => unreachable,
};
sigaction(SIG.ABRT, &sigact, null);

_ = linux.tkill(linux.gettid(), SIG.ABRT);

Expand Down Expand Up @@ -5678,10 +5676,13 @@ pub fn sigaltstack(ss: ?*stack_t, old_ss: ?*stack_t) SigaltstackError!void {
}

/// Examine and change a signal action.
pub fn sigaction(sig: u6, noalias act: ?*const Sigaction, noalias oact: ?*Sigaction) error{OperationNotSupported}!void {
pub fn sigaction(sig: u6, noalias act: ?*const Sigaction, noalias oact: ?*Sigaction) void {
switch (errno(system.sigaction(sig, act, oact))) {
.SUCCESS => return,
.INVAL => return error.OperationNotSupported,
// EINVAL means the signal is either invalid or some signal that cannot have its action
// changed. For POSIX, this means SIGKILL/SIGSTOP. For e.g. Solaris, this also includes the
// non-standard SIGWAITING, SIGCANCEL, and SIGLWP. Either way, programmer error.
.INVAL => unreachable,
else => unreachable,
}
}
Expand Down
12 changes: 6 additions & 6 deletions lib/std/posix/test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -862,10 +862,10 @@ test "sigaction" {
var old_sa: posix.Sigaction = undefined;

// Install the new signal handler.
try posix.sigaction(posix.SIG.USR1, &sa, null);
posix.sigaction(posix.SIG.USR1, &sa, null);

// Check that we can read it back correctly.
try posix.sigaction(posix.SIG.USR1, null, &old_sa);
posix.sigaction(posix.SIG.USR1, null, &old_sa);
try testing.expectEqual(&S.handler, old_sa.handler.sigaction.?);
try testing.expect((old_sa.flags & posix.SA.SIGINFO) != 0);

Expand All @@ -874,26 +874,26 @@ test "sigaction" {
try testing.expect(S.handler_called_count == 1);

// Check if passing RESETHAND correctly reset the handler to SIG_DFL
try posix.sigaction(posix.SIG.USR1, null, &old_sa);
posix.sigaction(posix.SIG.USR1, null, &old_sa);
try testing.expectEqual(posix.SIG.DFL, old_sa.handler.handler);

// Reinstall the signal w/o RESETHAND and re-raise
sa.flags = posix.SA.SIGINFO;
try posix.sigaction(posix.SIG.USR1, &sa, null);
posix.sigaction(posix.SIG.USR1, &sa, null);
try posix.raise(posix.SIG.USR1);
try testing.expect(S.handler_called_count == 2);

// Now set the signal to ignored
sa.handler = .{ .handler = posix.SIG.IGN };
sa.flags = 0;
try posix.sigaction(posix.SIG.USR1, &sa, null);
posix.sigaction(posix.SIG.USR1, &sa, null);

// Re-raise to ensure handler is actually ignored
try posix.raise(posix.SIG.USR1);
try testing.expect(S.handler_called_count == 2);

// Ensure that ignored state is returned when querying
try posix.sigaction(posix.SIG.USR1, null, &old_sa);
posix.sigaction(posix.SIG.USR1, null, &old_sa);
try testing.expectEqual(posix.SIG.IGN, old_sa.handler.handler.?);
}

Expand Down
3 changes: 1 addition & 2 deletions lib/std/start.zig
Original file line number Diff line number Diff line change
Expand Up @@ -638,8 +638,7 @@ fn maybeIgnoreSigpipe() void {
.mask = posix.empty_sigset,
.flags = 0,
};
posix.sigaction(posix.SIG.PIPE, &act, null) catch |err|
std.debug.panic("failed to set noop SIGPIPE handler: {s}", .{@errorName(err)});
posix.sigaction(posix.SIG.PIPE, &act, null);
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/crash_report.zig
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,7 @@ pub fn attachSegfaultHandler() void {
.flags = (posix.SA.SIGINFO | posix.SA.RESTART | posix.SA.RESETHAND),
};

debug.updateSegfaultHandler(&act) catch {
@panic("unable to install segfault handler, maybe adjust have_segfault_handling_support in std/debug.zig");
};
debug.updateSegfaultHandler(&act);
}

fn handleSegfaultPosix(sig: i32, info: *const posix.siginfo_t, ctx_ptr: ?*anyopaque) callconv(.C) noreturn {
Expand Down

0 comments on commit 9394db8

Please sign in to comment.