Fix miscompile of implement_buffer_content-macro, other approach

[avl]

This is an alternative approach to solving the problem #1949 .

I think it's 100% free of UB and totally sound. But it only works on nightly rust.

[est31]

I like this one much better than the other PR. Not sure if it fixes the UB though. cc @RalfJung is this code UB free now?

[RalfJung]

I'm afraid I am lacking context, and it is hard to say what soundness for a macro even means without knowing exactly how it is used.

Is there some self-contained piece of code (e.g. on the playground) that is representative for what happens here?

[RalfJung]

This is suspicious. The vtable must be non-null even in a raw pointer. In fact it must even be a valid pointer:

The following values are invalid (at their respective type): [...]
Invalid metadata in a wide reference, Box, or raw pointer:
dyn Trait metadata is invalid if it is not a pointer to a vtable for Trait that matches the actual dynamic trait the pointer or reference points to.

[RalfJung]

I think std::ptr::from_raw_parts might be unsound in its current form. EDIT: no it's fine, see below

[RalfJung]

Unless maybe these are all slices? But then you can use the stable ptr::slice_from_raw_parts

[RalfJung]

Okay I guess these must be all slices; for trait objects the metadata has type DynMetadata and that cannot be 0.

This is exactly why I need context -- it is very hard to understand macro code from a small patch like this, and I can't spend hours trying to figure this all out myself.^^

[est31]

Yeah it's a macro to implement custom buffer types. Those buffer types can take up any content. Copy-pasting the rustdoc:

Unsized types

In order to put some data in a buffer, it must implement the Content trait. This trait is
automatically implemented on all Sized types and on slices (like [u8]). This means that
you can create a Buffer<Foo> (if Foo is sized) or a Buffer<[u8]> for example without
worrying about it.

However unsized structs don't automatically implement this trait and you must call the
implement_buffer_content! macro on them. You must then use the empty_unsized constructor.

# #[macro_use] extern crate glium;
# fn example(display: glium::Display) {
# use std::mem;
# use glium::buffer::{BufferType, BufferMode};
struct Data {
    data: [f32],        // `[f32]` is unsized, therefore `Data` is unsized too
}

implement_buffer_content!(Data);    // without this, you can't put `Data` in a glium buffer

I'm not sure what the use case of this macro is. The documentation/tests/in-crate examples contain nothing more than custom wrappers of unsized slices. @avl what are your use cases for it? Maybe the best option is to remove it.

[avl]

My use case is a buffer which is a DST with some "header info" and then a list of elements. The number of elements is only known at runtime.

I think I could just as well use two different buffer-objects instead. I would have if I had known that this feature was problematic 😀. But the last couple of years the code has just worked.

[avl]

Okay I guess these must be all slices; for trait objects the metadata has type DynMetadata and that cannot be 0.

This is exactly why I need context -- it is very hard to understand macro code from a small patch like this, and I can't spend hours trying to figure this all out myself.^^

Here's a small self-contained example. I've read the rust docs and I think it should be sound. Unless there's some undocumented requirement somewhere.

#![feature(ptr_metadata)]
#![feature(layout_for_ptr)]

struct Packet {
    sequence_number: u32,     //total 16 byte header
    source_addres: u32,       
    destination_address: u32,
    size: u32,
    data_chunks: [u64]        // arbitrary number of 8 byte payload chunks
}

fn main() {
    let fake_ptr: *const Packet = std::ptr::from_raw_parts::<Packet>(std::ptr::null(), 0);
    // Safety: The metadata-part must be an initialized integer. It is a literal 0.
    let min_size = unsafe { std::mem::size_of_val_raw(fake_ptr) };

    let fake_ptr: *const Packet = std::ptr::from_raw_parts::<Packet>(std::ptr::null(), 1);
    // Safety: The metadata-part must be an initialized integer. It is a literal 1.
    let step = unsafe { std::mem::size_of_val_raw(fake_ptr) - min_size };
    
    // prints: min_size: 16, step: 8
    println!("min_size: {}, step: {}", min_size, step);   
}

[RalfJung]

I'm not very familiar with the new ptr_metadata API.

But if you only use this for slices / types where the "unsized tail" is a slice, then I agree you are fine here. This is basically just generalizing slice_from_raw_parts from slices to arbitrary types with a slice as their tail.

[est31]

As it seems there are valid use cases for this macro and it seems that this PR is not unsound, I'd like to merge this PR. However, it's using the pointer metadata APIs which are unstable. Thus the PR can't be merged at the moment.

I think due to the miscompilation I'll remove the macro for the time being and if the features stabilize, this PR can be merged (or feel free to close it and refile later).