changeset 10421:1669d8b5863a

aot verify: check if string constant is really a interned string; javadoc updates
author Bernhard Urban <bernhard.urban@jku.at>
date Wed, 19 Jun 2013 23:47:07 +0200
parents 672e126cf996
children b61e946bf0ef
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/AheadOfTimeVerificationPhase.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/LoadJavaMirrorWithKlassPhase.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java
diffstat 3 files changed, 33 insertions(+), 13 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/AheadOfTimeVerificationPhase.java	Wed Jun 19 23:46:56 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/AheadOfTimeVerificationPhase.java	Wed Jun 19 23:47:07 2013 +0200
@@ -27,8 +27,9 @@
 import com.oracle.graal.phases.*;
 
 /**
- * Checking for embedded oops in the graph. (Interned) Strings are an exception as they live in CDS
- * space.
+ * Checks for illegal object constants in a graph processed for AOT compilation. The only legal
+ * object constants are {@linkplain String#intern() interned} strings as they will be installed in
+ * the Class Data Sharing (CDS) space.
  * 
  * @see LoadJavaMirrorWithKlassPhase
  */
@@ -37,20 +38,30 @@
     @Override
     protected boolean verify(StructuredGraph graph) {
         for (ConstantNode node : graph.getNodes().filter(ConstantNode.class)) {
-            assert !isOop(node) || isNullReference(node) || isString(node) : "embedded oop: " + node;
+            assert !isObject(node) || isNullReference(node) || isInternedString(node) : "illegal object constant: " + node;
         }
         return true;
     }
 
-    private static boolean isOop(ConstantNode node) {
+    private static boolean isObject(ConstantNode node) {
         return node.kind() == Kind.Object;
     }
 
     private static boolean isNullReference(ConstantNode node) {
-        return isOop(node) && node.asConstant().asObject() == null;
+        return isObject(node) && node.asConstant().asObject() == null;
     }
 
-    private static boolean isString(ConstantNode node) {
-        return isOop(node) && node.asConstant().asObject() instanceof String;
+    private static boolean isInternedString(ConstantNode node) {
+        if (!isObject(node)) {
+            return false;
+        }
+
+        Object o = node.asConstant().asObject();
+        if (!(o instanceof String)) {
+            return false;
+        }
+
+        String s = (String) o;
+        return s == s.intern();
     }
 }
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/LoadJavaMirrorWithKlassPhase.java	Wed Jun 19 23:46:56 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/phases/LoadJavaMirrorWithKlassPhase.java	Wed Jun 19 23:47:07 2013 +0200
@@ -30,16 +30,19 @@
 import com.oracle.graal.nodes.extended.*;
 import com.oracle.graal.nodes.type.*;
 import com.oracle.graal.phases.*;
+import com.oracle.graal.phases.common.*;
 import com.oracle.graal.phases.tiers.*;
 
 /**
- * For AOT compilation we aren't allowed to use a class reference (javaMirror) directly. Instead the
- * class reference should be obtained from the klass object. The reason for this is, that in CDS a
- * klass object is mapped to a fixed address in memory, but the javaMirror is not (which lives in
- * the java heap).
+ * For AOT compilation we aren't allowed to use a {@link Class} reference ({@code javaMirror})
+ * directly. Instead the {@link Class} reference should be obtained from the {@code Klass} object.
+ * The reason for this is, that in Class Data Sharing (CDS) a {@code Klass} object is mapped to a
+ * fixed address in memory, but the {@code javaMirror} is not (which lives in the Java heap).
  * 
- * Lowering can introduce new ConstantNodes containing a class reference, thus this phase must be
- * applied after lowering.
+ * Lowering can introduce new {@link ConstantNode}s containing a {@link Class} reference, thus this
+ * phase must be applied after {@link LoweringPhase}.
+ * 
+ * @see AheadOfTimeVerificationPhase
  */
 public class LoadJavaMirrorWithKlassPhase extends BasePhase<PhaseContext> {
 
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java	Wed Jun 19 23:46:56 2013 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java	Wed Jun 19 23:47:07 2013 +0200
@@ -31,6 +31,8 @@
 import com.oracle.graal.graph.Graph.NodeChangedListener;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.calc.*;
+import com.oracle.graal.nodes.extended.*;
+import com.oracle.graal.nodes.java.*;
 import com.oracle.graal.nodes.spi.*;
 import com.oracle.graal.nodes.type.*;
 import com.oracle.graal.nodes.util.*;
@@ -85,6 +87,10 @@
          * @param workingSet the initial working set of nodes on which the canonicalizer works,
          *            should be an auto-grow node bitmap
          * @param customCanonicalizer
+         * @param canonicalizeReads flag to indicate if
+         *            {@link LoadFieldNode#canonical(CanonicalizerTool)} and
+         *            {@link ReadNode#canonical(CanonicalizerTool)} should canonicalize reads of
+         *            constant fields.
          */
         public Instance(MetaAccessProvider runtime, Assumptions assumptions, boolean canonicalizeReads, Iterable<Node> workingSet, CustomCanonicalizer customCanonicalizer) {
             this(runtime, assumptions, canonicalizeReads, workingSet, 0, customCanonicalizer);