Yet another bug with slice_patterns

[leonardinius]

I'm not particularly sure. It's either the bug with compiler failing to enforce lifetime rule or data corruption in Iterator flat_map filter_map (Updated).

---

I tried code found at https://github.com/leonardinius/git-wayback-machine.rs/tree/rust-bug:

Unfortunately I fail to reproduce the bug in different conditions and fail to create standalone code snippet to reproduce the bug.

How to repeat:

#shell
$ git clone https://github.com/leonardinius/git-wayback-machine.rs.git
$ cd git-wayback-machine.rs
$ git checkout rust-bug
$ cargo build
$ env RUST_LOG=debug  target/debug/git-wayback-machine 2>&1 | egrep "3 Page - Inspect 1-2|3 Page - Inspect 2-2"

---

I expected to see this happen:

Since they iterate the same vector data before and after flat_map (See history.rs#L62-L71) I expect the following lines to be as follows.

DEBUG:git_wayback_machine::history: 3 Page - Inspect 1-2: Entry: "9419034" "7 days ago" "Leonids Maslovs": "Personal e-mail?"                                                                     
DEBUG:git_wayback_machine::history: 3 Page - Inspect 2-2: Entry: "9419034" "7 days ago" "Leonids Maslovs": "Personal e-mail?"                                                                     

---

Instead, this happened: See lines below. Line 3 Page - Inspect 1-2: Entry: and 3 Page - Inspect 2-2: Entry: entries should read the same.

DEBUG:git_wayback_machine::history: 3 Page - Inspect 1-2: Entry: "368669f" "7 days ago" "Leonids Maslovs": "Try out execd command sample"                                                         
DEBUG:git_wayback_machine::history: 3 Page - Inspect 2-2: Entry: "3\u{2}\u{107}\u{7f}\u{0}" "\u{f}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{633}" "\u{7}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{233}\u{2
}\u{107}\u{7f}\u{0}": "\u{107}\u{7f}\u{0}\u{0}\n\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}\u{f3}\u{2}\u{107}\u{7f}\u{0}\u{0}\u{10}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0}"                                        

My assumptions / what went wrong:

• ? Data corruption has happened at flat_map
• ? Compiler didn't (but should) throw me away at history.rs#L62-L71 and history.rs#L73-L82, returning Option<Entry> without reference classifier

---

Meta

rustc --version --verbose:

rustc 1.3.0-nightly (912ab64a0 2015-06-25)                                                                                                                                                        
binary: rustc                                                                                                                                                                                     
commit-hash: 912ab64a0de2c121a1c9f10bb1dbe75983b78c73                                                                                                                                             
commit-date: 2015-06-25                                                                                                                                                                           
host: x86_64-unknown-linux-gnu                                                                                                                                                                    
release: 1.3.0-nightly

Backtrace:

No backtrace

---

Please do not hesitate to ask me for more information and standalone code snippets, however you might need to help me to create those ;)

[bluss]

You mean filter_map? Using ref on the closure's argument is suspect, maybe we can reproduce using that.

[bluss]

Oh no, it's probably make_entry that's entirely broken. Slice patterns are busted and you basically can't use them right now if you want safe rust. It's an unstable feature, fortunately.

[leonardinius]

@bluss Yes, you are right - filter_map, typo.

I'm not sure that is the case. I tried with different things and could not reproduce it.

I suspect the fact that compiler didn't error on returning Option<Entry> (dunno should be Option<Entry<'b>>) might be the root issue here.

I'm complete newbie in Rust, so I fail to easily come up with bug-reproducing snippet. You might need to provide me some guidance here ;(

[leonardinius]

@bluss So, you are saying it's more likely to be because of slice patterns being used and I should expect this kind of errors when using unstable features in nightly channel.

Makes sense.

Should I close this issue than? Or should it stay open as a reminder / something that will needs to be re-checked in next beta/stable?

[bluss]

Let it stay open unless it's already been reported. Your bug looks a bit different than the other soundness issues reported for slice patterns. My recommendation: don't enable the feature flag slice_patterns.

It is a serious memory safety issue. Being restricted to an unstable feature gate is better, but we still want rust to be safe even there.

[arielb1]

@leonardinius

slice_patterns is very buggy currently - just don't use it.

[steveklabnik]

Triage: this branch no longer builds:

error[E0529]: expected an array or slice, found `&[&str]`
  --> src/history.rs:79:16
   |
79 |         if let [commit, name, time, comment] = &parts[..] {
   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pattern cannot match with input type `&[&str]`
   |
   = help: the semantics of slice patterns changed recently; see issue #23121

Following that issue, if we patch it:

-        if let [commit, name, time, comment] = &parts[..] {
+        if let &[commit, name, time, comment] = &parts[..] {

then we get a different error:

error: borrowed value does not live long enough
  --> src/history.rs:65:36
   |
65 |                       .filter_map(|ref line| self.make_entry(line))
   |                                    ^^^^^^^^                      - temporary value only lives until here
   |                                    |
   |                                    does not live long enough
   |
   = note: borrowed value must be valid for the anonymous lifetime #1 defined on unknown free region bounded by scope CodeExtent(1511/CallSiteScope { fn_id: NodeId(566), body_id: NodeId(2401) })...

that error is, uh, not great. But I think that means the soundness issue is fixed?

[arielb1]

Sure enough. I basically fixed all of the crazy slice_patterns bugs.

[arielb1]

Reopening as E-needstest.

[mikhail-m1]

I simplified the example to the next

#![feature(slice_patterns)]

pub struct History<'a> { pub _s: &'a str }

impl<'a> History<'a> {
    pub fn get_page(&self) {
        for s in vec!["1|2".to_string()].into_iter().filter_map(|ref line| self.make_entry(line)) {  
            println!("{:?}", s);
        }
    }

    fn make_entry(&self, s: &'a String) -> Option<&str> {
        let parts: Vec<_> = s.split('|').collect();
        println!("{:?} -> {:?}", s, parts); // (1)

        if let [commit, ..] = &parts[..] { Some(commit) } else { None } //(2)
        //Some(parts[0]) // (3)
    }
}

fn main() {
    let h = History{_s: ""};
    h.get_page();
}

And it was not a bug in slice patterns, valgrind still detects error if uncomment (3) and comment (2). And invalid output is generated if comment (1) but valgrind doesn't detect anything.

I it tested on nightly-2015-06-25, for current version need to add & before [commit

[arielb1]

This of course still gives the expected lifetime error as in #26619 (comment) - and is not a bug in slice patterns. Do you think it is worth bisecting or adding a test?