Revert "Change JNI behavior related to hiddenapi."
This reverts commit 579db19af4f718c1aac5ca95c180a70c5114c6bd.
Bug: 178680596
Bug: 122551864
Bug: 184067905
Reason for revert: b/184067905
Change-Id: I1c989b6d937b818e4779d7dd9f35abd30be2aa58
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 4dbc357..745e7cf 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -9000,6 +9000,38 @@
return resolved;
}
+// Return the first accessible method from the list of interfaces implemented by
+// `klass`. For knowing if a method is accessible, we call through
+// `hiddenapi::ShouldDenyAccessToMember`.
+static ArtMethod* FindAccessibleInterfaceMethod(ObjPtr<mirror::Class> klass,
+ ObjPtr<mirror::DexCache> dex_cache,
+ ObjPtr<mirror::ClassLoader> class_loader,
+ ArtMethod* resolved_method,
+ PointerSize pointer_size)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ ObjPtr<mirror::IfTable> iftable = klass->GetIfTable();
+ for (int32_t i = 0, iftable_count = iftable->Count(); i < iftable_count; ++i) {
+ ObjPtr<mirror::PointerArray> methods = iftable->GetMethodArrayOrNull(i);
+ if (methods == nullptr) {
+ continue;
+ }
+ for (size_t j = 0, count = iftable->GetMethodArrayCount(i); j < count; ++j) {
+ if (resolved_method == methods->GetElementPtrSize<ArtMethod*>(j, pointer_size)) {
+ ObjPtr<mirror::Class> iface = iftable->GetInterface(i);
+ ArtMethod* interface_method = &iface->GetVirtualMethodsSlice(pointer_size)[j];
+ // Pass AccessMethod::kNone instead of kLinking to not warn on the
+ // access. We'll only warn later if we could not find a visible method.
+ if (!hiddenapi::ShouldDenyAccessToMember(interface_method,
+ hiddenapi::AccessContext(class_loader, dex_cache),
+ hiddenapi::AccessMethod::kNone)) {
+ return interface_method;
+ }
+ }
+ }
+ }
+ return nullptr;
+}
+
ArtMethod* ClassLinker::FindResolvedMethod(ObjPtr<mirror::Class> klass,
ObjPtr<mirror::DexCache> dex_cache,
ObjPtr<mirror::ClassLoader> class_loader,
@@ -9025,10 +9057,11 @@
// hiddenapi (typically it is declared up the hierarchy and is not an SDK
// method). Try to find an interface method from the implemented interfaces which is
// accessible.
- ArtMethod* itf_method = klass->FindAccessibleInterfaceMethod(
- resolved,
- hiddenapi::AccessContext(class_loader, dex_cache),
- image_pointer_size_);
+ ArtMethod* itf_method = FindAccessibleInterfaceMethod(klass,
+ dex_cache,
+ class_loader,
+ resolved,
+ image_pointer_size_);
if (itf_method == nullptr) {
// No interface method. Call ShouldDenyAccessToMember again but this time
// with AccessMethod::kLinking to ensure that an appropriate warning is
diff --git a/runtime/jni/jni_internal.cc b/runtime/jni/jni_internal.cc
index 8f1c81e..1847a25 100644
--- a/runtime/jni/jni_internal.cc
+++ b/runtime/jni/jni_internal.cc
@@ -163,26 +163,20 @@
// things not rendering correctly. E.g. b/16858794
static constexpr bool kWarnJniAbort = false;
-static hiddenapi::AccessContext GetJniAccessContext(Thread* self)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- // Construct AccessContext from the first calling class on stack.
- // If the calling class cannot be determined, e.g. unattached threads,
- // we conservatively assume the caller is trusted.
- ObjPtr<mirror::Class> caller = GetCallingClass(self, /* num_frames= */ 1);
- return caller.IsNull() ? hiddenapi::AccessContext(/* is_trusted= */ true)
- : hiddenapi::AccessContext(caller);
-}
-
template<typename T>
-ALWAYS_INLINE static bool ShouldDenyAccessToMember(
- T* member,
- Thread* self,
- hiddenapi::AccessMethod access_kind = hiddenapi::AccessMethod::kJNI)
+ALWAYS_INLINE static bool ShouldDenyAccessToMember(T* member, Thread* self)
REQUIRES_SHARED(Locks::mutator_lock_) {
return hiddenapi::ShouldDenyAccessToMember(
member,
- [self]() REQUIRES_SHARED(Locks::mutator_lock_) { return GetJniAccessContext(self); },
- access_kind);
+ [&]() REQUIRES_SHARED(Locks::mutator_lock_) {
+ // Construct AccessContext from the first calling class on stack.
+ // If the calling class cannot be determined, e.g. unattached threads,
+ // we conservatively assume the caller is trusted.
+ ObjPtr<mirror::Class> caller = GetCallingClass(self, /* num_frames */ 1);
+ return caller.IsNull() ? hiddenapi::AccessContext(/* is_trusted= */ true)
+ : hiddenapi::AccessContext(caller);
+ },
+ hiddenapi::AccessMethod::kJNI);
}
// Helpers to call instrumentation functions for fields. These take jobjects so we don't need to set
@@ -412,24 +406,8 @@
} else {
method = c->FindClassMethod(name, sig, pointer_size);
}
- if (method != nullptr &&
- ShouldDenyAccessToMember(method, soa.Self(), hiddenapi::AccessMethod::kNone)) {
- // The resolved method that we have found cannot be accessed due to
- // hiddenapi (typically it is declared up the hierarchy and is not an SDK
- // method). Try to find an interface method from the implemented interfaces which is
- // accessible.
- ArtMethod* itf_method = c->FindAccessibleInterfaceMethod(method,
- GetJniAccessContext(soa.Self()),
- pointer_size);
- if (itf_method == nullptr) {
- // No interface method. Call ShouldDenyAccessToMember again but this time
- // with AccessMethod::kJNI to ensure that an appropriate warning is
- // logged.
- ShouldDenyAccessToMember(method, soa.Self(), hiddenapi::AccessMethod::kJNI);
- method = nullptr;
- } else {
- // We found an interface method that is accessible, continue with the resolved method.
- }
+ if (method != nullptr && ShouldDenyAccessToMember(method, soa.Self())) {
+ method = nullptr;
}
if (method == nullptr || method->IsStatic() != is_static) {
ThrowNoSuchMethodError(soa, c, name, sig, is_static ? "static" : "non-static");
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index b1e8f2b..fb2b4a7 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -1794,34 +1794,5 @@
return res;
}
-ArtMethod* Class::FindAccessibleInterfaceMethod(
- ArtMethod* implementation_method,
- const hiddenapi::AccessContext& access_context,
- PointerSize pointer_size)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- ObjPtr<mirror::IfTable> iftable = GetIfTable();
- for (int32_t i = 0, iftable_count = iftable->Count(); i < iftable_count; ++i) {
- ObjPtr<mirror::PointerArray> methods = iftable->GetMethodArrayOrNull(i);
- if (methods == nullptr) {
- continue;
- }
- for (size_t j = 0, count = iftable->GetMethodArrayCount(i); j < count; ++j) {
- if (implementation_method == methods->GetElementPtrSize<ArtMethod*>(j, pointer_size)) {
- ObjPtr<mirror::Class> iface = iftable->GetInterface(i);
- ArtMethod* interface_method = &iface->GetVirtualMethodsSlice(pointer_size)[j];
- // Pass AccessMethod::kNone to not warn on the access. The caller is responsible
- // for eventually logging a warning.
- if (!hiddenapi::ShouldDenyAccessToMember(interface_method,
- access_context,
- hiddenapi::AccessMethod::kNone)) {
- return interface_method;
- }
- }
- }
- }
- return nullptr;
-}
-
-
} // namespace mirror
} // namespace art
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 351fb56..c2f1c59 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -923,15 +923,6 @@
PointerSize pointer_size)
REQUIRES_SHARED(Locks::mutator_lock_);
- // Return the first accessible method from the list of interfaces implemented by
- // this. For knowing if a method is accessible, we call through
- // `hiddenapi::ShouldDenyAccessToMember`.
- ArtMethod* FindAccessibleInterfaceMethod(
- ArtMethod* implementation_method,
- const hiddenapi::AccessContext& access_context,
- PointerSize pointer_size)
- REQUIRES_SHARED(Locks::mutator_lock_);
-
// Find a method with the given name and signature in a non-interface class.
//
// Search for the method in the class, following the JLS rules which conflict with the RI
diff --git a/test/817-hiddenapi/expected-stdout.txt b/test/817-hiddenapi/expected-stdout.txt
index 8db7853..6a5618e 100644
--- a/test/817-hiddenapi/expected-stdout.txt
+++ b/test/817-hiddenapi/expected-stdout.txt
@@ -1,2 +1 @@
JNI_OnLoad called
-JNI_OnLoad called
diff --git a/test/817-hiddenapi/src-art/Main.java b/test/817-hiddenapi/src-art/Main.java
index b83327e..a55066a 100644
--- a/test/817-hiddenapi/src-art/Main.java
+++ b/test/817-hiddenapi/src-art/Main.java
@@ -17,8 +17,6 @@
import dalvik.system.PathClassLoader;
import java.io.File;
import java.lang.reflect.Method;
-import java.nio.file.Files;
-import java.util.Arrays;
public class Main {
@@ -37,43 +35,6 @@
Class<?> cls = Class.forName("TestCase", true, childLoader);
Method m = cls.getDeclaredMethod("test");
m.invoke(null);
-
- // Create a new native library which 'childLoader' can load.
- String absoluteLibraryPath = getNativeLibFileName(args[0]);
-
- // Do the test for JNI code.
- m = cls.getDeclaredMethod("testNative", String.class);
- m.invoke(null, createNativeLibCopy(absoluteLibraryPath));
- }
-
- // Tries to find the absolute path of the native library whose basename is 'arg'.
- private static String getNativeLibFileName(String arg) throws Exception {
- String libName = System.mapLibraryName(arg);
- Method libPathsMethod = Runtime.class.getDeclaredMethod("getLibPaths");
- libPathsMethod.setAccessible(true);
- String[] libPaths = (String[]) libPathsMethod.invoke(Runtime.getRuntime());
- String nativeLibFileName = null;
- for (String p : libPaths) {
- String candidate = p + libName;
- if (new File(candidate).exists()) {
- nativeLibFileName = candidate;
- break;
- }
- }
- if (nativeLibFileName == null) {
- throw new IllegalStateException("Didn't find " + libName + " in " +
- Arrays.toString(libPaths));
- }
- return nativeLibFileName;
- }
-
- // Copy native library to a new file with a unique name so it does not
- // conflict with other loaded instance of the same binary file.
- private static String createNativeLibCopy(String nativeLibFileName) throws Exception {
- String tempFileName = System.mapLibraryName("hiddenapitest");
- File tempFile = new File(System.getenv("DEX_LOCATION"), tempFileName);
- Files.copy(new File(nativeLibFileName).toPath(), tempFile.toPath());
- return tempFile.getAbsolutePath();
}
private static final String DEX_PARENT_BOOT =
diff --git a/test/817-hiddenapi/src-ex/TestCase.java b/test/817-hiddenapi/src-ex/TestCase.java
index ed3ecc8..07dfabd 100644
--- a/test/817-hiddenapi/src-ex/TestCase.java
+++ b/test/817-hiddenapi/src-ex/TestCase.java
@@ -24,13 +24,4 @@
}
}
- public static void testNative(String library) {
- System.load(library);
- int value = testNativeInternal();
- if (value != 42) {
- throw new Error("Expected 42, got " + value);
- }
- }
-
- public static native int testNativeInternal();
}
diff --git a/test/817-hiddenapi/test_native.cc b/test/817-hiddenapi/test_native.cc
deleted file mode 100644
index 1c4dd8e..0000000
--- a/test/817-hiddenapi/test_native.cc
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * 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.
- */
-
-#include "jni.h"
-
-#include <android-base/logging.h>
-
-namespace art {
-
-extern "C" JNIEXPORT jint JNICALL Java_TestCase_testNativeInternal(JNIEnv* env,
- jclass) {
- jclass cls = env->FindClass("InheritAbstract");
- CHECK(cls != nullptr);
- jmethodID constructor = env->GetMethodID(cls, "<init>", "()V");
- CHECK(constructor != nullptr);
- jmethodID method_id = env->GetMethodID(cls, "methodPublicSdkNotInAbstractParent", "()I");
- if (method_id == nullptr) {
- return -1;
- }
- jobject obj = env->NewObject(cls, constructor);
- return env->CallIntMethod(obj, method_id);
-}
-
-} // namespace art
diff --git a/test/Android.bp b/test/Android.bp
index 7df6ead..af3aa21 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -657,7 +657,6 @@
"708-jit-cache-churn/jit.cc",
"720-thread-priority/thread_priority.cc",
"800-smali/jni.cc",
- "817-hiddenapi/test_native.cc",
"909-attach-agent/disallow_debugging.cc",
"1001-app-image-regions/app_image_regions.cc",
"1002-notify-startup/startup_interface.cc",