Skip to content

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

New issue

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

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

Already on GitHub? # to your account

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, covers all const contexts (static, const fn, ...) 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, covers all const contexts (static, const fn, ...) 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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) 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