Define apicheck function to use a Zig panic

[natecraddock]

From a quick glance at the Lua sources, it may be possible to override the use of assert.h for the API check with a custom function. If so, this would make api checks much more easy to debug with zig panics and stack traces.

And while we are here, should we make it possible to define LUA_USE_APICHECK manually (so it can be always on, or on in all safe modes?)

[robbielyman]

this is doable from consumer code:

pub fn main() void {
    // ...
    var act: std.posix.Sigaction = .{
        .handler = .{ .handler = handleAbrt },
        .mask = switch (builtin.os.tag) {
            .macos => 0,
            .linux => std.posix.empty_sigset,
            else => @compileError("os not supported"),
        },
        .flags = 0,
    };
    std.posix.sigaction(std.posix.SIG.ABRT, &act, null) catch {}; // if it doesn't work, we just crash the old way lol
    // ...
}

fn handleAbrt(_: c_int) callconv(.C) noreturn {
    @call(.always_inline, std.debug.panic, .{ "assertion failed!!", .{} });
}

[robbielyman]

here's a slightly better solution (zig 0.13, tested only on macOS so far):

pub fn main() void {
    // ...
    const act: if (builtin.mode == .Debug) std.posix.Sigaction = if (builtin.mode == .Debug) .{
        .handler = .{
            .handler = struct {
                fn handleAbrt(_: c_int) callconv(.C) noreturn {
                    std.debug.dumpCurrentStackTrace(@returnAddress());
                    std.process.exit(1);
                }
            }.handleAbrt,
         },
         .mask = if (builtin.os.tag == .linux) std.posix.empty_sigset else 0,
         .flags = 0,
     };
     if (builtin.mode == .Debug) try std.posix.sigaction(std.posix.SIG.ABRT, &act, null);
     // ...
}

this has the advantage that a stack trace is dumped in addition to the Lua library's assertion message. one disadvantage is that if other code (like for example std.debug.panicImpl) calls abort, you might see two stack traces. to guard against this, in seamstress v2 i put the stack trace dump inside a conditional which should only succeed if SIGABRT is raised by something other than std.debug.panicImpl

[natecraddock]

I got this working from consumer code a different way

// build.zig
const zlua = b.dependency("zlua", .{ .lua_user_h = b.path("src/user.h") };

// src/user.h
extern void zlua_assert(int e);

#define luai_apicheck(l,e) zlua_assert(e)

// main.zig
export fn zlua_assert(ok: bool) void {
    std.debug.assert(ok);
}

Then I try to access a stack index that is too high

Before

ziglua_test_raise: /home/nathan/dev/sandbox/ziglua-test-raise/zig-pkg/N-V-__8AAKEzFAAA695b9LXBhUSVK5MAV_VKSm1mEj3Acbze/src/lapi.c:64: TValue *index2value(lua_State *, int): Assertion `(idx <= ci->top.p - (ci->func.p + 1)) && "unacceptable index"' failed.
fish: Job 1, 'zig-out/bin/ziglua_test_raise' terminated by signal SIGABRT (Abort)

After

thread 23098 panic: reached unreachable code
/home/nathan/.local/bin/zig-0.16.0/lib/std/debug.zig:420:14: 0x1039aa9 in assert (std.zig)
    if (!ok) unreachable; // assertion failure
             ^
/home/nathan/dev/sandbox/ziglua-test-raise/src/main.zig:7:21: 0x121aa78 in zlua_assert (main.zig)
    std.debug.assert(ok);
                    ^
zig-pkg/N-V-__8AAKEzFAAA695b9LXBhUSVK5MAV_VKSm1mEj3Acbze/src/lapi.c:64:5: 0x121b456 in index2value (/home/nathan/dev/sandbox/ziglua-test-raise/zig-pkg/N-V-__8AAKEzFAAA695b9LXBhUSVK5MAV_VKSm1mEj3Acbze/src/lapi.c)
    api_check(L, idx <= ci->top.p - (ci->func.p + 1), "unacceptable index");
    ^
zig-pkg/N-V-__8AAKEzFAAA695b9LXBhUSVK5MAV_VKSm1mEj3Acbze/src/lapi.c:391:21: 0x121bd93 in lua_tointegerx (/home/nathan/dev/sandbox/ziglua-test-raise/zig-pkg/N-V-__8AAKEzFAAA695b9LXBhUSVK5MAV_VKSm1mEj3Acbze/src/lapi.c)
  const TValue *o = index2value(L, idx);
                    ^
/home/nathan/dev/ziglua/src/lib.zig:2788:48: 0x11e5d8e in toInteger (lib.zig)
                const result = c.lua_tointegerx(@ptrCast(lua), index, &success);
                                               ^
/home/nathan/dev/sandbox/ziglua-test-raise/src/main.zig:22:26: 0x11dfa56 in main (main.zig)
    _ = try lua.toInteger(10000);
                         ^
/home/nathan/.local/bin/zig-0.16.0/lib/std/start.zig:737:30: 0x11e03b7 in callMain (std.zig)
    return wrapMain(root.main(.{
                             ^
../sysdeps/nptl/libc_start_call_main.h:58:16: 0x7c8045a2a1c9 in __libc_start_call_main (../sysdeps/x86/libc-start.c)
../csu/libc-start.c:360:3: 0x7c8045a2a28a in __libc_start_main_impl (../sysdeps/x86/libc-start.c)
???:?:?: 0x121aaa4 in ??? (???)

I think this is very useful. I'd like to find a way to do this without using the user_h and maybe enable it by default

[robbielyman]

I agree, this would be a great addition! i guess the #define can be done with an addition to the compiler flags, but the extern declaration might be trickier.

[natecraddock]

I can think of two ways

1. Clang has a -include flag for #including an additional file before parsing. I've used it before but it feels kind of hacky and I'm not sure if it is integrated with zig build. So this might not actually be an option
2. Create a new run step that outputs the concatenation of the user's custom h file (if any) and the extern and #define and use that as the custom h file.

I think option 2 makes sense and shouldn't be too hard. I'll work on that