[Truffle] GetConstantNode should guard name when constant == null

jruby · Oct 15, 2015 · 25c265d · 25c265d
1 parent a8135a2
commit 25c265d
Showing 2 changed files with 41 additions and 6 deletions.
diff --git a/tool/truffle-findbugs-exclude.xml b/tool/truffle-findbugs-exclude.xml
@@ -82,6 +82,10 @@
     <Class name="org.jruby.truffle.nodes.constants.LookupConstantNode" />
     <Bug pattern="ES_COMPARING_PARAMETER_STRING_WITH_EQ" />
   </Match>
+  <Match>
+    <Class name="org.jruby.truffle.nodes.constants.GetConstantNode" />
+    <Bug pattern="ES_COMPARING_PARAMETER_STRING_WITH_EQ" />
+  </Match>
 
   <!-- Stuff where we're genuinely doing something complicated and we do know what we're doing -->
 

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/constants/GetConstantNode.java b/truffle/src/main/java/org/jruby/truffle/nodes/constants/GetConstantNode.java
@@ -17,6 +17,7 @@
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.object.DynamicObject;
 import com.oracle.truffle.api.source.SourceSection;
+import com.oracle.truffle.api.utilities.ConditionProfile;
 import org.jruby.truffle.nodes.RubyNode;
 import org.jruby.truffle.nodes.core.KernelNodes.RequireNode;
 import org.jruby.truffle.nodes.core.KernelNodesFactory;
@@ -32,6 +33,7 @@
 public abstract class GetConstantNode extends RubyNode {
 
     private final RestartableReadConstantNode readConstantNode;
+    private @Child CallDispatchHeadNode constMissingNode;
 
     public GetConstantNode(RubyContext context, SourceSection sourceSection, RestartableReadConstantNode readConstantNode) {
         super(context, sourceSection);
@@ -47,8 +49,8 @@ protected Object getConstant(DynamicObject module, String name, RubyConstant con
 
     @Specialization(guards = { "constant != null", "constant.isAutoload()" })
     protected Object autoloadConstant(VirtualFrame frame, DynamicObject module, String name, RubyConstant constant,
-            @Cached("createRequireNode()") RequireNode requireNode,
-            @Cached("deepCopyReadConstantNode()") RestartableReadConstantNode readConstantNode) {
+                                      @Cached("createRequireNode()") RequireNode requireNode,
+                                      @Cached("deepCopyReadConstantNode()") RestartableReadConstantNode readConstantNode) {
 
         final DynamicObject path = (DynamicObject) constant.getValue();
 
@@ -64,15 +66,35 @@ protected Object autoloadConstant(VirtualFrame frame, DynamicObject module, Stri
         }
     }
 
-    @Specialization(guards = "constant == null")
+    @Specialization(guards = {
+            "constant == null",
+            "guardName(name, cachedName, sameNameProfile)" })
     protected Object missingConstant(VirtualFrame frame, DynamicObject module, String name, Object constant,
-            @Cached("isValidConstantName(name)") boolean isValidConstantName,
-            @Cached("createConstMissingNode()") CallDispatchHeadNode constMissingNode,
-            @Cached("getSymbol(name)") DynamicObject symbolName) {
+                                     @Cached("name") String cachedName,
+                                     @Cached("isValidConstantName(name)") boolean isValidConstantName,
+                                     @Cached("createConstMissingNode()") CallDispatchHeadNode constMissingNode,
+                                     @Cached("getSymbol(name)") DynamicObject symbolName,
+                                     @Cached("createBinaryProfile()") ConditionProfile sameNameProfile) {
+        return missingConstantBody(frame, module, name, isValidConstantName, constMissingNode, symbolName);
+    }
+
+    @Specialization(guards = "constant == null")
+    protected Object missingConstantUncached(VirtualFrame frame, DynamicObject module, String name, Object constant) {
+        final boolean isValidConstantName = isValidConstantName(name);
+        if (constMissingNode == null) {
+            CompilerDirectives.transferToInterpreter();
+            constMissingNode = insert(createConstMissingNode());
+        }
+
+        return missingConstantBody(frame, module, name, isValidConstantName, constMissingNode, getSymbol(name));
+    }
+
+    private Object missingConstantBody(VirtualFrame frame, DynamicObject module, String name, boolean isValidConstantName, CallDispatchHeadNode constMissingNode, DynamicObject symbolName) {
         if (!isValidConstantName) {
             CompilerDirectives.transferToInterpreter();
             throw new RaiseException(getContext().getCoreLibrary().nameError(String.format("wrong constant name %s", name), name, this));
         }
+
         return constMissingNode.call(frame, module, "const_missing", null, symbolName);
     }
 
@@ -92,4 +114,13 @@ protected CallDispatchHeadNode createConstMissingNode() {
         return DispatchHeadNodeFactory.createMethodCall(getContext());
     }
 
+    protected boolean guardName(String name, String cachedName, ConditionProfile sameNameProfile) {
+        // This is likely as for literal constant lookup the name does not change and Symbols always return the same String.
+        if (sameNameProfile.profile(name == cachedName)) {
+            return true;
+        } else {
+            return name.equals(cachedName);
+        }
+    }
+
 }