ART: Change behavior for rethrowing init failures (2)

Always store the pending exception when making a class erroneous.
Instead of filtering by ExceptionInInitializerError, add an option
to the rethrow that enforces a NoClassDefFoundError, which is required
by the specification.

Use the libcore companion change to add the stored error (if any) as
a cause to the NoClassDefFoundError, which should significantly help
tracking down issues.

Fix run-test 008 to expect spec-compliant behavior. Test that a cause
has been set.

Bug: 25445103
Change-Id: I6a0dc54e78312283faf23415887eff387531407f

5 files changed, 20 insertions, 46 deletions

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 6053469016..cdb7712211 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -153,7 +153,7 @@ static void HandleEarlierVerifyError(Thread* self, ClassLinker* class_linker, mi
   self->AssertPendingException();
 }
 
-void ClassLinker::ThrowEarlierClassFailure(mirror::Class* c) {
+void ClassLinker::ThrowEarlierClassFailure(mirror::Class* c, bool wrap_in_no_class_def) {
   // 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
   // failed in verification, in which case v2 5.4.1 says we need to re-throw
@@ -178,10 +178,15 @@ void ClassLinker::ThrowEarlierClassFailure(mirror::Class* c) {
     self->SetException(pre_allocated);
   } else {
     if (c->GetVerifyError() != nullptr) {
+      // Rethrow stored error.
       HandleEarlierVerifyError(self, this, c);
-    } else {
-      self->ThrowNewException("Ljava/lang/NoClassDefFoundError;",
-                              PrettyDescriptor(c).c_str());
+    }
+    if (c->GetVerifyError() == nullptr || wrap_in_no_class_def) {
+      // If there isn't a recorded earlier error, or this is a repeat throw from initialization,
+      // the top-level exception must be a NoClassDefFoundError. The potentially already pending
+      // exception will be a cause.
+      self->ThrowNewWrappedException("Ljava/lang/NoClassDefFoundError;",
+                                     PrettyDescriptor(c).c_str());
     }
   }
 }
@@ -3423,7 +3428,7 @@ bool ClassLinker::InitializeClass(Thread* self, Handle<mirror::Class> klass,
 
     // Was the class already found to be erroneous? Done under the lock to match the JLS.
     if (klass->IsErroneous()) {
-      ThrowEarlierClassFailure(klass.Get());
+      ThrowEarlierClassFailure(klass.Get(), true);
       VlogClassInitializationFailure(klass);
       return false;
     }
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 73f9d4b864..a35ba3e6ef 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -848,9 +848,7 @@ class ClassLinker {
 
   // Throw the class initialization failure recorded when first trying to initialize the given
   // class.
-  // Note: Currently we only store the descriptor, so we cannot throw the exact throwable, only
-  //       a recreation with a custom string.
-  void ThrowEarlierClassFailure(mirror::Class* c)
+  void ThrowEarlierClassFailure(mirror::Class* c, bool wrap_in_no_class_def = false)
       SHARED_REQUIRES(Locks::mutator_lock_)
       REQUIRES(!dex_lock_);
 
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index 77275f007b..91e1cecbdf 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -94,42 +94,9 @@ void Class::SetStatus(Handle<Class> h_this, Status new_status, Thread* self) {
       }
     }
 
-    // Stash current exception.
-    StackHandleScope<1> hs(self);
-    Handle<mirror::Throwable> old_exception(hs.NewHandle(self->GetException()));
-    CHECK(old_exception.Get() != nullptr);
-    Class* eiie_class;
-    // Do't attempt to use FindClass if we have an OOM error since this can try to do more
-    // allocations and may cause infinite loops.
-    bool throw_eiie = (old_exception.Get() == nullptr);
-    if (!throw_eiie) {
-      std::string temp;
-      const char* old_exception_descriptor = old_exception->GetClass()->GetDescriptor(&temp);
-      throw_eiie = (strcmp(old_exception_descriptor, "Ljava/lang/OutOfMemoryError;") != 0);
-    }
-    if (throw_eiie) {
-      // Clear exception to call FindSystemClass.
-      self->ClearException();
-      eiie_class = Runtime::Current()->GetClassLinker()->FindSystemClass(
-          self, "Ljava/lang/ExceptionInInitializerError;");
-      CHECK(!self->IsExceptionPending());
-      // Only verification errors, not initialization problems, should set a verify error.
-      // This is to ensure that ThrowEarlierClassFailure will throw NoClassDefFoundError in that
-      // case.
-      Class* exception_class = old_exception->GetClass();
-      if (!eiie_class->IsAssignableFrom(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());
-        }
-      }
-    }
-
-    // Restore exception.
-    self->SetException(old_exception.Get());
+    // Remember the current exception.
+    CHECK(self->GetException() != nullptr);
+    h_this->SetVerifyError(self->GetException());
   }
   static_assert(sizeof(Status) == sizeof(uint32_t), "Size of status not equal to uint32");
   if (Runtime::Current()->IsActiveTransaction()) {
diff --git a/test/008-exceptions/expected.txt b/test/008-exceptions/expected.txt
index ea59a49677..083ecf7ced 100644
--- a/test/008-exceptions/expected.txt
+++ b/test/008-exceptions/expected.txt
@@ -9,7 +9,9 @@ Caused by: java.lang.NullPointerException: first throw
 	... 2 more
 Static Init
 BadError: This is bad by convention: BadInit
+java.lang.NoClassDefFoundError: BadInit
 BadError: This is bad by convention: BadInit
 Static BadInitNoStringInit
 BadErrorNoStringInit: This is bad by convention
+java.lang.NoClassDefFoundError: BadInitNoStringInit
 BadErrorNoStringInit: This is bad by convention
diff --git a/test/008-exceptions/src/Main.java b/test/008-exceptions/src/Main.java
index c0502047d0..9e3477a47b 100644
--- a/test/008-exceptions/src/Main.java
+++ b/test/008-exceptions/src/Main.java
@@ -98,8 +98,9 @@ public class Main {
             try {
                 BadInit.dummy = 1;
                 throw new IllegalStateException("Should not reach here.");
-            } catch (BadError e) {
+            } catch (NoClassDefFoundError e) {
                 System.out.println(e);
+                System.out.println(e.getCause());
             }
         } catch (Exception error) {
             error.printStackTrace();
@@ -120,8 +121,9 @@ public class Main {
             try {
                 BadInitNoStringInit.dummy = 1;
                 throw new IllegalStateException("Should not reach here.");
-            } catch (BadErrorNoStringInit e) {
+            } catch (NoClassDefFoundError e) {
                 System.out.println(e);
+                System.out.println(e.getCause());
             }
         } catch (Exception error) {
             error.printStackTrace();