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

Miri engine is too permissive with type-changing MIR assignments #70405

Closed
oli-obk opened this issue Mar 25, 2020 · 2 comments · Fixed by #70532
Closed

Miri engine is too permissive with type-changing MIR assignments #70405

oli-obk opened this issue Mar 25, 2020 · 2 comments · Fixed by #70532
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Mar 25, 2020

src.layout.details == dest.layout.details,
just checks whether the layout of the target and value of an assignment is the same. The only time the types can differ is when assigning a &mut T to a &T variable. We should instead employ a function like

fn mir_assign_valid_types(dst: Ty<'tcx>, src: Ty<'tcx>) {
    dst == src || match (&dst.kind, &src.kind) {
        (ty::Ref(_, _, dst_pointee), ty::Ref(_, _, src_pointee)) => dst_pointee == src_pointee,
        _ => false,
    }
}

(source #69700 (comment))

be sure to also add appropriate mutability checks to the patterns (mutable for the source, immutable for the dest)

@oli-obk oli-obk added A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-const-eval Area: Constant evaluation (MIR interpretation) labels Mar 25, 2020
@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 25, 2020
@RalfJung RalfJung changed the title Miri engine is too permissive with MIR assignments Miri engine is too permissive with type-changing MIR assignments Mar 26, 2020
@bors bors closed this as completed in 4f0a791 Apr 3, 2020
@RalfJung
Copy link
Member

RalfJung commented Apr 3, 2020

@eddyb do we want to keep this open, as you have some future plans for checking subtyping or some such thing?

@eddyb
Copy link
Member

eddyb commented Apr 3, 2020

I'm not sure, I think we should experiment with deeper lifetime erasure but I likely won't have the time to get involved myself any time soon. Maybe that should have its own issue though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants