Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FlatBufferBuilder (in C++ and possibly other languages) should consider aligning empty vectors to T. #8374

Closed
aardappel opened this issue Aug 16, 2024 · 3 comments

Comments

@aardappel
Copy link
Collaborator

Issue originally discovered here: dvidelabs/flatcc#287
(please read that fully for details).

The short of it is that a [T] (where sizeof(T) > 4, e.g. double) has so far always been aligned to 4 (for the size field) in the specific case of empty vectors.

This may seem benign since there is no T element to access, but at least in C/C++ this generates unaligned pointers (when calling e.g. Vector::data() whose mere existence, even if never accessed, can be undefined behavior (C) or generate an unspecified value (C++). While in practice no compiler/hardware causes trouble with this (currently), for the long term correctness of FlatBuffers it may be better to also align to T as the code originally intended.

Verifiers and other code must however always work with these unaligned empty vectors given how much data & code is in the wild.

For details of what that would entail, again, see above link.

@mikkelfj
Copy link
Contributor

I just wanted to add that unaligned empty vectors means vectors aligned to size 4 because the size field requires that alignment, and for what we know by now, the issue is only above 4. Hence verifiers should still require at least 4 bytes alignment.

Copy link
Contributor

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 github-actions bot added the stale label Feb 15, 2025
Copy link
Contributor

github-actions bot commented Mar 1, 2025

This issue was automatically closed due to no activity for 6 months plus the 14 day notice period.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants