[IR] Bug fix in ensure handling for Unrescuable exceptions

* Adding back missing Label instructions in a couple cases
  that were missing them -- bug introduced in c73cc11 and
  patches that modified break and non-local return handling.

* We currently build IR wherein we effectively require ensure
  exceptions to go through any intervening rescue block. This
  works great for regular exceptions. However, the interpreter
  had a bypass for Unrescuable and attempted to jump to the
  ensure block directly. However, this bypass breaks down in
  certain nested region scenarios. This bug was exposed by the
  break/non-local-return handling changes made in c73cc11 and
  related patches and is demonstrated by the following snippet:
-----------------------------------------------------------------
def foo
  loop do
    if true
      return 2
    end

    break 1
  end
end

puts foo
-----------------------------------------------------------------
BB [1:LBL_6]
BB [2:LBL_7]
	%self = recv_self
	check_arity(0, 0, -1)
	%block(0:0) = recv_closure
	thread_poll
	line_num(1)
BB [4:LBL_0]
	%v_0 = call(FUNCTIONAL, 'loop', %self, [], &Closure _CLOSURE_1[-:1])
	jump LBL_1
BB [5:LBL_2]
	%v_1 = recv_exception
	%v_0 = runtime_helper(handlePropagatedBreak, [%v_1])
BB [6:LBL_1]
	return(%v_0)
BB [7:LBL_5]
	%v_0 = recv_exception [no-typecheck]
	%v_1 = runtime_helper(handleNonlocalReturn, [%v_0])
	return(%v_1)
BB [9:LBL_8]

------ Rescue block map ------
BB 2 --> BB 7
BB 5 --> BB 7
BB 4 --> BB 5

------ Ensure block map ------
BB 5 --> BB 7

-----------------------------------------------------------------

  In this code, the non-local return in the loop causes an
  IRReturnJump (instanceof Unrescuable) to be thrown the closure
  corresponding to 'loop do ... end'. The body of 'foo' has
  a rescue block for the IRBreakJump it will receive from loop
  and an ensure block for the IRReturnJump it will receive from loop.

  At runtime, the interpreter tries to find an ensure block for the
  IRReturnJump that propagates up to 'foo'. But, it doesn't find out
  find one because the ensure map is setup assuming that all exceptions
  go through any inner rescue blocks -- and there is one to deal with
  the IRBreakJump (BB 4 --> BB 5)

  Note that this bug is not specific to break/returns -- it could
  have been triggered by other Unrescuables used in the JRuby runtime
  (ThreadExit, Continuation, etc.)

  This bug was exposed by crashing spec run for
  spec/ruby/library/csv/parse_spec.rb

* This patch fixes the bug by eliminating the bypass in the interpreter
  and routing all exceptions through any inner rescue blocks. However,
  Unrescuable types should bail immediately on entry and rethrow the
  exception for non-ensure blocks.

  This detection has currently been done in a hacky way (with a bunch of
  FIXMEs in IRBuilder.java and Interpreter.java) by using an existing
  checkType flag in ReceiveExceptionInstr.java but we could probably make
  the semantics explicit by one of (a) renaming the field (b) using a
  different flag to indicate that the instruction can deal with Unrescuables
  (c) a new instruction (d) additional explicit IR instructions with these
  checks. To be decided what the most appropriate solution is.

* One additional benefit of eliminating the bypass is that we no longer
  need the ensure map -- the rescue map has all information required for
  both rescues and ensures. To be cleaned up.

* But for now, this patch fixes the execution of the snippet
  above and lets the rspec proceed till the end, but it currently
  crashes while reporting failures (could be a related or a
  different bug/regression introduced sometime in the last year).

core/src/main/java/org/jruby/ir/IRBuilder.java

