Fix for exceptional exit from rescue not resetting $!

1.patch.patch

diff --git a/core/src/main/java/org/jruby/ir/IRBuilder.java b/core/src/main/java/org/jruby/ir/IRBuilder.java
index 4f2ec6a..0ebf86e 100644
--- a/core/src/main/java/org/jruby/ir/IRBuilder.java
+++ b/core/src/main/java/org/jruby/ir/IRBuilder.java
@@ -2201,6 +2201,13 @@ public class IRBuilder {
 
      * ****************************************************************/
     public Operand buildEnsureNode(EnsureNode ensureNode) {
+        // Save $! in a temp var so it can be restored when the exception gets handled.
+        Variable savedGlobalException = createTemporaryVariable();
+        addInstr(new GetGlobalVariableInstr(savedGlobalException, "$!"));
+
+        // prepare $!-clearing ensure block
+        EnsureBlockInfo lastErrorReset = resetLastErrorPreamble(savedGlobalException);
+
         Node bodyNode = ensureNode.getBodyNode();
 
         // ------------ Build the body of the ensure block ------------
@@ -2228,7 +2235,7 @@ public class IRBuilder {
         activeRescuers.push(ebi.dummyRescueBlockLabel);
 
         // Generate IR for code being protected
-        Operand rv = bodyNode instanceof RescueNode ? buildRescueInternal((RescueNode) bodyNode, ebi) : build(bodyNode);
+        Operand rv = bodyNode instanceof RescueNode ? buildRescueInternal((RescueNode) bodyNode, ebi, savedGlobalException) : build(bodyNode);
 
         // End of protected region
         addInstr(new ExceptionRegionEndMarkerInstr());
@@ -2266,6 +2273,9 @@ public class IRBuilder {
         // End label for the exception region
         addInstr(new LabelInstr(ebi.end));
 
+        // close out the $!-clearing region
+        resetLastErrorPostamble(lastErrorReset);
+
         return rv;
     }
 
@@ -3009,20 +3019,80 @@ public class IRBuilder {
     }
 
     public Operand buildRescue(RescueNode node) {
-        return buildRescueInternal(node, null);
+        // Save $! in a temp var so it can be restored when the exception gets handled.
+        Variable savedGlobalException = createTemporaryVariable();
+        addInstr(new GetGlobalVariableInstr(savedGlobalException, "$!"));
+
+        // prepare $!-clearing ensure block
+        EnsureBlockInfo lastErrorReset = resetLastErrorPreamble(savedGlobalException);
+
+        // build the rescue itself
+        Operand rv = buildRescueInternal(node, null, savedGlobalException);
+
+        // close out the $!-clearing region
+        resetLastErrorPostamble(lastErrorReset);
+
+        return rv;
     }
 
-    private Operand buildRescueInternal(RescueNode rescueNode, EnsureBlockInfo ensure) {
-        // Labels marking start, else, end of the begin-rescue(-ensure)-end block
-        Label rBeginLabel = ensure == null ? getNewLabel() : ensure.regionStart;
-        Label rEndLabel   = ensure == null ? getNewLabel() : ensure.end;
-        Label rescueLabel = getNewLabel(); // Label marking start of the first rescue code.
+    private void resetLastErrorPostamble(EnsureBlockInfo lastErrorReset) {
+        addInstr(new ExceptionRegionEndMarkerInstr());
+        activeRescuers.pop();
+
+        // We don't reset $! for normal exit, but we do need to jump past the outer finally
+        addInstr(new JumpInstr(lastErrorReset.end));
+
+        // Pop the current ensure block info node
+        activeEnsureBlockStack.pop();
+
+        // ------------ Emit the ensure body alongwith dummy rescue block ------------
+        // Now build the dummy rescue block that
+        // catches all exceptions thrown by the body
+        Variable exc2 = createTemporaryVariable();
+        addInstr(new LabelInstr(lastErrorReset.dummyRescueBlockLabel));
+        addInstr(new ReceiveJRubyExceptionInstr(exc2));
+
+        // Now emit the ensure body's stashed instructions
+        lastErrorReset.emitBody(this);
+
+        // Return (rethrow exception/end)
+        // rethrows the caught exception from the dummy ensure block
+        addInstr(new ThrowExceptionInstr(exc2));
+
+        // End label for the exception region
+        addInstr(new LabelInstr(lastErrorReset.end));
+    }
+
+    private EnsureBlockInfo resetLastErrorPreamble(Variable savedGlobalException) {
+        EnsureBlockInfo lastErrorReset = new EnsureBlockInfo(scope,
+                null,
+                getCurrentLoop(),
+                activeRescuers.peek());
+
+        lastErrorReset.addInstr(new PutGlobalVarInstr("$!", savedGlobalException));
+
+        // ------------ Build the protected region ------------
+        activeEnsureBlockStack.push(lastErrorReset);
+
+        // Start of protected region
+        addInstr(new LabelInstr(lastErrorReset.regionStart));
+        addInstr(new ExceptionRegionStartMarkerInstr(lastErrorReset.dummyRescueBlockLabel));
+        activeRescuers.push(lastErrorReset.dummyRescueBlockLabel);
+        return lastErrorReset;
+    }
+
+    private Operand buildRescueInternal(RescueNode rescueNode, EnsureBlockInfo ensure, Variable savedGlobalException) {
+        // Wrap the entire begin+rescue+ensure+else with $!-resetting "finally" logic
 
         // Save $! in a temp var so it can be restored when the exception gets handled.
-        Variable savedGlobalException = createTemporaryVariable();
         addInstr(new GetGlobalVariableInstr(savedGlobalException, "$!"));
         if (ensure != null) ensure.savedGlobalException = savedGlobalException;
 
+        // Labels marking start, else, end of the begin-rescue(-ensure)-end block
+        Label rBeginLabel = ensure == null ? getNewLabel() : ensure.regionStart;
+        Label rEndLabel   = ensure == null ? getNewLabel() : ensure.end;
+        Label rescueLabel = getNewLabel(); // Label marking start of the first rescue code.
+
         addInstr(new LabelInstr(rBeginLabel));
 
         // Placeholder rescue instruction that tells rest of the compiler passes the boundaries of the rescue block.
@@ -3097,7 +3167,6 @@ public class IRBuilder {
         // End label -- only if there is no ensure block!  With an ensure block, you end at ensureEndLabel.
         if (ensure == null) addInstr(new LabelInstr(rEndLabel));
 
-        activeRescueBlockStack.pop();
         return rv;
     }
 

2.ruby_code.rb

def foo
  raise
rescue
  1.times { return }
end

3.old_ir.txt

Instructions[-#0#foo,IRMethod,EnsureTempsAssigned]:
BB [1:LBL_8:-1]
	%v_1 = copy(nil)
	%v_4 = copy(%v_1)
	%v_5 = copy(%v_1)
	%v_2 = copy(%v_1)
	%v_3 = copy(%v_1)
	%v_6 = copy(%v_1)
	push_frame()
	push_binding()
BB [2:LBL_9:-1]
	%self = recv_self()
	check_arity(;req: 0, opt: 0, *r: false, kw: false)
	%v_1 = get_global_var($!)
BB [4:LBL_0:9]
	line_num(;n: 0)
	%v_2 = call_0o(%self ;n:raise, t:VA, cl:false)
	pop_binding()
	pop_frame()
	return(%v_2)
BB [5:LBL_2:16]
	%v_2 = recv_ruby_exc()
	%v_4 = inheritance_search_const(<Class:Object> ;name: StandardError, no_priv: false)
	%v_5 = rescue_eqq(%v_4, %v_2)
	b_true(LBL_4:23, %v_5)
BB [6:LBL_3:21]
	throw(%v_2)
BB [7:LBL_4:23]
	%v_2 = call(Fixnum:1, %self:CLOSURE foo_CLOSURE_1[-:0] ;n:times, t:NO, cl:true)
	put_global_var($!, %v_1)
	pop_binding()
	pop_frame()
	return(%v_2)
BB [9:LBL_7:31]
	%v_3 = recv_jruby_exc()
	%v_2 = runtime_helper(%v_3 ;method: HANDLE_NONLOCAL_RETURN)
	pop_binding()
	pop_frame()
	return(%v_2)
BB [11:LBL_10:-1]
BB [12:_GLOBAL_ENSURE_BLOCK__0:-1]
	%v_6 = recv_jruby_exc()
	pop_binding()
	pop_frame()
	throw(%v_6)


------ Rescue block map ------
BB 2 --> BB 9
BB 4 --> BB 5
BB 5 --> BB 9
BB 6 --> BB 9
BB 7 --> BB 9
BB 9 --> BB 12
BB 11 --> BB 12

4.new_ir.txt

Instructions[-#0#foo,IRMethod,EnsureTempsAssigned]:
BB [1:LBL_12:-1]
	%v_1 = copy(nil)
	%v_4 = copy(%v_1)
	%v_5 = copy(%v_1)
	%v_2 = copy(%v_1)
	%v_3 = copy(%v_1)
	%v_6 = copy(%v_1)
	push_frame()
	push_binding()
BB [2:LBL_13:-1]
	%self = recv_self()
	check_arity(;req: 0, opt: 0, *r: false, kw: false)
BB [4:LBL_0:9]
	%v_1 = get_global_var($!)
BB [5:LBL_4:12]
	line_num(;n: 0)
	%v_2 = call_0o(%self ;n:raise, t:VA, cl:false)
	%v_3 = copy(%v_2)
	jump(LBL_5:31)
BB [6:LBL_6:19]
	%v_2 = recv_ruby_exc()
	%v_4 = inheritance_search_const(<Class:Object> ;name: StandardError, no_priv: false)
	%v_5 = rescue_eqq(%v_4, %v_2)
	b_true(LBL_8:26, %v_5)
BB [7:LBL_7:24]
	throw(%v_2)
BB [8:LBL_8:26]
	%v_2 = call(Fixnum:1, %self:CLOSURE foo_CLOSURE_1[-:0] ;n:times, t:NO, cl:true)
	put_global_var($!, %v_1)
	%v_3 = copy(%v_2)
	jump(LBL_5:31)
BB [9:LBL_5:31]
	pop_binding()
	pop_frame()
	return(%v_3)
BB [10:LBL_3:34]
	%v_2 = recv_jruby_exc()
BB [11:LBL_1:36]
	put_global_var($!, %v_1)
	throw(%v_2)
BB [13:LBL_11:42]
	%v_3 = recv_jruby_exc()
	%v_2 = runtime_helper(%v_3 ;method: HANDLE_NONLOCAL_RETURN)
	pop_binding()
	pop_frame()
	return(%v_2)
BB [15:LBL_14:-1]
BB [16:_GLOBAL_ENSURE_BLOCK__0:-1]
	%v_6 = recv_jruby_exc()
	pop_binding()
	pop_frame()
	throw(%v_6)


------ Rescue block map ------
BB 2 --> BB 13
BB 4 --> BB 16
BB 5 --> BB 6
BB 6 --> BB 10
BB 7 --> BB 10
BB 8 --> BB 10
BB 9 --> BB 16
BB 10 --> BB 16
BB 11 --> BB 13
BB 13 --> BB 16
BB 15 --> BB 16