Merge branch '317853-fix-simpleverifier-merge' into 'master'

Resolve "SimpleVerifier merges types incorrectly when one type is an interface"

Closes #317853 and #315248

See merge request !206

asm-analysis/src/main/java/org/objectweb/asm/tree/analysis/SimpleVerifier.java

@@ -204,7 +204,17 @@ public class SimpleVerifier extends BasicVerifier {
         if (type.equals(NULL_TYPE)) {
           return true;
         } else if (type.getSort() == Type.OBJECT || type.getSort() == Type.ARRAY) {
-          return isAssignableFrom(expectedType, type);
+          if (isAssignableFrom(expectedType, type)) {
+            return true;
+          } else if (getClass(expectedType).isInterface()) {
+            // The merge of class or interface types can only yield class types (because it is not
+            // possible in general to find an unambiguous common super interface, due to multiple
+            // inheritance). Because of this limitation, we need to relax the subtyping check here
+            // if 'value' is an interface.
+            return Object.class.isAssignableFrom(getClass(type));
+          } else {
+            return false;
+          }
         } else {
           return false;
         }
@@ -343,11 +353,7 @@ public class SimpleVerifier extends BasicVerifier {
       }
       return false;
     }
-    Class<?> class1 = getClass(type1);
-    if (class1.isInterface()) {
-      class1 = Object.class;
-    }
-    return class1.isAssignableFrom(getClass(type2));
+    return getClass(type1).isAssignableFrom(getClass(type2));
   }
 
   /**

asm-analysis/src/test/java/org/objectweb/asm/tree/analysis/SimpleVerifierTest.java

@@ -27,6 +27,7 @@
 // THE POSSIBILITY OF SUCH DAMAGE.
 package org.objectweb.asm.tree.analysis;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -36,6 +37,7 @@ import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.Label;
@@ -458,6 +460,24 @@ public class SimpleVerifierTest extends AsmTest implements Opcodes {
     assertValid();
   }
 
+  @ParameterizedTest
+  @CsvSource({
+    "java/lang/String, java/lang/Number, java/lang/Object",
+    "java/lang/Integer, java/lang/Number, java/lang/Number",
+    "java/lang/Float, java/lang/Integer, java/lang/Number",
+    "java/lang/Long, java/util/List, java/lang/Object",
+    "java/util/Map, java/util/List, java/lang/Object"
+  })
+  public void testMergeObjectTypes(
+      final String internalName1, final String internalName2, final String expectedInternalName) {
+    BasicValue value1 = new BasicValue(Type.getObjectType(internalName1));
+    BasicValue value2 = new BasicValue(Type.getObjectType(internalName2));
+    BasicValue expectedValue = new BasicValue(Type.getObjectType(expectedInternalName));
+    SimpleVerifier verifier = new SimpleVerifier();
+    assertEquals(expectedValue, verifier.merge(value1, value2));
+    assertEquals(expectedValue, verifier.merge(value2, value1));
+  }
+
   @Test
   public void testClassNotFound() {
     Label loopLabel = new Label();
@@ -473,7 +493,7 @@ public class SimpleVerifierTest extends AsmTest implements Opcodes {
   }
 
   @Test
-  void testIsAssignableFrom() {
+  public void testIsAssignableFrom() {
     Type baseType = Type.getObjectType("C");
     Type superType = Type.getObjectType("D");
     Type interfaceType = Type.getObjectType("I");
@@ -511,6 +531,38 @@ public class SimpleVerifierTest extends AsmTest implements Opcodes {
     }.test();
   }
 
+  /**
+   * Checks that the merge of an ArrayList and an SQLException can be returned as an Iterable. The
+   * merged type is recomputed by SimpleVerifier as Object (because of limitations of the merging
+   * algorithm, due to multiple interface inheritance), but the subtyping check is relaxed if the
+   * super type is an interface type.
+   *
+   * @throws AnalyzerException
+   */
+  @Test
+  public void testIsAssignableFromInterface() throws AnalyzerException {
+    methodNode =
+        new MethodNode(
+            ACC_PUBLIC | ACC_STATIC,
+            "m",
+            "(Ljava/util/ArrayList;Ljava/sql/SQLException;)Ljava/lang/Iterable;",
+            null,
+            null);
+    methodNode.visitCode();
+    methodNode.visitVarInsn(ALOAD, 0);
+    Label elseLabel = new Label();
+    methodNode.visitJumpInsn(IFNULL, elseLabel);
+    methodNode.visitVarInsn(ALOAD, 0);
+    Label endIfLabel = new Label();
+    methodNode.visitJumpInsn(GOTO, endIfLabel);
+    methodNode.visitLabel(elseLabel);
+    methodNode.visitVarInsn(ALOAD, 1);
+    methodNode.visitLabel(endIfLabel);
+    methodNode.visitInsn(ARETURN);
+    methodNode.visitMaxs(1, 2);
+    assertValid();
+  }
+
   /**
    * Tests that the precompiled classes can be successfully analyzed with a SimpleVerifier.
    *