Skip to content

Commit 4b63f94

Browse files
rootbeeralexrp
authored andcommitted
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.
1 parent a9ff2d5 commit 4b63f94

File tree

2 files changed

+62
-41
lines changed

2 files changed

+62
-41
lines changed

lib/std/os/linux.zig

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1786,25 +1786,41 @@ pub fn sigaction(sig: u6, noalias act: ?*const Sigaction, noalias oact: ?*Sigact
17861786

17871787
const usize_bits = @typeInfo(usize).int.bits;
17881788

1789-
pub fn sigaddset(set: *sigset_t, sig: u6) void {
1790-
const s = sig - 1;
1791-
// shift in musl: s&8*sizeof *set->__bits-1
1792-
const shift = @as(u5, @intCast(s & (usize_bits - 1)));
1793-
const val = @as(u32, @intCast(1)) << shift;
1794-
(set.*)[@as(usize, @intCast(s)) / usize_bits] |= val;
1789+
pub const sigset_t = [1024 / 32]u32;
1790+
1791+
const sigset_len = @typeInfo(sigset_t).array.len;
1792+
1793+
/// Empty set to initialize sigset_t instances from.
1794+
pub const empty_sigset: sigset_t = [_]u32{0} ** sigset_len;
1795+
1796+
pub const filled_sigset: sigset_t = [_]u32{0x7fff_ffff} ++ [_]u32{0} ** (sigset_len - 1);
1797+
1798+
pub const all_mask: sigset_t = [_]u32{0xffff_ffff} ** sigset_len;
1799+
1800+
fn sigset_bit_index(sig: usize) struct { word: usize, mask: u32 } {
1801+
assert(sig > 0);
1802+
assert(sig < NSIG);
1803+
const bit = sig - 1;
1804+
const shift = @as(u5, @truncate(bit % 32));
1805+
return .{
1806+
.word = bit / 32,
1807+
.mask = @as(u32, 1) << shift,
1808+
};
17951809
}
17961810

1797-
pub fn sigdelset(set: *sigset_t, sig: u6) void {
1798-
const s = sig - 1;
1799-
// shift in musl: s&8*sizeof *set->__bits-1
1800-
const shift = @as(u5, @intCast(s & (usize_bits - 1)));
1801-
const val = @as(u32, @intCast(1)) << shift;
1802-
(set.*)[@as(usize, @intCast(s)) / usize_bits] ^= val;
1811+
pub fn sigaddset(set: *sigset_t, sig: usize) void {
1812+
const index = sigset_bit_index(sig);
1813+
(set.*)[index.word] |= index.mask;
18031814
}
18041815

1805-
pub fn sigismember(set: *const sigset_t, sig: u6) bool {
1806-
const s = sig - 1;
1807-
return ((set.*)[@as(usize, @intCast(s)) / usize_bits] & (@as(usize, @intCast(1)) << @intCast(s & (usize_bits - 1)))) != 0;
1816+
pub fn sigdelset(set: *sigset_t, sig: usize) void {
1817+
const index = sigset_bit_index(sig);
1818+
(set.*)[index.word] ^= index.mask;
1819+
}
1820+
1821+
pub fn sigismember(set: *const sigset_t, sig: usize) bool {
1822+
const index = sigset_bit_index(sig);
1823+
return ((set.*)[index.word] & index.mask) != 0;
18081824
}
18091825

18101826
pub fn getsockname(fd: i32, noalias addr: *sockaddr, noalias len: *socklen_t) usize {
@@ -5402,10 +5418,6 @@ pub const TFD = switch (native_arch) {
54025418
/// As signal numbers are sequential, NSIG is one greater than the largest defined signal number.
54035419
pub const NSIG = if (is_mips) 128 else 65;
54045420

5405-
pub const sigset_t = [1024 / 32]u32;
5406-
5407-
pub const all_mask: sigset_t = [_]u32{0xffffffff} ** @typeInfo(sigset_t).array.len;
5408-
54095421
const k_sigaction_funcs = struct {
54105422
const handler = ?*align(1) const fn (i32) callconv(.c) void;
54115423
const restorer = *const fn () callconv(.c) void;
@@ -5446,10 +5458,6 @@ pub const Sigaction = extern struct {
54465458
restorer: ?*const fn () callconv(.c) void = null,
54475459
};
54485460

5449-
const sigset_len = @typeInfo(sigset_t).array.len;
5450-
pub const empty_sigset = [_]u32{0} ** sigset_len;
5451-
pub const filled_sigset = [_]u32{(1 << (31 & (usize_bits - 1))) - 1} ++ [_]u32{0} ** (sigset_len - 1);
5452-
54535461
pub const SFD = struct {
54545462
pub const CLOEXEC = 1 << @bitOffsetOf(O, "CLOEXEC");
54555463
pub const NONBLOCK = 1 << @bitOffsetOf(O, "NONBLOCK");

lib/std/os/linux/test.zig

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -128,28 +128,41 @@ test "fadvise" {
128128
test "sigset_t" {
129129
var sigset = linux.empty_sigset;
130130

131-
try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), false);
132-
try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), false);
133-
134-
linux.sigaddset(&sigset, linux.SIG.USR1);
135-
136-
try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), true);
137-
try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), false);
138-
139-
linux.sigaddset(&sigset, linux.SIG.USR2);
140-
141-
try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), true);
142-
try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), true);
131+
// See that none are set, then set each one, see that they're all set, then
132+
// remove them all, and then see that none are set.
133+
for (1..linux.NSIG) |i| {
134+
try expectEqual(linux.sigismember(&sigset, @truncate(i)), false);
135+
}
136+
for (1..linux.NSIG) |i| {
137+
linux.sigaddset(&sigset, @truncate(i));
138+
}
139+
for (1..linux.NSIG) |i| {
140+
try expectEqual(linux.sigismember(&sigset, @truncate(i)), true);
141+
try expectEqual(linux.sigismember(&linux.empty_sigset, @truncate(i)), false);
142+
}
143+
for (1..linux.NSIG) |i| {
144+
linux.sigdelset(&sigset, @truncate(i));
145+
}
146+
for (1..linux.NSIG) |i| {
147+
try expectEqual(linux.sigismember(&sigset, @truncate(i)), false);
148+
}
143149

144-
linux.sigdelset(&sigset, linux.SIG.USR1);
150+
linux.sigaddset(&sigset, 1);
151+
try expectEqual(sigset[0], 1);
152+
try expectEqual(sigset[1], 0);
145153

146-
try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), false);
147-
try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), true);
154+
linux.sigaddset(&sigset, 31);
155+
try expectEqual(sigset[0], 0x4000_0001);
156+
try expectEqual(sigset[1], 0);
148157

149-
linux.sigdelset(&sigset, linux.SIG.USR2);
158+
linux.sigaddset(&sigset, 36);
159+
try expectEqual(sigset[0], 0x4000_0001);
160+
try expectEqual(sigset[1], 0x8);
150161

151-
try expectEqual(linux.sigismember(&sigset, linux.SIG.USR1), false);
152-
try expectEqual(linux.sigismember(&sigset, linux.SIG.USR2), false);
162+
linux.sigaddset(&sigset, 64);
163+
try expectEqual(sigset[0], 0x4000_0001);
164+
try expectEqual(sigset[1], 0x8000_0008);
165+
try expectEqual(sigset[2], 0);
153166
}
154167

155168
test {

0 commit comments

Comments
 (0)