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:17:27 UTC

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

Author: markt
Date: Sat Mar  7 19:17:27 2015
New Revision: 1664897

URL: http://svn.apache.org/r1664897
Log:
Fix BCEL-193
Patch by Jérôme Leroux
The verifier should not check for run time compatibility of objects assigned to arrays.

Added:
    commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/VerifierArrayAccessTestCase.java   (with props)
    commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/
    commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArrayAccess01.java   (with props)
    commons/proper/bcel/trunk/src/test/resources/org/apache/bcel/verifier/tests/TestArrayAccess02.class   (with props)
    commons/proper/bcel/trunk/src/test/resources/org/apache/bcel/verifier/tests/TestArrayAccess03.class   (with props)
    commons/proper/bcel/trunk/src/test/resources/org/apache/bcel/verifier/tests/TestArrayAccess04.class   (with props)
Modified:
    commons/proper/bcel/trunk/src/changes/changes.xml
    commons/proper/bcel/trunk/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.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=1664897&r1=1664896&r2=1664897&view=diff
==============================================================================
--- commons/proper/bcel/trunk/src/changes/changes.xml (original)
+++ commons/proper/bcel/trunk/src/changes/changes.xml Sat Mar  7 19:17:27 2015
@@ -1,4 +1,4 @@
-<?xml version="1.0"?>
+<?xml version="1.0" encoding="UTF-8"?>
 <!--
 
    Licensed to the Apache Software Foundation (ASF) under one or more
@@ -77,6 +77,10 @@ The <action> type attribute can be add,u
       <action issue="BCEL-194" type="fix" due-to="Mark Roberts">
         Removed the 'index' variable from the LocalVariableGen's hash code.
       </action>
+      <action issue="BCEL-193" type="fix" dev="markt" due-to="Jérôme Leroux">
+        The verifier should not check for run time compatibility of objects
+        assigned to arrays.
+      </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/verifier/structurals/InstConstraintVisitor.java
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java?rev=1664897&r1=1664896&r2=1664897&view=diff
==============================================================================
--- commons/proper/bcel/trunk/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java (original)
+++ commons/proper/bcel/trunk/src/main/java/org/apache/bcel/verifier/structurals/InstConstraintVisitor.java Sat Mar  7 19:17:27 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;
 
 
@@ -71,14 +71,14 @@ public class InstConstraintVisitor exten
 
     /**
      * The ConstantPoolGen we're working on.
-     * 
+     *
      * @see #setConstantPoolGen(ConstantPoolGen cpg)
      */
     private ConstantPoolGen cpg = null;
 
     /**
      * The MethodGen we're working on.
-     * 
+     *
      * @see #setMethodGen(MethodGen mg)
      */
     private MethodGen mg = null;
@@ -129,7 +129,7 @@ public class InstConstraintVisitor exten
     /**
      * Sets the ConstantPoolGen instance needed for constraint
      * checking prior to execution.
-     */    
+     */
     public void setConstantPoolGen(ConstantPoolGen cpg){
         this.cpg = cpg;
     }
@@ -276,7 +276,7 @@ public class InstConstraintVisitor exten
          // visitLoadClass(o) has been called before: Every FieldOrMethod
          // implements LoadClass.
          // visitCPInstruction(o) has been called before.
-        // A FieldInstruction may be: GETFIELD, GETSTATIC, PUTFIELD, PUTSTATIC 
+        // A FieldInstruction may be: GETFIELD, GETSTATIC, PUTFIELD, PUTSTATIC
             Constant c = cpg.getConstant(o.getIndex());
             if (!(c instanceof ConstantFieldref)){
                 constraintViolated(o, "Index '"+o.getIndex()+"' should refer to a CONSTANT_Fieldref_info structure, but refers to '"+c+"'.");
@@ -453,7 +453,7 @@ public class InstConstraintVisitor exten
         if (arrayrefOfArrayType(o, arrayref)){
             if (! (((ArrayType) arrayref).getElementType() instanceof ReferenceType)){
                 constraintViolated(o, "The 'arrayref' does not refer to an array with elements of a ReferenceType but to an array of "+((ArrayType) arrayref).getElementType()+".");
-            }    
+            }
             //referenceTypeIsInitialized(o, (ReferenceType) (((ArrayType) arrayref).getElementType()));
         }
     }
@@ -463,7 +463,6 @@ public class InstConstraintVisitor exten
      */
     @Override
     public void visitAASTORE(AASTORE o){
-        try {
         Type arrayref = stack().peek(2);
         Type index    = stack().peek(1);
         Type value    = stack().peek(0);
@@ -475,18 +474,12 @@ public class InstConstraintVisitor exten
             //referenceTypeIsInitialized(o, (ReferenceType) value);
         }
         // Don't bother further with "referenceTypeIsInitialized()", there are no arrays
-        // of an uninitialized object type. 
+        // of an uninitialized object type.
         if (arrayrefOfArrayType(o, arrayref)){
             if (! (((ArrayType) arrayref).getElementType() instanceof ReferenceType)){
                 constraintViolated(o, "The 'arrayref' does not refer to an array with elements of a ReferenceType but to an array of "+((ArrayType) arrayref).getElementType()+".");
             }
-            if (! ((ReferenceType)value).isAssignmentCompatibleWith(((ArrayType) arrayref).getElementType())){
-                constraintViolated(o, "The type of 'value' ('"+value+"') is not assignment compatible to the components of the array 'arrayref' refers to. ('"+((ArrayType) arrayref).getElementType()+"')");
-            }
-        }
-        } catch (ClassNotFoundException e) {
-        // FIXME: maybe not the best way to handle this
-        throw new AssertionViolatedException("Missing class: " + e, e);
+            // No check for array element assignment compatibility. This is done at runtime.
         }
     }
 
@@ -743,7 +736,7 @@ public class InstConstraintVisitor exten
         indexOfInt(o, stack().peek());
         if (stack().peek(1) == Type.NULL){
             return;
-        } 
+        }
         if (! (stack().peek(1) instanceof ArrayType)){
             constraintViolated(o, "Stack next-to-top must be of type double[] but is '"+stack().peek(1)+"'.");
         }
@@ -764,7 +757,7 @@ public class InstConstraintVisitor exten
         indexOfInt(o, stack().peek(1));
         if (stack().peek(2) == Type.NULL){
             return;
-        } 
+        }
         if (! (stack().peek(2) instanceof ArrayType)){
             constraintViolated(o, "Stack next-to-next-to-top must be of type double[] but is '"+stack().peek(2)+"'.");
         }
@@ -1059,7 +1052,7 @@ public class InstConstraintVisitor exten
         indexOfInt(o, stack().peek());
         if (stack().peek(1) == Type.NULL){
             return;
-        } 
+        }
         if (! (stack().peek(1) instanceof ArrayType)){
             constraintViolated(o, "Stack next-to-top must be of type float[] but is '"+stack().peek(1)+"'.");
         }
@@ -1080,7 +1073,7 @@ public class InstConstraintVisitor exten
         indexOfInt(o, stack().peek(1));
         if (stack().peek(2) == Type.NULL){
             return;
-        } 
+        }
         if (! (stack().peek(2) instanceof ArrayType)){
             constraintViolated(o, "Stack next-to-next-to-top must be of type float[] but is '"+stack().peek(2)+"'.");
         }
@@ -1291,7 +1284,7 @@ public class InstConstraintVisitor exten
                     //      "Wider" object types don't allow us to check for things like that below.
                     //constraintViolated(o, "The referenced field has the ACC_PROTECTED modifier, and it's a member of the current class or a superclass of the current class. However, the referenced object type '"+stack().peek()+"' is not the current class or a subclass of the current class.");
                 }
