Skip to content

Commit

Permalink
Force tag to the same position in KStringInner
Browse files Browse the repository at this point in the history
tag() relies on KStringInner having a tag field at the same offset in every union variant, and every union variant is repr(C) so that this holds (provided that `B` has the same size, anyway -- since it's a sealed trait, that is guaranteed for any constructed instance, though it's a bit subtle).

However, the union _itself_ does not have a guaranteed layout unless one uses repr(C). It's a bit silly, but technically every single variant could be in a different location, causing the tags not to line up within the union even though they do within the structs individually. See e.g. https://rust-lang.github.io/unsafe-code-guidelines/layout/unions.html

The fixes would be:

1. use a struct with a tag field first, and then a union field second. (Or even an enum, as is done with the non-`unsafe` implementation).
2. use a repr(C) union instead of a repr(Rust) union.
3. Maybe add some assertions so that even with repr(Rust), the layout is as expected?

I opted to do (2) as a quick fix to send out.
  • Loading branch information
ssbr authored Jul 22, 2024
1 parent dc4a475 commit 04322e1
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ mod inner {
mod inner {
use super::*;

#[repr(C)]
pub(super) union KStringInner<B> {
tag: TagVariant,
singleton: SingletonVariant,
Expand Down

0 comments on commit 04322e1

Please sign in to comment.