@@ -851,13 +851,24 @@ private void handleNonlocalReturnInMethod(IRScope s) {
         Label rEndLabel   = s.getNewLabel();
         Label gebLabel    = s.getNewLabel();
 
-        // protect the entire body as it exists now with the global ensure block
+        // Protect the entire body as it exists now with the global ensure block
+        //
+        // Add label and marker instruction in reverse order to the beginning
+        // so that the label ends up being the first instr.
         s.addInstrAtBeginning(new ExceptionRegionStartMarkerInstr(rBeginLabel, rEndLabel, gebLabel, gebLabel));
+        s.addInstrAtBeginning(new LabelInstr(rBeginLabel));
         s.addInstr(new ExceptionRegionEndMarkerInstr());
 
         // Receive exceptions (could be anything, but the handler only processes IRReturnJumps)
         s.addInstr(new LabelInstr(gebLabel));
         Variable exc = s.getNewTemporaryVariable();
+        // FIXME: This should be rethrowable-exception-instr
+        // (for ensure blocks and can receive Unrescuable exceptions)
+        //
+        // UGLY HACK: For now, we are going to piggyback on top of the
+        // no-type-checking field which is indicating the same thing
+        // but worth thinking over and either adding a new flag or a
+        // new instruction
         s.addInstr(new ReceiveExceptionInstr(exc, false));  // no type-checking
 
         // Handle break using runtime helper
@@ -891,6 +902,7 @@ private void receiveBreakException(IRScope s, Operand block, CallInstr callInstr
         Label rescueLabel = s.getNewLabel();
 
         // Protected region
+        s.addInstr(new LabelInstr(rBeginLabel));
         s.addInstr(new ExceptionRegionStartMarkerInstr(rBeginLabel, rEndLabel, null, rescueLabel));
         s.addInstr(callInstr);
         s.addInstr(new JumpInstr(rEndLabel));
@@ -2280,6 +2292,13 @@ public Operand buildEnsureNode(EnsureNode ensureNode, IRScope s) {
         Label rethrowExcLabel = s.getNewLabel();
         Variable exc = s.getNewTemporaryVariable();
         s.addInstr(new LabelInstr(ebi.dummyRescueBlockLabel));
+        // FIXME: This should be rethrowable-exception-instr
+        // (for ensure blocks and can receive Unrescuable exceptions)
+        //
+        // UGLY HACK: For now, we are going to piggyback on top of the
+        // no-type-checking field which is indicating the same thing
+        // but worth thinking over and either adding a new flag or a
+        // new instruction
         s.addInstr(new ReceiveExceptionInstr(exc, false)); // Dont check type since we are simply throwing it back
         s.addInstr(new SetReturnAddressInstr(ebi.returnAddr, rethrowExcLabel));
 

core/src/main/java/org/jruby/ir/interpreter/Interpreter.java

@@ -533,6 +533,19 @@ private static void receiveArg(ThreadContext context, Instr i, Operation operati
             break;
         case RECV_EXCEPTION: {
             ReceiveExceptionInstr rei = (ReceiveExceptionInstr)instr;
+            // FIXME: HACK for now .. make semantics explicit rather
+            // than piggybacking on top of the checkType field
+            //
+            // Unrescuable:
+            //    IRReturnJump, ThreadKill, RubyContinuation, MainExitException, etc.
+            //    These cannot be rescued -- only run ensure blocks
+            //
+            // Others:
+            //    IRBreakJump, Ruby exceptions, errors, and other java exceptions.
+            //    These can be rescued -- run rescue blocks
+            if (rei.checkType && (exception instanceof Unrescuable)) {
+                Helpers.throwException((Throwable)exception);
+            }
             result = (exception instanceof RaiseException && rei.checkType) ? ((RaiseException)exception).getException() : exception;
             break;
         }
@@ -766,17 +779,9 @@ private static IRubyObject interpret(ThreadContext context, IRubyObject self,
                 }
                 }
             } catch (Throwable t) {
-                // Unrescuable:
-                //    IRReturnJump, ThreadKill, RubyContinuation, MainExitException, etc.
-                //    These cannot be rescued -- only run ensure blocks
-                //
-                // Others:
-                //    IRBreakJump, Ruby exceptions, errors, and other java exceptions.
-                //    These can be rescued -- run rescue blocks
-
                 if (debug) LOG.info("in scope: " + scope + ", caught Java throwable: " + t + "; excepting instr: " + instr);
-                ipc = (t instanceof Unrescuable) ? scope.getEnsurerPC(instr) : scope.getRescuerPC(instr);
-                if (debug) LOG.info("ipc for rescuer/ensurer: " + ipc);
+                ipc = scope.getRescuerPC(instr);
+                if (debug) LOG.info("ipc for rescuer: " + ipc);
 
                 if (ipc == -1) {
                     Helpers.throwException((Throwable)t);