[Truffle] Fix semantics for Kernel#public_send.

* It simply only allows calls to public methods. * See #4125. * public_send does not seems omitted from backtrace like send is.
jruby · Sep 7, 2016 · 1dc8c8e · 1dc8c8e
1 parent 157321f
commit 1dc8c8e
Show file tree

Hide file tree

Showing 9 changed files with 27 additions and 22 deletions.
diff --git a/spec/truffle/tags/core/kernel/public_send_tags.txt b/spec/truffle/tags/core/kernel/public_send_tags.txt
diff --git a/truffle/src/main/java/org/jruby/truffle/core/CoreLibrary.java b/truffle/src/main/java/org/jruby/truffle/core/CoreLibrary.java
@@ -282,7 +282,6 @@ public class CoreLibrary {
     private final Map<Errno, DynamicObject> errnoClasses = new HashMap<>();
 
     @CompilationFinal private InternalMethod basicObjectSendMethod;
-    @CompilationFinal private InternalMethod kernelPublicSendMethod;
 
     @CompilationFinal private GlobalVariableStorage loadPathStorage;
     @CompilationFinal private GlobalVariableStorage loadedFeaturesStorage;
@@ -803,7 +802,6 @@ public void addCoreMethods(PrimitiveManager primitiveManager) {
         coreMethodNodeManager.allMethodInstalled();
 
         basicObjectSendMethod = getMethod(basicObjectClass, "__send__");
-        kernelPublicSendMethod = getMethod(kernelModule, "public_send");
     }
 
     private InternalMethod getMethod(DynamicObject module, String name) {
@@ -1430,8 +1428,7 @@ public boolean isLoaded() {
 
     public boolean isSend(InternalMethod method) {
         CallTarget callTarget = method.getCallTarget();
-        return callTarget == basicObjectSendMethod.getCallTarget() ||
-                callTarget == kernelPublicSendMethod.getCallTarget();
+        return callTarget == basicObjectSendMethod.getCallTarget();
     }
 
     public DynamicObjectFactory getIntRangeFactory() {

diff --git a/truffle/src/main/java/org/jruby/truffle/core/kernel/KernelNodes.java b/truffle/src/main/java/org/jruby/truffle/core/kernel/KernelNodes.java
@@ -36,7 +36,9 @@
 import com.oracle.truffle.api.profiles.ConditionProfile;
 import com.oracle.truffle.api.source.Source;
 import com.oracle.truffle.api.source.SourceSection;
+
 import jnr.constants.platform.Errno;
+
 import org.jcodings.Encoding;
 import org.jcodings.specific.UTF8Encoding;
 import org.jruby.common.IRubyWarnings;
@@ -100,6 +102,8 @@
 import org.jruby.truffle.language.control.JavaException;
 import org.jruby.truffle.language.control.RaiseException;
 import org.jruby.truffle.language.dispatch.CallDispatchHeadNode;
+import org.jruby.truffle.language.dispatch.DispatchAction;
+import org.jruby.truffle.language.dispatch.DispatchHeadNode;
 import org.jruby.truffle.language.dispatch.DispatchHeadNodeFactory;
 import org.jruby.truffle.language.dispatch.DoesRespondDispatchHeadNode;
 import org.jruby.truffle.language.dispatch.MissingBehavior;
@@ -1398,13 +1402,11 @@ public DynamicObject publicMethods(Object self, boolean includeAncestors) {
     @CoreMethod(names = "public_send", needsBlock = true, required = 1, rest = true)
     public abstract static class PublicSendNode extends CoreMethodArrayArgumentsNode {
 
-        @Child private CallDispatchHeadNode dispatchNode;
+        @Child private DispatchHeadNode dispatchNode;
 
         public PublicSendNode(RubyContext context, SourceSection sourceSection) {
             super(context, sourceSection);
-
-            dispatchNode = new CallDispatchHeadNode(context, false,
-                    MissingBehavior.CALL_METHOD_MISSING);
+            dispatchNode = new DispatchHeadNode(context, false, true, MissingBehavior.CALL_METHOD_MISSING, DispatchAction.CALL_METHOD);
         }
 
         @Specialization
@@ -1414,7 +1416,7 @@ public Object send(VirtualFrame frame, Object self, Object name, Object[] args,
 
         @Specialization
         public Object send(VirtualFrame frame, Object self, Object name, Object[] args, DynamicObject block) {
-            return dispatchNode.callWithBlock(frame, self, name, block, args);
+            return dispatchNode.dispatch(frame, self, name, block, args);
         }
 
     }

diff --git a/truffle/src/main/java/org/jruby/truffle/interop/RubyMessageResolution.java b/truffle/src/main/java/org/jruby/truffle/interop/RubyMessageResolution.java
@@ -85,7 +85,7 @@ private DispatchHeadNode getDispatchNode() {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
                 findContextNode = insert(RubyLanguage.INSTANCE.unprotectedCreateFindContextNode());
                 final RubyContext context = RubyLanguage.INSTANCE.unprotectedFindContext(findContextNode);
-                dispatchNode = insert(new DispatchHeadNode(context, true, MissingBehavior.CALL_METHOD_MISSING, DispatchAction.CALL_METHOD));
+                dispatchNode = insert(new DispatchHeadNode(context, true, false, MissingBehavior.CALL_METHOD_MISSING, DispatchAction.CALL_METHOD));
             }
 
             return dispatchNode;
@@ -117,7 +117,7 @@ private DispatchHeadNode getDispatchHeadNode() {
                 CompilerDirectives.transferToInterpreterAndInvalidate();
                 findContextNode = insert(RubyLanguage.INSTANCE.unprotectedCreateFindContextNode());
                 final RubyContext context = RubyLanguage.INSTANCE.unprotectedFindContext(findContextNode);
-                dispatchHeadNode = insert(new DispatchHeadNode(context, true, MissingBehavior.CALL_METHOD_MISSING, DispatchAction.CALL_METHOD));
+                dispatchHeadNode = insert(new DispatchHeadNode(context, true, false, MissingBehavior.CALL_METHOD_MISSING, DispatchAction.CALL_METHOD));
             }
 
             return dispatchHeadNode;

diff --git a/truffle/src/main/java/org/jruby/truffle/language/dispatch/CallDispatchHeadNode.java b/truffle/src/main/java/org/jruby/truffle/language/dispatch/CallDispatchHeadNode.java
@@ -39,7 +39,7 @@ public static CallDispatchHeadNode createMethodCallIgnoreVisibility() {
     }
 
     public CallDispatchHeadNode(RubyContext context, boolean ignoreVisibility, MissingBehavior missingBehavior) {
-        super(context, ignoreVisibility, missingBehavior, DispatchAction.CALL_METHOD);
+        super(context, ignoreVisibility, false, missingBehavior, DispatchAction.CALL_METHOD);
     }
 
     public Object call(VirtualFrame frame, Object receiver, Object method, Object... arguments) {

diff --git a/truffle/src/main/java/org/jruby/truffle/language/dispatch/DispatchHeadNode.java b/truffle/src/main/java/org/jruby/truffle/language/dispatch/DispatchHeadNode.java
@@ -18,6 +18,7 @@ public class DispatchHeadNode extends Node {
 
     protected final RubyContext context;
     protected final boolean ignoreVisibility;
+    protected final boolean onlyCallPublic;
     protected final MissingBehavior missingBehavior;
     protected final DispatchAction dispatchAction;
 
@@ -26,10 +27,11 @@ public class DispatchHeadNode extends Node {
     public DispatchHeadNode(
             RubyContext context,
             boolean ignoreVisibility,
-            MissingBehavior missingBehavior,
-            DispatchAction dispatchAction) {
+            boolean onlyCallPublic,
+            MissingBehavior missingBehavior, DispatchAction dispatchAction) {
         this.context = context;
         this.ignoreVisibility = ignoreVisibility;
+        this.onlyCallPublic = onlyCallPublic;
         this.missingBehavior = missingBehavior;
         this.dispatchAction = dispatchAction;
         first = new UnresolvedDispatchNode(context, ignoreVisibility, missingBehavior, dispatchAction);

diff --git a/truffle/src/main/java/org/jruby/truffle/language/dispatch/DispatchNode.java b/truffle/src/main/java/org/jruby/truffle/language/dispatch/DispatchNode.java
@@ -53,8 +53,6 @@ protected InternalMethod lookup(
             Object receiver,
             String name,
             boolean ignoreVisibility) {
-        assert ignoreVisibility || RubyGuards.isRubyClass(callerClass);
-
         final InternalMethod method = ModuleOperations.lookupMethod(coreLibrary().getMetaClass(receiver), name);
 
         // If no method was found, use #method_missing
@@ -68,10 +66,17 @@ protected InternalMethod lookup(
         }
 
         // Check visibility
-        if (!ignoreVisibility && !method.isVisibleTo(callerClass)) {
-            return null;
+        if (!ignoreVisibility) {
+            if (getHeadNode().onlyCallPublic) {
+                if (!method.getVisibility().isPublic()) {
+                    return null;
+                }
+            } else if (!method.isVisibleTo(callerClass)) {
+                return null;
+            }
         }
 
+
         return method;
     }
 

diff --git a/truffle/src/main/java/org/jruby/truffle/language/dispatch/DoesRespondDispatchHeadNode.java b/truffle/src/main/java/org/jruby/truffle/language/dispatch/DoesRespondDispatchHeadNode.java
@@ -19,7 +19,7 @@ public static DoesRespondDispatchHeadNode create() {
     }
 
     public DoesRespondDispatchHeadNode(RubyContext context, boolean ignoreVisibility) {
-        super(context, ignoreVisibility, MissingBehavior.RETURN_MISSING, DispatchAction.RESPOND_TO_METHOD);
+        super(context, ignoreVisibility, false, MissingBehavior.RETURN_MISSING, DispatchAction.RESPOND_TO_METHOD);
     }
 
     /**

diff --git a/truffle/src/main/java/org/jruby/truffle/language/dispatch/UnresolvedDispatchNode.java b/truffle/src/main/java/org/jruby/truffle/language/dispatch/UnresolvedDispatchNode.java
@@ -108,7 +108,7 @@ private DispatchNode doUnboxedObject(
             Object methodName) {
         final DynamicObject callerClass;
 
-        if (ignoreVisibility) {
+        if (ignoreVisibility || getHeadNode().onlyCallPublic) {
             callerClass = null;
         } else {
             callerClass = coreLibrary().getMetaClass(RubyArguments.getSelf(frame));
@@ -149,7 +149,7 @@ private DispatchNode doDynamicObject(
             Object[] argumentsObjects) {
         final DynamicObject callerClass;
 
-        if (ignoreVisibility) {
+        if (ignoreVisibility || getHeadNode().onlyCallPublic) {
             callerClass = null;
         } else if (getDispatchAction() == DispatchAction.RESPOND_TO_METHOD) {
             final FrameInstance instance = getContext().getCallStack().getCallerFrameIgnoringSend();