Skip to content

Tracking issue for RFC 2645, "Transparent Unions" (formerly: and Enums) #60405

Open

Description

@Centril

This is a tracking issue for the RFC "Transparent Unions and Enums" (rust-lang/rfcs#2645).

Steps:

Unresolved questions:

The role of #[repr(transparent)] in nonnull-style optimizations is not entirely clear. Specifically, it is unclear whether the user can rely on these optimizations to be performed when they make a type transparent. Transparent unions somewhat complicate the matter. General concensus seems to be that the compiler is free to decide where and when to perform nonnull-style optimizations on unions (regardless of whether or not the union is transaprent), and no guarantees are made to the user about when and if those optimizations will be applied. It is still an open question exactly what guarantees (if any) Rust makes about transparent structs (and enums) and nonnull-style optimizations.

This RFC doesn't propose any changes to transparent structs, and so does not strictly depend on this question being resolved. But since this RFC is attempting to round out the #[repr(transparent)] feature, it seems reasonable to dedicate some time to attempting to round out the guarantees about #[repr(transparent)] on structs.

Also it is not clear if transparent unions can even be implemented on LLVM without seriously restricting our semantics for unions overall.

Activity

  1. added
    B-RFC-approvedBlocker: Approved by a merged RFC but not yet implemented.
    T-langRelevant to the language team
    C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFC
    on Apr 30, 2019
  2. mjbshaw commented on Apr 30, 2019

    @mjbshaw
    Contributor

    I plan on attempting to implement the feature by adapting my previous implementation. I'd appreciate it if someone could take a look at that commit and let me know if it's horribly wrong :)

  3. Centril commented on Apr 30, 2019

    @Centril
    ContributorAuthor

    @pnkfelix Up for reviewing ^--- ?

  4. added a commit that references this issue on Jun 11, 2019
  5. added
    B-RFC-implementedBlocker: Approved by a merged RFC and implemented but not stabilized.
    on Jun 11, 2019
  6. 67 remaining items

  7. hanna-kruppe commented on May 7, 2020

    @hanna-kruppe
    Contributor

    Why is that not a problem if you omit #[repr(transparent)]?

    Since repr(Rust) unions have no ABI commitments, we can just represent them as a literal byte array. That allows each byte to be poison-or-not independently.

  8. mjbshaw commented on May 7, 2020

    @mjbshaw
    Contributor

    @hanna-kruppe That's true, but I'm not sure it's useful to allow such granularity for poison because you still can't read f. You can only memcpy the non-poison bytes of f, which I'm not sure Rust should guarantee you can do because it's such a big footgun (and I can't think of any real value it provides).

  9. hanna-kruppe commented on May 7, 2020

    @hanna-kruppe
    Contributor

    That goes more into your second question (why/how this observation matters), which I deliberately didn't go into because it's a more complex topic and my time is currently very limited.

  10. RalfJung commented on May 7, 2020

    @RalfJung
    Member

    Why is this so bad? You just poisoned f; I'm not sure what else you'd expect to happen, with or without #[repr(transparent)].

    #[repr(transparent)]
    union U { f: u32 }
    
    let mut u = U { f: 0 };
    (&mut u as *mut _ as *mut MaybeUninit<u8>).add(1).write(MaybeUninit::uninit());
    
    let u2 = u;
    let v = (&u2 as *const _ as *const u8).read();
    println!("{}", v);

    The Rust semantics as they exist in my head and as it is drafted here would guarantee that this program prints 0. This is what "unions are just bags of bytes" means.

    But it turns out LLVM actually says "nope this is UB as all of u2 is poisoned". So, the desired semantics is not implementable with LLVM while upholding repr(transparent) guarantees.

    Of course we could try to adjust our semantics, but (a) that will make the semantics of unions significantly more complicated, and (b) it seems like a shame that LLVM would force us to cripple our semantics like that, for no good reason. If LLVM's type system was not quite so restrictive, we could just tell LLVM to load 4 bytes at once and preserve which byte is poison and which is not. But LLVM conflates "bytes" with "integers that have arithmetic operations", and then a lot of sadness ensues.

  11. mjbshaw commented on May 8, 2020

    @mjbshaw
    Contributor

    Thanks for the demo, @RalfJung. I personally think this should have the same behavior as #[repr(transparent)] struct S { f: u32 }. I'll take some time to read through the draft you linked to, though.

  12. RalfJung commented on May 8, 2020

    @RalfJung
    Member

    @mjbshaw do you still think that if we replace U by MaybeUninit<u32>? Since MaybeUninit is repr(transparent), that's basically the same type.

  13. alercah commented on Jul 13, 2022

    @alercah

    Doesn't writing MaybeUninit::uninit() still write undef and not poison? Unless I'm quite mistaken, in this playground, it's undef and not poison that gets passed to write_to_float. And since undef is tracked bitwise, this all works out doesn't it?

    Of course, if LLVM does change uninitialized memory to poison, then we would run into this problem, without bitwise semantics.

  14. RalfJung commented on Jul 14, 2022

    @RalfJung
    Member

    Indeed, this was written assuming LLVM would switch to poison eventually.

    Cc #94428

  15. alercah commented on Oct 8, 2022

    @alercah

    Is the concern in this comment about poison spreading to entire union fields not equally valid for #[repr(C)] unions that must be passed as integers per platform ABIs?

  16. RalfJung commented on Oct 8, 2022

    @RalfJung
    Member

    Yeah sure, that's the same situation.

    Judging from this discussion, it seems like LLVM will get a "freezing load" operation. Whenever we have a load that would allow partially uninit data, we could use the freezing load and be sure that the data is preserved correctly. This does lose some information, but at least it would resolve the concern about spreading poison to neighboring bytes.

  17. clarfonthey commented on Dec 3, 2022

    @clarfonthey
    Contributor

    So, poking around the various open issues and PRs and this looks like the best place to mention this.

    I mentioned in #101179 that I think that allowing DSTs in MaybeUninit might be a way to improve the API for slices (by allowing MaybeUninit<[T]>), but it appears that DSTs are still not allowed in unions, even #[repr(transparent)] ones like MaybeUninit. The closest this was to being implemented was #47650 which got closed because the author didn't have time to finish it.

    Was there ever an explicit reason to disallow this, or was it just not implemented/considered since no one had discussed it much?

  18. RalfJung commented on Dec 3, 2022

    @RalfJung
    Member

    The problem is custom DST. If CStr becomes a DST, then size_of_val might have to actually read the data behind the reference to determine the length of the value -- but clearly with a MaybeUninit<CStr> that would be bad.

  19. clarfonthey commented on Dec 4, 2022

    @clarfonthey
    Contributor

    Ah, right -- we haven't fully eliminated the possibility of thin DSTs. I guess that we're still unsure what a proper custom DST RFC would look like, although my guess is that CStr will be a thorn in the side for most of them. C strings really are the worst. (I say this sarcastically, although I'm not surprised to find another reason why they're problematic.)

    My gut feeling is to say that any RFC which would permit thin DSTs (which, as demonstrated by CStr, would be something we want) is that we'd also want a mechanism to filter them out, precisely for cases like this. But I guess that this isn't a strong enough argument to be able to simply allow them now and deal with the consequences later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-reprArea: the `#[repr(stuff)]` attributeB-RFC-approvedBlocker: Approved by a merged RFC but not yet implemented.B-RFC-implementedBlocker: Approved by a merged RFC and implemented but not stabilized.B-unstableBlocker: Implemented in the nightly compiler and unstable.C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCS-tracking-design-concernsStatus: There are blocking design concerns.T-langRelevant to the language team

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions