[GR-36029] [GR-36060] Fix problems in case of incompatible class chan…

…ges. PullRequest: graal/13397
oracle · Jan 12, 2023 · 6615100 · 6615100
2 parents f61545c + c515c45
commit 6615100
Show file tree

Hide file tree

Showing 29 changed files with 446 additions and 128 deletions.
diff --git a/compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java b/compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java
@@ -1595,6 +1595,14 @@ protected GuardingNode maybeEmitExplicitStoreCheck(ValueNode array, JavaKind ele
         return emitBytecodeExceptionCheck(condition, true, BytecodeExceptionKind.ARRAY_STORE, value);
     }
 
+    protected ValueNode maybeEmitIncompatibleClassChangeErrorCheck(ValueNode receiver, InvokeKind invokeKind, ResolvedJavaType referencedType) {
+        if (invokeKind != InvokeKind.Interface || receiver.getStackKind() != JavaKind.Object || !needsIncompatibleClassChangeErrorCheck()) {
+            return receiver;
+        }
+        GraalError.guarantee(referencedType != null, "Interface calls must have a referenceType");
+        return emitIncompatibleClassChangeCheck(receiver, referencedType);
+    }
+
     @Override
     public AbstractBeginNode emitBytecodeExceptionCheck(LogicNode condition, boolean passingOnTrue, BytecodeExceptionKind exceptionKind, ValueNode... arguments) {
         AbstractBeginNode result = GraphBuilderContext.super.emitBytecodeExceptionCheck(condition, passingOnTrue, exceptionKind, arguments);
@@ -1664,7 +1672,7 @@ void genInvokeStatic(JavaMethod target) {
             }
 
             ValueNode[] args = frameState.popArguments(resolvedTarget.getSignature().getParameterCount(false));
-            Invoke invoke = appendInvoke(InvokeKind.Static, resolvedTarget, args);
+            Invoke invoke = appendInvoke(InvokeKind.Static, resolvedTarget, args, null);
             if (invoke != null && classInit[0] != null) {
                 invoke.setClassInit(classInit[0]);
             }
@@ -1689,7 +1697,7 @@ protected void genInvokeInterface(int cpi, int opcode) {
     protected void genInvokeInterface(JavaType referencedType, JavaMethod target) {
         if (callTargetIsResolved(target) && (referencedType == null || referencedType instanceof ResolvedJavaType)) {
             ValueNode[] args = frameState.popArguments(target.getSignature().getParameterCount(true));
-            Invoke invoke = appendInvoke(InvokeKind.Interface, (ResolvedJavaMethod) target, args);
+            Invoke invoke = appendInvoke(InvokeKind.Interface, (ResolvedJavaMethod) target, args, (ResolvedJavaType) referencedType);
             if (invoke != null) {
                 invoke.callTarget().setReferencedType((ResolvedJavaType) referencedType);
             }
@@ -1732,7 +1740,7 @@ protected void genInvokeVirtual(ResolvedJavaMethod resolvedTarget) {
         }
 
         ValueNode[] args = frameState.popArguments(resolvedTarget.getSignature().getParameterCount(true));
-        appendInvoke(InvokeKind.Virtual, resolvedTarget, args);
+        appendInvoke(InvokeKind.Virtual, resolvedTarget, args, null);
     }
 
     private boolean genDynamicInvokeHelper(ResolvedJavaMethod target, int cpi, int opcode) {
@@ -1751,9 +1759,9 @@ private boolean genDynamicInvokeHelper(ResolvedJavaMethod target, int cpi, int o
         boolean hasReceiver = (opcode == INVOKEDYNAMIC) ? false : !target.isStatic();
         ValueNode[] args = frameState.popArguments(target.getSignature().getParameterCount(hasReceiver));
         if (hasReceiver) {
-            appendInvoke(InvokeKind.Virtual, target, args);
+            appendInvoke(InvokeKind.Virtual, target, args, null);
         } else {
-            appendInvoke(InvokeKind.Static, target, args);
+            appendInvoke(InvokeKind.Static, target, args, null);
         }
 
         return true;
@@ -1769,7 +1777,7 @@ void genInvokeSpecial(JavaMethod target) {
             assert target != null;
             assert target.getSignature() != null;
             ValueNode[] args = frameState.popArguments(target.getSignature().getParameterCount(true));
-            appendInvoke(InvokeKind.Special, (ResolvedJavaMethod) target, args);
+            appendInvoke(InvokeKind.Special, (ResolvedJavaMethod) target, args, null);
         } else {
             handleUnresolvedInvoke(target, InvokeKind.Special);
         }
@@ -1816,11 +1824,16 @@ public JavaType getInvokeReturnType() {
 
     @Override
     public Invoke handleReplacedInvoke(InvokeKind invokeKind, ResolvedJavaMethod targetMethod, ValueNode[] args, boolean inlineEverything) {
+        GraalError.guarantee(invokeKind != InvokeKind.Interface, "Interface invoke needs a referencedType");
+        return handleReplacedInvoke(invokeKind, targetMethod, args, inlineEverything, null);
+    }
+
+    public Invoke handleReplacedInvoke(InvokeKind invokeKind, ResolvedJavaMethod targetMethod, ValueNode[] args, boolean inlineEverything, ResolvedJavaType referencedType) {
         boolean previous = forceInliningEverything;
         forceInliningEverything = previous || inlineEverything;
         try {
             setBciCanBeDuplicated(true);
-            return appendInvoke(invokeKind, targetMethod, args);
+            return appendInvoke(invokeKind, targetMethod, args, referencedType);
         } finally {
             setBciCanBeDuplicated(false);
             forceInliningEverything = previous;
@@ -1834,7 +1847,20 @@ public void handleReplacedInvoke(CallTargetNode callTarget, JavaKind resultType)
         createNonInlinedInvoke(exceptionEdgeAction, bci(), callTarget, resultType);
     }
 
-    protected Invoke appendInvoke(InvokeKind initialInvokeKind, ResolvedJavaMethod initialTargetMethod, ValueNode[] args) {
+    protected Invoke appendInvoke(InvokeKind initialInvokeKind, ResolvedJavaMethod initialTargetMethod, ValueNode[] args, ResolvedJavaType referencedType) {
+        if (!parsingIntrinsic() && DeoptALot.getValue(options)) {
+            append(new DeoptimizeNode(DeoptimizationAction.None, RuntimeConstraint));
+            JavaKind resultType = initialTargetMethod.getSignature().getReturnKind();
+            frameState.pushReturn(resultType, ConstantNode.defaultForKind(resultType, graph));
+            return null;
+        }
+
+        if (initialInvokeKind.hasReceiver()) {
+            args[0] = maybeEmitExplicitNullCheck(args[0]);
+            /* This check must be done before any de-virtualization of the invoke. */
+            args[0] = maybeEmitIncompatibleClassChangeErrorCheck(args[0], initialInvokeKind, referencedType);
+        }
+
         ResolvedJavaMethod targetMethod = initialTargetMethod;
         InvokeKind invokeKind = initialInvokeKind;
         if (initialInvokeKind.isIndirect()) {
@@ -1847,16 +1873,7 @@ protected Invoke appendInvoke(InvokeKind initialInvokeKind, ResolvedJavaMethod i
         }
 
         JavaKind resultType = targetMethod.getSignature().getReturnKind();
-        if (!parsingIntrinsic() && DeoptALot.getValue(options)) {
-            append(new DeoptimizeNode(DeoptimizationAction.None, RuntimeConstraint));
-            frameState.pushReturn(resultType, ConstantNode.defaultForKind(resultType, graph));
-            return null;
-        }
-
         JavaType returnType = maybeEagerlyResolve(targetMethod.getSignature().getReturnType(method.getDeclaringClass()), targetMethod.getDeclaringClass());
-        if (invokeKind.hasReceiver()) {
-            args[0] = maybeEmitExplicitNullCheck(args[0]);
-        }
 
         if (initialInvokeKind == InvokeKind.Special && !targetMethod.isConstructor()) {
             emitCheckForInvokeSuperSpecial(args);
@@ -1956,11 +1973,7 @@ protected Invoke appendInvoke(InvokeKind initialInvokeKind, ResolvedJavaMethod i
      *            declared in a interface
      */
     private void emitCheckForDeclaringClassChange(ResolvedJavaType declaringClass, ValueNode[] args) {
-        ValueNode receiver = args[0];
-        TypeReference checkedType = TypeReference.createTrusted(graph.getAssumptions(), declaringClass);
-        LogicNode condition = genUnique(createInstanceOf(checkedType, receiver, null));
-        FixedGuardNode fixedGuard = append(new FixedGuardNode(condition, ClassCastException, None, false));
-        args[0] = append(PiNode.create(receiver, StampFactory.object(checkedType, true), fixedGuard));
+        args[0] = emitIncompatibleClassChangeCheck(args[0], declaringClass);
     }
 
     /**
@@ -1977,14 +1990,22 @@ protected void emitCheckForInvokeSuperSpecial(ValueNode[] args) {
         ResolvedJavaType callingClass = method.getDeclaringClass();
         callingClass = getHostClass(callingClass);
         if (callingClass.isInterface()) {
-            ValueNode receiver = args[0];
-            TypeReference checkedType = TypeReference.createTrusted(graph.getAssumptions(), callingClass);
-            LogicNode condition = genUnique(createInstanceOf(checkedType, receiver, null));
-            FixedGuardNode fixedGuard = append(new FixedGuardNode(condition, ClassCastException, None, false));
-            args[0] = append(PiNode.create(receiver, StampFactory.object(checkedType, true), fixedGuard));
+            args[0] = emitIncompatibleClassChangeCheck(args[0], callingClass);
         }
     }
 
+    protected ValueNode emitIncompatibleClassChangeCheck(ValueNode object, ResolvedJavaType checkedType) {
+        TypeReference checkedTypeRef = TypeReference.createTrusted(graph.getAssumptions(), checkedType);
+        LogicNode condition = genUnique(InstanceOfNode.create(checkedTypeRef, object));
+        ValueNode guardingNode;
+        if (needsExplicitException()) {
+            guardingNode = emitBytecodeExceptionCheck(condition, true, BytecodeExceptionKind.INCOMPATIBLE_CLASS_CHANGE);
+        } else {
+            guardingNode = append(new FixedGuardNode(condition, ClassCastException, None, false));
+        }
+        return append(PiNode.create(object, StampFactory.object(checkedTypeRef, true), guardingNode));
+    }
+
     @SuppressWarnings("deprecation")
     private static ResolvedJavaType getHostClass(ResolvedJavaType type) {
         ResolvedJavaType hostClass = type.getHostClass();
@@ -4723,6 +4744,10 @@ protected boolean needsExplicitStoreCheckException(ValueNode array, ValueNode va
         return needsExplicitException();
     }
 
+    protected boolean needsIncompatibleClassChangeErrorCheck() {
+        return false;
+    }
+
     @Override
     public boolean needsExplicitException() {
         BytecodeExceptionMode exceptionMode = graphBuilderConfig.getBytecodeExceptionMode();

diff --git a/...graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/extended/BytecodeExceptionNode.java b/...graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/extended/BytecodeExceptionNode.java
@@ -101,6 +101,11 @@ public enum BytecodeExceptionKind {
          */
         ARRAY_STORE(1, ArrayStoreException.class),
 
+        /**
+         * Represents a {@link IncompatibleClassChangeError}. No arguments are allowed.
+         */
+        INCOMPATIBLE_CLASS_CHANGE(0, IncompatibleClassChangeError.class),
+
         /** Represents a {@link AssertionError} without arguments. */
         ASSERTION_ERROR_NULLARY(0, AssertionError.class),
 

diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/BigBang.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/BigBang.java
@@ -129,4 +129,9 @@ default void onTypeInitialized(AnalysisType type) {
     default void afterAnalysis() {
 
     }
+
+    @SuppressWarnings("unused")
+    default AnalysisMethod fallbackResolveConcreteMethod(AnalysisType resolvingType, AnalysisMethod method) {
+        return null;
+    }
 }
diff --git a/...src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AccessFieldTypeFlow.java b/...src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AccessFieldTypeFlow.java
@@ -38,7 +38,7 @@ public abstract class AccessFieldTypeFlow extends TypeFlow<BytecodePosition> {
 
     protected AccessFieldTypeFlow(BytecodePosition accessLocation, AnalysisField field) {
         /* The declared type of a field access node is the field declared type. */
-        super(accessLocation, field.getType());
+        super(accessLocation, filterUncheckedInterface(field.getType()));
         this.field = field;
     }
 

diff --git a/...com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ActualParameterTypeFlow.java b/...com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ActualParameterTypeFlow.java
@@ -34,7 +34,7 @@
  */
 public class ActualParameterTypeFlow extends TypeFlow<ValueNode> {
     public ActualParameterTypeFlow(AnalysisType declaredType) {
-        super(null, declaredType);
+        super(null, filterUncheckedInterface(declaredType));
     }
 
     @Override

diff --git a/...rc/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ActualReturnTypeFlow.java b/...rc/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/ActualReturnTypeFlow.java
@@ -33,11 +33,11 @@ public class ActualReturnTypeFlow extends TypeFlow<BytecodePosition> {
     private InvokeTypeFlow invokeFlow;
 
     public ActualReturnTypeFlow(AnalysisType declaredType) {
-        super(null, declaredType);
+        this(null, declaredType);
     }
 
     public ActualReturnTypeFlow(BytecodePosition source, AnalysisType declaredType) {
-        super(source, declaredType);
+        super(source, filterUncheckedInterface(declaredType));
     }
 
     public ActualReturnTypeFlow(ActualReturnTypeFlow original, MethodFlowsGraph methodFlows) {

diff --git a/...atevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldTypeFlow.java b/...atevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FieldTypeFlow.java
@@ -61,7 +61,7 @@ private static TypeState initialFieldState(AnalysisField field) {
     private volatile FieldFilterTypeFlow filterFlow;
 
     public FieldTypeFlow(AnalysisField field, AnalysisType type) {
-        super(field, type, initialFieldState(field));
+        super(field, filterUncheckedInterface(type), initialFieldState(field));
     }
 
     public FieldTypeFlow(AnalysisField field, AnalysisType type, AnalysisObject object) {

diff --git a/...src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalParamTypeFlow.java b/...src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalParamTypeFlow.java
@@ -38,7 +38,7 @@ public class FormalParamTypeFlow extends TypeFlow<BytecodePosition> {
     protected final int position;
 
     public FormalParamTypeFlow(BytecodePosition sourcePosition, AnalysisType declaredType, int position) {
-        super(sourcePosition, declaredType);
+        super(sourcePosition, filterUncheckedInterface(declaredType));
         this.position = position;
     }
 

diff --git a/...rc/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReturnTypeFlow.java b/...rc/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/FormalReturnTypeFlow.java
@@ -31,7 +31,7 @@
 
 public class FormalReturnTypeFlow extends TypeFlow<BytecodePosition> {
     public FormalReturnTypeFlow(BytecodePosition source, AnalysisType declaredType) {
-        super(source, declaredType);
+        super(source, filterUncheckedInterface(declaredType));
     }
 
     public FormalReturnTypeFlow(FormalReturnTypeFlow original, MethodFlowsGraph methodFlows) {

diff --git a/.../src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetLoadTypeFlow.java b/.../src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetLoadTypeFlow.java
@@ -144,7 +144,7 @@ public String toString() {
     public abstract static class AbstractUnsafeLoadTypeFlow extends OffsetLoadTypeFlow {
 
         AbstractUnsafeLoadTypeFlow(BytecodePosition loadLocation, AnalysisType objectType, AnalysisType componentType, TypeFlow<?> objectFlow) {
-            super(loadLocation, objectType, componentType, objectFlow);
+            super(loadLocation, objectType, filterUncheckedInterface(componentType), objectFlow);
         }
 
         AbstractUnsafeLoadTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, AbstractUnsafeLoadTypeFlow original) {

diff --git a/...src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetStoreTypeFlow.java b/...src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/OffsetStoreTypeFlow.java
@@ -183,7 +183,7 @@ public String toString() {
     public abstract static class AbstractUnsafeStoreTypeFlow extends OffsetStoreTypeFlow {
 
         AbstractUnsafeStoreTypeFlow(BytecodePosition storeLocation, AnalysisType objectType, AnalysisType componentType, TypeFlow<?> objectFlow, TypeFlow<?> valueFlow) {
-            super(storeLocation, objectType, componentType, objectFlow, valueFlow);
+            super(storeLocation, objectType, filterUncheckedInterface(componentType), objectFlow, valueFlow);
         }
 
         AbstractUnsafeStoreTypeFlow(PointsToAnalysis bb, MethodFlowsGraph methodFlows, OffsetStoreTypeFlow original) {

diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/TypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/TypeFlow.java
@@ -568,6 +568,28 @@ public TypeState declaredTypeFilter(PointsToAnalysis bb, TypeState newState) {
         return TypeState.forIntersection(bb, newState, declaredType.getAssignableTypes(true));
     }
 
+    /**
+     * In Java, interface types are not checked by the bytecode verifier. So even when, e.g., a
+     * method parameter has the declared type Comparable, any Object can be passed in. We therefore
+     * need to filter out interface types, as well as arrays of interface types, in many places
+     * where we use the declared type.
+     *
+     * Places where interface types need to be filtered: method parameters, method return values,
+     * and field loads (including unsafe memory loads).
+     * 
+     * Places where interface types need not be filtered: array element loads (because all array
+     * stores have an array store check).
+     */
+    public static AnalysisType filterUncheckedInterface(AnalysisType type) {
+        if (type != null) {
+            AnalysisType elementalType = type.getElementalType();
+            if (elementalType.isInterface()) {
+                return type.getUniverse().objectType().getArrayClass(type.getArrayDimension());
+            }
+        }
+        return type;
+    }
+
     public void update(PointsToAnalysis bb) {
         TypeState curState = getState();
         for (TypeFlow<?> use : getUses()) {

diff --git a/...ratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java b/...ratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java
@@ -318,6 +318,10 @@ public AnalysisType getArrayClass(int dim) {
         return result;
     }
 
+    public int getArrayDimension() {
+        return dimension;
+    }
+
     public void cleanupAfterAnalysis() {
         instantiatedTypes = null;
         instantiatedTypesNonNull = null;
@@ -1015,7 +1019,7 @@ public AnalysisType getComponentType() {
     }
 
     @Override
-    public ResolvedJavaType getElementalType() {
+    public AnalysisType getElementalType() {
         return elementalType;
     }
 
@@ -1046,6 +1050,9 @@ public AnalysisMethod resolveConcreteMethod(ResolvedJavaMethod method, ResolvedJ
             ResolvedJavaType substCallerType = substMethod.getDeclaringClass();
 
             Object newResolvedMethod = universe.lookup(wrapped.resolveConcreteMethod(substMethod, substCallerType));
+            if (newResolvedMethod == null) {
+                newResolvedMethod = getUniverse().getBigbang().fallbackResolveConcreteMethod(this, (AnalysisMethod) method);
+            }
             if (newResolvedMethod == null) {
                 newResolvedMethod = NULL_METHOD;
             }
@@ -1254,7 +1261,7 @@ public ResolvedJavaType getHostClass() {
         return universe.lookup(wrapped.getHostClass());
     }
 
-    AnalysisUniverse getUniverse() {
+    public AnalysisUniverse getUniverse() {
         return universe;
     }
 

diff --git a/...rc/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ObjectTreePrinter.java b/...rc/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ObjectTreePrinter.java
@@ -434,7 +434,7 @@ static class WorkListEntry {
     }
 
     private boolean suppressType(AnalysisType type) {
-        AnalysisType elementalType = (AnalysisType) type.getElementalType();
+        AnalysisType elementalType = type.getElementalType();
         String elementalTypeName = elementalType.toJavaName(true);
 
         if (expandTypeMatcher.matches(elementalTypeName)) {