How to check if non-null pointer is not null?

This is weird, but I am exposing my Zig function to C code. My Zig function has a pointer argument (a pointer to function even), and that pointer is non-nullable in the Zig time system:

export fn f(callback: *const fn () callconv(.c) void) callconv(.c) void {
}

But of course I expect the muppet on the other side of FFI boundary (:wave:) to pass null by mistake. So I want to assert that callback isn’t null.

assert(callback != null);

obviously doesn’t type-check.

assert(@intFromPtr(callback) != 0);

works in debug, but gets optimized away in release safe which is expected and desirable … Except for this single case where I do want to have the assert in.

I could declare my arg as nullable, but that’s wrong, because it is non-nullable, and passing null would be a programming error.

Any practical tips here?

1 Like

I would push back a bit on this. Because you are in C, it is nullable. All pointers are by default. So passing null may be an error, but is a possibility. In this case, I think modeling the type based on the FFI boundary instead of what you want it to be would be more accurate.

So whether or not you like it, the function signature in C is actually:

export fn f(callback: ?*const fn () callconv(.c) void) callconv(.c) void {
}
13 Likes

Safe bet none of us like it :smiley:

But then the assert is just

  const real_callback = callback.?;

Name has no thought given to it at all and is probably terrible, but you get the idea.

If you want to have code that deals with the value being null, you can’t assert in the type system that is never going to be null.

5 Likes

I guess, what I want is @assertValid(val: anytype) builtin that inserts type-specific safety checks. C.f. How to check if deserialized exhaustive enum value is legal?

Maybe

fn f(callback: *const fn () callconv(.c) void) callconv(.c) void {
    _ = callback;
}

fn fNullCheck(callback: ?*const fn () callconv(.c) void) callconv(.c) void {
    f(callback.?);
}

comptime {
    @export(&switch (builtin.mode) {
        .Debug, .ReleaseSafe => fNullCheck,
        .ReleaseFast, .ReleaseSmall => f,
    }, .{ .name = "f" });
}
3 Likes

If it can be null, it is nullable. Nothing on the other side of the FFI will respect Zig’s opinion on what non-nullable means. I would expose a 2nd function (that takes a nullable) to the FFI (to respect what the FFI can actually provide, because it can’t provide a non-null pointer) assert there, and then call your ā€œinternalā€ function that is able to enforce the rules Zig provides.

Edit: what @rpkak suggested :slight_smile:

1 Like

We may not like it, but that is the reality of the situation. When one is dealing with C, one has to model it with C’s type system, not Zig’s.

3 Likes

I’d say that’s a good reason for a ā€œproperā€ assert which panics and is also active in debug release mode.

pub fn assertEx(ok: bool, reason: []const u8) void {
    if (!ok) @panic(reason);
}

PS: and maybe use a [*c] pointer… apparently that can be zero?

2 Likes

I think the problem is the llvm ptr nonnull attribute, which allows the optimization. Because of this the arg has to be nullable in ReleaseSafe, so that the check is not optimized away.

Counter point, I like it :D.

Some rando off the street (the C ABI) is constantly trying to bring his pet raccoon into the restaurant. I could pretend he’s gonna follow my rules, but I’d much rather have a bouncer that checks him for raccoons at the door, before he get’s into my nice clean restaurant (zig ABI) :smiley:

8 Likes

assertEx is longer than if by eight characters…

6 Likes

I guess another intuition pump here would be pointer alignment? AFAICT, both Zig and C require pointers to be aligned, but, again, there’s nothing to guarantee alignment across FFI boundary, so it is generally a good idea to assert pointer alignment on the Zig side. But declaring any pointer at the FFI as align(1) would be unpleasant busy-work.

It’s not in the standard, but compilers often have non-standard extensions to define nullability in C APIs (for instance the macOS system headers are full of such annotations).

Also, an assert(ptr) will hint static analyzers that ptr is assumed to be non-null from that point on (or something like __builtin_assume() which is basically equivalent to Zig’s assert - e.g. panic in debug mode, optimize in release mode).

So the conventions and tooling is all there, it’s just not (yet?) in the standard (I bet we’ll see a _Nonnull or _Nullable in one of the next C standards - e.g. in one or two decades :wink:

4 Likes

If passing null is a bug, then we want the check to be an assertion, so it gets removed in release non-safe builds and the compiler can leverage this assumption.
If it’s not a bug, just an error that can naturally arise during the course of a correct program, then yes, the signature should reflect that, and the function should signal the error somehow, like by returning an error code.
Checking the pointer is non null is something Zig already does in its safe builds. The problem here is that, in this case, it is assuming someone already checked the invariant when calling the code, which would be true if the caller was coming from Zig itself, as the assertion would be inserted by the compiler at the call site. But since the caller is not from Zig, we can’t guarantee that the invariant has already been asserted.
Perhaps the best solution here is for Zig to safety-check the arguments of an exported function at the callee’s prologue.

3 Likes

Thanks, @LucasSantos91, that’s definitely the semantics I expect here, but I failed to put it to words with precision, because I live in the Safe bubble :rofl:

2 Likes

One somewhat non-conventional and very work-intensive idea would be to code-generate the C API wrapper from a ā€˜semantically richer’ native Zig API, e.g. all the export functions would be code-generated Zig code which would have some auto-generated checks (like checking the raw C pointers for non-null) or could even do some type transforms, like passing a C string pointer as []const u8 slice into the native Zig function, or exposing a Zig slice parameter as a pointer/size pair in the C API.

…such a ā€˜safe C shim generator’ would actually be a nice general tool :slight_smile:

5 Likes

…old topic I know… but unless the app ships in release-safe-mode, this behaviour is not all that useful since such problems tend to never show up during inhouse testing but only once in a full-moon among thousands of users (that’s why it is also a good idea to keep C asserts active in shipped code, and to have a proper error reporting system setup which uploads minidumps or at least a stack trace) :slight_smile:

1 Like

I looked up the documentation on C pointers, because this thread reminded me they exist, and I was wondering if the correct-by-doctrine type might be [*c], but maybe not:

This type is to be avoided whenever possible. The only valid reason for using a C pointer is in auto-generated code from translating C code.

I’m not sure how I feel about that. An export function which is getting called from C, and takes a pointer, is going to get a [*c] no matter how it’s documented in the signature. Granted that the only difference between that and a ?* is that Zig lets you do more things (because C does) with a [*c], but it also will coerce straight to a *, with a safe-mode null check, and without the need to write .? after it.

Another strength is that in Zig, when we say ?*, we usually mean ?*. There’s a little mental circle: look at the ?*, then see a .? cast, think ā€œhmmmā€, then look at export and think ā€œohhhhh okā€. But if the signature says [*c] that’s clear: it’s export, the pointer is from the wild and coming in hot, we’re going to tame it at the boundary.

I nominate ā€œor as the correct[1] pointer signature for an export function parameter, which then immediately casts it into Zig terms for internal useā€ as a second valid use for [*c]. While being open to reasons why that might be bad.

Edit: to @floooh’s point about keeping the assert in all modes, it can just as well be compared to null with a @panic if so. This is more about documenting ā€œthis pointer will carry C semantics when we receive itā€.


  1. By which I mean: accurately reflects reality. ā†©ļøŽ

2 Likes

…hmmm interesting… I generally use [*c] in the auto-generated sokol-zig bindings. But I guess since it’s generated code (just not via translateC) it’s a valid use case… I actually like that annotation to make it clear to the reader that this is a pointer with C pointer semantics.

1 Like