Preserve JNI bindings across structural redefinition
We were incorrectly clearing the JNI bindings of native methods on
classes which are structurally redefined. This can cause major issues
with code which uses JNIEnv::RegisterNatives to setup native methods.
This change copies the required information from the old to the new
method. Since the RegisterNatives method can be run asynchronously we
need to wait until we gain the final SuspendAll to actually read and
copy the registration.
Test: ./test.py --host
Bug: 158476592
Change-Id: Ie8a33234bb852a46e5275502c842553637c8a2c6
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index 05d198b..707c700 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -2758,6 +2758,7 @@
// Instead we need to update everything else.
// Just replace the class and be done with it.
art::Locks::mutator_lock_->AssertExclusiveHeld(driver_->self_);
+ art::ClassLinker* cl = driver_->runtime_->GetClassLinker();
art::ScopedAssertNoThreadSuspension sants(__FUNCTION__);
art::ObjPtr<art::mirror::Class> orig(holder.GetMirrorClass());
art::ObjPtr<art::mirror::Class> replacement(holder.GetNewClassObject());
@@ -2799,12 +2800,24 @@
old_instance,
old_instance->GetClass());
}
- // Mark old class obsolete.
- for (auto old_class : old_classes->Iterate()) {
+ // Mark old class and methods obsolete. Copy over any native implementation as well.
+ for (auto [old_class, new_class] : art::ZipLeft(old_classes->Iterate(), new_classes->Iterate())) {
old_class->SetObsoleteObject();
- // Mark methods obsolete. We need to wait until later to actually clear the jit data.
+ // Mark methods obsolete and copy native implementation. We need to wait
+ // until later to actually clear the jit data. We copy the native
+ // implementation here since we don't want to race with any threads doing
+ // RegisterNatives.
for (art::ArtMethod& m : old_class->GetMethods(art::kRuntimePointerSize)) {
+ if (m.IsNative()) {
+ art::ArtMethod* new_method =
+ new_class->FindClassMethod(m.GetNameView(), m.GetSignature(), art::kRuntimePointerSize);
+ DCHECK(new_class->GetMethodsSlice(art::kRuntimePointerSize).Contains(new_method))
+ << "Could not find method " << m.PrettyMethod() << " declared in new class!";
+ DCHECK(new_method->IsNative());
+ new_method->SetEntryPointFromJni(m.GetEntryPointFromJni());
+ }
m.SetIsObsolete();
+ cl->SetEntryPointsForObsoleteMethod(&m);
if (m.IsInvokable()) {
m.SetDontCompile();
}
diff --git a/test/2035-structural-native-method/expected.txt b/test/2035-structural-native-method/expected.txt
new file mode 100644
index 0000000..c782b8f
--- /dev/null
+++ b/test/2035-structural-native-method/expected.txt
@@ -0,0 +1,3 @@
+value is 42
+value is 42
+non-native value is 1337
diff --git a/test/2035-structural-native-method/info.txt b/test/2035-structural-native-method/info.txt
new file mode 100644
index 0000000..9467187
--- /dev/null
+++ b/test/2035-structural-native-method/info.txt
@@ -0,0 +1,5 @@
+Tests structural redefinition with register-natives
+
+Regression test for b/158476592. Structural redefinition was incorrectly
+clearing the JNI bindings of native methods. This could interfere with
+classes using 'JNIEnv::RegisterNatives'.
diff --git a/test/2035-structural-native-method/run b/test/2035-structural-native-method/run
new file mode 100755
index 0000000..ff387ff
--- /dev/null
+++ b/test/2035-structural-native-method/run
@@ -0,0 +1,17 @@
+#!/bin/bash
+#
+# Copyright 2020 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.
+
+./default-run "$@" --jvmti --runtime-option -Xopaque-jni-ids:true
diff --git a/test/2035-structural-native-method/src-art/Main.java b/test/2035-structural-native-method/src-art/Main.java
new file mode 100644
index 0000000..4bcd725
--- /dev/null
+++ b/test/2035-structural-native-method/src-art/Main.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2020 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.
+ */
+
+public class Main {
+ public static void main(String[] args) throws Exception {
+ art.Test2035.run();
+ }
+}
diff --git a/test/2035-structural-native-method/src-art/art/Redefinition.java b/test/2035-structural-native-method/src-art/art/Redefinition.java
new file mode 120000
index 0000000..81eaf31
--- /dev/null
+++ b/test/2035-structural-native-method/src-art/art/Redefinition.java
@@ -0,0 +1 @@
+../../../jvmti-common/Redefinition.java
\ No newline at end of file
diff --git a/test/2035-structural-native-method/src-art/art/Test2035.java b/test/2035-structural-native-method/src-art/art/Test2035.java
new file mode 100644
index 0000000..b95bff6
--- /dev/null
+++ b/test/2035-structural-native-method/src-art/art/Test2035.java
@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2020 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.
+ */
+
+package art;
+
+import java.util.Base64;
+import java.lang.reflect.Method;
+
+public class Test2035 {
+ public static class Transform {
+ public Transform() {}
+
+ public native long getValue();
+ }
+ /*
+ * base64 encoded class/dex file for
+ * Base64 generated using:
+ * % javac Test2035.java
+ * % d8 Test2035\$Transform.class
+ * % base64 classes.dex| sed 's:^:":' | sed 's:$:" +:'
+ *
+ * package art;
+ * public static class Transform {
+ * public Transform() {
+ * }
+ * public native long getValue();
+ * public long nonNativeValue() {
+ * return 1337;
+ * };
+ * }
+ */
+ private static final byte[] DEX_BYTES =
+ Base64.getDecoder()
+ .decode(
+ "ZGV4CjAzNQAIm/YHNPSI0ggIbrKz6Jg/IBYl2Kq0TXS8AwAAcAAAAHhWNBIAAAAAAAAAABADAAAQ"
+ + "AAAAcAAAAAcAAACwAAAAAgAAAMwAAAAAAAAAAAAAAAQAAADkAAAAAQAAAAQBAACYAgAAJAEAAGAB"
+ + "AABoAQAAawEAAIUBAACVAQAAuQEAANkBAADtAQAA/AEAAAcCAAAKAgAAFwIAACECAAAnAgAANwIA"
+ + "AD4CAAABAAAAAgAAAAMAAAAEAAAABQAAAAYAAAAJAAAAAQAAAAAAAAAAAAAACQAAAAYAAAAAAAAA"
+ + "AQABAAAAAAABAAAACwAAAAEAAAANAAAABQABAAAAAAABAAAAAQAAAAUAAAAAAAAABwAAAAADAADb"
+ + "AgAAAAAAAAMAAQAAAAAAVAEAAAMAAAAWADkFEAAAAAEAAQABAAAAWAEAAAQAAABwEAMAAAAOAA0A"
+ + "DgAJAA48AAAAAAY8aW5pdD4AAUoAGExhcnQvVGVzdDIwMzUkVHJhbnNmb3JtOwAOTGFydC9UZXN0"
+ + "MjAzNTsAIkxkYWx2aWsvYW5ub3RhdGlvbi9FbmNsb3NpbmdDbGFzczsAHkxkYWx2aWsvYW5ub3Rh"
+ + "dGlvbi9Jbm5lckNsYXNzOwASTGphdmEvbGFuZy9PYmplY3Q7AA1UZXN0MjAzNS5qYXZhAAlUcmFu"
+ + "c2Zvcm0AAVYAC2FjY2Vzc0ZsYWdzAAhnZXRWYWx1ZQAEbmFtZQAObm9uTmF0aXZlVmFsdWUABXZh"
+ + "bHVlAIsBfn5EOHsiY29tcGlsYXRpb24tbW9kZSI6ImRlYnVnIiwiaGFzLWNoZWNrc3VtcyI6ZmFs"
+ + "c2UsIm1pbi1hcGkiOjEsInNoYS0xIjoiOGNkYTg3OGE1MjJiMjJkMWQ2YTljNGQ0MjY5M2Y0OTAw"
+ + "MjJmZTQ2YiIsInZlcnNpb24iOiIyLjIuMS1kZXYifQACAwEOGAICBAIKBAkMFwgAAAECAIGABLwC"
+ + "AYECAAEBpAIAAAAAAAAAAgAAAMwCAADSAgAA9AIAAAAAAAAAAAAAAAAAAA4AAAAAAAAAAQAAAAAA"
+ + "AAABAAAAEAAAAHAAAAACAAAABwAAALAAAAADAAAAAgAAAMwAAAAFAAAABAAAAOQAAAAGAAAAAQAA"
+ + "AAQBAAABIAAAAgAAACQBAAADIAAAAgAAAFQBAAACIAAAEAAAAGABAAAEIAAAAgAAAMwCAAAAIAAA"
+ + "AQAAANsCAAADEAAAAgAAAPACAAAGIAAAAQAAAAADAAAAEAAAAQAAABADAAA=");
+
+ public static void run() throws Exception {
+ Redefinition.setTestConfiguration(Redefinition.Config.COMMON_REDEFINE);
+ doTest();
+ }
+
+ public static void doTest() throws Exception {
+ LinkClassMethods(Transform.class);
+ Transform t = new Transform();
+ System.out.println("value is " + t.getValue());
+ Redefinition.doCommonStructuralClassRedefinition(Transform.class, DEX_BYTES);
+ System.out.println("value is " + t.getValue());
+ System.out.println(
+ "non-native value is " + Transform.class.getDeclaredMethod("nonNativeValue").invoke(t));
+ }
+
+ public static native void LinkClassMethods(Class<?> k);
+}
diff --git a/test/2035-structural-native-method/src/Main.java b/test/2035-structural-native-method/src/Main.java
new file mode 100644
index 0000000..e0477b0
--- /dev/null
+++ b/test/2035-structural-native-method/src/Main.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2020 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.
+ */
+
+public class Main {
+ public static void main(String[] args) throws Exception {
+ System.out.println("FAIL: Test is only for art!");
+ }
+}
diff --git a/test/2035-structural-native-method/structural-native.cc b/test/2035-structural-native-method/structural-native.cc
new file mode 100644
index 0000000..bf51c8b
--- /dev/null
+++ b/test/2035-structural-native-method/structural-native.cc
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2020 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 <stdio.h>
+
+#include <vector>
+
+#include "android-base/logging.h"
+#include "android-base/macros.h"
+#include "jni.h"
+#include "jvmti.h"
+
+// Test infrastructure
+#include "jvmti_helper.h"
+#include "scoped_local_ref.h"
+#include "test_env.h"
+
+namespace art {
+namespace Test2035StructuralNativeMethod {
+
+jlong JNICALL TransformNativeMethod(JNIEnv* env ATTRIBUTE_UNUSED, jclass klass ATTRIBUTE_UNUSED) {
+ return 42;
+}
+
+extern "C" JNIEXPORT void JNICALL Java_art_Test2035_LinkClassMethods(
+ JNIEnv* env, jclass klass ATTRIBUTE_UNUSED, jclass target) {
+ JNINativeMethod meth{"getValue", "()J", reinterpret_cast<void*>(TransformNativeMethod)};
+ env->RegisterNatives(target, &meth, 1);
+}
+
+
+} // namespace Test2035StructuralNativeMethod
+} // namespace art
diff --git a/test/Android.bp b/test/Android.bp
index 49cbf3e..62b2153 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -372,6 +372,7 @@
"1976-hello-structural-static-methods/structural_transform_methods.cc",
"2005-pause-all-redefine-multithreaded/pause-all.cc",
"2009-structural-local-ref/local-ref.cc",
+ "2035-structural-native-method/structural-native.cc",
],
// Use NDK-compatible headers for ctstiagent.
header_libs: [
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 10c05f9..aaa3f3f 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1199,7 +1199,8 @@
"2009-structural-local-ref",
"2011-stack-walk-concurrent-instrument",
"2012-structural-redefinition-failures-jni-id",
- "2033-shutdown-mechanics"],
+ "2033-shutdown-mechanics",
+ "2035-structural-native-method"],
"variant": "jvm",
"description": ["Doesn't run on RI."]
},
