Skip to content

Audit FreeBSD structs to match header files/ABI #12002

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 1 commit into from
Sep 15, 2022
Merged

Audit FreeBSD structs to match header files/ABI #12002

merged 1 commit into from
Sep 15, 2022

Conversation

The-King-of-Toasters
Copy link
Contributor

@The-King-of-Toasters The-King-of-Toasters commented Jul 5, 2022

Required for #1759.

I wrote a simple project that verified the sizes against the headers at compile time:

const std = @import("std");
// Vendored from std/c/freebsd.zig.
// Change the first line to `@import("std");` to compile
const freebsd = @import("freebsd.zig");
const c = @cImport({
    @cInclude("sys/types.h");
    @cInclude("sys/socket.h");
    @cInclude("sys/event.h");
    @cInclude("sys/umtx.h");
    @cInclude("sys/link_elf.h");
    @cInclude("sys/fcntl.h");
    @cInclude("sys/stat.h");
    @cInclude("sys/timespec.h");
    @cInclude("sys/dirent.h");
    @cInclude("sys/socket.h");
    @cInclude("sys/signal.h");
    @cInclude("sys/ttycom.h");
    @cInclude("sys/ucontext.h");
    @cInclude("sys/stack.h");
    @cInclude("sys/poll.h");

    @cInclude("netinet/in.h");
    @cInclude("sys/un.h");
    @cInclude("netinet/if_ether.h");
    @cInclude("netdb.h");

    @cInclude("pthread.h");
    @cInclude("semaphore.h");
});

fn assertType(comptime ZigType: anytype, comptime CType: anytype) void {
    std.debug.assert(@sizeOf(ZigType) == @sizeOf(CType));
    std.debug.assert(@bitSizeOf(ZigType) == @bitSizeOf(CType));
    std.debug.assert(@alignOf(ZigType) == @alignOf(CType));
}

fn assert(comptime T: []const u8) void {
    return assertType(@field(freebsd, T), @field(c, T));
}

comptime {
    assert("sf_hdtr");
    assert("pthread_mutex_t");
    assert("pthread_cond_t");
    assert("pthread_rwlock_t");
    assert("pthread_attr_t");
    assert("sem_t");
    assert("_umtx_time");
    assertType(freebsd.Kevent, c.struct_kevent);
    assert("dl_phdr_info");
    assertType(freebsd.Flock, c.struct_flock);
    assert("msghdr");
    assertType(freebsd.Stat, c.struct_stat);
    assert("timespec");
    assert("timeval");
    assert("dirent");
    assert("sockaddr");
    assertType(freebsd.sockaddr.in, c.sockaddr_in);
    assertType(freebsd.sockaddr.in6, c.sockaddr_in6);
    assertType(freebsd.sockaddr.un, c.sockaddr_un);
    assert("sigval");
    assert("sigset_t");
    assert("winsize");
    assertType(freebsd.Sigaction, c.struct_sigaction);
    assert("siginfo_t");
    assert("mcontext_t");
    assert("ucontext_t");
    assert("stack_t");
    assert("addrinfo");
    assert("pollfd");
    //     @compileError(std.fmt.comptimePrint("{d} {d}\n", .{
    //         @sizeOf(freebsd.mcontext_t),
    //         @sizeOf(c.mcontext_t),
    //     }));
}

This was ran on a FreeBSD 13.1-RELEASE amd64 system using zig version 0.10.0-dev.2842+d65e248ed.

I added some doc-comments from the headers where I could. Also included is the mcontext_t definition for aarch64 systems when that gets supported down the line.

@The-King-of-Toasters
Copy link
Contributor Author

The-King-of-Toasters commented Jul 5, 2022

I had to change the event loop code to deal with a change in Kevent.udata's type. Would appreciate the feedback from the owner.

EDIT: I'm backing this out, as all Kevent targets have the same issue. This should be dealt with in a followup pr for all of them.

Copy link
Member

@ifreund ifreund left a comment

Choose a reason for hiding this comment

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

Nice work, this is a cool way to catch some errors.

Note that it only works for the target run on though, for example FreeBSD's stat structure seems to have extra fields when targeting i386: https://github.com/freebsd/freebsd-src/blob/d80d73493767111b7569e831a93061014c682274/sys/sys/stat.h#L170

I didn't have time to review everything in depth, but here's a start

The big outliers were `Stat` and `mcontext_t`. Also adds doc-comments
from the headers where possible.
@The-King-of-Toasters
Copy link
Contributor Author

Updating this PR to audit the kinfo_file structure that was added in #12082. I've also backed out the arm mcontext bits, as I couldn't actually get zig to compile on an aarch64 vm. I'll leave that for #3939.

@andrewrk andrewrk merged commit 7ccb94f into ziglang:master Sep 15, 2022
@andrewrk
Copy link
Member

Thank you and nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants