ART: Change behavior for rethrowing init failures
Allow to store a Throwable instance or a throwable class. Handle
rethrow accordingly.
Bug: 25444180
Change-Id: I703c2c6eaf34ad0e3bc0f5a104d65f2ff1b212ca
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index da70456..6053469 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -104,12 +104,13 @@
va_end(args);
}
-bool ClassLinker::HasInitWithString(Thread* self, const char* descriptor) {
+static bool HasInitWithString(Thread* self, ClassLinker* class_linker, const char* descriptor)
+ SHARED_REQUIRES(Locks::mutator_lock_) {
ArtMethod* method = self->GetCurrentMethod(nullptr);
StackHandleScope<1> hs(self);
Handle<mirror::ClassLoader> class_loader(hs.NewHandle(method != nullptr ?
method->GetDeclaringClass()->GetClassLoader() : nullptr));
- mirror::Class* exception_class = FindClass(self, descriptor, class_loader);
+ mirror::Class* exception_class = class_linker->FindClass(self, descriptor, class_loader);
if (exception_class == nullptr) {
// No exc class ~ no <init>-with-string.
@@ -119,10 +120,39 @@
}
ArtMethod* exception_init_method = exception_class->FindDeclaredDirectMethod(
- "<init>", "(Ljava/lang/String;)V", image_pointer_size_);
+ "<init>", "(Ljava/lang/String;)V", class_linker->GetImagePointerSize());
return exception_init_method != nullptr;
}
+// Helper for ThrowEarlierClassFailure. Throws the stored error.
+static void HandleEarlierVerifyError(Thread* self, ClassLinker* class_linker, mirror::Class* c)
+ SHARED_REQUIRES(Locks::mutator_lock_) {
+ mirror::Object* obj = c->GetVerifyError();
+ DCHECK(obj != nullptr);
+ self->AssertNoPendingException();
+ if (obj->IsClass()) {
+ // Previous error has been stored as class. Create a new exception of that type.
+
+ // It's possible the exception doesn't have a <init>(String).
+ std::string temp;
+ const char* descriptor = obj->AsClass()->GetDescriptor(&temp);
+
+ if (HasInitWithString(self, class_linker, descriptor)) {
+ self->ThrowNewException(descriptor, PrettyDescriptor(c).c_str());
+ } else {
+ self->ThrowNewException(descriptor, nullptr);
+ }
+ } else {
+ // Previous error has been stored as an instance. Just rethrow.
+ mirror::Class* throwable_class =
+ self->DecodeJObject(WellKnownClasses::java_lang_Throwable)->AsClass();
+ mirror::Class* error_class = obj->GetClass();
+ CHECK(throwable_class->IsAssignableFrom(error_class));
+ self->SetException(obj->AsThrowable());
+ }
+ self->AssertPendingException();
+}
+
void ClassLinker::ThrowEarlierClassFailure(mirror::Class* c) {
// The class failed to initialize on a previous attempt, so we want to throw
// a NoClassDefFoundError (v2 2.17.5). The exception to this rule is if we
@@ -131,8 +161,11 @@
Runtime* const runtime = Runtime::Current();
if (!runtime->IsAotCompiler()) { // Give info if this occurs at runtime.
std::string extra;
- if (c->GetVerifyErrorClass() != nullptr) {
- extra = PrettyDescriptor(c->GetVerifyErrorClass());
+ if (c->GetVerifyError() != nullptr) {
+ mirror::Class* descr_from = c->GetVerifyError()->IsClass()
+ ? c->GetVerifyError()->AsClass()
+ : c->GetVerifyError()->GetClass();
+ extra = PrettyDescriptor(descr_from);
}
LOG(INFO) << "Rejecting re-init on previously-failed class " << PrettyClass(c) << ": " << extra;
}
@@ -144,17 +177,8 @@
mirror::Throwable* pre_allocated = runtime->GetPreAllocatedNoClassDefFoundError();
self->SetException(pre_allocated);
} else {
- if (c->GetVerifyErrorClass() != nullptr) {
- // TODO: change the verifier to store an _instance_, with a useful detail message?
- // It's possible the exception doesn't have a <init>(String).
- std::string temp;
- const char* descriptor = c->GetVerifyErrorClass()->GetDescriptor(&temp);
-
- if (HasInitWithString(self, descriptor)) {
- self->ThrowNewException(descriptor, PrettyDescriptor(c).c_str());
- } else {
- self->ThrowNewException(descriptor, nullptr);
- }
+ if (c->GetVerifyError() != nullptr) {
+ HandleEarlierVerifyError(self, this, c);
} else {
self->ThrowNewException("Ljava/lang/NoClassDefFoundError;",
PrettyDescriptor(c).c_str());
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 392efd2..73f9d4b 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -854,9 +854,6 @@
SHARED_REQUIRES(Locks::mutator_lock_)
REQUIRES(!dex_lock_);
- bool HasInitWithString(Thread* self, const char* descriptor)
- SHARED_REQUIRES(Locks::mutator_lock_) REQUIRES(!dex_lock_);
-
bool CanWeInitializeClass(mirror::Class* klass, bool can_init_statics, bool can_init_parents)
SHARED_REQUIRES(Locks::mutator_lock_);
diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index 04b8900..2c086c5 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -515,7 +515,7 @@
addOffset(OFFSETOF_MEMBER(mirror::Class, sfields_), "sFields");
addOffset(OFFSETOF_MEMBER(mirror::Class, status_), "status");
addOffset(OFFSETOF_MEMBER(mirror::Class, super_class_), "superClass");
- addOffset(OFFSETOF_MEMBER(mirror::Class, verify_error_class_), "verifyErrorClass");
+ addOffset(OFFSETOF_MEMBER(mirror::Class, verify_error_), "verifyError");
addOffset(OFFSETOF_MEMBER(mirror::Class, virtual_methods_), "virtualMethods");
addOffset(OFFSETOF_MEMBER(mirror::Class, vtable_), "vtable");
};
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 19ee7f4..174de0e 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -520,15 +520,6 @@
}
}
-inline void Class::SetVerifyErrorClass(Class* klass) {
- CHECK(klass != nullptr) << PrettyClass(this);
- if (Runtime::Current()->IsActiveTransaction()) {
- SetFieldObject<true>(OFFSET_OF_OBJECT_MEMBER(Class, verify_error_class_), klass);
- } else {
- SetFieldObject<false>(OFFSET_OF_OBJECT_MEMBER(Class, verify_error_class_), klass);
- }
-}
-
template<VerifyObjectFlags kVerifyFlags>
inline uint32_t Class::GetAccessFlags() {
// Check class is loaded/retired or this is java.lang.String that has a
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index 9d01a1d..77275f0 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -57,6 +57,15 @@
java_lang_Class_.VisitRootIfNonNull(visitor, RootInfo(kRootStickyClass));
}
+inline void Class::SetVerifyError(mirror::Object* error) {
+ CHECK(error != nullptr) << PrettyClass(this);
+ if (Runtime::Current()->IsActiveTransaction()) {
+ SetFieldObject<true>(OFFSET_OF_OBJECT_MEMBER(Class, verify_error_), error);
+ } else {
+ SetFieldObject<false>(OFFSET_OF_OBJECT_MEMBER(Class, verify_error_), error);
+ }
+}
+
void Class::SetStatus(Handle<Class> h_this, Status new_status, Thread* self) {
Status old_status = h_this->GetStatus();
ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
@@ -109,7 +118,13 @@
// case.
Class* exception_class = old_exception->GetClass();
if (!eiie_class->IsAssignableFrom(exception_class)) {
- h_this->SetVerifyErrorClass(exception_class);
+ // Store the exception class when this is the AoT compiler. Don't store full exceptions,
+ // as they need to be trimmed (native components are not storable in an image).
+ if (Runtime::Current()->IsAotCompiler()) {
+ h_this->SetVerifyError(exception_class);
+ } else {
+ h_this->SetVerifyError(old_exception.Get());
+ }
}
}
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 8219d69..c4339b9 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -1015,9 +1015,9 @@
void SetClinitThreadId(pid_t new_clinit_thread_id) SHARED_REQUIRES(Locks::mutator_lock_);
- Class* GetVerifyErrorClass() SHARED_REQUIRES(Locks::mutator_lock_) {
+ Object* GetVerifyError() SHARED_REQUIRES(Locks::mutator_lock_) {
// DCHECK(IsErroneous());
- return GetFieldObject<Class>(OFFSET_OF_OBJECT_MEMBER(Class, verify_error_class_));
+ return GetFieldObject<Class>(OFFSET_OF_OBJECT_MEMBER(Class, verify_error_));
}
uint16_t GetDexClassDefIndex() SHARED_REQUIRES(Locks::mutator_lock_) {
@@ -1158,7 +1158,7 @@
SHARED_REQUIRES(Locks::mutator_lock_);
private:
- void SetVerifyErrorClass(Class* klass) SHARED_REQUIRES(Locks::mutator_lock_);
+ void SetVerifyError(Object* klass) SHARED_REQUIRES(Locks::mutator_lock_);
template <bool throw_on_failure, bool use_referrers_cache>
bool ResolvedFieldAccessTest(Class* access_to, ArtField* field,
@@ -1230,8 +1230,9 @@
// check for interfaces and return null.
HeapReference<Class> super_class_;
- // If class verify fails, we must return same error on subsequent tries.
- HeapReference<Class> verify_error_class_;
+ // If class verify fails, we must return same error on subsequent tries. We may store either
+ // the class of the error, or an actual instance of Throwable here.
+ HeapReference<Object> verify_error_;
// Virtual method table (vtable), for use by "invoke-virtual". The vtable from the superclass is
// copied in, and virtual methods from our class either replace those from the super or are
diff --git a/test/008-exceptions/expected.txt b/test/008-exceptions/expected.txt
index 92c79dc..ea59a49 100644
--- a/test/008-exceptions/expected.txt
+++ b/test/008-exceptions/expected.txt
@@ -1,12 +1,15 @@
Got an NPE: second throw
java.lang.NullPointerException: second throw
- at Main.catchAndRethrow(Main.java:58)
- at Main.exceptions_007(Main.java:41)
- at Main.main(Main.java:49)
+ at Main.catchAndRethrow(Main.java:77)
+ at Main.exceptions_007(Main.java:59)
+ at Main.main(Main.java:67)
Caused by: java.lang.NullPointerException: first throw
- at Main.throwNullPointerException(Main.java:65)
- at Main.catchAndRethrow(Main.java:55)
+ at Main.throwNullPointerException(Main.java:84)
+ at Main.catchAndRethrow(Main.java:74)
... 2 more
Static Init
-BadError: This is bad by convention
-BadError: This is bad by convention
+BadError: This is bad by convention: BadInit
+BadError: This is bad by convention: BadInit
+Static BadInitNoStringInit
+BadErrorNoStringInit: This is bad by convention
+BadErrorNoStringInit: This is bad by convention
diff --git a/test/008-exceptions/src/Main.java b/test/008-exceptions/src/Main.java
index 7f6d0c5..c050204 100644
--- a/test/008-exceptions/src/Main.java
+++ b/test/008-exceptions/src/Main.java
@@ -14,20 +14,38 @@
* limitations under the License.
*/
-// An exception that doesn't have a <init>(String) method.
+// An error class.
class BadError extends Error {
- public BadError() {
- super("This is bad by convention");
+ public BadError(String s) {
+ super("This is bad by convention: " + s);
}
}
-// A class that throws BadException during static initialization.
+// A class that throws BadError during static initialization.
class BadInit {
static int dummy;
static {
System.out.println("Static Init");
if (true) {
- throw new BadError();
+ throw new BadError("BadInit");
+ }
+ }
+}
+
+// An error that doesn't have a <init>(String) method.
+class BadErrorNoStringInit extends Error {
+ public BadErrorNoStringInit() {
+ super("This is bad by convention");
+ }
+}
+
+// A class that throws BadErrorNoStringInit during static initialization.
+class BadInitNoStringInit {
+ static int dummy;
+ static {
+ System.out.println("Static BadInitNoStringInit");
+ if (true) {
+ throw new BadErrorNoStringInit();
}
}
}
@@ -48,6 +66,7 @@
public static void main (String args[]) {
exceptions_007();
exceptionsRethrowClassInitFailure();
+ exceptionsRethrowClassInitFailureNoStringInit();
}
private static void catchAndRethrow() {
@@ -86,4 +105,26 @@
error.printStackTrace();
}
}
+
+ private static void exceptionsRethrowClassInitFailureNoStringInit() {
+ try {
+ try {
+ BadInitNoStringInit.dummy = 1;
+ throw new IllegalStateException("Should not reach here.");
+ } catch (BadErrorNoStringInit e) {
+ System.out.println(e);
+ }
+
+ // Check if it works a second time.
+
+ try {
+ BadInitNoStringInit.dummy = 1;
+ throw new IllegalStateException("Should not reach here.");
+ } catch (BadErrorNoStringInit e) {
+ System.out.println(e);
+ }
+ } catch (Exception error) {
+ error.printStackTrace();
+ }
+ }
}
