Control flow bug when using "return"

[JulianGCalderon]

Inside a while loop, when the last pushed basic block has a return terminator, the remaining buffered statements (not yet pushed) are added as an outer basic block.

Minimal Reproducible Example

Given the following code:

fn main() -> i32 {
  let mut foo: i32 = 0;

  if false {
    return 1;
  } else {
    foo = 2;
  }

  return 3;
}  

It gets parsed (roughly) as:

(FunctionBody
    (LetStmt)
    (IfExpr
      condition: (Expression)
      content: (Block) ;; then block
      else: (Block)
    )
    (ReturnStmt)
)

But lowered (roughly) to IR as:

(FnBody
  (BasicBlock
    statements: (
      (StorageLive)
      (Assign) ;; foo = 0
      (Assign) ;; condition temp var
    )
    terminator: (SwitchInt) ;; goto 1 or 2
  )
  (BasicBlock
    statements: (
      (Assign) ;; return pointer
    )
    terminator: (Return)
  )
  (BasicBlock
    statements: (
      (Assign) ;; foo = 2
      (Assign) ;; return pointer
    )
    terminator: (Return)
  )
)

The problem is that the else block gets pushed in the same block as the final return.

In this particular scenario there are no side effects, but this allows for unwanted behaviour.

Bug with While Loop

In the following example, the return of the program is 1 even though the program should hang.

fn main() -> i32 {
  while true {
    if false {
      return 0;
    };
  }

  return 1;
}  

After lowering the if, the last pushed basic block has a return terminator. Then, no empty block is pushed as the last block of the while block. This means that the else branch points outside the while loop

(FnBody
  (BasicBlock
    statements: ( )
    terminator: (Goto) ;; goto next
  )
  (BasicBlock
    statements: (
      (Assign) ;; while condition temp var
    )
    terminator: (SwitchInt) ;; goto 2 or 4
  )
  (BasicBlock
    statements: (
      (Assign) ;; if condition temp var
    )
    terminator: (SwitchInt) ;; goto 3 or 4
  )
  (BasicBlock
    statements: (
      (Assign) ;; return pointer
    )
    terminator: (Return)
  )
  ;; There is an empty basic block missing here, it should point to the start of the loop.
  (BasicBlock
    statements: (
      (Assign) ;; return pointer
    )
    terminator: (Return)
  )
)

Bug with Nested Ifs

In the following example, the expected output is 0, but it's actually 2

mod Example {
    fn main() -> i32 {
        let foo: i32 = 0;

        if true {
            if false {
                return 1;
            }
        } else {
            foo = 2;
        }

        return foo;
    }
}

After lowering the nested if, the last pushed block has a return terminator. This means that the else branch gets pushed with the final return statement (therefore, gets executed).

(FnBody
  (BasicBlock
    statements: (
      (StorageLive)
      (Assign) ;; foo = 0;
      (Assign) ;; condition temp var
    )
    terminator: (SwitchInt) ;; goto 1 or 3
  )
  (BasicBlock
    statements: (
      (Assign) ;; nested condition temp var
    )
    terminator: (SwitchInt) ;; goto 2 or 3
  )
  (BasicBlock
    statements: (
      (Assign) ;; return pointer
    )
    terminator: (Return)
  )
  ;; foo assign should be here, as the else branch
  (BasicBlock
    statements: (
      (Assign) ;; foo = 2
      (Assign) ;; return pointer
    )
    terminator: (Return)
  )
)

Proposed Solution

One simple way to fix this is, when lowering a block (while, for, if/else), never leave buffered statements to the callee (not yet pushed). Another alternative I can think of is to delegate the responsability of taking this edge cases into account to the callee (although more error prone)