-            } 
+            }
         }
 
         // TODO: Could go into Pass 3a.
@@ -1410,7 +1403,7 @@ public class InstConstraintVisitor exten
         indexOfInt(o, stack().peek());
         if (stack().peek(1) == Type.NULL){
             return;
-        } 
+        }
         if (! (stack().peek(1) instanceof ArrayType)){
             constraintViolated(o, "Stack next-to-top must be of type int[] but is '"+stack().peek(1)+"'.");
         }
@@ -1444,7 +1437,7 @@ public class InstConstraintVisitor exten
         indexOfInt(o, stack().peek(1));
         if (stack().peek(2) == Type.NULL){
             return;
-        } 
+        }
         if (! (stack().peek(2) instanceof ArrayType)){
             constraintViolated(o, "Stack next-to-next-to-top must be of type int[] but is '"+stack().peek(2)+"'.");
         }
@@ -1653,7 +1646,7 @@ public class InstConstraintVisitor exten
         if (!(stack().peek() instanceof ReferenceType)){
             constraintViolated(o, "The value at the stack top is not of a ReferenceType, but of type '"+stack().peek()+"'.");
         }
-        referenceTypeIsInitialized(o, (ReferenceType) (stack().peek()) );    
+        referenceTypeIsInitialized(o, (ReferenceType) (stack().peek()) );
     }
 
     /**
@@ -1664,7 +1657,7 @@ public class InstConstraintVisitor exten
         if (!(stack().peek() instanceof ReferenceType)){
             constraintViolated(o, "The value at the stack top is not of a ReferenceType, but of type '"+stack().peek()+"'.");
         }
-        referenceTypeIsInitialized(o, (ReferenceType) (stack().peek()) );    
+        referenceTypeIsInitialized(o, (ReferenceType) (stack().peek()) );
     }
 
     /**
@@ -1829,7 +1822,7 @@ public class InstConstraintVisitor exten
         //       instead of "wider cast object types" generated during verification.
         //if ( ! Repository.implementationOf(objref_classname, theInterface) ){
         //    constraintViolated(o, "The 'objref' item '"+objref+"' does not implement '"+theInterface+"' as expected.");
-        //}    
+        //}
 
         int counted_count = 1; // 1 for the objectref
         for (int i=0; i<nargs; i++){
@@ -1912,7 +1905,7 @@ public class InstConstraintVisitor exten
                 }
             }
 
-            objref_classname = ((ObjectType) objref).getClassName();        
+            objref_classname = ((ObjectType) objref).getClassName();
         }
         else{
             if (!(objref instanceof UninitializedObjectType)){
@@ -1925,7 +1918,7 @@ public class InstConstraintVisitor exten
         String theClass = o.getClassName(cpg);
         if ( ! Repository.instanceOf(objref_classname, theClass) ){
             constraintViolated(o, "The 'objref' item '"+objref+"' does not implement '"+theClass+"' as expected.");
-        }    
+        }
 
         } catch (ClassNotFoundException e) {
         // FIXME: maybe not the best way to handle this
@@ -2056,7 +2049,7 @@ public class InstConstraintVisitor exten
 
         if ( ! Repository.instanceOf(objref_classname, theClass) ){
             constraintViolated(o, "The 'objref' item '"+objref+"' does not implement '"+theClass+"' as expected.");
-        }    
+        }
         } catch (ClassNotFoundException e) {
         // FIXME: maybe not the best way to handle this
         throw new AssertionViolatedException("Missing class: " + e, e);
@@ -2241,7 +2234,7 @@ public class InstConstraintVisitor exten
         indexOfInt(o, stack().peek());
         if (stack().peek(1) == Type.NULL){
             return;
-        } 
+        }
         if (! (stack().peek(1) instanceof ArrayType)){
             constraintViolated(o, "Stack next-to-top must be of type long[] but is '"+stack().peek(1)+"'.");
         }
@@ -2275,7 +2268,7 @@ public class InstConstraintVisitor exten
         indexOfInt(o, stack().peek(1));
         if (stack().peek(2) == Type.NULL){
             return;
-        } 
+        }
         if (! (stack().peek(2) instanceof ArrayType)){
             constraintViolated(o, "Stack next-to-next-to-top must be of type long[] but is '"+stack().peek(2)+"'.");
         }
@@ -2577,7 +2570,7 @@ public class InstConstraintVisitor exten
         //e.g.: Don't instantiate interfaces
         if (! obj.referencesClass()){
             constraintViolated(o, "Expecting a class type (ObjectType) to work on. Found: '"+obj+"'.");
-        }        
+        }
     }
 
     /**
@@ -2701,7 +2694,7 @@ public class InstConstraintVisitor exten
                             objreftype.subclassOf(curr) ) ){
                     constraintViolated(o, "The referenced field has the ACC_PROTECTED modifier, and it's a member of the current class or a superclass of the current class. However, the referenced object type '"+stack().peek()+"' is not the current class or a subclass of the current class.");
                 }
-            } 
+            }
         }
 
         // TODO: Could go into Pass 3a.
@@ -2815,7 +2808,7 @@ public class InstConstraintVisitor exten
         indexOfInt(o, stack().peek());
         if (stack().peek(1) == Type.NULL){
             return;
-        } 
+        }
         if (! (stack().peek(1) instanceof ArrayType)){
             constraintViolated(o, "Stack next-to-top must be of type short[] but is '"+stack().peek(1)+"'.");
         }
@@ -2836,7 +2829,7 @@ public class InstConstraintVisitor exten
         indexOfInt(o, stack().peek(1));
         if (stack().peek(2) == Type.NULL){
             return;
-        } 
+        }
         if (! (stack().peek(2) instanceof ArrayType)){
             constraintViolated(o, "Stack next-to-next-to-top must be of type short[] but is '"+stack().peek(2)+"'.");
         }

Added: commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/VerifierArrayAccessTestCase.java
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/VerifierArrayAccessTestCase.java?rev=1664897&view=auto
==============================================================================
--- commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/VerifierArrayAccessTestCase.java (added)
+++ commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/VerifierArrayAccessTestCase.java Sat Mar  7 19:17:27 2015
@@ -0,0 +1,34 @@
+/*
+ * 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;
+
+
+public class VerifierArrayAccessTestCase extends AbstractVerifierTestCase {
+    
+    public void testInvalidArrayAccess() {
+        assertVerifyRejected("TestArrayAccess03", "Verification of an arraystore instruction on an object must fail.");
+        assertVerifyRejected("TestArrayAccess04", "Verification of an arraystore instruction of an int on an array of references must fail.");
+    }
+    
+    public void testValidArrayAccess() {
+        assertVerifyOK("TestArrayAccess01", "Verification of an arraystore instruction on an array that is not compatible with the stored element must pass.");
+        assertVerifyOK("TestArrayAccess02", "Verification of an arraystore instruction on an array that is not compatible with the stored element must pass.");
+    }
+    
+}

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

Added: commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArrayAccess01.java
URL: http://svn.apache.org/viewvc/commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArrayAccess01.java?rev=1664897&view=auto
==============================================================================
--- commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArrayAccess01.java (added)
+++ commons/proper/bcel/trunk/src/test/java/org/apache/bcel/verifier/tests/TestArrayAccess01.java Sat Mar  7 19:17:27 2015
@@ -0,0 +1,32 @@
+/*
+ * 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 TestArrayAccess01 extends XTestArray01{
+
+    public static void test(){
+        XTestArray01[] array = new TestArrayAccess01[1];
+        array[0] = new XTestArray01();
+    }
+   
+}
+
+class XTestArray01 {
+    
+}

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

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

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

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

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

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

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