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

When nested subfield of a union is moved, move checker incorrectly permits access to its "uncle" fields #41126

Closed
petrochenkov opened this issue Apr 6, 2017 · 1 comment
Labels
A-borrow-checker Area: The borrow checker I-needs-decision Issue: In need of a decision. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented Apr 6, 2017

#![feature(untagged_unions)]
#![allow(unused)]

#[allow(unions_with_drop_fields)]
union U {
    x: (Vec<u8>, Vec<u8>),
    y: Vec<u8>,
}

fn main() { unsafe {
    let u = U { x: (Vec::new(), Vec::new()) };
    let a = u.x.0;
    let a = u.y; // This is incorrectly accepted despite u.y being "collaterally moved"
}}

When implementing move checking for unions I incorrectly assumed that moves automatically "propagate" to parent "loan paths", like in borrow checker, e.g.

    let a = &mut u.x.0;
    let a = &u.y; // This currently gives correct error due to u.y being "collaterally borrowed"

, this turns out to not be the case.
This case was missed in tests, so the error went unnoticed.

I have some quick fix, will submit tomorrow.

@petrochenkov petrochenkov added A-borrow-checker Area: The borrow checker I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness labels Apr 6, 2017
@arielb1 arielb1 added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. I-needs-decision Issue: In need of a decision. labels Apr 8, 2017
@arielb1
Copy link
Contributor

arielb1 commented Apr 8, 2017

I think we want more consensus on the interaction of unions and the borrow checker. I didn't see any discussion of it on the RFC (rust-lang/rfcs#1897).

bors added a commit that referenced this issue Apr 14, 2017
Fix move checking for nested union fields

Fixes #41126
r? @arielb1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker I-needs-decision Issue: In need of a decision. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants