Reland "Change JNI behavior related to hiddenapi."
This reverts commit dbcf4a2a597309f33914fb60dc09e1056032794a.
Bug: 178680596
Bug: 122551864
Bug: 184067905
Reason for revert: Limit the lookup of an accessible interface to an
interface part of the SDK.
Change-Id: If08269908044bc0f2abe1967b6d952b1e828179b
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index df49a3a..23dd0a2 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -9004,38 +9004,6 @@
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,
@@ -9060,12 +9028,8 @@
// 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 = FindAccessibleInterfaceMethod(klass,
- dex_cache,
- class_loader,
- resolved,
- image_pointer_size_);
+ // part of the SDK.
+ ArtMethod* itf_method = klass->FindAccessibleInterfaceMethod(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 1847a25..f483ad3 100644
--- a/runtime/jni/jni_internal.cc
+++ b/runtime/jni/jni_internal.cc
@@ -163,20 +163,26 @@
// 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)
+ALWAYS_INLINE static bool ShouldDenyAccessToMember(
+ T* member,
+ Thread* self,
+ hiddenapi::AccessMethod access_kind = hiddenapi::AccessMethod::kJNI)
REQUIRES_SHARED(Locks::mutator_lock_) {
return hiddenapi::ShouldDenyAccessToMember(
member,
- [&]() 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);
+ [self]() REQUIRES_SHARED(Locks::mutator_lock_) { return GetJniAccessContext(self); },
+ access_kind);
}
// Helpers to call instrumentation functions for fields. These take jobjects so we don't need to set
@@ -406,8 +412,22 @@
} else {
method = c->FindClassMethod(name, sig, pointer_size);
}
- if (method != nullptr && ShouldDenyAccessToMember(method, soa.Self())) {
- method = nullptr;
+ 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, 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 || 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 fb2b4a7..c1593a7 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -1794,5 +1794,29 @@
return res;
}
+ArtMethod* Class::FindAccessibleInterfaceMethod(ArtMethod* implementation_method,
+ 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];
+ // If the interface method is part of the public SDK, return it.
+ if ((hiddenapi::GetRuntimeFlags(interface_method) & kAccPublicApi) != 0) {
+ return interface_method;
+ }
+ }
+ }
+ }
+ return nullptr;
+}
+
+
} // namespace mirror
} // namespace art
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index c2f1c59..fdc1be9 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -923,6 +923,12 @@
PointerSize pointer_size)
REQUIRES_SHARED(Locks::mutator_lock_);
+ // Return the first public SDK method from the list of interfaces implemented by
+ // this class.
+ ArtMethod* FindAccessibleInterfaceMethod(ArtMethod* implementation_method,
+ 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/check b/test/817-hiddenapi/check
new file mode 100755
index 0000000..8c21ab4
--- /dev/null
+++ b/test/817-hiddenapi/check
@@ -0,0 +1,27 @@
+#!/bin/bash
+#
+# 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.
+
+# Inputs:
+# $1: Test's expected standard output
+# $2: Test's actual standard output
+# $3: Test's expected standard error
+# $4: Test's actual standard error
+
+# On gcstress configurations, an extra "JNI_OnUnload called" line may
+# be emitted. If so, remove it.
+sed -e '${/^JNI_OnUnload called$/d;}' "$2" > "$2.tmp"
+
+./default-check "$1" "$2.tmp" "$3" "$4"
diff --git a/test/817-hiddenapi/expected-stdout.txt b/test/817-hiddenapi/expected-stdout.txt
index 6a5618e..8db7853 100644
--- a/test/817-hiddenapi/expected-stdout.txt
+++ b/test/817-hiddenapi/expected-stdout.txt
@@ -1 +1,2 @@
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 a55066a..b83327e 100644
--- a/test/817-hiddenapi/src-art/Main.java
+++ b/test/817-hiddenapi/src-art/Main.java
@@ -17,6 +17,8 @@
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 {
@@ -35,6 +37,43 @@
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 07dfabd..ed3ecc8 100644
--- a/test/817-hiddenapi/src-ex/TestCase.java
+++ b/test/817-hiddenapi/src-ex/TestCase.java
@@ -24,4 +24,13 @@
}
}
+ 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
new file mode 100644
index 0000000..1c4dd8e
--- /dev/null
+++ b/test/817-hiddenapi/test_native.cc
@@ -0,0 +1,37 @@
+/*
+ * 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 44f84cd..33fa5d8 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -663,6 +663,7 @@
"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",