Fix reporting of certain verify errors.

The code was assuming that the reference type could always be inferred
from the error code, but in two cases it couldn't.  This resulted in a
weird string appearing where the class name should be in the exception.
The type is now explicitly stuffed into the replacement instruction.

I added one additional test to 075; with this, plus 003 and 077, I think
we have full coverage.

For bug 2084560.

diff --git a/tests/075-verification-error/expected.txt b/tests/075-verification-error/expected.txt
index 3e8dbd0..6e4f584 100644 (file)
--- a/tests/075-verification-error/expected.txt
+++ b/tests/075-verification-error/expected.txt
@@ -8,4 +8,5 @@ Got expected IllegalAccessError (sfield)
 Got expected IllegalAccessError (method)
 Got expected IllegalAccessError (smethod)
 Got expected IllegalAccessError (meth-class)
+Got expected IllegalAccessError (field-class)
 Got expected IllegalAccessError (meth-meth)

diff --git a/tests/075-verification-error/src/Main.java b/tests/075-verification-error/src/Main.java
index 8a0dd15..f357242 100644 (file)
--- a/tests/075-verification-error/src/Main.java
+++ b/tests/075-verification-error/src/Main.java
@@ -121,13 +121,22 @@ public class Main {
         try {
             /* accessible static method in an inaccessible class */
             InaccessibleClass.test();
-            System.err.println("ERROR: bad access succeeded\n");
+            System.err.println("ERROR: bad meth-class access succeeded\n");
         } catch (IllegalAccessError iae) {
             System.out.println("Got expected IllegalAccessError (meth-class)");
             if (VERBOSE) System.out.println("--- " + iae);
         }
 
         try {
+            /* accessible static field in an inaccessible class */
+            int blah = InaccessibleClass.blah;
+            System.err.println("ERROR: bad field-class access succeeded\n");
+        } catch (IllegalAccessError iae) {
+            System.out.println("Got expected IllegalAccessError (field-class)");
+            if (VERBOSE) System.out.println("--- " + iae);
+        }
+
+        try {
             /* inaccessible static method in an accessible class */
             InaccessibleMethod.test();
             System.err.println("ERROR: bad access succeeded\n");

diff --git a/tests/075-verification-error/src/other/InaccessibleClass.java b/tests/075-verification-error/src/other/InaccessibleClass.java
index 79ad335..254eaa3 100644 (file)
--- a/tests/075-verification-error/src/other/InaccessibleClass.java
+++ b/tests/075-verification-error/src/other/InaccessibleClass.java
@@ -18,5 +18,7 @@ package other;
 
 public class InaccessibleClass {
     public static void test() {}
+
+    public static int blah = 5;
 }
 

diff --git a/tests/075-verification-error/src2/other/InaccessibleClass.java b/tests/075-verification-error/src2/other/InaccessibleClass.java
index c598910..a3cfbff 100644 (file)
--- a/tests/075-verification-error/src2/other/InaccessibleClass.java
+++ b/tests/075-verification-error/src2/other/InaccessibleClass.java
@@ -18,5 +18,7 @@ package other;
 
 /*package*/ class InaccessibleClass {
     public static void test() {}
+
+    public static int blah = 5;
 }
 

diff --git a/vm/DalvikVersion.h b/vm/DalvikVersion.h
index 26b52be..efbb393 100644 (file)
--- a/vm/DalvikVersion.h
+++ b/vm/DalvikVersion.h
@@ -32,6 +32,6 @@
  * way classes load changes, e.g. field ordering or vtable layout.  Changing
  * this guarantees that the optimized form of the DEX file is regenerated.
  */
-#define DALVIK_VM_BUILD         16
+#define DALVIK_VM_BUILD         17
 
 #endif /*_DALVIK_VERSION*/

diff --git a/vm/analysis/CodeVerify.c b/vm/analysis/CodeVerify.c
index a1862bb..f945f23 100644 (file)
--- a/vm/analysis/CodeVerify.c
+++ b/vm/analysis/CodeVerify.c
@@ -2934,6 +2934,7 @@ static void verifyFilledNewArrayRegs(const Method* meth,
 static bool replaceFailingInstruction(Method* meth, InsnFlags* insnFlags,
     int insnIdx, VerifyError failure)
 {
+    VerifyErrorRefType refType;
     const u2* oldInsns = meth->insns + insnIdx;
     u2 oldInsn = *oldInsns;
     bool result = false;
@@ -2954,9 +2955,10 @@ static bool replaceFailingInstruction(Method* meth, InsnFlags* insnFlags,
     case OP_INSTANCE_OF:
     case OP_NEW_INSTANCE:
     case OP_NEW_ARRAY:
-
     case OP_FILLED_NEW_ARRAY:           // insn[1] == class ref, 3 bytes
     case OP_FILLED_NEW_ARRAY_RANGE:
+        refType = VERIFY_ERROR_REF_CLASS;
+        break;
 
     case OP_IGET:                       // insn[1] == field ref, 2 bytes
     case OP_IGET_BOOLEAN:
@@ -2986,6 +2988,8 @@ static bool replaceFailingInstruction(Method* meth, InsnFlags* insnFlags,
     case OP_SPUT_SHORT:
     case OP_SPUT_WIDE:
     case OP_SPUT_OBJECT:
+        refType = VERIFY_ERROR_REF_FIELD;
+        break;
 
     case OP_INVOKE_VIRTUAL:             // insn[1] == method ref, 3 bytes
     case OP_INVOKE_VIRTUAL_RANGE:
@@ -2997,7 +3001,9 @@ static bool replaceFailingInstruction(Method* meth, InsnFlags* insnFlags,
     case OP_INVOKE_STATIC_RANGE:
     case OP_INVOKE_INTERFACE:
     case OP_INVOKE_INTERFACE_RANGE:
+        refType = VERIFY_ERROR_REF_METHOD;
         break;
+
     default:
         /* could handle this in a generic way, but this is probably safer */
         LOG_VFY("GLITCH: verifier asked to replace opcode 0x%02x\n",
@@ -3022,7 +3028,8 @@ static bool replaceFailingInstruction(Method* meth, InsnFlags* insnFlags,
     }
 
     /* encode the opcode, with the failure code in the high byte */
-    newInsns[0] = OP_THROW_VERIFICATION_ERROR | (failure << 8);
+    newInsns[0] = OP_THROW_VERIFICATION_ERROR |
+        (failure << 8) | (refType << (8 + kVerifyErrorRefTypeShift));
 
     result = true;
 

diff --git a/vm/analysis/DexOptimize.h b/vm/analysis/DexOptimize.h
index 8ae2af5..afcb3cb 100644 (file)
--- a/vm/analysis/DexOptimize.h
+++ b/vm/analysis/DexOptimize.h
@@ -43,16 +43,31 @@ typedef enum VerifyError {
     VERIFY_ERROR_NONE = 0,      /* no error; must be zero */
     VERIFY_ERROR_GENERIC,       /* VerifyError */
 
-    VERIFY_ERROR_NO_CLASS,      /* NoClassDefFoundError (ref=class) */
-    VERIFY_ERROR_NO_FIELD,      /* NoSuchFieldError (ref=field) */
-    VERIFY_ERROR_NO_METHOD,     /* NoSuchMethodError (ref=method) */
-    VERIFY_ERROR_ACCESS_CLASS,  /* IllegalAccessError (ref=class) */
-    VERIFY_ERROR_ACCESS_FIELD,  /* IllegalAccessError (ref=field) */
-    VERIFY_ERROR_ACCESS_METHOD, /* IllegalAccessError (ref=method) */
-    VERIFY_ERROR_CLASS_CHANGE,  /* IncompatibleClassChangeError (ref=class) */
-    VERIFY_ERROR_INSTANTIATION, /* InstantiationError (ref=class) */
+    VERIFY_ERROR_NO_CLASS,      /* NoClassDefFoundError */
+    VERIFY_ERROR_NO_FIELD,      /* NoSuchFieldError */
+    VERIFY_ERROR_NO_METHOD,     /* NoSuchMethodError */
+    VERIFY_ERROR_ACCESS_CLASS,  /* IllegalAccessError */
+    VERIFY_ERROR_ACCESS_FIELD,  /* IllegalAccessError */
+    VERIFY_ERROR_ACCESS_METHOD, /* IllegalAccessError */
+    VERIFY_ERROR_CLASS_CHANGE,  /* IncompatibleClassChangeError */
+    VERIFY_ERROR_INSTANTIATION, /* InstantiationError */
 } VerifyError;
 
+/*
+ * Identifies the type of reference in the instruction that generated the
+ * verify error (e.g. VERIFY_ERROR_ACCESS_CLASS could come from a method,
+ * field, or class reference).
+ *
+ * This must fit in two bits.
+ */
+typedef enum VerifyErrorRefType {
+    VERIFY_ERROR_REF_CLASS  = 0,
+    VERIFY_ERROR_REF_FIELD  = 1,
+    VERIFY_ERROR_REF_METHOD = 2,
+} VerifyErrorRefType;
+
+#define kVerifyErrorRefTypeShift 6
+
 #define VERIFY_OK(_failure) ((_failure) == VERIFY_ERROR_NONE)
 
 /*

diff --git a/vm/interp/Interp.c b/vm/interp/Interp.c
index f45e21a..233ee3f 100644 (file)
--- a/vm/interp/Interp.c
+++ b/vm/interp/Interp.c
@@ -698,10 +698,22 @@ Method* dvmInterpFindInterfaceMethod(ClassObject* thisClass, u4 methodIdx,
  * Each returns a newly-allocated string.
  */
 #define kThrowShow_accessFromClass     1
-static char* classNameFromIndex(const Method* method, int ref, int flags)
+static char* classNameFromIndex(const Method* method, int ref,
+    VerifyErrorRefType refType, int flags)
 {
     static const int kBufLen = 256;
     const DvmDex* pDvmDex = method->clazz->pDvmDex;
+
+    if (refType == VERIFY_ERROR_REF_FIELD) {
+        /* get class ID from field ID */
+        const DexFieldId* pFieldId = dexGetFieldId(pDvmDex->pDexFile, ref);
+        ref = pFieldId->classIdx;
+    } else if (refType == VERIFY_ERROR_REF_METHOD) {
+        /* get class ID from method ID */
+        const DexMethodId* pMethodId = dexGetMethodId(pDvmDex->pDexFile, ref);
+        ref = pMethodId->classIdx;
+    }
+
     const char* className = dexStringByTypeIdx(pDvmDex->pDexFile, ref);
     char* dotClassName = dvmDescriptorToDot(className);
     if (flags == 0)
@@ -722,7 +734,8 @@ static char* classNameFromIndex(const Method* method, int ref, int flags)
     free(dotClassName);
     return result;
 }
-static char* fieldNameFromIndex(const Method* method, int ref, int flags)
+static char* fieldNameFromIndex(const Method* method, int ref,
+    VerifyErrorRefType refType, int flags)
 {
     static const int kBufLen = 256;
     const DvmDex* pDvmDex = method->clazz->pDvmDex;
@@ -730,6 +743,11 @@ static char* fieldNameFromIndex(const Method* method, int ref, int flags)
     const char* className;
     const char* fieldName;
 
+    if (refType != VERIFY_ERROR_REF_FIELD) {
+        LOGW("Expected ref type %d, got %d\n", VERIFY_ERROR_REF_FIELD, refType);
+        return NULL;    /* no message */
+    }
+
     pFieldId = dexGetFieldId(pDvmDex->pDexFile, ref);
     className = dexStringByTypeIdx(pDvmDex->pDexFile, pFieldId->classIdx);
     fieldName = dexStringById(pDvmDex->pDexFile, pFieldId->nameIdx);
@@ -749,7 +767,8 @@ static char* fieldNameFromIndex(const Method* method, int ref, int flags)
     free(dotName);
     return result;
 }
-static char* methodNameFromIndex(const Method* method, int ref, int flags)
+static char* methodNameFromIndex(const Method* method, int ref,
+    VerifyErrorRefType refType, int flags)
 {
     static const int kBufLen = 384;
     const DvmDex* pDvmDex = method->clazz->pDvmDex;
@@ -757,6 +776,11 @@ static char* methodNameFromIndex(const Method* method, int ref, int flags)
     const char* className;
     const char* methodName;
 
+    if (refType != VERIFY_ERROR_REF_METHOD) {
+        LOGW("Expected ref type %d, got %d\n", VERIFY_ERROR_REF_METHOD,refType);
+        return NULL;    /* no message */
+    }
+
     pMethodId = dexGetMethodId(pDvmDex->pDexFile, ref);
     className = dexStringByTypeIdx(pDvmDex->pDexFile, pMethodId->classIdx);
     methodName = dexStringById(pDvmDex->pDexFile, pMethodId->nameIdx);
@@ -786,47 +810,52 @@ static char* methodNameFromIndex(const Method* method, int ref, int flags)
  * This is used by the invoke-verification-error instruction.  It always
  * throws an exception.
  *
- * "kind" indicates the kind of failure encountered by the verifier.  The
- * meaning of "ref" is kind-specific; it's usually an index to a
- * class, field, or method reference.
+ * "kind" indicates the kind of failure encountered by the verifier.  It
+ * has two parts, an error code and an indication of the reference type.
  */
 void dvmThrowVerificationError(const Method* method, int kind, int ref)
 {
+    const int typeMask = 0xff << kVerifyErrorRefTypeShift;
+    VerifyError errorKind = kind & ~typeMask;
+    VerifyErrorRefType refType = kind >> kVerifyErrorRefTypeShift;
     const char* exceptionName = "Ljava/lang/VerifyError;";
     char* msg = NULL;
 
-    switch ((VerifyError) kind) {
+    switch ((VerifyError) errorKind) {
     case VERIFY_ERROR_NO_CLASS:
         exceptionName = "Ljava/lang/NoClassDefFoundError;";
-        msg = classNameFromIndex(method, ref, 0);
+        msg = classNameFromIndex(method, ref, refType, 0);
         break;
     case VERIFY_ERROR_NO_FIELD:
         exceptionName = "Ljava/lang/NoSuchFieldError;";
-        msg = fieldNameFromIndex(method, ref, 0);
+        msg = fieldNameFromIndex(method, ref, refType, 0);
         break;
     case VERIFY_ERROR_NO_METHOD:
         exceptionName = "Ljava/lang/NoSuchMethodError;";
-        msg = methodNameFromIndex(method, ref, 0);
+        msg = methodNameFromIndex(method, ref, refType, 0);
         break;
     case VERIFY_ERROR_ACCESS_CLASS:
         exceptionName = "Ljava/lang/IllegalAccessError;";
-        msg = classNameFromIndex(method, ref, kThrowShow_accessFromClass);
+        msg = classNameFromIndex(method, ref, refType,
+            kThrowShow_accessFromClass);
         break;
     case VERIFY_ERROR_ACCESS_FIELD:
         exceptionName = "Ljava/lang/IllegalAccessError;";
-        msg = fieldNameFromIndex(method, ref, kThrowShow_accessFromClass);
+        msg = fieldNameFromIndex(method, ref, refType,
+            kThrowShow_accessFromClass);
         break;
     case VERIFY_ERROR_ACCESS_METHOD:
         exceptionName = "Ljava/lang/IllegalAccessError;";
-        msg = methodNameFromIndex(method, ref, kThrowShow_accessFromClass);
+        msg = methodNameFromIndex(method, ref, refType,
+            kThrowShow_accessFromClass);
         break;
     case VERIFY_ERROR_CLASS_CHANGE:
         exceptionName = "Ljava/lang/IncompatibleClassChangeError;";
-        msg = classNameFromIndex(method, ref, 0);
+        msg = classNameFromIndex(method, ref, refType, 0);
         break;
     case VERIFY_ERROR_INSTANTIATION:
         exceptionName = "Ljava/lang/InstantiationError;";
-        msg = classNameFromIndex(method, ref, 0);
+        msg = classNameFromIndex(method, ref, refType, 0);
         break;
 
     case VERIFY_ERROR_GENERIC: