Allow redefined methods/classes to access fields
Previously we were not updating the ArtFields of redefined classes.
This meant that the fields of redefined classes could not be accessed.
Bug: 32369913
Test: ./test/run-test --host 917
Test: mma -j40 test-art-host
Change-Id: If039e883e9a57970a0cffbc7dcbbaaf2d44490aa
diff --git a/runtime/openjdkjvmti/ti_redefine.cc b/runtime/openjdkjvmti/ti_redefine.cc
index d0349b9..98c2c5f 100644
--- a/runtime/openjdkjvmti/ti_redefine.cc
+++ b/runtime/openjdkjvmti/ti_redefine.cc
@@ -392,19 +392,14 @@
}
}
-// Performs updates to class that will allow us to verify it.
-bool Redefiner::UpdateClass(art::ObjPtr<art::mirror::Class> mclass,
- art::ObjPtr<art::mirror::DexCache> new_dex_cache) {
+bool Redefiner::UpdateMethods(art::ObjPtr<art::mirror::Class> mclass,
+ art::ObjPtr<art::mirror::DexCache> new_dex_cache,
+ const art::DexFile::ClassDef& class_def) {
art::ClassLinker* linker = runtime_->GetClassLinker();
art::PointerSize image_pointer_size = linker->GetImagePointerSize();
- const art::DexFile::ClassDef* class_def = art::OatFile::OatDexFile::FindClassDef(
- *dex_file_, class_sig_, art::ComputeModifiedUtf8Hash(class_sig_));
- if (class_def == nullptr) {
- RecordFailure(ERR(INVALID_CLASS_FORMAT), "Unable to find ClassDef!");
- return false;
- }
- const art::DexFile::TypeId& declaring_class_id = dex_file_->GetTypeId(class_def->class_idx_);
+ const art::DexFile::TypeId& declaring_class_id = dex_file_->GetTypeId(class_def.class_idx_);
const art::DexFile& old_dex_file = mclass->GetDexFile();
+ // Update methods.
for (art::ArtMethod& method : mclass->GetMethods(image_pointer_size)) {
const art::DexFile::StringId* new_name_id = dex_file_->FindStringId(method.GetName());
art::dex::TypeIndex method_return_idx =
@@ -431,10 +426,54 @@
uint32_t dex_method_idx = dex_file_->GetIndexForMethodId(*method_id);
method.SetDexMethodIndex(dex_method_idx);
linker->SetEntryPointsToInterpreter(&method);
- method.SetCodeItemOffset(dex_file_->FindCodeItemOffset(*class_def, dex_method_idx));
+ method.SetCodeItemOffset(dex_file_->FindCodeItemOffset(class_def, dex_method_idx));
method.SetDexCacheResolvedMethods(new_dex_cache->GetResolvedMethods(), image_pointer_size);
method.SetDexCacheResolvedTypes(new_dex_cache->GetResolvedTypes(), image_pointer_size);
}
+ return true;
+}
+
+bool Redefiner::UpdateFields(art::ObjPtr<art::mirror::Class> mclass) {
+ // TODO The IFields & SFields pointers should be combined like the methods_ arrays were.
+ for (auto fields_iter : {mclass->GetIFields(), mclass->GetSFields()}) {
+ for (art::ArtField& field : fields_iter) {
+ std::string declaring_class_name;
+ const art::DexFile::TypeId* new_declaring_id =
+ dex_file_->FindTypeId(field.GetDeclaringClass()->GetDescriptor(&declaring_class_name));
+ const art::DexFile::StringId* new_name_id = dex_file_->FindStringId(field.GetName());
+ const art::DexFile::TypeId* new_type_id = dex_file_->FindTypeId(field.GetTypeDescriptor());
+ // TODO Handle error, cleanup.
+ CHECK(new_name_id != nullptr && new_type_id != nullptr && new_declaring_id != nullptr);
+ const art::DexFile::FieldId* new_field_id =
+ dex_file_->FindFieldId(*new_declaring_id, *new_name_id, *new_type_id);
+ CHECK(new_field_id != nullptr);
+ // We only need to update the index since the other data in the ArtField cannot be updated.
+ field.SetDexFieldIndex(dex_file_->GetIndexForFieldId(*new_field_id));
+ }
+ }
+ return true;
+}
+
+// Performs updates to class that will allow us to verify it.
+bool Redefiner::UpdateClass(art::ObjPtr<art::mirror::Class> mclass,
+ art::ObjPtr<art::mirror::DexCache> new_dex_cache) {
+ const art::DexFile::ClassDef* class_def = art::OatFile::OatDexFile::FindClassDef(
+ *dex_file_, class_sig_, art::ComputeModifiedUtf8Hash(class_sig_));
+ if (class_def == nullptr) {
+ RecordFailure(ERR(INVALID_CLASS_FORMAT), "Unable to find ClassDef!");
+ return false;
+ }
+ if (!UpdateMethods(mclass, new_dex_cache, *class_def)) {
+ // TODO Investigate appropriate error types.
+ RecordFailure(ERR(INTERNAL), "Unable to update class methods.");
+ return false;
+ }
+ if (!UpdateFields(mclass)) {
+ // TODO Investigate appropriate error types.
+ RecordFailure(ERR(INTERNAL), "Unable to update class fields.");
+ return false;
+ }
+
// Update the class fields.
// Need to update class last since the ArtMethod gets its DexFile from the class (which is needed
// to call GetReturnTypeDescriptor and GetParameterTypeList above).
diff --git a/runtime/openjdkjvmti/ti_redefine.h b/runtime/openjdkjvmti/ti_redefine.h
index c819acd..73cfc2b 100644
--- a/runtime/openjdkjvmti/ti_redefine.h
+++ b/runtime/openjdkjvmti/ti_redefine.h
@@ -159,6 +159,14 @@
art::ObjPtr<art::mirror::LongArray> original_cookie)
REQUIRES(art::Locks::mutator_lock_);
+ bool UpdateFields(art::ObjPtr<art::mirror::Class> mclass)
+ REQUIRES(art::Locks::mutator_lock_);
+
+ bool UpdateMethods(art::ObjPtr<art::mirror::Class> mclass,
+ art::ObjPtr<art::mirror::DexCache> new_dex_cache,
+ const art::DexFile::ClassDef& class_def)
+ REQUIRES(art::Locks::mutator_lock_);
+
bool UpdateClass(art::ObjPtr<art::mirror::Class> mclass,
art::ObjPtr<art::mirror::DexCache> new_dex_cache)
REQUIRES(art::Locks::mutator_lock_);
diff --git a/test/917-fields-transformation/build b/test/917-fields-transformation/build
new file mode 100755
index 0000000..898e2e5
--- /dev/null
+++ b/test/917-fields-transformation/build
@@ -0,0 +1,17 @@
+#!/bin/bash
+#
+# Copyright 2016 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-build "$@" --experimental agents
diff --git a/test/917-fields-transformation/expected.txt b/test/917-fields-transformation/expected.txt
new file mode 100644
index 0000000..bcdd201
--- /dev/null
+++ b/test/917-fields-transformation/expected.txt
@@ -0,0 +1,12 @@
+Result is Hello
+take1 is Hello
+take2 is Goodbye
+Result is start
+take1 is start
+take2 is end
+Result is Goodbye
+take1 is Hello
+take2 is Goodbye
+Result is end
+take1 is start
+take2 is end
diff --git a/test/917-fields-transformation/info.txt b/test/917-fields-transformation/info.txt
new file mode 100644
index 0000000..4cd1bd9
--- /dev/null
+++ b/test/917-fields-transformation/info.txt
@@ -0,0 +1 @@
+Tests field access after class redefinition support in the jvmti plugin.
diff --git a/test/917-fields-transformation/run b/test/917-fields-transformation/run
new file mode 100755
index 0000000..a434b63
--- /dev/null
+++ b/test/917-fields-transformation/run
@@ -0,0 +1,43 @@
+#!/bin/bash
+#
+# Copyright 2016 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.
+
+plugin=libopenjdkjvmtid.so
+agent=libtiagentd.so
+lib=tiagentd
+if [[ "$@" == *"-O"* ]]; then
+ agent=libtiagent.so
+ plugin=libopenjdkjvmti.so
+ lib=tiagent
+fi
+
+if [[ "$@" == *"--jvm"* ]]; then
+ arg="jvm"
+else
+ arg="art"
+ if [[ "$@" != *"--debuggable"* ]]; then
+ other_args=" -Xcompiler-option --debuggable "
+ else
+ other_args=""
+ fi
+fi
+
+./default-run "$@" --experimental agents \
+ --experimental runtime-plugins \
+ --runtime-option -agentpath:${agent}=917-fields-transformation,${arg} \
+ --android-runtime-option -Xplugin:${plugin} \
+ --android-runtime-option -Xfully-deoptable \
+ ${other_args} \
+ --args ${lib}
diff --git a/test/917-fields-transformation/src/Main.java b/test/917-fields-transformation/src/Main.java
new file mode 100644
index 0000000..5378bb7
--- /dev/null
+++ b/test/917-fields-transformation/src/Main.java
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2016 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 java.util.Base64;
+public class Main {
+
+ // base64 encoded class/dex file for
+ // class Transform {
+ // public String take1;
+ // public String take2;
+ //
+ // public Transform(String a, String b) {
+ // take1 = a;
+ // take2 = b;
+ // }
+ //
+ // public String getResult() {
+ // return take2;
+ // }
+ // }
+ private static final byte[] CLASS_BYTES = Base64.getDecoder().decode(
+ "yv66vgAAADQAFwoABQARCQAEABIJAAQAEwcAFAcAFQEABXRha2UxAQASTGphdmEvbGFuZy9TdHJp" +
+ "bmc7AQAFdGFrZTIBAAY8aW5pdD4BACcoTGphdmEvbGFuZy9TdHJpbmc7TGphdmEvbGFuZy9TdHJp" +
+ "bmc7KVYBAARDb2RlAQAPTGluZU51bWJlclRhYmxlAQAJZ2V0UmVzdWx0AQAUKClMamF2YS9sYW5n" +
+ "L1N0cmluZzsBAApTb3VyY2VGaWxlAQAOVHJhbnNmb3JtLmphdmEMAAkAFgwABgAHDAAIAAcBAAlU" +
+ "cmFuc2Zvcm0BABBqYXZhL2xhbmcvT2JqZWN0AQADKClWACAABAAFAAAAAgABAAYABwAAAAEACAAH" +
+ "AAAAAgABAAkACgABAAsAAAAzAAIAAwAAAA8qtwABKiu1AAIqLLUAA7EAAAABAAwAAAASAAQAAAAU" +
+ "AAQAFQAJABYADgAXAAEADQAOAAEACwAAAB0AAQABAAAABSq0AAOwAAAAAQAMAAAABgABAAAAGgAB" +
+ "AA8AAAACABA=");
+ private static final byte[] DEX_BYTES = Base64.getDecoder().decode(
+ "ZGV4CjAzNQAGUTBb4jIABRlaI9rejdk7RCfyqR2kmNSkAgAAcAAAAHhWNBIAAAAAAAAAAAQCAAAM" +
+ "AAAAcAAAAAQAAACgAAAAAwAAALAAAAACAAAA1AAAAAMAAADkAAAAAQAAAPwAAACIAQAAHAEAAFwB" +
+ "AABkAQAAZwEAAHQBAACIAQAAnAEAAKwBAACvAQAAtAEAAMgBAADTAQAA2gEAAAIAAAADAAAABAAA" +
+ "AAYAAAABAAAAAgAAAAAAAAAGAAAAAwAAAAAAAAAHAAAAAwAAAFQBAAAAAAIACgAAAAAAAgALAAAA" +
+ "AAACAAAAAAAAAAAACQAAAAEAAQAAAAAAAAAAAAAAAAABAAAAAAAAAAUAAAAAAAAA8AEAAAAAAAAD" +
+ "AAMAAQAAAOEBAAAIAAAAcBACAAAAWwEAAFsCAQAOAAIAAQAAAAAA6wEAAAMAAABUEAEAEQAAAAIA" +
+ "AAACAAIABjxpbml0PgABTAALTFRyYW5zZm9ybTsAEkxqYXZhL2xhbmcvT2JqZWN0OwASTGphdmEv" +
+ "bGFuZy9TdHJpbmc7AA5UcmFuc2Zvcm0uamF2YQABVgADVkxMABJlbWl0dGVyOiBqYWNrLTQuMTkA" +
+ "CWdldFJlc3VsdAAFdGFrZTEABXRha2UyABQCAAAHDjwtLQAaAAcOAAACAQEAAQEBAIGABJwCAQG8" +
+ "AgAADQAAAAAAAAABAAAAAAAAAAEAAAAMAAAAcAAAAAIAAAAEAAAAoAAAAAMAAAADAAAAsAAAAAQA" +
+ "AAACAAAA1AAAAAUAAAADAAAA5AAAAAYAAAABAAAA/AAAAAEgAAACAAAAHAEAAAEQAAABAAAAVAEA" +
+ "AAIgAAAMAAAAXAEAAAMgAAACAAAA4QEAAAAgAAABAAAA8AEAAAAQAAABAAAABAIAAA==");
+
+ public static void main(String[] args) {
+ System.loadLibrary(args[1]);
+ doTest(new Transform("Hello", "Goodbye"),
+ new Transform("start", "end"));
+ }
+
+ private static void printTransform(Transform t) {
+ System.out.println("Result is " + t.getResult());
+ System.out.println("take1 is " + t.take1);
+ System.out.println("take2 is " + t.take2);
+ }
+ public static void doTest(Transform t1, Transform t2) {
+ printTransform(t1);
+ printTransform(t2);
+ doCommonClassRedefinition(Transform.class, CLASS_BYTES, DEX_BYTES);
+ printTransform(t1);
+ printTransform(t2);
+ }
+
+ // Transforms the class
+ private static native void doCommonClassRedefinition(Class<?> target,
+ byte[] class_file,
+ byte[] dex_file);
+}
diff --git a/test/917-fields-transformation/src/Transform.java b/test/917-fields-transformation/src/Transform.java
new file mode 100644
index 0000000..6fe6223
--- /dev/null
+++ b/test/917-fields-transformation/src/Transform.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2016 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 Transform {
+ public String take1;
+ public String take2;
+
+ public Transform(String take1, String take2) {
+ this.take1 = take1;
+ this.take2 = take2;
+ }
+
+ public String getResult() {
+ return take1;
+ }
+}
diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk
index c02999b..bf5bdb1 100644
--- a/test/Android.run-test.mk
+++ b/test/Android.run-test.mk
@@ -280,6 +280,7 @@
911-get-stack-trace \
912-classes \
913-heaps \
+ 917-fields-transformation \
ifneq (,$(filter target,$(TARGET_TYPES)))
ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,target,$(RUN_TYPES),$(PREBUILD_TYPES), \
diff --git a/test/ti-agent/common_load.cc b/test/ti-agent/common_load.cc
index 2795cbc..3886148 100644
--- a/test/ti-agent/common_load.cc
+++ b/test/ti-agent/common_load.cc
@@ -66,6 +66,7 @@
{ "911-get-stack-trace", Test911GetStackTrace::OnLoad, nullptr },
{ "912-classes", Test912Classes::OnLoad, nullptr },
{ "913-heaps", Test913Heaps::OnLoad, nullptr },
+ { "917-fields-transformation", common_redefine::OnLoad, nullptr },
};
static AgentLib* FindAgent(char* name) {
