False positive const-UB check on `NonNull` pointer

[JakobDegen]

I tried this code:

use std::ptr::NonNull;

const X: NonNull<()> = {
    let r: &'static u16 = &0;
    
    let p = unsafe { std::mem::transmute::<_, *mut ()>(r) };
    let p = p.wrapping_byte_add(3);
    unsafe { NonNull::new_unchecked(p) }
};

I expected to see this happen: Compiles. This code cannot be UB, as the alignment on the static guarantees that the produced pointer has odd address and therefore is non-null.

Instead, this happened:

error[E0080]: it is undefined behavior to use this value
 --> src/lib.rs:3:1
  |
3 | const X: NonNull<()> = {
  | ^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a potentially null pointer, but expected something that cannot possibly fail to be greater or equal to 1
  |
  = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
  = note: the raw bytes of the constant (size: 8, align: 8) {
              ╾───alloc3+0x3<imm>───╼                         │ ╾──────╼
          }

For more information about this error, try `rustc --explain E0080`.

The check seems to know that it might sometimes be wrong, but this isn't a case where the rules around UB are unclear - the rules around what pointers you can put in a NonNull are very clear.

Meta

All versions of Rust since 1.61 where the necessary operations were stabilized in const

[scottmcm]

What's the situation in which you want to do this?

The plan is for NonNull::new to compile-fail on this soon too, as "well don't do that", so if this is something you actually need for something it'd be good to hear why.

Or is it just a "please change the error message to say something else"?

[JakobDegen]

@scottmcm this comes up from doing a sequence of two memory optimizations. The first one is this type. For our purposes we can treat it as a Box<[u64]> but with the following representation: The value itself is a pointer to the heap allocated slice data, and the length is stored before that data (so at pointer - 8). For the size zero case, we can then point one-past-the-end of a statically allocate u64. The benefit of this over Box<[u64]> is that it saves 8 bytes in the length zero case.

The issue comes up once I wanted to use the low bits of that pointer to additionally bitpack in some more data. So, imagine that I wanted to store a (ThinBoxSlice, bool), but represented as thin_box_slice_ptr | bool_val. A ([], True) of that type ought to be statically allocatable, but runs into the issue described here, since the bit-packing ends up adding one to what was already a one-past-the-end pointer.

That being said, now that I'm thinking about this though, I don't think that this should be hard to fix - it should be completely fine to replace the if offset > allocation_size with a if offset > allocation_size && offset % allocation_align == 0 - that results in what I think is actually a correct check.

[RalfJung]

This code cannot be UB, as the alignment on the static guarantees that the produced pointer has odd address and therefore is non-null.

Yeah we could make our null-check logic more complicated to account for this. That would immediately become a stable guarantee though, now that is_null can be called on stable.

[RalfJung]

#133700 should fix this :)