jruby · Apr 26, 2016 · Apr 26, 2016
Showing with 67 additions and 11 deletions.

+4 −4 core/src/main/java/org/jruby/ir/Operation.java

+11 −2 core/src/main/java/org/jruby/ir/instructions/CallBase.java

+30 −5 core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java

+22 −0 spec/compiler/general_spec.rb
diff --git a/core/src/main/java/org/jruby/ir/Operation.java b/core/src/main/java/org/jruby/ir/Operation.java
@@ -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;

diff --git a/core/src/main/java/org/jruby/ir/instructions/CallBase.java b/core/src/main/java/org/jruby/ir/instructions/CallBase.java
@@ -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) {

diff --git a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java
@@ -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.

diff --git a/spec/compiler/general_spec.rb b/spec/compiler/general_spec.rb
@@ -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