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

Corrupted memory after updated to 1.12 #36936

Closed
paulusx opened this issue Oct 3, 2016 · 5 comments
Closed

Corrupted memory after updated to 1.12 #36936

paulusx opened this issue Oct 3, 2016 · 5 comments
Assignees
Labels
regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@paulusx
Copy link

paulusx commented Oct 3, 2016

Hello!
I see some garbage in memory:

    use super::yaml_rust::{YamlLoader, Yaml};
    use std::collections::HashMap;

    #[test]
    fn test_yaml() {
        let test_data = r#"
Property1:
  data: Value1

Property2:
  data: Value2

Property3:
  data: Value3

"#;
        let mut check_data = HashMap::new();
        check_data.insert("Property1","Value1");
        check_data.insert("Property2","Value2");
        check_data.insert("Property3","Value3");

        let scheme = &(YamlLoader::load_from_str(test_data).unwrap() as Vec<Yaml>)[0];
        for (key, value) in {
            if let Yaml::Hash(ref x) = *scheme {
                x // if write here "x.clone()", OK
            } else {
                panic!("Bad format")
            }
        } {

            println!("{:?}: {:?};", key, value);
            if check_data[key.as_str().unwrap()] != value["data"].as_str().unwrap() {
                panic!("\nkey: {:?} ,data: {:?} ≠ {:?}\n",
                       key.as_str().unwrap(),
                       check_data[key.as_str().unwrap()],
                       value["data"].as_str().unwrap()
                );

            }
        }
    }

I expect correct return (like with rust 1.11), but I have panic:

Rust 1.12

$ rustup run stable cargo test test_yaml
   Compiling yaml-rust v0.3.3 (https://github.com/chyh1990/yaml-rust.git#f7d2897b)
   Compiling …
    Finished debug [unoptimized + debuginfo] target(s) in 6.13 secs
     Running target/debug/…

running 1 test
test test_yaml ... FAILED

failures:

---- test_yaml stdout ----
        String("Property1"): Hash({String("data"): String("Value1")});
String("Property2"): Hash({String("data"): String("Value2")});
String("String(\"3"): Hash({String("data"): String("datang")});
thread 'test_yaml' panicked at 'no entry found for key', ../src/libcore/option.rs:700
note: Run with `RUST_BACKTRACE=1` for a backtrace.

failures:
    scheme::test::test_yaml

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured

error: test failed

Rust 1.11

$ rustup run 1.11.0 cargo test test_yaml
   Compiling yaml-rust v0.3.3 (https://github.com/chyh1990/yaml-rust.git#f7d2897b)
   Compiling …
     Running target/debug/…

running 1 test
test test_yaml ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured

Data corrupted always in last record.
Thanks!

@alexcrichton
Copy link
Member

This looks like something about drop and drop flags is going awry wrt as expressions? I think this is a minimization of what you're seeing:

struct A;                     

impl Drop for A {             
    fn drop(&mut self) {      
        println!("hello");    
    }                         
}                             

fn foo() -> A { A }           

fn main() {                   
    let a = &(foo() as A);    
    println!("here: {:p}", a);
}                             

That prints "hello" before "here", which means we're allowing use of a value after it's been destroyed.

@alexcrichton alexcrichton added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Oct 3, 2016
@alexcrichton
Copy link
Member

@paulusx if you're looking for a quick fix then removing as Vec<Yaml> will fix the code above

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Oct 3, 2016

Here's a shorter repro without any println!s (for smaller and simpler code post-expansion):

#![crate_type="lib"]

struct A(u32);

impl Drop for A {
    fn drop(&mut self) {
        self.0 = 0;
    }
}

fn foo() -> u32 {
    let a = &(A(1) as A);
    a.0
}

The too-early drop is already present in the debug-mode MIR:

    bb0: {
        /* snip */
        _2 = A::A(const 1u32,);          // scope 0 at <anon>:12:15: 12:19
        _1 = &_2;                        // scope 0 at <anon>:12:13: 12:25
        drop(_2) -> bb1;                 // scope 0 at <anon>:12:15: 12:19
    }

    bb1: {
        /* snip */
        _4 = ((*_1).0: u32);             // scope 1 at <anon>:13:5: 13:8
        /* ... */

@michaelwoerister
Copy link
Member

cc @rust-lang/compiler

@arielb1
Copy link
Contributor

arielb1 commented Oct 3, 2016

Will fix.

@arielb1 arielb1 self-assigned this Oct 3, 2016
arielb1 added a commit to arielb1/rust that referenced this issue Oct 3, 2016
that made no sense (see test), and was incompatible with borrowck.

Fixes rust-lang#36936.
@arielb1 arielb1 added the I-wrong label Oct 3, 2016
bors added a commit that referenced this issue Oct 5, 2016
stop having identity casts be lexprs

that made no sense (see test), and was incompatible with borrowck.

Fixes #36936.

beta-nominated since (bad) regression.

r? @eddyb
alexcrichton pushed a commit to alexcrichton/rust that referenced this issue Oct 11, 2016
that made no sense (see test), and was incompatible with borrowck.

Fixes rust-lang#36936.
brson pushed a commit to brson/rust that referenced this issue Oct 18, 2016
that made no sense (see test), and was incompatible with borrowck.

Fixes rust-lang#36936.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants