You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ma...@apache.org on 2015/03/07 20:38:35 UTC

svn commit: r1664899 - in /commons/proper/bcel/trunk/src: changes/ main/java/org/apache/bcel/generic/ main/java/org/apache/bcel/verifier/structurals/ test/java/org/apache/bcel/verifier/ test/java/org/apache/bcel/verifier/tests/ test/resources/org/apach...

Author: markt
Date: Sat Mar  7 19:38:34 2015
New Revision: 1664899

URL: http://svn.apache.org/r1664899
Log:
Fix BCEL-188
Patch by Jérôme Leroux
Correct verification of the return value of a method.

Added:
    commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArray01.java   (with props)
    commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestReturn02.java   (with props)
    commons/proper/bcel/trunk/src/test/resources/org/apache/bcel/verifier/tests/TestReturn03.class   (with props)
Modified:
    commons/proper/bcel/trunk/src/changes/changes.xml
    commons/proper/bcel/trunk/src/main/java/org/apache/bcel/generic/ReferenceType.java
    commons/proper/bcel/trunk/src/main/java/org/apache/bcel/generic/Type.java
    commons/proper/bcel/trunk/src/main/java/org/apache/bcel/verifier/structurals/Pass3bVerifier.java
    commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/VerifierReturnTestCase.java

Modified: commons/proper/bcel/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/changes/changes.xml?rev=1664899&r1=1664898&r2=1664899&view=diff
==============================================================================
--- commons/proper/bcel/trunk/src/changes/changes.xml (original)
+++ commons/proper/bcel/trunk/src/changes/changes.xml Sat Mar  7 19:38:34 2015
@@ -81,6 +81,9 @@ The <action> type attribute can be add,u
         The verifier should not check for run time compatibility of objects
         assigned to arrays.
       </action>
+      <action issue="BCEL-188" type="fix" dev="markt" due-to="Jérôme Leroux">
+        Correct verification of the return value of a method.
+      </action>
       <action issue="BCEL-186" type="fix" dev="sebb">
         Performance degradation with the UTF8 cache
         getInstance no longer uses cache

