Skip to content

Add sigaltstack syscall for Linux #2546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions std/c/darwin.zig
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,5 @@ pub extern "c" fn mach_port_deallocate(task: ipc_space_t, name: mach_port_name_t
pub fn sigaddset(set: *sigset_t, signo: u5) void {
set.* |= u32(1) << (signo - 1);
}

pub extern "c" fn sigaltstack(ss: ?*stack_t, old_ss: ?*stack_t) c_int;
4 changes: 4 additions & 0 deletions std/c/freebsd.zig
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
const std = @import("../std.zig");
use std.c;

extern "c" fn __error() *c_int;
pub const _errno = __error;

pub extern "c" fn getdents(fd: c_int, buf_ptr: [*]u8, nbytes: usize) usize;
pub extern "c" fn sigaltstack(ss: ?*stack_t, old_ss: ?*stack_t) c_int;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow I didn't catch this the first time.

Suggested change
pub extern "c" fn sigaltstack(ss: ?*stack_t, old_ss: ?*stack_t) c_int;
pub extern "c" fn signaltstack(ss: ?*stack_t, old_ss: ?*stack_t) c_int;

(the other ones too)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait what? It's signal alternate stack also in FreeBSD OpenBSD and Linux

Copy link
Member

@andrewrk andrewrk May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my mistake. I saw /home/vsts/work/1/s/std/os.zig:2458:25: error: container 'c' has no member called 'sigaltstack' and made an incorrect assumption.

So this looks like a bug with use then? damn

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this looks like a bug with use then? damn

ah right of course. it was just the same darwin bug on all 3 of the CI because they each test cross compilation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this looks like a bug with use then? damn

Nope, I just forgot to add the declaration in darwin.zig

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well I feel silly. sorry for all the noise

2 changes: 2 additions & 0 deletions std/c/linux.zig
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,5 @@ pub extern "c" fn getauxval(__type: c_ulong) c_ulong;

pub const dl_iterate_phdr_callback = extern fn (info: *dl_phdr_info, size: usize, data: ?*c_void) c_int;
pub extern "c" fn dl_iterate_phdr(callback: dl_iterate_phdr_callback, data: ?*c_void) c_int;

pub extern "c" fn sigaltstack(ss: ?*stack_t, old_ss: ?*stack_t) c_int;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these can go in std/c.zig yeah? they're the same on every platform

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are, but they're not supported by every platform and that's why they've been duplicated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that's better than a link error if accidentally used. Makes sense to me.

4 changes: 4 additions & 0 deletions std/c/netbsd.zig
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
const std = @import("../std.zig");
use std.c;

extern "c" fn __errno() *c_int;
pub const _errno = __errno;

pub extern "c" fn getdents(fd: c_int, buf_ptr: [*]u8, nbytes: usize) usize;
pub extern "c" fn sigaltstack(ss: ?*stack_t, old_ss: ?*stack_t) c_int;
23 changes: 23 additions & 0 deletions std/os.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2433,6 +2433,29 @@ pub fn unexpectedErrno(err: usize) UnexpectedError {
return error.Unexpected;
}

pub const SigaltstackError = error{
/// The supplied stack size was less than MINSIGSTKSZ.
SizeTooSmall,

/// Attempted to change the signal stack while it was active.
PermissionDenied,
Unexpected,
};

pub fn sigaltstack(ss: ?*stack_t, old_ss: ?*stack_t) SigaltstackError!void {
if (windows.is_the_target or uefi.is_the_target or wasi.is_the_target)
@compileError("std.os.sigaltstack not available for this target");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this if and explicit compile error is just fine, but not required. everywhere else just lets the natural compile error happen if you call the function on an unsupported target.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicit is better than implicit, I believe that goes for error messages too.


switch (errno(system.sigaltstack(ss, old_ss))) {
0 => return,
EFAULT => unreachable,
EINVAL => unreachable,
ENOMEM => return error.SizeTooSmall,
EPERM => return error.PermissionDenied,
else => |err| return unexpectedErrno(err),
}
}

test "" {
_ = @import("os/darwin.zig");
_ = @import("os/freebsd.zig");
Expand Down
12 changes: 12 additions & 0 deletions std/os/bits/darwin.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1078,3 +1078,15 @@ pub const EQFULL = 106;

/// Must be equal largest errno
pub const ELAST = 106;

pub const SIGSTKSZ = 131072;
pub const MINSIGSTKSZ = 32768;

pub const SS_ONSTACK = 1;
pub const SS_DISABLE = 4;

pub const stack_t = extern struct {
ss_sp: [*]u8,
ss_size: isize,
ss_flags: i32,
};
16 changes: 16 additions & 0 deletions std/os/bits/freebsd.zig
Original file line number Diff line number Diff line change
Expand Up @@ -835,3 +835,19 @@ pub const ENOTRECOVERABLE = 95; // State not recoverable
pub const EOWNERDEAD = 96; // Previous owner died

pub const ELAST = 96; // Must be equal largest errno

pub const MINSIGSTKSZ = switch (builtin.arch) {
.i386, .x86_64 => 2048,
.arm, .aarch64 => 4096,
else => @compileError("MINSIGSTKSZ not defined for this architecture"),
};
pub const SIGSTKSZ = MINSIGSTKSZ + 32768;

pub const SS_ONSTACK = 1;
pub const SS_DISABLE = 4;

pub const stack_t = extern struct {
ss_sp: [*]u8,
ss_size: isize,
ss_flags: i32,
};
21 changes: 21 additions & 0 deletions std/os/bits/linux.zig
Original file line number Diff line number Diff line change
Expand Up @@ -929,3 +929,24 @@ pub fn CPU_COUNT(set: cpu_set_t) cpu_count_t {
//#define CPU_COUNT(set) CPU_COUNT_S(sizeof(cpu_set_t),set)
//#define CPU_ZERO(set) CPU_ZERO_S(sizeof(cpu_set_t),set)
//#define CPU_EQUAL(s1,s2) CPU_EQUAL_S(sizeof(cpu_set_t),s1,s2)

pub const MINSIGSTKSZ = switch (builtin.arch) {
.i386, .x86_64, .arm => 2048,
.aarch64 => 5120,
else => @compileError("MINSIGSTKSZ not defined for this architecture"),
};
pub const SIGSTKSZ = switch (builtin.arch) {
.i386, .x86_64, .arm => 8192,
.aarch64 => 16384,
else => @compileError("SIGSTKSZ not defined for this architecture"),
};

pub const SS_ONSTACK = 1;
pub const SS_DISABLE = 2;
pub const SS_AUTODISARM = 1 << 31;

pub const stack_t = extern struct {
ss_sp: [*]u8,
ss_flags: i32,
ss_size: isize,
};
12 changes: 12 additions & 0 deletions std/os/bits/netbsd.zig
Original file line number Diff line number Diff line change
Expand Up @@ -723,3 +723,15 @@ pub const ENOLINK = 95; // Link has been severed
pub const EPROTO = 96; // Protocol error

pub const ELAST = 96; // Must equal largest errno

pub const MINSIGSTKSZ = 8192;
pub const SIGSTKSZ = MINSIGSTKSZ + 32768;

pub const SS_ONSTACK = 1;
pub const SS_DISABLE = 4;

pub const stack_t = extern struct {
ss_sp: [*]u8,
ss_size: isize,
ss_flags: i32,
};
4 changes: 4 additions & 0 deletions std/os/linux.zig
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,10 @@ pub fn capset(hdrp: *cap_user_header_t, datap: *const cap_user_data_t) usize {
return syscall2(SYS_capset, @ptrToInt(hdrp), @ptrToInt(datap));
}

pub fn sigaltstack(ss: ?*stack_t, old_ss: ?*stack_t) usize {
return syscall2(SYS_sigaltstack, @ptrToInt(ss), @ptrToInt(old_ss));
}

// XXX: This should be weak
extern const __ehdr_start: elf.Ehdr = undefined;

Expand Down
11 changes: 11 additions & 0 deletions std/os/test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,14 @@ test "realpath" {
var buf: [std.fs.MAX_PATH_BYTES]u8 = undefined;
testing.expectError(error.FileNotFound, fs.realpath("definitely_bogus_does_not_exist1234", &buf));
}

test "sigaltstack" {
Copy link
Member

@andrewrk andrewrk May 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's misspelled here too and below oops. my mistake

if (builtin.os == .windows or builtin.os == .wasi) return error.SkipZigTest;

var st: os.stack_t = undefined;
try os.sigaltstack(null, &st);
// Setting a stack size less than MINSIGSTKSZ returns ENOMEM
st.ss_flags = 0;
st.ss_size = 1;
testing.expectError(error.SizeTooSmall, os.sigaltstack(&st, null));
}
4 changes: 4 additions & 0 deletions std/os/uefi.zig
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
// TODO this is where the extern declarations go. For example, see
// inc/efilib.h in gnu-efi-code

const builtin = @import("builtin");

pub const is_the_target = builtin.os == .uefi;