ABI break: z_stream::zalloc and z_stream::zfree should be able to set to None (null)

[crlf0710]

No description provided.

[crlf0710]

rust-lang/rust#66059 is gonna turn some code into panic, i think.

[RalfJung]

Oh so these are function pointers (fn() types)? Indeed instantiating those with mem::zeroed() or mem::uninitialized() is immediate UB. If you want a nullable function pointer, use Option<fn()>; that one you can zero-initialize.

[crlf0710]

@RalfJung Yes, and that will be a breaking change for this crate.

[joshtriplett]

Unfortunately, this would indeed be a breaking ABI change in libz-sys, and that would be a major disruption to the ecosystem. I'm going to leave this issue open, to document the problem, but I don't think it's going to be fixable.

[RalfJung]

This is a UB bug, and as our UB diagnostics get better this might become a panic. Specifically, lines like this will panic.

[joshtriplett]

@RalfJung The UB isn't in this crate, though. It's in any crate that creates a zeroed z_stream.

I would love to fix this, but I don't see how that could be possible without breaking ABI.

[RalfJung]

@joshtriplett

It's in any crate that creates a zeroed z_stream.

Fair. This crate makes it impossible to avoid the UB though.

I don't see how that could be possible without breaking ABI.

Not sure how feasible that is, but there could be a new z_stream2 struct with the fixed types, and corresponding versions of z_stream-taking functions? Many of those seem to be by-pointer anyway, and z_streamp works for both z_stream and z_stream2 as it is just a raw pointer.

[joshtriplett]

@RalfJung

Fair. This crate makes it impossible to avoid the UB though.

You can avoid the UB by setting the pointers to a valid value. You just can't set the pointers to NULL and rely on the defaults.

Not sure how feasible that is, but there could be a new z_stream2 struct with the fixed types, and corresponding versions of z_stream-taking functions? Many of those seem to be by-pointer anyway, and z_streamp works for both z_stream and z_stream2 as it is just a raw pointer.

Interesting. Would it be UB to transmute a raw pointer to a z_stream2 (or some_module::z_stream) to a raw pointer to z_stream? Or is that OK as long as nothing dereferences that pointer and that pointer is passed directly to C?

[RalfJung]

You can avoid the UB by setting the pointers to a valid value. You just can't set the pointers to NULL and rely on the defaults.

Right. So you cannot use the entire zlib API (the defaults) and you cannot use mem::zeroed to prepare the zlib config struct -- not sure if that was ever an intended use-case, and not sure how often it happens, I just know there is at least one crate to do this.

Interesting. Would it be UB to transmute a raw pointer to a z_stream2 (or some_module::z_stream) to a raw pointer to z_stream? Or is that OK as long as nothing dereferences that pointer and that pointer is passed directly to C?

Raw pointers may point to anything, including nothing at all. They do not even have to be dereferencable, or aligned, or anything like that. That's their key reason to exist in my view -- to opt out of all the guarantees that come with references.

You don't even need to transmute for that, you can just cast the raw pointer in safe code. And as you say that is okay as long as the NULL fn ptr is not loaded on the Rust side.