Modified: commons/proper/bcel/trunk/src/main/java/org/apache/bcel/generic/ReferenceType.java
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/main/java/org/apache/bcel/generic/ReferenceType.java?rev=1664899&r1=1664898&r2=1664899&view=diff
==============================================================================
--- commons/proper/bcel/trunk/src/main/java/org/apache/bcel/generic/ReferenceType.java (original)
+++ commons/proper/bcel/trunk/src/main/java/org/apache/bcel/generic/ReferenceType.java Sat Mar  7 19:38:34 2015
@@ -57,7 +57,7 @@ public abstract class ReferenceType exte
      */
     public boolean isCastableTo( Type t ) throws ClassNotFoundException {
         if (this.equals(Type.NULL)) {
-            return true; // If this is ever changed in isAssignmentCompatible()
+            return t instanceof ReferenceType; // If this is ever changed in isAssignmentCompatible()
         }
         return isAssignmentCompatibleWith(t);
         /* Yes, it's true: It's the same definition.

Modified: commons/proper/bcel/trunk/src/main/java/org/apache/bcel/generic/Type.java
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/main/java/org/apache/bcel/generic/Type.java?rev=1664899&r1=1664898&r2=1664899&view=diff
==============================================================================
--- commons/proper/bcel/trunk/src/main/java/org/apache/bcel/generic/Type.java (original)
+++ commons/proper/bcel/trunk/src/main/java/org/apache/bcel/generic/Type.java Sat Mar  7 19:38:34 2015
@@ -23,8 +23,10 @@ import java.util.List;
 import org.apache.bcel.Constants;
 import org.apache.bcel.classfile.ClassFormatException;
 import org.apache.bcel.classfile.Utility;
+import org.apache.bcel.verifier.structurals.LocalVariables;
+import org.apache.bcel.verifier.structurals.OperandStack;
 
-/** 
+/**
  * Abstract super class for all possible java types, namely basic types
  * such as int, object types like String and array types, e.g. int[]
  *
@@ -106,6 +108,19 @@ public abstract class Type implements ja
         return type;
     }
 
+    /**
+     * boolean, short and char variable are considered as int in the stack or local variable area.
+     * Returns {@link Type#INT} for {@link Type#BOOLEAN}, {@link Type#SHORT} or {@link Type#CHAR}, otherwise
+     * returns the given type.
+     * @see OperandStack#push(Type)
+     * @see LocalVariables#set(int, Type)
+     */
+    public Type normalizeForStackOrLocal(){
+        if (this == Type.BOOLEAN || this == Type.BYTE || this == Type.SHORT || this == Type.CHAR){
+            return Type.INT;
+        }
+        return this;
+    }
 
     /**
      * @return stack size of this type (2 for long and double, 0 for void, 1 otherwise)

Modified: commons/proper/bcel/trunk/src/main/java/org/apache/bcel/verifier/structurals/Pass3bVerifier.java
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/main/java/org/apache/bcel/verifier/structurals/Pass3bVerifier.java?rev=1664899&r1=1664898&r2=1664899&view=diff
==============================================================================
--- commons/proper/bcel/trunk/src/main/java/org/apache/bcel/verifier/structurals/Pass3bVerifier.java (original)
+++ commons/proper/bcel/trunk/src/main/java/org/apache/bcel/verifier/structurals/Pass3bVerifier.java Sat Mar  7 19:38:34 2015
@@ -14,7 +14,7 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  *
- */ 
+ */
 package org.apache.bcel.verifier.structurals;
 
 
@@ -30,14 +30,12 @@ import org.apache.bcel.Repository;
 import org.apache.bcel.classfile.JavaClass;
 import org.apache.bcel.classfile.Method;
 import org.apache.bcel.generic.ConstantPoolGen;
-import org.apache.bcel.generic.GETFIELD;
 import org.apache.bcel.generic.InstructionHandle;
-import org.apache.bcel.generic.InvokeInstruction;
 import org.apache.bcel.generic.JsrInstruction;
-import org.apache.bcel.generic.LoadInstruction;
 import org.apache.bcel.generic.MethodGen;
 import org.apache.bcel.generic.ObjectType;
 import org.apache.bcel.generic.RET;
+import org.apache.bcel.generic.ReferenceType;
 import org.apache.bcel.generic.ReturnInstruction;
 import org.apache.bcel.generic.ReturnaddressType;
 import org.apache.bcel.generic.Type;
@@ -53,7 +51,7 @@ import org.apache.bcel.verifier.exc.Veri
  * so-called structural verification as described in The Java Virtual Machine
  * Specification, 2nd edition.
  * More detailed information is to be found at the do_verify() method's
- * documentation. 
+ * documentation.
  *
  * @version $Id$
  * @author Enver Haase
@@ -253,56 +251,42 @@ public final class Pass3bVerifier extend
                     }
                 }
                 //see JVM $4.8.2
-                //TODO implement all based on stack 
                 Type returnedType = null;
