Fixes for "null instanceof" and class initialization
First issue was that the codegen seemed to expect r0 to contain 0 for
a false result in the null object case. This is a quick fix to make
that true. Given that the code is doing the work of a null check and
pulling out the object's class, the code should probably just pass the
class to the helper function, making this a case of IsAssignableFrom,
not instanceof.
Second issues were related to missing EnsureInitialized calls in two
different code paths, one coming from reflection, one coming from
managed code. New Class::AllocObject assert should help keep us out of
trouble in the future in this area, although perhaps Heap::AllocObject
should have a check as well.
Change-Id: Ib7975b6457481c1ac85135d38f42c6061e6443a0
diff --git a/Android.mk b/Android.mk
index a27d526..a27b2f2 100644
--- a/Android.mk
+++ b/Android.mk
@@ -156,7 +156,7 @@
# oatdump targets
.PHONY: dump-oat
-dump-oat: dump-oat-core dump-oat-boot
+dump-oat: dump-oat-core dump-oat-boot dump-oat-Calculator
.PHONY: dump-oat-core
dump-oat-core: $(TARGET_CORE_OAT) $(OATDUMP)
diff --git a/src/check_jni.cc b/src/check_jni.cc
index 6ff0fd7..2208d14 100644
--- a/src/check_jni.cc
+++ b/src/check_jni.cc
@@ -236,9 +236,10 @@
return;
}
+ CHECK(fid != NULL) << PrettyTypeOf(o);
Field* f = DecodeField(fid);
Class* f_type = f->GetType();
- // check invariant that all jfieldIDs have resovled types
+ // check invariant that all jfieldIDs have resolved types
DCHECK(f_type != NULL);
Class* c = o->GetClass();
if (c->FindInstanceField(f->GetName()->ToModifiedUtf8(), f_type) == NULL) {
diff --git a/src/compiler/codegen/arm/Thumb2/Gen.cc b/src/compiler/codegen/arm/Thumb2/Gen.cc
index c9d1bb9..e3893d3 100644
--- a/src/compiler/codegen/arm/Thumb2/Gen.cc
+++ b/src/compiler/codegen/arm/Thumb2/Gen.cc
@@ -707,6 +707,7 @@
// At this point, r2 has class
loadValueDirectFixed(cUnit, rlSrc, r3); /* Ref */
/* When taken r0 has NULL which can be used for store directly */
+ loadConstant(cUnit, r0, 0); /* Assume false */
ArmLIR* branch1 = genCmpImmBranch(cUnit, kArmCondEq, r3, 0);
/* load object->clazz */
DCHECK_EQ(Object::ClassOffset().Int32Value(), 0);
diff --git a/src/java_lang_Class.cc b/src/java_lang_Class.cc
index 4fe3191..68b83f2 100644
--- a/src/java_lang_Class.cc
+++ b/src/java_lang_Class.cc
@@ -327,7 +327,7 @@
if (o == NULL) {
return JNI_FALSE;
}
- return Object::InstanceOf(o, c) ? JNI_TRUE : JNI_FALSE;
+ return o->InstanceOf(c) ? JNI_TRUE : JNI_FALSE;
}
jboolean Class_isInterface(JNIEnv* env, jobject javaThis) {
diff --git a/src/java_lang_reflect_Constructor.cc b/src/java_lang_reflect_Constructor.cc
index 10a8f07..6cc8ea7 100644
--- a/src/java_lang_reflect_Constructor.cc
+++ b/src/java_lang_reflect_Constructor.cc
@@ -40,6 +40,11 @@
return NULL;
}
+ if (!Runtime::Current()->GetClassLinker()->EnsureInitialized(c, true)) {
+ DCHECK(Thread::Current()->IsExceptionPending());
+ return NULL;
+ }
+
Object* receiver = c->AllocObject();
if (receiver == NULL) {
return NULL;
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index fd6d314..0436500 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -691,12 +691,12 @@
ScopedJniThreadState ts(env);
CHECK_NE(static_cast<jclass>(NULL), clazz); // TODO: ReportJniError
if (jobj == NULL) {
- // NB. JNI is different from regular Java instanceof in this respect
+ // Note: JNI is different from regular Java instanceof in this respect
return JNI_TRUE;
} else {
Object* obj = Decode<Object*>(ts, jobj);
Class* klass = Decode<Class*>(ts, clazz);
- return Object::InstanceOf(obj, klass) ? JNI_TRUE : JNI_FALSE;
+ return obj->InstanceOf(klass) ? JNI_TRUE : JNI_FALSE;
}
}
diff --git a/src/object.cc b/src/object.cc
index cfc18bf..5017c23 100644
--- a/src/object.cc
+++ b/src/object.cc
@@ -807,6 +807,7 @@
DCHECK(!IsAbstract()) << PrettyClass(this);
DCHECK(!IsInterface()) << PrettyClass(this);
DCHECK(!IsPrimitive()) << PrettyClass(this);
+ DCHECK(!Runtime::Current()->IsStarted() || IsInitializing()) << PrettyClass(this);
return Heap::AllocObject(this, this->object_size_);
}
diff --git a/src/object.h b/src/object.h
index 1a137bd..47aa98d 100644
--- a/src/object.h
+++ b/src/object.h
@@ -191,11 +191,9 @@
// C++ mirror of java.lang.Object
class MANAGED Object {
public:
- static bool InstanceOf(const Object* object, const Class* klass) {
- if (object == NULL) {
- return false;
- }
- return object->InstanceOf(klass);
+ static uint32_t InstanceOfFromCode(const Object* object, const Class* klass) {
+ DCHECK(object != NULL);
+ return object->InstanceOf(klass) ? 1 : 0;
}
static MemberOffset ClassOffset() {
@@ -1295,6 +1293,11 @@
return GetStatus() >= kStatusVerified;
}
+ // Returns true if the class is initializing.
+ bool IsInitializing() const {
+ return GetStatus() >= kStatusInitializing;
+ }
+
// Returns true if the class is initialized.
bool IsInitialized() const {
return GetStatus() == kStatusInitialized;
diff --git a/src/object_test.cc b/src/object_test.cc
index d6705d7..f263872 100644
--- a/src/object_test.cc
+++ b/src/object_test.cc
@@ -328,18 +328,15 @@
ASSERT_TRUE(X != NULL);
ASSERT_TRUE(Y != NULL);
- EXPECT_FALSE(Object::InstanceOf(NULL, X));
- EXPECT_FALSE(Object::InstanceOf(NULL, Y));
-
Object* x = X->AllocObject();
Object* y = Y->AllocObject();
ASSERT_TRUE(x != NULL);
ASSERT_TRUE(y != NULL);
- EXPECT_TRUE(Object::InstanceOf(x, X));
- EXPECT_FALSE(Object::InstanceOf(x, Y));
- EXPECT_TRUE(Object::InstanceOf(y, X));
- EXPECT_TRUE(Object::InstanceOf(y, Y));
+ EXPECT_EQ(1U, Object::InstanceOfFromCode(x, X));
+ EXPECT_EQ(0U, Object::InstanceOfFromCode(x, Y));
+ EXPECT_EQ(1U, Object::InstanceOfFromCode(y, X));
+ EXPECT_EQ(1U, Object::InstanceOfFromCode(y, Y));
EXPECT_TRUE(x->InstanceOf(X));
EXPECT_FALSE(x->InstanceOf(Y));
diff --git a/src/runtime.cc b/src/runtime.cc
index ca1d4f8..eb5e498 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -359,14 +359,17 @@
}
void Runtime::Start() {
- started_ = true;
-
InitNativeMethods();
Thread::FinishStartup();
class_linker_->RunRootClinits();
+ // Class::AllocObject asserts that all objects allocated better be
+ // initialized after Runtime::IsStarted is true, so this needs to
+ // come after ClassLinker::RunRootClinits.
+ started_ = true;
+
StartDaemonThreads();
}
diff --git a/src/thread.cc b/src/thread.cc
index 4e3e8d1..d79e1c5 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -230,6 +230,11 @@
return NULL; // Failure
}
}
+ if (!klass->IsInitialized()
+ && !Runtime::Current()->GetClassLinker()->EnsureInitialized(klass, true)) {
+ DCHECK(Thread::Current()->IsExceptionPending());
+ return NULL; // Failure
+ }
return klass->AllocObject();
}
@@ -497,7 +502,7 @@
pSetObjStatic = Field::SetObjStaticFromCode;
pInitializeTypeFromCode = InitializeTypeFromCode;
pResolveMethodFromCode = ResolveMethodFromCode;
- pInstanceofNonTrivialFromCode = Object::InstanceOf;
+ pInstanceofNonTrivialFromCode = Object::InstanceOfFromCode;
pLockObjectFromCode = LockObjectFromCode;
pFindInstanceFieldFromCode = Field::FindInstanceFieldFromCode;
pCheckSuspendFromCode = artCheckSuspendFromCode;
diff --git a/src/thread.h b/src/thread.h
index 01cc1cc..123a376 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -210,7 +210,7 @@
Object* (*pGetObjStatic)(uint32_t, const Method*);
void (*pSetObjStatic)(uint32_t, const Method*, Object*);
void (*pCanPutArrayElementFromCode)(void*, void*);
- bool (*pInstanceofNonTrivialFromCode) (const Object*, const Class*);
+ uint32_t (*pInstanceofNonTrivialFromCode) (const Object*, const Class*);
void (*pCheckCastFromCode) (void*, void*);
Method* (*pFindInterfaceMethodInCache)(Class*, uint32_t, const Method*, struct DvmDex*);
void (*pUnlockObjectFromCode)(void*, void*);
diff --git a/test/IntMath/IntMath.java b/test/IntMath/IntMath.java
index 7b0d5f8..21f84bb 100644
--- a/test/IntMath/IntMath.java
+++ b/test/IntMath/IntMath.java
@@ -54,6 +54,10 @@
IntMathBase a = new IntMathBase();
IntMath b = new IntMath();
+ if (!(null instanceof IntMathBase)) {
+ x = x + 42;
+ }
+
if (a instanceof IntMathBase) {
x = x * 2;
}
@@ -1116,7 +1120,7 @@
}
res = instanceTest(10);
- if (res == 1352) {
+ if (res == 1436) {
System.out.println("instanceTest PASSED");
} else {
System.out.println("instanceTest FAILED: " + res);