Skip to content

Commit

Permalink
Add another layer of try/finally around rescue/ensure to reset $!.
Browse files Browse the repository at this point in the history
Fixes #2794.
  • Loading branch information
headius committed May 8, 2015
1 parent 5da3695 commit 2880c73
Showing 1 changed file with 78 additions and 9 deletions.
87 changes: 78 additions & 9 deletions core/src/main/java/org/jruby/ir/IRBuilder.java
Expand Up @@ -2201,6 +2201,13 @@ Exception region start marker_2 (protected by whichever block handles exceptions
* ****************************************************************/
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 ------------
Expand Down Expand Up @@ -2228,7 +2235,7 @@ public Operand buildEnsureNode(EnsureNode ensureNode) {
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());
Expand Down Expand Up @@ -2266,6 +2273,9 @@ public Operand buildEnsureNode(EnsureNode ensureNode) {
// End label for the exception region
addInstr(new LabelInstr(ebi.end));

// close out the $!-clearing region
resetLastErrorPostamble(lastErrorReset);

return rv;
}

Expand Down Expand Up @@ -3009,20 +3019,80 @@ public Operand buildRegexp(RegexpNode reNode) {
}

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.
Expand Down Expand Up @@ -3097,7 +3167,6 @@ private Operand buildRescueInternal(RescueNode rescueNode, EnsureBlockInfo ensur
// 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;
}

Expand Down

0 comments on commit 2880c73

Please sign in to comment.