Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: b4b36dd80c83
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: d30e1eb56963
Choose a head ref
  • 2 commits
  • 4 files changed
  • 1 contributor

Commits on Apr 26, 2016

  1. Put block arg processing in rescuable block and mark as exception.

    Fixes #3643
    
    Theere were a few problems here.
    
    * The code for a bare lambda{} was not getting proper GEB due to
      an old GEB (culled) remaining set as the globalEnsureBB in the
      CFG. This caused later code that wanted to add a GEB to assume
      one was already present.
    * The instructions added by ACP for block arg processing were
      added to the entryBB, which never has an exception edge. They
      were moved to a separate "prologueBB" that is handled by GEB.
    * None of the block arg prep instructions were marked as
      potentially raising exceptions.
    
    This ultimately resulted in a frame being left on the stack, so
    the first super of `execute` (from #3643) was fine, but the
    second would start supering from the wrong point in the stack.
    headius committed Apr 26, 2016
    Copy the full SHA
    6682704 View commit details
  2. eval forms with no arguments can't be in-situ string evals.

    ...so they don't need to deopt.
    headius committed Apr 26, 2016
    Copy the full SHA
    d30e1eb View commit details
8 changes: 4 additions & 4 deletions core/src/main/java/org/jruby/ir/Operation.java
Original file line number Diff line number Diff line change
@@ -221,10 +221,10 @@ public enum Operation {
TOGGLE_BACKTRACE(OpFlags.f_is_book_keeping_op | OpFlags.f_has_side_effect),
UPDATE_BLOCK_STATE(OpFlags.f_is_book_keeping_op | OpFlags.f_has_side_effect),

PREPARE_BLOCK_ARGS(OpFlags.f_is_book_keeping_op | OpFlags.f_has_side_effect),
PREPARE_SINGLE_BLOCK_ARG(OpFlags.f_is_book_keeping_op | OpFlags.f_has_side_effect),
PREPARE_FIXED_BLOCK_ARGS(OpFlags.f_is_book_keeping_op | OpFlags.f_has_side_effect),
PREPARE_NO_BLOCK_ARGS(OpFlags.f_is_book_keeping_op | OpFlags.f_has_side_effect);
PREPARE_BLOCK_ARGS(OpFlags.f_is_book_keeping_op | OpFlags.f_has_side_effect | OpFlags.f_can_raise_exception),
PREPARE_SINGLE_BLOCK_ARG(OpFlags.f_is_book_keeping_op | OpFlags.f_has_side_effect | OpFlags.f_can_raise_exception),
PREPARE_FIXED_BLOCK_ARGS(OpFlags.f_is_book_keeping_op | OpFlags.f_has_side_effect | OpFlags.f_can_raise_exception),
PREPARE_NO_BLOCK_ARGS(OpFlags.f_is_book_keeping_op | OpFlags.f_has_side_effect | OpFlags.f_can_raise_exception);

public final OpClass opClass;
private int flags;
13 changes: 11 additions & 2 deletions core/src/main/java/org/jruby/ir/instructions/CallBase.java
Original file line number Diff line number Diff line change
@@ -252,8 +252,17 @@ private boolean computeEvalFlag() {
String mname = getName();
// checking for "call" is conservative. It can be eval only if the receiver is a Method
// CON: Removed "call" check because we didn't do it in 1.7 and it deopts all callers of Method or Proc objects.
if (/*mname.equals("call") ||*/ mname.equals("eval") || mname.equals("module_eval") ||
mname.equals("class_eval") || mname.equals("instance_eval")) return true;
// CON: eval forms with no arguments are block or block pass, and do not need to deopt
if (
(mname.equals("eval") ||
mname.equals("module_eval") ||
mname.equals("class_eval") ||
mname.equals("instance_eval")
) &&
getArgsCount() != 0) {

return true;
}

// Calls to 'send' where the first arg is either unknown or is eval or send (any others?)
if (potentiallySend(mname) && argsCount >= 1) {
Original file line number Diff line number Diff line change
@@ -99,21 +99,22 @@ public Object execute(IRScope scope, Object... data) {

entryBB.insertInstr(insertIndex++, new UpdateBlockExecutionStateInstr(Self.SELF));

Signature sig = ((IRClosure)scope).getSignature();
BasicBlock prologueBB = createPrologueBlock(cfg);

// Add the right kind of arg preparation instruction
Signature sig = ((IRClosure)scope).getSignature();
int arityValue = sig.arityValue();
if (arityValue == 0) {
entryBB.addInstr(PrepareNoBlockArgsInstr.INSTANCE);
prologueBB.addInstr(PrepareNoBlockArgsInstr.INSTANCE);
} else {
if (sig.isFixed()) {
if (arityValue == 1) {
entryBB.addInstr(PrepareSingleBlockArgInstr.INSTANCE);
prologueBB.addInstr(PrepareSingleBlockArgInstr.INSTANCE);
} else {
entryBB.addInstr(PrepareFixedBlockArgsInstr.INSTANCE);
prologueBB.addInstr(PrepareFixedBlockArgsInstr.INSTANCE);
}
} else {
entryBB.addInstr(PrepareBlockArgsInstr.INSTANCE);
prologueBB.addInstr(PrepareBlockArgsInstr.INSTANCE);
}
}
} else {
@@ -189,6 +190,30 @@ public Object execute(IRScope scope, Object... data) {
return null;
}

// We create an extra BB after entryBB for some ACP instructions which can possibly throw
// an exception. We want to keep them out of entryBB so we have a safe place to put
// stuff before exception without needing to worry about weird flow control.
// FIXME: We need to centralize prologue logic in case there's other places we want to use it
private BasicBlock createPrologueBlock(CFG cfg) {
BasicBlock entryBB = cfg.getEntryBB();

BasicBlock oldStart = cfg.getOutgoingDestinationOfType(entryBB, CFG.EdgeType.FALL_THROUGH);
BasicBlock prologueBB = new BasicBlock(cfg, cfg.getScope().getNewLabel());
cfg.removeEdge(entryBB, oldStart);
cfg.addBasicBlock(prologueBB);
cfg.addEdge(entryBB, prologueBB, CFG.EdgeType.FALL_THROUGH);
cfg.addEdge(prologueBB, oldStart, CFG.EdgeType.FALL_THROUGH);

// If there's already a GEB, make sure we have an edge to it and use it to rescue these instrs
if (cfg.getGlobalEnsureBB() != null) {
BasicBlock geb = cfg.getGlobalEnsureBB();
cfg.addEdge(prologueBB, geb, CFG.EdgeType.EXCEPTION);
cfg.setRescuerBB(prologueBB, geb);
}

return prologueBB;
}

@Override
public boolean invalidate(IRScope scope) {
// Cannot add call protocol instructions after we've added them once.
22 changes: 22 additions & 0 deletions spec/compiler/general_spec.rb
Original file line number Diff line number Diff line change
@@ -1112,5 +1112,27 @@ def a; __callee__; end
$VERBOSE = verbose
end
end

it "maintains frame stack integrity through a bare lambda (GH #3643)" do
code = '
module GH3643
class A
def x(proc)
instance_eval(&proc) rescue nil
:ok
end
end
A.prepend(Module.new { def x(proc); super; super; end })
def self.foo
A.new.x(lambda{})
end
foo
end
'

run(code) do |x|
x.should == :ok
end
end
end
end