-                InstructionHandle ihPrev = null;
-                ihPrev = ih.getPrev();
-
-                if (ihPrev != null)
-                {
-                    if( ihPrev.getInstruction() instanceof InvokeInstruction )
-                    {
-                        returnedType = ((InvokeInstruction)ihPrev.getInstruction()).getType(m.getConstantPool());
-                    }
-                    if( ihPrev.getInstruction() instanceof LoadInstruction )
-                    {
-                        int index = ((LoadInstruction)ihPrev.getInstruction()).getIndex();
-                        returnedType = lvs.get(index);
-                    }
-                    if( ihPrev.getInstruction() instanceof GETFIELD )
-                    {
-                        returnedType = ((GETFIELD)ihPrev.getInstruction()).getType(m.getConstantPool());
-                    }
+                OperandStack inStack = ic.getInFrame().getStack();
+                if (inStack.size() >= 1) {
+                    returnedType = inStack.peek();
+                } else {
+                    returnedType = Type.VOID;
                 }
 
-                if( returnedType != null )
-                {
-                    if( returnedType instanceof ObjectType )
-                    {
-                        try
-                        {
-                            if( !((ObjectType)returnedType).isAssignmentCompatibleWith(m.getReturnType()) )
-                            {
-                                throw new StructuralCodeConstraintException("Returned type "+returnedType+" does not match Method's return type "+m.getReturnType());
+                if (returnedType != null) {
+                    if (returnedType instanceof ReferenceType) {
+                        try {
+                            if (!((ReferenceType) returnedType).isCastableTo(m.getReturnType())) {
+                                invalidReturnTypeError(returnedType, m);
                             }
-                        }
-                        catch (ClassNotFoundException e)
-                        {
-                            //dont know what do do now, so raise RuntimeException
+                        } catch (ClassNotFoundException e) {
+                            // Don't know what do do now, so raise RuntimeException
                             throw new RuntimeException(e);
                         }
-                    }
-                    else if( !returnedType.equals(m.getReturnType()) )
-                    {
-                        throw new StructuralCodeConstraintException("Returned type "+returnedType+" does not match Method's return type "+m.getReturnType());
+                    } else if (!returnedType.equals(m.getReturnType().normalizeForStackOrLocal())) {
+                        invalidReturnTypeError(returnedType, m);
                     }
                 }
             }
-        }while ((ih = ih.getNext()) != null);
+        } while ((ih = ih.getNext()) != null);
 
      }
 
     /**
+     * Throws an exception indicating the returned type is not compatible with the return type of the given method
+     * @throws StructuralCodeConstraintException always
+     */
+    public void invalidReturnTypeError(Type returnedType, MethodGen m){
+        throw new StructuralCodeConstraintException("Returned type "+returnedType+" does not match Method's return type "+m.getReturnType());
+    }
+
+    /**
      * Pass 3b implements the data flow analysis as described in the Java Virtual
      * Machine Specification, Second Edition.
       * Later versions will use LocalVariablesInfo objects to verify if the

Modified: commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/VerifierReturnTestCase.java
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/VerifierReturnTestCase.java?rev=1664899&r1=1664898&r2=1664899&view=diff
==============================================================================
--- commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/VerifierReturnTestCase.java (original)
+++ commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/VerifierReturnTestCase.java Sat Mar  7 19:38:34 2015
@@ -21,6 +21,11 @@ public class VerifierReturnTestCase exte
 
     public void testInvalidReturn() {
         assertVerifyRejected("TestReturn01", "Verification of a void method that returns an object must fail.");
+        assertVerifyRejected("TestReturn03", "Verification of an int method that returns null must fail.");
     }
 
+    public void testValidReturn() {
+        assertVerifyOK("TestReturn02", "Verification of a method that returns a newly created object must pass.");
+        assertVerifyOK("TestArray01", "Verification of a method that returns an array must pass.");
+    }
 }

Added: commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArray01.java
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArray01.java?rev=1664899&view=auto
==============================================================================
--- commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArray01.java (added)
+++ commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArray01.java Sat Mar  7 19:38:34 2015
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package org.apache.bcel.verifier.tests;
+
+import java.io.Serializable;
+
+public class TestArray01{
+
+    public static Object test1(){
+        String[] a = new String[4];
+        a[0] = "";
+        a.equals(null);
+        test2(a);
+        test3(a);
+        test4(a);
+        return a;
+    }
+
+    @SuppressWarnings("unused")
+    public static void test2(Object o){
+    }
+
+    @SuppressWarnings("unused")
+    public static void test3(Serializable o){
+    }
+
+    @SuppressWarnings("unused")
+    public static void test4(Cloneable o){
+    }
+
+    public static Serializable test5(){
+        return new Object[1];
+    }
+
+    public static Cloneable test6(){
+        return new Object[1];
+    }
+
+    public static Object foo(String s){
+        return s;
+    }
+}
\ No newline at end of file

Propchange: commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArray01.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestReturn02.java
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestReturn02.java?rev=1664899&view=auto
==============================================================================
--- commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestReturn02.java (added)
+++ commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestReturn02.java Sat Mar  7 19:38:34 2015
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * 
+ */
+package org.apache.bcel.verifier.tests;
+
+public class TestReturn02 {
+
+    public static String test1(char[] data, int offset, int count) {
+        return new String(data, offset, count);
+    }
+    
+    public static Object test2(){
+        return new Object();
+    }
+    
+    public static boolean test3(){
+        return true;
+    }
+    
+    public static byte test4(){
+        return 1;
+    }
+    
+    public static short test5(){
+        return 1;
+    }
+    
+    public static char test6(){
+        return 'a';
+    }
+    
+    public static int test7(){
+        return 1;
+    }
+    
+    public static long test8(){
+        return 1l;
+    }
+    
+    public static float test9(){
+        return 1.0f;
+    }
+    
+    public static double test10(){
+        return 1.0;
+    }
+    
+    public static Object test11(){
+        return null;
+    }
+}

Propchange: commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestReturn02.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: commons/proper/bcel/trunk/src/test/resources/org/apache/bcel/verifier/tests/TestReturn03.class
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/test/resources/org/apache/bcel/verifier/tests/TestReturn03.class?rev=1664899&view=auto
==============================================================================
Binary file - no diff available.

Propchange: commons/proper/bcel/trunk/src/test/resources/org/apache/bcel/verifier/tests/TestReturn03.class
------------------------------------------------------------------------------
    svn:mime-type = application/octet-stream