Repository: goby-lang/goby (go)

Issue:
Top level `break` instruction causes panic on REPL
The following Goby code from https://github.com/goby-lang/goby/blob/master/vm/statement_test.go#L13 causes panic only on REPL:

```ruby
x = [1, 2, 3]
y = 0
x.each do |i|
  y += i
  if i == 2
    break
  end
end
y
```

```
» y
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x68 pc=0x43ef404]

goroutine 1 [running]:
github.com/goby-lang/goby/vm.(*VM).REPLExec(0xc4200a6780, 0xc4201e69c0, 0x5, 0x8)
	/Users/hachi8833/deve/golang/gopath/sys/src/github.com/goby-lang/goby/vm/repl.go:38 +0x4d4
github.com/goby-lang/goby/igb.StartIgb(0x4577c2d, 0x5)
	/Users/hachi8833/deve/golang/gopath/sys/src/github.com/goby-lang/goby/igb/repl.go:231 +0xd23
main.main()
	/Users/hachi8833/deve/golang/gopath/sys/src/github.com/goby-lang/goby/goby.go:29 +0x954
```

Note that the code works fine when running as a file.

And the following works fine on REPL; looks like the bug only occurs on top level `break` instruction.

```ruby
» def foo
¤   x = [1, 2, 3]
¤   y = 0
¤   x.each do |i|
¤     y += i
¤     if i == 2
¤       break
¤     end
¤   end
¤   y
» end
#»
» foo
#» 3
```

FYI: complied with Go 1.9.3.

Pull request description:
Fix #584
The cause of this issue is that Goby pops the incorrect call frame when evaluating a `break` statement inside a built-in method's block.

When evaluating a built-in method's call frame, Goby should pop it after the evaluation is finished. And we do this by calling `t.callFrameStack.pop()` directly, which makes sense in most cases.  

However, because the "break" instruction pops out 3 call frames every time (which is necessary, please read the comments inside `Break` instruction for more detail), it'd pop the built-in method's frame in advance. So when Goby finishes the built-in frame's evaluation later and tries to pop the frame from the stack, it actually pops the top frame, which later causes the nil pointer error.



Patch:
diff --git a/vm/thread.go b/vm/thread.go
index 1b73b2c..5cc12b6 100644
--- a/vm/thread.go
+++ b/vm/thread.go
@@ -152,7 +152,18 @@ func (t *Thread) evalCallFrame(cf callFrame) {
 		t.Stack.Push(&Pointer{Target: result})
 		//fmt.Println(t.callFrameStack.inspect())
 		//fmt.Println("-----------------------")
-		t.callFrameStack.pop()
+
+		// this check is to prevent popping out a call frame that should be kept
+		// usually the top call frame would be the `cf` here
+		// but in some rare cases it could be other call frame, and if we pop it here we'll have some troubles in the later execution
+		//
+		// one example of the problem would be https://github.com/goby-lang/goby/issues/584
+		// the cause of this issue is that the "break" instruction always pops 3 call frames
+		// (this is necessary, please read the comments inside `Break` instruction for more detail)
+		// and because we already popped the `cf` during the `Break` instruction, what we pop here would be the top-level call frame, which causes the program to crash
+		if t.callFrameStack.top() == cf {
+			t.callFrameStack.pop()
+		}
 	}
 
 	t.removeUselessBlockFrame(cf)


Test patch:
diff --git a/vm/vm_test.go b/vm/vm_test.go
index f36b1ee..af67e9f 100644
--- a/vm/vm_test.go
+++ b/vm/vm_test.go
@@ -15,6 +15,24 @@ func TestVM_REPLExec(t *testing.T) {
 		inputs   []string
 		expected interface{}
 	}{
+		{
+			[]string{
+				`
+x = [1, 2, 3]
+y = 0
+`,
+				`
+x.each do |i|
+  y += i
+  if i == 2
+    break
+  end
+end
+`,
+				`y`,
+			},
+			3,
+		},
 		{
 			[]string{
 				`