jruby · Jul 12, 2018 · Jul 12, 2018 · Jul 12, 2018 · Jul 12, 2018 · Jul 12, 2018
diff --git a/core/src/main/java/org/jruby/RubyBinding.java b/core/src/main/java/org/jruby/RubyBinding.java
@@ -153,7 +153,7 @@ public IRubyObject local_variable_get(ThreadContext context, IRubyObject symbol)
 
         if (slot == -1) throw context.runtime.newNameError("local variable `" + name +  "' not defined for " + inspect(), name);
 
-        return evalScope.getValue(slot & 0xffff, slot >> 16);
+        return evalScope.getValueOrNil(slot & 0xffff, slot >> 16, context.nil);
     }
 
     @JRubyMethod

diff --git a/core/src/main/java/org/jruby/RubyString.java b/core/src/main/java/org/jruby/RubyString.java
@@ -3496,26 +3496,11 @@ private void replaceInternal19(int beg, int len, RubyString repl) {
     /** rb_str_aref, rb_str_aref_m
      *
      */
-    public IRubyObject op_aref(ThreadContext context, IRubyObject arg1, IRubyObject arg2) {
-        return op_aref19(context, arg1, arg2);
-    }
-
-    public IRubyObject op_aref(ThreadContext context, IRubyObject arg) {
-        return op_aref19(context, arg);
-    }
-
-    @JRubyMethod(name = {"[]", "slice"}, reads = BACKREF, writes = BACKREF)
-    public IRubyObject op_aref19(ThreadContext context, IRubyObject arg1, IRubyObject arg2) {
-        Ruby runtime = context.runtime;
-        if (arg1 instanceof RubyRegexp) return subpat(context, (RubyRegexp) arg1, arg2);
-        return substr19(runtime, RubyNumeric.num2int(arg1), RubyNumeric.num2int(arg2));
-    }
-
     @JRubyMethod(name = {"[]", "slice"}, reads = BACKREF, writes = BACKREF)
-    public IRubyObject op_aref19(ThreadContext context, IRubyObject arg) {
+    public IRubyObject op_aref(ThreadContext context, IRubyObject arg) {
         Ruby runtime = context.runtime;
         if (arg instanceof RubyFixnum) {
-            return op_aref19(runtime, RubyNumeric.fix2int((RubyFixnum)arg));
+            return op_aref(runtime, RubyNumeric.fix2int((RubyFixnum)arg));
         } else if (arg instanceof RubyRegexp) {
             return subpat(context, (RubyRegexp) arg);
         } else if (arg instanceof RubyString) {
@@ -3535,7 +3520,14 @@ public IRubyObject op_aref19(ThreadContext context, IRubyObject arg) {
                 return begLen == null ? runtime.getNil() : substr19(runtime, begLen[0], begLen[1]);
             }
         }
-        return op_aref19(runtime, RubyNumeric.num2int(arg));
+        return op_aref(runtime, RubyNumeric.num2int(arg));
+    }
+
+    @JRubyMethod(name = {"[]", "slice"}, reads = BACKREF, writes = BACKREF)
+    public IRubyObject op_aref(ThreadContext context, IRubyObject arg1, IRubyObject arg2) {
+        Ruby runtime = context.runtime;
+        if (arg1 instanceof RubyRegexp) return subpat(context, (RubyRegexp) arg1, arg2);
+        return substr19(runtime, RubyNumeric.num2int(arg1), RubyNumeric.num2int(arg2));
     }
 
     @JRubyMethod
@@ -3548,7 +3540,7 @@ public IRubyObject byteslice(ThreadContext context, IRubyObject arg) {
         return byteARef(context.runtime, arg);
     }
 
-    private IRubyObject op_aref19(Ruby runtime, int idx) {
+    private IRubyObject op_aref(Ruby runtime, int idx) {
         IRubyObject str = substr19(runtime, idx, 1);
         return !str.isNil() && ((RubyString) str).value.getRealSize() == 0 ? runtime.getNil() : str;
     }
@@ -3617,18 +3609,10 @@ private IRubyObject subpat(ThreadContext context, RubyRegexp regex) {
     /** rb_str_aset, rb_str_aset_m
      *
      */
-    public IRubyObject op_aset(ThreadContext context, IRubyObject arg0, IRubyObject arg1) {
-        return op_aset19(context, arg0, arg1);
-    }
-
-    public IRubyObject op_aset(ThreadContext context, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2) {
-        return op_aset19(context, arg0, arg1, arg2);
-    }
-
     @JRubyMethod(name = "[]=", reads = BACKREF)
-    public IRubyObject op_aset19(ThreadContext context, IRubyObject arg0, IRubyObject arg1) {
+    public IRubyObject op_aset(ThreadContext context, IRubyObject arg0, IRubyObject arg1) {
         if (arg0 instanceof RubyFixnum) {
-            return op_aset19(context, RubyNumeric.fix2int((RubyFixnum)arg0), arg1);
+            return op_aset(context, RubyNumeric.fix2int((RubyFixnum)arg0), arg1);
         } else if (arg0 instanceof RubyRegexp) {
             subpatSet(context, (RubyRegexp) arg0, null, arg1);
             return arg1;
@@ -3654,16 +3638,16 @@ public IRubyObject op_aset19(ThreadContext context, IRubyObject arg0, IRubyObjec
                 return arg1;
             }
         }
-        return op_aset19(context, RubyNumeric.num2int(arg0), arg1);
+        return op_aset(context, RubyNumeric.num2int(arg0), arg1);
     }
 
-    private IRubyObject op_aset19(ThreadContext context, int idx, IRubyObject arg1) {
+    private IRubyObject op_aset(ThreadContext context, int idx, IRubyObject arg1) {
         StringSupport.replaceInternal19(context.runtime, idx, 1, this, arg1.convertToString());
         return arg1;
     }
 
     @JRubyMethod(name = "[]=", reads = BACKREF)
-    public IRubyObject op_aset19(ThreadContext context, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2) {
+    public IRubyObject op_aset(ThreadContext context, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2) {
         if (arg0 instanceof RubyRegexp) {
             subpatSet(context, (RubyRegexp)arg0, arg1, arg2);
         } else {
@@ -3695,14 +3679,14 @@ public IRubyObject slice_bang(ThreadContext context, IRubyObject arg0) {
         if (result.isNil()) {
             modifyCheck(); // keep cr ?
         } else {
-            op_aset19(context, arg0, RubyString.newEmptyString(context.runtime));
+            op_aset(context, arg0, RubyString.newEmptyString(context.runtime));
         }
         return result;
     }
 
     @JRubyMethod(name = "slice!", reads = BACKREF, writes = BACKREF)
     public IRubyObject slice_bang(ThreadContext context, IRubyObject arg0, IRubyObject arg1) {
-        IRubyObject result = op_aref19(context, arg0, arg1);
+        IRubyObject result = op_aref(context, arg0, arg1);
         if (result.isNil()) {
             modifyCheck(); // keep cr ?
         } else {
@@ -6524,4 +6508,24 @@ public IRubyObject op_equal19(ThreadContext context, IRubyObject other) {
         return op_equal(context, other);
     }
 
+    @Deprecated
+    public IRubyObject op_aref19(ThreadContext context, IRubyObject arg1, IRubyObject arg2) {
+        return op_aref(context, arg1, arg2);
+    }
+
+    @Deprecated
+    public IRubyObject op_aref19(ThreadContext context, IRubyObject arg) {
+        return op_aref(context, arg);
+    }
+
+    @Deprecated
+    public IRubyObject op_aset19(ThreadContext context, IRubyObject arg0, IRubyObject arg1) {
+        return op_aset(context, arg0, arg1);
+    }
+
+    @Deprecated
+    public IRubyObject op_aset19(ThreadContext context, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2) {
+        return op_aset(context, arg0, arg1, arg2);
+    }
+
 }
diff --git a/core/src/main/java/org/jruby/RubySymbol.java b/core/src/main/java/org/jruby/RubySymbol.java
@@ -570,12 +570,12 @@ public IRubyObject match_p(ThreadContext context, IRubyObject other, IRubyObject
 
     @JRubyMethod(name = {"[]", "slice"})
     public IRubyObject op_aref(ThreadContext context, IRubyObject arg) {
-        return newShared(context.runtime).op_aref19(context, arg);
+        return newShared(context.runtime).op_aref(context, arg);
     }
 
     @JRubyMethod(name = {"[]", "slice"})
     public IRubyObject op_aref(ThreadContext context, IRubyObject arg1, IRubyObject arg2) {
-        return newShared(context.runtime).op_aref19(context, arg1, arg2);
+        return newShared(context.runtime).op_aref(context, arg1, arg2);
     }
 
     @JRubyMethod(name = {"length", "size"})

diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/CompiledIRMethod.java b/core/src/main/java/org/jruby/internal/runtime/methods/CompiledIRMethod.java
@@ -1,6 +1,10 @@
 package org.jruby.internal.runtime.methods;
 
 import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+
+import com.headius.invokebinder.Binder;
 import org.jruby.RubyModule;
 import org.jruby.internal.runtime.AbstractIRMethod;
 import org.jruby.ir.IRMethod;
@@ -10,6 +14,7 @@
 import org.jruby.runtime.ArgumentDescriptor;
 import org.jruby.runtime.Block;
 import org.jruby.runtime.Helpers;
+import org.jruby.runtime.Signature;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.Visibility;
 import org.jruby.runtime.builtin.IRubyObject;
@@ -30,7 +35,24 @@ public CompiledIRMethod(MethodHandle variable, IRScope method, Visibility visibi
     public CompiledIRMethod(MethodHandle variable, MethodHandle specific, int specificArity, IRScope method,
                             Visibility visibility, RubyModule implementationClass, boolean hasKwargs) {
         super(method, visibility, implementationClass);
-        this.variable = variable;
+
+        if (hasKwargs) {
+            MethodType type = variable.type();
+            int params = type.parameterCount();
+            MethodHandle frobnicate = Binder
+                    .from(type.changeReturnType(IRubyObject[].class))
+                    .dropLast(params - 4)
+                    .drop(1, 2)
+                    .append(signature)
+                    .invoke(FROBNICATE);
+            this.variable = Binder
+                    .from(type)
+                    .fold(frobnicate)
+                    .permute(type.parameterType(params - 1) == Block.class ? new int[] {1, 2, 3, 0, 5, 6, 7, 8} : new int[] {1, 2, 3, 0, 5, 6, 7})
+                    .invoke(variable);
+        } else {
+            this.variable = variable;
+        }
         this.specific = specific;
         // deopt unboxing if we have to process kwargs hash (although this really has nothing to do with arg
         // unboxing -- it was a simple path to hacking this in).
@@ -43,6 +65,10 @@ public CompiledIRMethod(MethodHandle variable, MethodHandle specific, int specif
         setHandle(variable);
     }
 
+    private static final MethodHandle FROBNICATE = Binder
+            .from(IRubyObject[].class, ThreadContext.class, IRubyObject[].class, Signature.class)
+            .invokeStaticQuiet(MethodHandles.lookup(), IRRuntimeHelpers.class, "frobnicateKwargsArgument");
+
     public MethodHandle getHandleFor(int arity) {
         if (specificArity != -1 && arity == specificArity) {
             return specific;
@@ -69,8 +95,6 @@ public InterpreterContext ensureInstrsReady() {
 
     @Override
     public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
-        if (hasKwargs) args = IRRuntimeHelpers.frobnicateKwargsArgument(context, args, signature);
-
         try {
             return (IRubyObject) this.variable.invokeExact(context, staticScope, self, args, block, implementationClass.getMethodLocation(), name);
         }
@@ -134,8 +158,6 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz
 
     @Override
     public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args) {
-        if (hasKwargs) args = IRRuntimeHelpers.frobnicateKwargsArgument(context, args, signature);
-
         try {
             return (IRubyObject) this.variable.invokeExact(context, staticScope, self, args, Block.NULL_BLOCK, implementationClass.getMethodLocation(), name);
         }

diff --git a/core/src/main/java/org/jruby/ir/operands/TemporaryLocalVariable.java b/core/src/main/java/org/jruby/ir/operands/TemporaryLocalVariable.java
@@ -59,6 +59,18 @@ public Object retrieve(ThreadContext context, IRubyObject self, StaticScope curr
         // both here and in DynamicScope var lookups.  To be done later.
         //
         // I dont like this at all.  This feels ugly!
+
+        /* CON FIXME: Update: this logic hides misbehaving native methods that return null (we currently forbid this).
+
+           Such a method was discovered while running tests with the JIT enabled. The JIT called the method and assigned
+           its result to a temporary local variable. Normally this would be safe because as SSS mentions above the
+           ALVLSI pass has run to guarantee uninitialized locals produce nil, but this is an unexpected null result from
+           a Ruby method call, and where the interpreter ignores the null the JIT propagates it unguarded.
+
+           * We do need a more robust way to mitigate the presence of rogue null in the system. @NotNull annotation?
+           * The interpreter pays for this null check even in the "full" IR because they share this instruction.
+
+           */
         Object o = temp[offset];
         return o == null ? context.nil : o;
     }

diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
@@ -535,7 +535,7 @@ public static void checkArity(ThreadContext context, StaticScope scope, Object[]
         }
     }
 
-    public static IRubyObject[] frobnicateKwargsArgument(ThreadContext context, IRubyObject[] args, 
+    public static IRubyObject[] frobnicateKwargsArgument(ThreadContext context, IRubyObject[] args,
         org.jruby.runtime.Signature signature) {
         return frobnicateKwargsArgument(context, args, signature.required());
     }

diff --git a/core/src/main/java/org/jruby/ir/targets/Bootstrap.java b/core/src/main/java/org/jruby/ir/targets/Bootstrap.java
@@ -382,7 +382,7 @@ static MethodHandle buildGenericHandle(InvokeSite site, DynamicMethod method, Ru
         }
 
         if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {
-            LOG.info(site.name() + "\tbound indirectly " + Bootstrap.logMethod(method));
+            LOG.info(site.name() + "\tbound indirectly " + method + ", " + Bootstrap.logMethod(method));
         }
 
         return binder.invokeVirtualQuiet(LOOKUP, "call").handle();
@@ -509,10 +509,6 @@ static MethodHandle buildJittedHandle(InvokeSite site, DynamicMethod method, boo
 
         if (compiledIRMethod != null) {
 
-            // Temporary fix for missing kwargs dup+splitting logic from frobnicate, called by CompiledIRMethod but
-            // skipped by indy's direct binding.
-            if (compiledIRMethod.hasKwargs()) return null;
-
             // attempt IR direct binding
             // TODO: this will have to expand when we start specializing arities
 
@@ -552,7 +548,7 @@ static MethodHandle buildJittedHandle(InvokeSite site, DynamicMethod method, boo
             mh = binder.invoke(mh).handle();
 
             if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {
-                LOG.info(site.name() + "\tbound to jitted method " + Bootstrap.logMethod(method));
+                LOG.info(site.name() + "\tbound directly to jitted method " + Bootstrap.logMethod(method));
             }
         }
 
@@ -580,6 +576,9 @@ static MethodHandle buildNativeHandle(InvokeSite site, DynamicMethod method, boo
                         binder = SmartBinder.from(lookup(), site.signature);
                     } else {
                         // arity mismatch...leave null and use DynamicMethod.call below
+                        if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {
+                            LOG.info(site.name() + "\tdid not match the primary arity for a native method " + Bootstrap.logMethod(method));
+                        }
                     }
                 } else {
                     // varargs
@@ -722,7 +721,7 @@ private static MethodHandle createJavaHandle(InvokeSite site, DynamicMethod meth
         // This logic does not handle closure conversion yet
         if (site.fullSignature.lastArgType() == Block.class) {
             if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {
-                LOG.info(site.name() + "\tattempted passed a closure calling Java method: " + Bootstrap.logMethod(method));
+                LOG.info(site.name() + "\tpassed a closure to Java method " + nativeCall + ": " + Bootstrap.logMethod(method));
             }
             return null;
         }
@@ -1113,7 +1112,7 @@ public static CallSite prepareBlock(Lookup lookup, String name, MethodType type,
     }
 
     static String logMethod(DynamicMethod method) {
-        return "[#" + method.getSerialNumber() + " " + method.getImplementationClass() + "]";
+        return "[#" + method.getSerialNumber() + " " + method.getImplementationClass().getMethodLocation() + "]";
     }
 
     static String logBlock(Block block) {

diff --git a/core/src/test/java/org/jruby/TestRegexpCache.java b/core/src/test/java/org/jruby/TestRegexpCache.java
@@ -55,7 +55,7 @@ public void testByteListCacheKeySharing() {
         assertNotNull( RubyRegexp.patternCache.get(strBytes) );
 
         // str[0] = 'R'
-        str.op_aset19(runtime.getCurrentContext(), runtime.newFixnum(0), RubyString.newString(runtime, "R"));
+        str.op_aset(runtime.getCurrentContext(), runtime.newFixnum(0), RubyString.newString(runtime, "R"));
 
         assertEquals(ByteList.create("Regexp"), strBytes);