Using the iterator inside the for loop causes UB

[bluss]

Experimenting with using the iterator inside the loop (awsome feature), found weird bug

fn main() {
    let mut it = range(0u, 5);
    for i in it {
        println!("{}", i)
        if i == -23 {
            it.next();
        }
    }
}

Using rustc 0.12.0-pre (66a0b52 2014-07-25 20:36:10 +0000)

Actual Output:

1
1
1
1
1

Expected Output:

1
2
3
4
5

Bug needs optimization level 2 or higher -- so it is probably undefined behavior-related.

The actual condition doesn't matter, I just picked one that would never be true.

[bluss]

cc @pcwalton

[japaric]

@blake2-ppc

This looks like a regression in the borrow checker. You're not supposed to be able to mutably borrow it inside the for loop, because the for i in it already mutably borrows it.

You can confirm this is a regression by testing your program in the playpen, but compiling it with the 0.11 snapshot, where it gets flagged as incorrect (reason: double mutable borrow).

[thestinger]

It doesn't have to borrow it as &mut, it's only calling an &mut self method on it and the return value isn't tied to that self. It was borrowing it as &mut before due to the AST expansion hack.

[eddyb]

Interestingly enough, trying to reduce the IR by isolating the println call produces the correct result even with optimizations.

Yes, it does seem an optimized println call is important for this bug to manifest.
It reuses a pointer into the value of the Option<uint> that next returns, as part of the for loop, for printing.
However... It all seems fine until you notice the LLVM lifetime intrinsics: that Option gets llvm.lifetime.start called for it before the loop (because of that pointer reuse thing) then after each iteration. There is no llvm.lifetime.end call for it.
cc @dotdash

On a second thought, there are no such intrinsics without optimizations so they must've been added by inlining or other LLVM passes. I'll leave this to the experts.

[dotdash]

Looks like a mis-optimization.

; ModuleID = '<stdin>'
target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define void @test(i1 %c) {
entry-block:
  %x = alloca i8
  br label %loop_in

loop_in:                                          ; preds = %loop_out, %loop_body, %entry-block
  call void @llvm.lifetime.start(i64 8, i8* %x)
  br label %loop_body

loop_body:                                        ; preds = %loop_in
  br i1 %c, label %loop_in, label %loop_out

loop_out:                                         ; preds = %loop_body
  call void @llvm.lifetime.end(i64 8, i8* %x)
  br label %loop_in

out:                                              ; No predecessors!
  ret void
}

; Function Attrs: nounwind
declare void @llvm.lifetime.start(i64, i8* nocapture) unnamed_addr #0

; Function Attrs: nounwind
declare void @llvm.lifetime.end(i64, i8* nocapture) unnamed_addr #0

attributes #0 = { nounwind }

Looks like this after SimplifyCFG:

; ModuleID = 'test.no-opt.bc'
target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define void @test(i1 %c) {
entry-block:
  %x = alloca i8
  br label %loop_body

loop_body:                                        ; preds = %loop_body, %entry-block
  call void @llvm.lifetime.start(i64 8, i8* %x)
  br label %loop_body
}

; Function Attrs: nounwind
declare void @llvm.lifetime.start(i64, i8* nocapture) unnamed_addr #0

; Function Attrs: nounwind
declare void @llvm.lifetime.end(i64, i8* nocapture) unnamed_addr #0

attributes #0 = { nounwind }

[dotdash]

The above on its own isn't enough. In addition to that the loop rotate pass moves the lifetime start from the start of the loop to the end, and that kills it.

[dotdash]

Fix submitted upstream at http://reviews.llvm.org/D4699

[luqmana]

Fixed by #16080.