flatbuffers icon indicating copy to clipboard operation
flatbuffers copied to clipboard

Rust Push::alignment Incorrect for Structs

Open tustvold opened this issue 2 years ago • 5 comments

Consider a struct of the form

struct FieldNode {
  length: long;
  null_count: long;
}

The following code is generated for Push

impl<'b> flatbuffers::Push for FieldNode {
    type Output = FieldNode;
    #[inline]
    unsafe fn push(&self, dst: &mut [u8], _written_len: usize) {
        let src =
            ::core::slice::from_raw_parts(self as *const FieldNode as *const u8, Self::size());
        dst.copy_from_slice(src);
    }
}

This therefore uses the default impl of Push::alignment which is

fn alignment() -> PushAlignment {
    PushAlignment::new(align_of::<Self::Output>())
}

Unfortunately the definition of FieldNode is

pub struct FieldNode(pub [u8; 16]);

Which has an alignment of 1.

The net result is that the writer does not provide the correct alignment guarantees for structs, which causes the verifiers of some implementations to fail - https://github.com/apache/arrow-rs/issues/5052.

tustvold avatar Nov 08 '23 01:11 tustvold

@CasperN perhaps you have some thoughts on this?

tustvold avatar Nov 13 '23 14:11 tustvold

@tustvold btw, meanwhile based on your findings, I did a "hack" in flatbuffer create_vector/create_vector_from_iter and changed the alignment to 8.

evgenyx00 avatar Nov 13 '23 14:11 evgenyx00

So the codegen in the official Rust backend is generating code that produces wrong data (probably coming from misunderstanding of Rust alignment rules) and none of the maintainers care?

jpochyla avatar Dec 27 '23 10:12 jpochyla

This issue is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

github-actions[bot] avatar Jun 26 '24 20:06 github-actions[bot]

This is still an issue, adding a comment to avoid closure of this problem.

evgenyx00 avatar Jun 30 '24 08:06 evgenyx00