Always check for an exception after a class lookup.
This means we need to stop the lookup, as an exception is pending.
Test: 831-unverified-bcp
Bug: 195766785
Merged-In: I8aa65f6bbaae83eff0be7ca5d82e0c0a548b5b60
(cherry picked from commit 36d182089a4ced3603c7cdcb61b2d92a5a76a471)
Change-Id: I3e414f75e7081eb93b8cb5af89859d25492edf10
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 8b9e42a..e21a004 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -2621,6 +2621,26 @@
return ClassPathEntry(nullptr, nullptr);
}
+// Helper macro to make sure each class loader lookup call handles the case the
+// class loader is not recognized, or the lookup threw an exception.
+#define RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION(call_, result_, thread_) \
+do { \
+ auto local_call = call_; \
+ if (!local_call) { \
+ return false; \
+ } \
+ auto local_result = result_; \
+ if (local_result != nullptr) { \
+ return true; \
+ } \
+ auto local_thread = thread_; \
+ if (local_thread->IsExceptionPending()) { \
+ /* Pending exception means there was an error other than */ \
+ /* ClassNotFound that must be returned to the caller. */ \
+ return false; \
+ } \
+} while (0)
+
bool ClassLinker::FindClassInSharedLibraries(ScopedObjectAccessAlreadyRunnable& soa,
Thread* self,
const char* descriptor,
@@ -2640,12 +2660,10 @@
MutableHandle<mirror::ClassLoader> temp_loader = hs.NewHandle<mirror::ClassLoader>(nullptr);
for (auto loader : shared_libraries.Iterate<mirror::ClassLoader>()) {
temp_loader.Assign(loader);
- if (!FindClassInBaseDexClassLoader(soa, self, descriptor, hash, temp_loader, result)) {
- return false; // One of the shared libraries is not supported.
- }
- if (*result != nullptr) {
- return true; // Found the class up the chain.
- }
+ RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION(
+ FindClassInBaseDexClassLoader(soa, self, descriptor, hash, temp_loader, result),
+ *result,
+ self);
}
return true;
}
@@ -2658,7 +2676,8 @@
/*out*/ ObjPtr<mirror::Class>* result) {
// Termination case: boot class loader.
if (IsBootClassLoader(soa, class_loader.Get())) {
- *result = FindClassInBootClassLoaderClassPath(self, descriptor, hash);
+ RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION(
+ FindClassInBootClassLoaderClassPath(self, descriptor, hash, result), *result, self);
return true;
}
@@ -2668,26 +2687,24 @@
// - shared libraries
// - class loader dex files
- // Handles as RegisterDexFile may allocate dex caches (and cause thread suspension).
+ // Create a handle as RegisterDexFile may allocate dex caches (and cause thread suspension).
StackHandleScope<1> hs(self);
Handle<mirror::ClassLoader> h_parent(hs.NewHandle(class_loader->GetParent()));
- if (!FindClassInBaseDexClassLoader(soa, self, descriptor, hash, h_parent, result)) {
- return false; // One of the parents is not supported.
- }
- if (*result != nullptr) {
- return true; // Found the class up the chain.
- }
-
- if (!FindClassInSharedLibraries(soa, self, descriptor, hash, class_loader, result)) {
- return false; // One of the shared library loader is not supported.
- }
- if (*result != nullptr) {
- return true; // Found the class in a shared library.
- }
-
- // Search the current class loader classpath.
- *result = FindClassInBaseDexClassLoaderClassPath(soa, descriptor, hash, class_loader);
- return !soa.Self()->IsExceptionPending();
+ RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION(
+ FindClassInBaseDexClassLoader(soa, self, descriptor, hash, h_parent, result),
+ *result,
+ self);
+ RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION(
+ FindClassInSharedLibraries(soa, self, descriptor, hash, class_loader, result),
+ *result,
+ self);
+ RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION(
+ FindClassInBaseDexClassLoaderClassPath(soa, descriptor, hash, class_loader, result),
+ *result,
+ self);
+ // We did not find a class, but the class loader chain was recognized, so we
+ // return true.
+ return true;
}
if (IsDelegateLastClassLoader(soa, class_loader)) {
@@ -2696,37 +2713,27 @@
// - shared libraries
// - class loader dex files
// - parent
- *result = FindClassInBootClassLoaderClassPath(self, descriptor, hash);
- if (*result != nullptr) {
- return true; // The class is part of the boot class path.
- }
- if (self->IsExceptionPending()) {
- // Pending exception means there was an error other than ClassNotFound that must be returned
- // to the caller.
- return false;
- }
+ RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION(
+ FindClassInBootClassLoaderClassPath(self, descriptor, hash, result), *result, self);
+ RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION(
+ FindClassInSharedLibraries(soa, self, descriptor, hash, class_loader, result),
+ *result,
+ self);
+ RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION(
+ FindClassInBaseDexClassLoaderClassPath(soa, descriptor, hash, class_loader, result),
+ *result,
+ self);
- if (!FindClassInSharedLibraries(soa, self, descriptor, hash, class_loader, result)) {
- return false; // One of the shared library loader is not supported.
- }
- if (*result != nullptr) {
- return true; // Found the class in a shared library.
- }
-
- *result = FindClassInBaseDexClassLoaderClassPath(soa, descriptor, hash, class_loader);
- if (*result != nullptr) {
- return true; // Found the class in the current class loader
- }
- if (self->IsExceptionPending()) {
- // Pending exception means there was an error other than ClassNotFound that must be returned
- // to the caller.
- return false;
- }
-
- // Handles as RegisterDexFile may allocate dex caches (and cause thread suspension).
+ // Create a handle as RegisterDexFile may allocate dex caches (and cause thread suspension).
StackHandleScope<1> hs(self);
Handle<mirror::ClassLoader> h_parent(hs.NewHandle(class_loader->GetParent()));
- return FindClassInBaseDexClassLoader(soa, self, descriptor, hash, h_parent, result);
+ RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION(
+ FindClassInBaseDexClassLoader(soa, self, descriptor, hash, h_parent, result),
+ *result,
+ self);
+ // We did not find a class, but the class loader chain was recognized, so we
+ // return true.
+ return true;
}
// Unsupported class loader.
@@ -2734,6 +2741,8 @@
return false;
}
+#undef RETURN_IF_UNRECOGNIZED_OR_FOUND_OR_EXCEPTION
+
namespace {
// Matches exceptions caught in DexFile.defineClass.
@@ -2761,36 +2770,38 @@
// Finds the class in the boot class loader.
// If the class is found the method returns the resolved class. Otherwise it returns null.
-ObjPtr<mirror::Class> ClassLinker::FindClassInBootClassLoaderClassPath(Thread* self,
- const char* descriptor,
- size_t hash) {
- ObjPtr<mirror::Class> result = nullptr;
+bool ClassLinker::FindClassInBootClassLoaderClassPath(Thread* self,
+ const char* descriptor,
+ size_t hash,
+ /*out*/ ObjPtr<mirror::Class>* result) {
ClassPathEntry pair = FindInClassPath(descriptor, hash, boot_class_path_);
if (pair.second != nullptr) {
ObjPtr<mirror::Class> klass = LookupClass(self, descriptor, hash, nullptr);
if (klass != nullptr) {
- result = EnsureResolved(self, descriptor, klass);
+ *result = EnsureResolved(self, descriptor, klass);
} else {
- result = DefineClass(self,
- descriptor,
- hash,
- ScopedNullHandle<mirror::ClassLoader>(),
- *pair.first,
- *pair.second);
+ *result = DefineClass(self,
+ descriptor,
+ hash,
+ ScopedNullHandle<mirror::ClassLoader>(),
+ *pair.first,
+ *pair.second);
}
- if (result == nullptr) {
+ if (*result == nullptr) {
CHECK(self->IsExceptionPending()) << descriptor;
FilterDexFileCaughtExceptions(self, this);
}
}
- return result;
+ // The boot classloader is always a known lookup.
+ return true;
}
-ObjPtr<mirror::Class> ClassLinker::FindClassInBaseDexClassLoaderClassPath(
+bool ClassLinker::FindClassInBaseDexClassLoaderClassPath(
ScopedObjectAccessAlreadyRunnable& soa,
const char* descriptor,
size_t hash,
- Handle<mirror::ClassLoader> class_loader) {
+ Handle<mirror::ClassLoader> class_loader,
+ /*out*/ ObjPtr<mirror::Class>* result) {
DCHECK(IsPathOrDexClassLoader(soa, class_loader) ||
IsInMemoryDexClassLoader(soa, class_loader) ||
IsDelegateLastClassLoader(soa, class_loader))
@@ -2810,17 +2821,17 @@
};
VisitClassLoaderDexFiles(soa, class_loader, find_class_def);
- ObjPtr<mirror::Class> klass = nullptr;
if (class_def != nullptr) {
- klass = DefineClass(soa.Self(), descriptor, hash, class_loader, *dex_file, *class_def);
- if (UNLIKELY(klass == nullptr)) {
+ *result = DefineClass(soa.Self(), descriptor, hash, class_loader, *dex_file, *class_def);
+ if (UNLIKELY(*result == nullptr)) {
CHECK(soa.Self()->IsExceptionPending()) << descriptor;
FilterDexFileCaughtExceptions(soa.Self(), this);
} else {
DCHECK(!soa.Self()->IsExceptionPending());
}
}
- return klass;
+ // A BaseDexClassLoader is always a known lookup.
+ return true;
}
ObjPtr<mirror::Class> ClassLinker::FindClass(Thread* self,
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 547753a..5faf760 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -1024,20 +1024,26 @@
// dex files and does not recurse into its parent.
// The method checks that the provided class loader is either a PathClassLoader or a
// DexClassLoader.
- // If the class is found the method returns the resolved class. Otherwise it returns null.
- ObjPtr<mirror::Class> FindClassInBaseDexClassLoaderClassPath(
+ // If the class is found the method updates `result`.
+ // The method always returns true, to notify to the caller a
+ // BaseDexClassLoader has a known lookup.
+ bool FindClassInBaseDexClassLoaderClassPath(
ScopedObjectAccessAlreadyRunnable& soa,
const char* descriptor,
size_t hash,
- Handle<mirror::ClassLoader> class_loader)
+ Handle<mirror::ClassLoader> class_loader,
+ /*out*/ ObjPtr<mirror::Class>* result)
REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(!Locks::dex_lock_);
// Finds the class in the boot class loader.
- // If the class is found the method returns the resolved class. Otherwise it returns null.
- ObjPtr<mirror::Class> FindClassInBootClassLoaderClassPath(Thread* self,
- const char* descriptor,
- size_t hash)
+ // If the class is found the method updates `result`.
+ // The method always returns true, to notify to the caller the
+ // boot class loader has a known lookup.
+ bool FindClassInBootClassLoaderClassPath(Thread* self,
+ const char* descriptor,
+ size_t hash,
+ /*out*/ ObjPtr<mirror::Class>* result)
REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(!Locks::dex_lock_);
diff --git a/test/831-unverified-bcp/expected-stderr.txt b/test/831-unverified-bcp/expected-stderr.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/831-unverified-bcp/expected-stderr.txt
diff --git a/test/831-unverified-bcp/expected-stdout.txt b/test/831-unverified-bcp/expected-stdout.txt
new file mode 100644
index 0000000..6a5618e
--- /dev/null
+++ b/test/831-unverified-bcp/expected-stdout.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/831-unverified-bcp/info.txt b/test/831-unverified-bcp/info.txt
new file mode 100644
index 0000000..b4f7aeb
--- /dev/null
+++ b/test/831-unverified-bcp/info.txt
@@ -0,0 +1,2 @@
+Regression test for class resolution, where the class linker would not check if
+an exception was pending after looking up a class in the boot classpath.
diff --git a/test/831-unverified-bcp/smali-ex/NonVerifiedClass.smali b/test/831-unverified-bcp/smali-ex/NonVerifiedClass.smali
new file mode 100644
index 0000000..986f70a
--- /dev/null
+++ b/test/831-unverified-bcp/smali-ex/NonVerifiedClass.smali
@@ -0,0 +1,23 @@
+# Copyright 2021 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+.class public LNonVerifiedClass;
+.super Ljava/util/Objects;
+
+.method public constructor <init>()V
+ .registers 1
+ invoke-direct {p0}, Ljava/util/Objects;-><init>()V
+ return-void
+.end method
+
diff --git a/test/831-unverified-bcp/src/Main.java b/test/831-unverified-bcp/src/Main.java
new file mode 100644
index 0000000..853d8ed
--- /dev/null
+++ b/test/831-unverified-bcp/src/Main.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import dalvik.system.PathClassLoader;
+import java.io.File;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+import java.nio.file.Files;
+import java.util.Arrays;
+
+public class Main {
+
+ public static void main(String[] args) throws Exception {
+ System.loadLibrary(args[0]);
+ appendToBootClassLoader(OTHER_DEX, /* isCorePlatform */ false);
+
+ try {
+ Class.forName("NonVerifiedClass");
+ throw new Error("Expected VerifyError");
+ } catch (VerifyError e) {
+ // Expected.
+ }
+ }
+
+ private static native int appendToBootClassLoader(String dexPath, boolean isCorePlatform);
+
+ private static final String OTHER_DEX =
+ new File(System.getenv("DEX_LOCATION"), "831-unverified-bcp-ex.jar").getAbsolutePath();
+}
+
+// Define the class also in the classpath, to trigger the AssertNoPendingException crash.
+class NonVerifiedClass {
+}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index d9b089d..dc96854 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1142,6 +1142,7 @@
"820-vdex-multidex",
"821-many-args",
"822-hiddenapi-future",
+ "831-unverified-bcp",
"999-redefine-hiddenapi",
"1000-non-moving-space-stress",
"1001-app-image-regions",
