Skip to content

Commit 41eaa41

Browse files
committed
std.os.abort patch cleanups
* move global into function scope * clarify comments * avoid unnecessary usage of std.atomic API * switch on error instead of `catch unreachable` * call linux.gettid() instead of going through higher level API and doing unnecessary casting
1 parent da6e76a commit 41eaa41

File tree

1 file changed

+22
-37
lines changed

1 file changed

+22
-37
lines changed

lib/std/os.zig

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -250,15 +250,6 @@ pub var argv: [][*:0]u8 = if (builtin.link_libc) undefined else switch (builtin.
250250
else => undefined,
251251
};
252252

253-
/// Atomic to guard correct program teardown in abort()
254-
var abort_entered = impl: {
255-
if (builtin.single_threaded) {
256-
break :impl {};
257-
} else {
258-
break :impl std.atomic.Atomic(bool).init(false);
259-
}
260-
};
261-
262253
/// To obtain errno, call this function with the return value of the
263254
/// system function call. For some systems this will obtain the value directly
264255
/// from the return code; for others it will use a thread-local errno variable.
@@ -451,7 +442,7 @@ fn getRandomBytesDevURandom(buf: []u8) !void {
451442
/// Causes abnormal process termination.
452443
/// If linking against libc, this calls the abort() libc function. Otherwise
453444
/// it raises SIGABRT followed by SIGKILL and finally lo
454-
/// assume: Current signal handler for SIGABRT does **not call abort**.
445+
/// Invokes the current signal handler for SIGABRT, if any.
455446
pub fn abort() noreturn {
456447
@setCold(true);
457448
// MSVCRT abort() sometimes opens a popup window which is undesirable, so
@@ -464,49 +455,44 @@ pub fn abort() noreturn {
464455
windows.kernel32.ExitProcess(3);
465456
}
466457
if (!builtin.link_libc and builtin.os.tag == .linux) {
467-
// Linux man page wants to first "unblock SIG.ABRT", but this is a footgun
458+
// The Linux man page says that the libc abort() function
459+
// "first unblocks the SIGABRT signal", but this is a footgun
468460
// for user-defined signal handlers that want to restore some state in
469-
// some program sections and crash in others
470-
471-
// user installed SIGABRT handler is run, if installed
461+
// some program sections and crash in others.
462+
// So, the user-installed SIGABRT handler is run, if present.
472463
raise(SIG.ABRT) catch {};
473464

474-
// disable all signal handlers
465+
// Disable all signal handlers.
475466
sigprocmask(SIG.BLOCK, &linux.all_mask, null);
476467

477-
// ensure teardown by one thread
468+
// Only one thread may proceed to the rest of abort().
478469
if (!builtin.single_threaded) {
479-
while (abort_entered.compareAndSwap(false, true, .SeqCst, .SeqCst)) |_| {}
470+
const global = struct {
471+
var abort_entered: bool = false;
472+
};
473+
while (@cmpxchgWeak(bool, &global.abort_entered, false, true, .SeqCst, .SeqCst)) |_| {}
480474
}
481475

482-
// install default handler to terminate
476+
// Install default handler so that the tkill below will terminate.
483477
const sigact = Sigaction{
484478
.handler = .{ .sigaction = SIG.DFL },
485479
.mask = undefined,
486480
.flags = undefined,
487481
.restorer = undefined,
488482
};
489-
sigaction(SIG.ABRT, &sigact, null) catch unreachable;
483+
sigaction(SIG.ABRT, &sigact, null) catch |err| switch (err) {
484+
error.OperationNotSupported => unreachable,
485+
};
490486

491-
// make sure we have a pending SIGABRT queued
492-
const tid = std.Thread.getCurrentId();
493-
_ = linux.tkill(@intCast(i32, tid), SIG.ABRT);
487+
_ = linux.tkill(linux.gettid(), SIG.ABRT);
494488

495-
// SIG.ABRT signal will run default handler
496489
const sigabrtmask: linux.sigset_t = [_]u32{0} ** 31 ++ [_]u32{1 << (SIG.ABRT - 1)};
497-
sigprocmask(SIG.UNBLOCK, &sigabrtmask, null); // [32]u32
498-
499-
// Beyond this point should be unreachable
500-
501-
// abnormal termination without using signal handler
502-
const nullptr: *allowzero volatile u8 = @intToPtr(*allowzero volatile u8, 0);
503-
nullptr.* = 0;
490+
sigprocmask(SIG.UNBLOCK, &sigabrtmask, null);
504491

505-
// try SIGKILL, which is no abnormal termination as defined by POSIX and ISO C
492+
// Beyond this point should be unreachable.
493+
@intToPtr(*allowzero volatile u8, 0).* = 0;
506494
raise(SIG.KILL) catch {};
507-
508-
// pid 1 might not be signalled in some containers
509-
exit(127);
495+
exit(127); // Pid 1 might not be signalled in some containers.
510496
}
511497
if (builtin.os.tag == .uefi) {
512498
exit(0); // TODO choose appropriate exit code
@@ -5488,13 +5474,12 @@ pub fn sigaction(sig: u6, noalias act: ?*const Sigaction, noalias oact: ?*Sigact
54885474
}
54895475
}
54905476

5491-
/// Set the thread signal mask
5492-
/// Invalid masks are checked in Debug and ReleaseFast
5477+
/// Sets the thread signal mask.
54935478
pub fn sigprocmask(flags: u32, noalias set: ?*const sigset_t, noalias oldset: ?*sigset_t) void {
54945479
switch (errno(system.sigprocmask(flags, set, oldset))) {
54955480
.SUCCESS => return,
54965481
.FAULT => unreachable,
5497-
.INVAL => unreachable, // main purpose: debug InvalidValue error
5482+
.INVAL => unreachable,
54985483
else => unreachable,
54995484
}
55005485
}

0 commit comments

Comments
 (0)