Skip to content

-Zvalidate-mir: check that all referenced locals exist #73356

Closed
@jonas-schievink

Description

@jonas-schievink
Contributor

When encountering a Local, there must be a corresponding LocalDecl in the MIR body. If there isn't, later passes can panic due to indexing out of bounds of the LocalDecl array. The validation pass could detect this problem earlier.

Activity

added
C-enhancementCategory: An issue proposing an enhancement or a PR with one.
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
A-MIRArea: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html
on Jun 14, 2020
jonas-schievink

jonas-schievink commented on Jun 14, 2020

@jonas-schievink
ContributorAuthor

Instructions:

Extend the validation pass in src/librustc_mir/transform/validate.rs by implementing visit_local for the TypeChecker MIR visitor:

impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {

The Visitor trait is documented here.

The method should check that the local exists in self.body.local_decls, and fail with a descriptive error message if not (by calling self.fail, not by panicking directly).

added
E-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
on Jun 14, 2020
yerke

yerke commented on Jun 14, 2020

@yerke
Contributor

@rustbot claim

self-assigned this
on Jun 14, 2020
jonas-schievink

jonas-schievink commented on Jun 14, 2020

@jonas-schievink
ContributorAuthor

@yerke FYI, the validation pass currently fails on even libcore until #73175 is merged, so you need to either wait for that, or cherry-pick some commits from there to get it to work.

You can test your changes by running RUSTFLAGS_NOT_BOOTSTRAP="-Zvalidate-mir" ./x.py test --stage=1 src/test/ui (the validator only runs on very little code by default, -Zvalidate-mir turns it on for everything).

yerke

yerke commented on Jun 15, 2020

@yerke
Contributor

@jonas-schievink I wanted to double check, running RUSTFLAGS_NOT_BOOTSTRAP="-Zvalidate-mir" ./x.py test --stage=1 src/test/ui should fail when I run it, right? It actually passed for me after I merged in your branch from #73359.
I wonder if I should try clean and build stage 1 again. :\

jonas-schievink

jonas-schievink commented on Jun 15, 2020

@jonas-schievink
ContributorAuthor

It should pass with #73359 applied, but fail on current master.

yerke

yerke commented on Jun 15, 2020

@yerke
Contributor

@jonas-schievink So your PR fixes this issue (#73356) as well?

jonas-schievink

jonas-schievink commented on Jun 15, 2020

@jonas-schievink
ContributorAuthor

Ah, no, this issue is just about adding additional validation. It shouldn't actually detect any locals that don't exist. So the command should work both before and after implementing this. If it fails after implementing this, something else is going on that we should look into.

pitaj

pitaj commented on Oct 15, 2020

@pitaj
Contributor

@yerke are you still working on this?

yerke

yerke commented on Oct 15, 2020

@yerke
Contributor

No, sorry. Didn’t have time for it. Go ahead.

jonas-schievink

jonas-schievink commented on Nov 7, 2020

@jonas-schievink
ContributorAuthor

@rustbot release-assignment

removed their assignment
on Nov 7, 2020
natsukagami

natsukagami commented on Nov 8, 2020

@natsukagami

Would like to work on this, but it seems there is another check, not on the declaration like this one, but on the memory liveness. This should cover the case that the local is not declared (I think), or would it not?

// Uses of locals must occur while the local's storage is allocated.
self.storage_liveness.seek_after_primary_effect(location);
let locals_with_storage = self.storage_liveness.get();
if !locals_with_storage.contains(*local) {
self.fail(location, format!("use of local {:?}, which has no storage here", local));
}

self-assigned this
on Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-MIRArea: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.htmlC-enhancementCategory: An issue proposing an enhancement or a PR with one.E-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @pitaj@jonas-schievink@yerke@natsukagami@camelid

    Issue actions

      -Zvalidate-mir: check that all referenced locals exist · Issue #73356 · rust-lang/rust