Commit 4b63f94b4e

Pat Tullmann <pat.github@tullmann.org>
2025-04-08 07:21:03
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.
master
1 parent a9ff2d5
Changed files (2)
lib
std
os
linux
test.zig
linux.zig
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);
-
-    linux.sigaddset(&sigset, linux.SIG.USR1);
-
-    try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), true);
-    try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), false);
-
-    linux.sigaddset(&sigset, linux.SIG.USR2);
-
-    try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), true);
-    try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), true);
+    // 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.sigdelset(&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), false);
-    try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), true);
+    linux.sigaddset(&sigset, 31);
+    try expectEqual(sigset[0], 0x4000_0001);
+    try expectEqual(sigset[1], 0);
 
-    linux.sigdelset(&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), 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 {
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");