From 4b63f94b4ecf282c259c24a09d6c4ab7490b6cab Mon Sep 17 00:00:00 2001 From: Pat Tullmann Date: Mon, 7 Apr 2025 22:21:03 -0700 Subject: [PATCH] Fix sigaddset/sigdelset bit-fiddling math The code was using u32 and usize interchangably, which doesn't work on 64-bit systems. This: `pub const sigset_t = [1024 / 32]u32;` is not consistent with this: `const shift = @as(u5, @intCast(s & (usize_bits - 1)));` However, normal signal numbers are less than 31, so the bad math doesn't matter much. Also, despite support for 1024 signals in the set, only setting signals between 1 and NSIG (which is mostly 65, but sometimes 128) is defined. The existing tests only exercised signal numbers in the first 31 bits so they didn't trip over this: The C library `sigaddset` will return `EINVAL` if given an out of bounds signal number. I made the Zig code just silently ignore any out of bounds signal numbers. Moved all the `sigset` related declarations next to each in the source, too. The `filled_sigset` seems non-standard to me. I think it is meant to be used like `empty_sigset`, but it only contains 31 set signals, which seems wrong (should be 64 or 128, aka `NSIG`). It's also unused. The oddly named but similar `all_mask` is used (by posix.zig) but sets all 1024 bits (which I understood to be undefined behavior but seems to work just fine). For comparison the musl `sigfillset` fills in 65 bits or 128 bits. --- lib/std/os/linux.zig | 54 ++++++++++++++++++++++----------------- lib/std/os/linux/test.zig | 49 ++++++++++++++++++++++------------- 2 files changed, 62 insertions(+), 41 deletions(-) diff --git a/lib/std/os/linux.zig b/lib/std/os/linux.zig index 60885d1ba5..6ede68e71a 100644 --- a/lib/std/os/linux.zig +++ b/lib/std/os/linux.zig @@ -1786,25 +1786,41 @@ pub fn sigaction(sig: u6, noalias act: ?*const Sigaction, noalias oact: ?*Sigact const usize_bits = @typeInfo(usize).int.bits; -pub fn sigaddset(set: *sigset_t, sig: u6) void { - const s = sig - 1; - // shift in musl: s&8*sizeof *set->__bits-1 - const shift = @as(u5, @intCast(s & (usize_bits - 1))); - const val = @as(u32, @intCast(1)) << shift; - (set.*)[@as(usize, @intCast(s)) / usize_bits] |= val; +pub const sigset_t = [1024 / 32]u32; + +const sigset_len = @typeInfo(sigset_t).array.len; + +/// Empty set to initialize sigset_t instances from. +pub const empty_sigset: sigset_t = [_]u32{0} ** sigset_len; + +pub const filled_sigset: sigset_t = [_]u32{0x7fff_ffff} ++ [_]u32{0} ** (sigset_len - 1); + +pub const all_mask: sigset_t = [_]u32{0xffff_ffff} ** sigset_len; + +fn sigset_bit_index(sig: usize) struct { word: usize, mask: u32 } { + assert(sig > 0); + assert(sig < NSIG); + const bit = sig - 1; + const shift = @as(u5, @truncate(bit % 32)); + return .{ + .word = bit / 32, + .mask = @as(u32, 1) << shift, + }; } -pub fn sigdelset(set: *sigset_t, sig: u6) void { - const s = sig - 1; - // shift in musl: s&8*sizeof *set->__bits-1 - const shift = @as(u5, @intCast(s & (usize_bits - 1))); - const val = @as(u32, @intCast(1)) << shift; - (set.*)[@as(usize, @intCast(s)) / usize_bits] ^= val; +pub fn sigaddset(set: *sigset_t, sig: usize) void { + const index = sigset_bit_index(sig); + (set.*)[index.word] |= index.mask; } -pub fn sigismember(set: *const sigset_t, sig: u6) bool { - const s = sig - 1; - return ((set.*)[@as(usize, @intCast(s)) / usize_bits] & (@as(usize, @intCast(1)) << @intCast(s & (usize_bits - 1)))) != 0; +pub fn sigdelset(set: *sigset_t, sig: usize) void { + const index = sigset_bit_index(sig); + (set.*)[index.word] ^= index.mask; +} + +pub fn sigismember(set: *const sigset_t, sig: usize) bool { + const index = sigset_bit_index(sig); + return ((set.*)[index.word] & index.mask) != 0; } pub fn getsockname(fd: i32, noalias addr: *sockaddr, noalias len: *socklen_t) usize { @@ -5402,10 +5418,6 @@ pub const TFD = switch (native_arch) { /// As signal numbers are sequential, NSIG is one greater than the largest defined signal number. pub const NSIG = if (is_mips) 128 else 65; -pub const sigset_t = [1024 / 32]u32; - -pub const all_mask: sigset_t = [_]u32{0xffffffff} ** @typeInfo(sigset_t).array.len; - const k_sigaction_funcs = struct { const handler = ?*align(1) const fn (i32) callconv(.c) void; const restorer = *const fn () callconv(.c) void; @@ -5446,10 +5458,6 @@ pub const Sigaction = extern struct { restorer: ?*const fn () callconv(.c) void = null, }; -const sigset_len = @typeInfo(sigset_t).array.len; -pub const empty_sigset = [_]u32{0} ** sigset_len; -pub const filled_sigset = [_]u32{(1 << (31 & (usize_bits - 1))) - 1} ++ [_]u32{0} ** (sigset_len - 1); - pub const SFD = struct { pub const CLOEXEC = 1 << @bitOffsetOf(O, "CLOEXEC"); pub const NONBLOCK = 1 << @bitOffsetOf(O, "NONBLOCK"); diff --git a/lib/std/os/linux/test.zig b/lib/std/os/linux/test.zig index 6d2dbe8bc9..27d156e9cc 100644 --- a/lib/std/os/linux/test.zig +++ b/lib/std/os/linux/test.zig @@ -128,28 +128,41 @@ test "fadvise" { test "sigset_t" { var sigset = linux.empty_sigset; - try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), false); - try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), false); + // See that none are set, then set each one, see that they're all set, then + // remove them all, and then see that none are set. + for (1..linux.NSIG) |i| { + try expectEqual(linux.sigismember(&sigset, @truncate(i)), false); + } + for (1..linux.NSIG) |i| { + linux.sigaddset(&sigset, @truncate(i)); + } + for (1..linux.NSIG) |i| { + try expectEqual(linux.sigismember(&sigset, @truncate(i)), true); + try expectEqual(linux.sigismember(&linux.empty_sigset, @truncate(i)), false); + } + for (1..linux.NSIG) |i| { + linux.sigdelset(&sigset, @truncate(i)); + } + for (1..linux.NSIG) |i| { + try expectEqual(linux.sigismember(&sigset, @truncate(i)), false); + } - linux.sigaddset(&sigset, linux.SIG.USR1); + linux.sigaddset(&sigset, 1); + try expectEqual(sigset[0], 1); + try expectEqual(sigset[1], 0); - try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), true); - try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), false); + linux.sigaddset(&sigset, 31); + try expectEqual(sigset[0], 0x4000_0001); + try expectEqual(sigset[1], 0); - linux.sigaddset(&sigset, linux.SIG.USR2); + linux.sigaddset(&sigset, 36); + try expectEqual(sigset[0], 0x4000_0001); + try expectEqual(sigset[1], 0x8); - try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), true); - try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), true); - - linux.sigdelset(&sigset, linux.SIG.USR1); - - try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), false); - try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), true); - - linux.sigdelset(&sigset, linux.SIG.USR2); - - try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), false); - try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), false); + linux.sigaddset(&sigset, 64); + try expectEqual(sigset[0], 0x4000_0001); + try expectEqual(sigset[1], 0x8000_0008); + try expectEqual(sigset[2], 0); } test {