Skip to content

trans codegen injects aliasing of l-values via match ident { ident => ... } #23698

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

Closed
pnkfelix opened this issue Mar 25, 2015 · 3 comments · Fixed by #23702
Closed

trans codegen injects aliasing of l-values via match ident { ident => ... } #23698

pnkfelix opened this issue Mar 25, 2015 · 3 comments · Fixed by #23702

Comments

@pnkfelix
Copy link
Member

trans codegen feeds the zeroed discriminant back into match in a loop

Some examples, courtesy of @eddyb and @dotdash

fn main() {
    let mut result = Box::new(42);
    let mut i = 0;
    loop {
        match result {
            x => {
                println!("A>>> {:p}", &*x);
                println!("A: {:?}", x);
                result = Box::new(42);
                if i > 10 { break; }
                i += 1;
            }
        }
    }
}

playpen here yields:

A>>> 0x7fedb4423008
A: 42
A>>> 0x0
playpen: application terminated abnormally with signal 4 (Illegal instruction)

Another one, courtesy of @eddyb:

fn main() {
    let mut result = (Box::new(12), 34);

    // Run in a loop to demonstrate the zeroing:
    let mut i = 0;
    while i < 4 {
        match result {
            x => {
                println!("{:p} {}", &*x.0, x.1);
                result = (Box::new(12), 34);
                i += 1;
            }
        }
    }
}

playpen yields:

0x7f05c5823008 34
0x0 0
0x0 0
0x0 0
@eddyb
Copy link
Member

eddyb commented Mar 25, 2015

Not sure if the title should be changed, but this is an aliasing lvalue bug:

fn main() {
    let mut a = Box::new(1);
    match a {
        mut b => {
            a = Box::new(2);
            println!("{:p} {:p}", &mut a, &mut b);
        }
    }
}

playpen yields 0x7fff8c5fe240 0x7fff8c5fe240

@dotdash (doener on IRC) pointed out that the by-value pattern is reusing the location of result for x while result is being mutated, which led to the more specific zeroing issue (x was being dropped, taking the new result value with it).

EDIT: Do note the local variables are aliasing, not the boxes contained within.
Any type that moves can be used in place of Box<i32> to get the same effect.

EDIT2: Simplified the test case by removing the unnecessary loop.

@pnkfelix pnkfelix changed the title trans codegen feeds the zeroed discriminant back into match in a loop trans codegen injects aliasing of l-values via match ident { ident => ... } Mar 25, 2015
@pnkfelix
Copy link
Member Author

here is a further reduced example, well-suited for e.g. duplication in no_std:

#[derive(Debug)]
enum E { C, A, B }
// force E to be zeroed when it goes out of scope
impl Drop for E { fn drop(&mut self) { } }

fn main() {
    let mut result = E::A;
    match result {
        mut x => {
            result = E::B;
            println!("{:p} {:p}", &mut x, &mut result);
        }
    }
    println!("result: {:?}", result);
}

playpen yields:

<anon>:2:10: 2:11 warning: variant is never used: `C`, #[warn(dead_code)] on by default
<anon>:2 enum E { C, A, B }
                  ^
0x7fff05ac3690 0x7fff05ac3690
result: C

@dotdash
Copy link
Contributor

dotdash commented Mar 25, 2015

Probably have a fix. The reassignment checker had a bug

dotdash added a commit to dotdash/rust that referenced this issue Mar 25, 2015
…criminant

The reassignment checker effectively only checks whether the last
assignment in a body affects the discriminant, but it should of course
check all the assignments.

Fixes rust-lang#23698
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 25, 2015
 The reassignment checker effectively only checks whether the last
assignment in a body affects the discriminant, but it should of course
check all the assignments.

Fixes rust-lang#23698
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants