Abort transaction when Class.forName() fails.
And update VmClassLoader.findLoadedClass implementation in
UnstartedRuntime which has erroneously diverged since
https://android-review.googlesource.com/145075 .
Also prevent transactional interpreter from transfering
control to a catch handler for aborted transactions.
Also clean up Transaction::kAbortExceptionDescriptor naming
and some unused parameters.
Test: TransactionTest.CatchClassForNameAbortClass
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Change-Id: Ibfc544283f5434efbaab238d11a6152ed2578050
diff --git a/dex2oat/driver/compiler_driver.cc b/dex2oat/driver/compiler_driver.cc
index def5af9..fdf52d2 100644
--- a/dex2oat/driver/compiler_driver.cc
+++ b/dex2oat/driver/compiler_driver.cc
@@ -2238,7 +2238,7 @@
// compiler and will be pruned by ImageWriter.
Handle<mirror::Class> exception_class =
hs.NewHandle(class_linker->FindClass(self,
- Transaction::kAbortExceptionSignature,
+ Transaction::kAbortExceptionDescriptor,
class_loader));
bool exception_initialized =
class_linker->EnsureInitialized(self, exception_class, true, true);
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 0586ad9..302551f 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -303,14 +303,13 @@
// any value.
DCHECK(Runtime::Current()->AreNonStandardExitsEnabled());
JValue ret = JValue();
- bool res = PerformNonStandardReturn<MonitorState::kNoMonitorsLocked>(
+ PerformNonStandardReturn<MonitorState::kNoMonitorsLocked>(
self,
shadow_frame,
ret,
instrumentation,
accessor.InsSize(),
0);
- DCHECK(res) << "Expected to perform non-standard return!";
return ret;
}
if (UNLIKELY(self->IsExceptionPending())) {
@@ -321,14 +320,13 @@
JValue ret = JValue();
if (UNLIKELY(shadow_frame.GetForcePopFrame())) {
DCHECK(Runtime::Current()->AreNonStandardExitsEnabled());
- bool res = PerformNonStandardReturn<MonitorState::kNoMonitorsLocked>(
+ PerformNonStandardReturn<MonitorState::kNoMonitorsLocked>(
self,
shadow_frame,
ret,
instrumentation,
accessor.InsSize(),
0);
- DCHECK(res) << "Expected to perform non-standard return!";
}
return ret;
}
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index 65bff8e..959df00 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -158,7 +158,8 @@
template <bool kMonitorCounting>
static NO_INLINE void UnlockHeldMonitors(Thread* self, ShadowFrame* shadow_frame)
REQUIRES_SHARED(Locks::mutator_lock_) {
- DCHECK(shadow_frame->GetForcePopFrame());
+ DCHECK(shadow_frame->GetForcePopFrame() ||
+ Runtime::Current()->IsTransactionAborted());
// Unlock all monitors.
if (kMonitorCounting && shadow_frame->GetMethod()->MustCountLocks()) {
// Get the monitors from the shadow-frame monitor-count data.
@@ -194,7 +195,7 @@
};
template<MonitorState kMonitorState>
-static inline ALWAYS_INLINE WARN_UNUSED bool PerformNonStandardReturn(
+static inline ALWAYS_INLINE void PerformNonStandardReturn(
Thread* self,
ShadowFrame& frame,
JValue& result,
@@ -202,28 +203,23 @@
uint16_t num_dex_inst,
uint32_t dex_pc) REQUIRES_SHARED(Locks::mutator_lock_) {
static constexpr bool kMonitorCounting = (kMonitorState == MonitorState::kCountingMonitors);
- if (UNLIKELY(frame.GetForcePopFrame())) {
- ObjPtr<mirror::Object> thiz(frame.GetThisObject(num_dex_inst));
- StackHandleScope<1> hs(self);
- Handle<mirror::Object> h_thiz(hs.NewHandle(thiz));
- DCHECK(Runtime::Current()->AreNonStandardExitsEnabled());
- if (UNLIKELY(self->IsExceptionPending())) {
- LOG(WARNING) << "Suppressing exception for non-standard method exit: "
- << self->GetException()->Dump();
- self->ClearException();
- }
- if (kMonitorState != MonitorState::kNoMonitorsLocked) {
- UnlockHeldMonitors<kMonitorCounting>(self, &frame);
- }
- DoMonitorCheckOnExit<kMonitorCounting>(self, &frame);
- result = JValue();
- if (UNLIKELY(NeedsMethodExitEvent(instrumentation))) {
- SendMethodExitEvents(
- self, instrumentation, frame, h_thiz.Get(), frame.GetMethod(), dex_pc, result);
- }
- return true;
+ ObjPtr<mirror::Object> thiz(frame.GetThisObject(num_dex_inst));
+ StackHandleScope<1u> hs(self);
+ Handle<mirror::Object> h_thiz(hs.NewHandle(thiz));
+ if (UNLIKELY(self->IsExceptionPending())) {
+ LOG(WARNING) << "Suppressing exception for non-standard method exit: "
+ << self->GetException()->Dump();
+ self->ClearException();
}
- return false;
+ if (kMonitorState != MonitorState::kNoMonitorsLocked) {
+ UnlockHeldMonitors<kMonitorCounting>(self, &frame);
+ }
+ DoMonitorCheckOnExit<kMonitorCounting>(self, &frame);
+ result = JValue();
+ if (UNLIKELY(NeedsMethodExitEvent(instrumentation))) {
+ SendMethodExitEvents(
+ self, instrumentation, frame, h_thiz.Get(), frame.GetMethod(), dex_pc, result);
+ }
}
// Handles all invoke-XXX/range instructions except for invoke-polymorphic[/range].
diff --git a/runtime/interpreter/interpreter_switch_impl-inl.h b/runtime/interpreter/interpreter_switch_impl-inl.h
index 538ac12..d7a1b1b 100644
--- a/runtime/interpreter/interpreter_switch_impl-inl.h
+++ b/runtime/interpreter/interpreter_switch_impl-inl.h
@@ -55,13 +55,37 @@
public:
#define HANDLER_ATTRIBUTES ALWAYS_INLINE FLATTEN WARN_UNUSED REQUIRES_SHARED(Locks::mutator_lock_)
+ HANDLER_ATTRIBUTES bool CheckTransactionAbort() {
+ if (transaction_active && Runtime::Current()->IsTransactionAborted()) {
+ // Transaction abort cannot be caught by catch handlers.
+ // Preserve the abort exception while doing non-standard return.
+ StackHandleScope<1u> hs(Self());
+ Handle<mirror::Throwable> abort_exception = hs.NewHandle(Self()->GetException());
+ DCHECK(abort_exception != nullptr);
+ DCHECK(abort_exception->GetClass()->DescriptorEquals(Transaction::kAbortExceptionDescriptor));
+ Self()->ClearException();
+ PerformNonStandardReturn<kMonitorState>(Self(),
+ shadow_frame_,
+ ctx_->result,
+ Instrumentation(),
+ Accessor().InsSize(),
+ inst_->GetDexPc(Insns()));
+ Self()->SetException(abort_exception.Get());
+ ExitInterpreterLoop();
+ return false;
+ }
+ return true;
+ }
+
HANDLER_ATTRIBUTES bool CheckForceReturn() {
- if (PerformNonStandardReturn<kMonitorState>(Self(),
- shadow_frame_,
- ctx_->result,
- Instrumentation(),
- Accessor().InsSize(),
- inst_->GetDexPc(Insns()))) {
+ if (shadow_frame_.GetForcePopFrame()) {
+ DCHECK(Runtime::Current()->AreNonStandardExitsEnabled());
+ PerformNonStandardReturn<kMonitorState>(Self(),
+ shadow_frame_,
+ ctx_->result,
+ Instrumentation(),
+ Accessor().InsSize(),
+ inst_->GetDexPc(Insns()));
ExitInterpreterLoop();
return false;
}
@@ -71,6 +95,9 @@
HANDLER_ATTRIBUTES bool HandlePendingException() {
DCHECK(Self()->IsExceptionPending());
Self()->AllowThreadSuspension();
+ if (!CheckTransactionAbort()) {
+ return false;
+ }
if (!CheckForceReturn()) {
return false;
}
diff --git a/runtime/interpreter/unstarted_runtime.cc b/runtime/interpreter/unstarted_runtime.cc
index 2c040ab..73fffe8 100644
--- a/runtime/interpreter/unstarted_runtime.cc
+++ b/runtime/interpreter/unstarted_runtime.cc
@@ -127,24 +127,17 @@
}
// Helper function to deal with class loading in an unstarted runtime.
-static void UnstartedRuntimeFindClass(Thread* self, Handle<mirror::String> className,
- Handle<mirror::ClassLoader> class_loader, JValue* result,
- const std::string& method_name, bool initialize_class,
- bool abort_if_not_found)
+static void UnstartedRuntimeFindClass(Thread* self,
+ Handle<mirror::String> className,
+ Handle<mirror::ClassLoader> class_loader,
+ JValue* result,
+ bool initialize_class)
REQUIRES_SHARED(Locks::mutator_lock_) {
CHECK(className != nullptr);
std::string descriptor(DotToDescriptor(className->ToModifiedUtf8().c_str()));
ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
ObjPtr<mirror::Class> found = class_linker->FindClass(self, descriptor.c_str(), class_loader);
- if (found == nullptr && abort_if_not_found) {
- if (!self->IsExceptionPending()) {
- AbortTransactionOrFail(self, "%s failed in un-started runtime for class: %s",
- method_name.c_str(),
- PrettyDescriptor(descriptor.c_str()).c_str());
- }
- return;
- }
if (found != nullptr && initialize_class) {
StackHandleScope<1> hs(self);
HandleWrapperObjPtr<mirror::Class> h_class = hs.NewHandleWrapper(&found);
@@ -164,9 +157,23 @@
static void CheckExceptionGenerateClassNotFound(Thread* self)
REQUIRES_SHARED(Locks::mutator_lock_) {
if (self->IsExceptionPending()) {
- // If it is not the transaction abort exception, wrap it.
- std::string type(mirror::Object::PrettyTypeOf(self->GetException()));
- if (type != Transaction::kAbortExceptionDescriptor) {
+ Runtime* runtime = Runtime::Current();
+ DCHECK_EQ(runtime->IsTransactionAborted(),
+ self->GetException()->GetClass()->DescriptorEquals(
+ Transaction::kAbortExceptionDescriptor))
+ << self->GetException()->GetClass()->PrettyDescriptor();
+ if (runtime->IsActiveTransaction()) {
+ // The boot class path at run time may contain additional dex files with
+ // the required class definition(s). We cannot throw a normal exception at
+ // compile time because a class initializer could catch it and successfully
+ // initialize a class differently than when executing at run time.
+ // If we're not aborting the transaction yet, abort now. b/183691501
+ if (!runtime->IsTransactionAborted()) {
+ AbortTransactionF(self, "ClassNotFoundException");
+ }
+ } else {
+ // If not in a transaction, it cannot be the transaction abort exception. Wrap it.
+ DCHECK(!runtime->IsTransactionAborted());
self->ThrowNewWrappedException("Ljava/lang/ClassNotFoundException;",
"ClassNotFoundException");
}
@@ -206,8 +213,7 @@
ShadowFrame* shadow_frame,
JValue* result,
size_t arg_offset,
- bool long_form,
- const char* caller) {
+ bool long_form) {
ObjPtr<mirror::String> class_name = GetClassName(self, shadow_frame, arg_offset);
if (class_name == nullptr) {
return;
@@ -239,20 +245,18 @@
h_class_name,
ScopedNullHandle<mirror::ClassLoader>(),
result,
- caller,
- initialize_class,
- false);
+ initialize_class);
CheckExceptionGenerateClassNotFound(self);
}
void UnstartedRuntime::UnstartedClassForName(
Thread* self, ShadowFrame* shadow_frame, JValue* result, size_t arg_offset) {
- UnstartedClassForNameCommon(self, shadow_frame, result, arg_offset, false, "Class.forName");
+ UnstartedClassForNameCommon(self, shadow_frame, result, arg_offset, /*long_form=*/ false);
}
void UnstartedRuntime::UnstartedClassForNameLong(
Thread* self, ShadowFrame* shadow_frame, JValue* result, size_t arg_offset) {
- UnstartedClassForNameCommon(self, shadow_frame, result, arg_offset, true, "Class.forName");
+ UnstartedClassForNameCommon(self, shadow_frame, result, arg_offset, /*long_form=*/ true);
}
void UnstartedRuntime::UnstartedClassGetPrimitiveClass(
@@ -271,7 +275,7 @@
void UnstartedRuntime::UnstartedClassClassForName(
Thread* self, ShadowFrame* shadow_frame, JValue* result, size_t arg_offset) {
- UnstartedClassForNameCommon(self, shadow_frame, result, arg_offset, true, "Class.classForName");
+ UnstartedClassForNameCommon(self, shadow_frame, result, arg_offset, /*long_form=*/ true);
}
void UnstartedRuntime::UnstartedClassNewInstance(
@@ -710,13 +714,27 @@
StackHandleScope<2> hs(self);
Handle<mirror::String> h_class_name(hs.NewHandle(class_name));
Handle<mirror::ClassLoader> h_class_loader(hs.NewHandle(class_loader));
- UnstartedRuntimeFindClass(self, h_class_name, h_class_loader, result,
- "VMClassLoader.findLoadedClass", false, false);
+ UnstartedRuntimeFindClass(self,
+ h_class_name,
+ h_class_loader,
+ result,
+ /*initialize_class=*/ false);
// This might have an error pending. But semantics are to just return null.
if (self->IsExceptionPending()) {
- // If it is an InternalError, keep it. See CheckExceptionGenerateClassNotFound.
- std::string type(mirror::Object::PrettyTypeOf(self->GetException()));
- if (type != "java.lang.InternalError") {
+ Runtime* runtime = Runtime::Current();
+ DCHECK_EQ(runtime->IsTransactionAborted(),
+ self->GetException()->GetClass()->DescriptorEquals(
+ Transaction::kAbortExceptionDescriptor))
+ << self->GetException()->GetClass()->PrettyDescriptor();
+ if (runtime->IsActiveTransaction()) {
+ // If we're not aborting the transaction yet, abort now. b/183691501
+ // See CheckExceptionGenerateClassNotFound() for more detailed explanation.
+ if (!runtime->IsTransactionAborted()) {
+ AbortTransactionF(self, "ClassNotFoundException");
+ }
+ } else {
+ // If not in a transaction, it cannot be the transaction abort exception. Clear it.
+ DCHECK(!runtime->IsTransactionAborted());
self->ClearException();
}
}
diff --git a/runtime/interpreter/unstarted_runtime.h b/runtime/interpreter/unstarted_runtime.h
index 4a71645..8b31f8f 100644
--- a/runtime/interpreter/unstarted_runtime.h
+++ b/runtime/interpreter/unstarted_runtime.h
@@ -93,8 +93,7 @@
ShadowFrame* shadow_frame,
JValue* result,
size_t arg_offset,
- bool long_form,
- const char* caller) REQUIRES_SHARED(Locks::mutator_lock_);
+ bool long_form) REQUIRES_SHARED(Locks::mutator_lock_);
static void InitializeInvokeHandlers(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_);
static void InitializeJNIHandlers(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_);
diff --git a/runtime/interpreter/unstarted_runtime_test.cc b/runtime/interpreter/unstarted_runtime_test.cc
index 77075a6..ecee216 100644
--- a/runtime/interpreter/unstarted_runtime_test.cc
+++ b/runtime/interpreter/unstarted_runtime_test.cc
@@ -216,7 +216,7 @@
void PrepareForAborts() REQUIRES_SHARED(Locks::mutator_lock_) {
ObjPtr<mirror::Object> result = Runtime::Current()->GetClassLinker()->FindClass(
Thread::Current(),
- Transaction::kAbortExceptionSignature,
+ Transaction::kAbortExceptionDescriptor,
ScopedNullHandle<mirror::ClassLoader>());
CHECK(result != nullptr);
}
diff --git a/runtime/transaction.cc b/runtime/transaction.cc
index 5d9456b..0293d23 100644
--- a/runtime/transaction.cc
+++ b/runtime/transaction.cc
@@ -21,6 +21,7 @@
#include "aot_class_linker.h"
#include "base/mutex-inl.h"
#include "base/stl_util.h"
+#include "dex/descriptors_names.h"
#include "gc/accounting/card_table-inl.h"
#include "gc/heap.h"
#include "gc_root-inl.h"
@@ -90,16 +91,16 @@
void Transaction::ThrowAbortError(Thread* self, const std::string* abort_message) {
const bool rethrow = (abort_message == nullptr);
if (kIsDebugBuild && rethrow) {
- CHECK(IsAborted()) << "Rethrow " << Transaction::kAbortExceptionDescriptor
+ CHECK(IsAborted()) << "Rethrow " << DescriptorToDot(Transaction::kAbortExceptionDescriptor)
<< " while transaction is not aborted";
}
if (rethrow) {
// Rethrow an exception with the earlier abort message stored in the transaction.
- self->ThrowNewWrappedException(Transaction::kAbortExceptionSignature,
+ self->ThrowNewWrappedException(Transaction::kAbortExceptionDescriptor,
GetAbortMessage().c_str());
} else {
// Throw an exception with the given abort message.
- self->ThrowNewWrappedException(Transaction::kAbortExceptionSignature,
+ self->ThrowNewWrappedException(Transaction::kAbortExceptionDescriptor,
abort_message->c_str());
}
}
diff --git a/runtime/transaction.h b/runtime/transaction.h
index 184280f..2fd0e2c 100644
--- a/runtime/transaction.h
+++ b/runtime/transaction.h
@@ -45,8 +45,7 @@
class Transaction final {
public:
- static constexpr const char* kAbortExceptionDescriptor = "dalvik.system.TransactionAbortError";
- static constexpr const char* kAbortExceptionSignature = "Ldalvik/system/TransactionAbortError;";
+ static constexpr const char* kAbortExceptionDescriptor = "Ldalvik/system/TransactionAbortError;";
Transaction(bool strict, mirror::Class* root);
~Transaction();
diff --git a/runtime/transaction_test.cc b/runtime/transaction_test.cc
index 91eafab..ee7c591 100644
--- a/runtime/transaction_test.cc
+++ b/runtime/transaction_test.cc
@@ -54,7 +54,7 @@
ASSERT_TRUE(h_klass->IsInitialized());
h_klass.Assign(class_linker_->FindSystemClass(soa.Self(),
- Transaction::kAbortExceptionSignature));
+ Transaction::kAbortExceptionDescriptor));
ASSERT_TRUE(h_klass != nullptr);
class_linker_->EnsureInitialized(soa.Self(), h_klass, true, true);
ASSERT_TRUE(h_klass->IsInitialized());
@@ -599,6 +599,18 @@
testTransactionAbort("LTransaction$MultipleNativeCallAbortClass;");
}
+// Tests failing class initialization due to Class.forName() not finding the class,
+// even if an "all" catch handler catches the exception thrown when aborting the transaction.
+TEST_F(TransactionTest, CatchClassForNameAbortClass) {
+ testTransactionAbort("LTransaction$CatchClassForNameAbortClass;");
+}
+
+// Same as CatchClassForNameAbortClass but the class initializer tries to do the work twice.
+// This would trigger a DCHECK() if we continued executing bytecode with an aborted transaction.
+TEST_F(TransactionTest, CatchClassForNameAbortClassTwice) {
+ testTransactionAbort("LTransaction$CatchClassForNameAbortClassTwice;");
+}
+
// Tests failing class initialization due to allocating instance of finalizable class.
TEST_F(TransactionTest, FinalizableAbortClass) {
testTransactionAbort("LTransaction$FinalizableAbortClass;");
diff --git a/test/Transaction/Transaction.java b/test/Transaction/Transaction.java
index e7085c1..6a4f5ef 100644
--- a/test/Transaction/Transaction.java
+++ b/test/Transaction/Transaction.java
@@ -74,6 +74,31 @@
}
}
+ static class CatchClassForNameAbortClass {
+ static {
+ try {
+ Class.forName("non.existent.TestClass");
+ } catch (Throwable e) {
+ // ignore exception.
+ }
+ }
+ }
+
+ static class CatchClassForNameAbortClassTwice {
+ static {
+ try {
+ Class.forName("non.existent.TestClass");
+ } catch (Throwable e) {
+ // ignore exception.
+ }
+ try {
+ Class.forName("non.existent.TestClass");
+ } catch (Throwable e) {
+ // ignore exception.
+ }
+ }
+ }
+
// Helper class to abort transaction: finalizable class with natve methods.
static class AbortHelperClass {
public void finalize() throws Throwable {