Fix redefinition causing corrupt j.l.r.Field objects
During (non-structural) redefinition we change the dex-field-indexs of
all the ArtField's of the class being redefined. These ids are used by
the java/lang/reflect/Field class to avoid having to store a pointer
to the ArtField. This meant that when a class was redefined any
existing j.l.r.Field objects pointing to its fields are rendered
invalid, due to the dex_field_index no longer being present on the
class.
To fix this we simply walk the heap during class redefinition and
update any j.l.r.Field objects of the redefined class that we find.
Test: ./test.py --host
Bug: 147310999
Bug: 147338896
Change-Id: I3b038b4e669f8b7b4e804238248b4d03780ce2ca
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc
index dccc226..8a8fe30 100644
--- a/openjdkjvmti/ti_redefine.cc
+++ b/openjdkjvmti/ti_redefine.cc
@@ -105,6 +105,7 @@
#include "mirror/dex_cache.h"
#include "mirror/executable-inl.h"
#include "mirror/field-inl.h"
+#include "mirror/field.h"
#include "mirror/method.h"
#include "mirror/method_handle_impl-inl.h"
#include "mirror/object.h"
@@ -2560,6 +2561,7 @@
}
void Redefiner::ClassRedefinition::UpdateFields(art::ObjPtr<art::mirror::Class> mclass) {
+ std::unordered_map<uint32_t, uint32_t> dex_field_index_map;
// 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) {
@@ -2572,10 +2574,33 @@
const art::dex::FieldId* new_field_id =
dex_file_->FindFieldId(*new_declaring_id, *new_name_id, *new_type_id);
CHECK(new_field_id != nullptr);
+ uint32_t new_field_index = dex_file_->GetIndexForFieldId(*new_field_id);
+ // We need to keep track of the changes to the index so we can update any
+ // java.lang.reflect.Field objects that happen to be on the heap.
+ dex_field_index_map[field.GetDexFieldIndex()] = new_field_index;
// 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));
+ field.SetDexFieldIndex(new_field_index);
}
}
+ // walk the heap to update any java/lang/reflect/Field objects that refer to any of these fields.
+ // TODO We could combine this and do one heap-walk for all redefined classes.
+ art::ObjPtr<art::mirror::Class> reflect_field(art::GetClassRoot<art::mirror::Field>());
+ auto vis = [&](art::ObjPtr<art::mirror::Object> obj) REQUIRES_SHARED(art::Locks::mutator_lock_) {
+ // j.l.r.Field is a final class.
+ if (obj->GetClass() != reflect_field) {
+ return;
+ }
+ art::ObjPtr<art::mirror::Field> fld(art::down_cast<art::mirror::Field*>(obj.Ptr()));
+ // Only this class is getting new indexs.
+ if (fld->GetDeclaringClass() != mclass) {
+ return;
+ }
+ auto it = dex_field_index_map.find(fld->GetDexFieldIndex());
+ if (it != dex_field_index_map.end()) {
+ fld->SetDexFieldIndex</*kTransactionActive=*/false>(it->second);
+ }
+ };
+ driver_->runtime_->GetHeap()->VisitObjectsPaused(vis);
}
void Redefiner::ClassRedefinition::CollectNewFieldAndMethodMappings(
diff --git a/runtime/mirror/field.h b/runtime/mirror/field.h
index 2ed3452..e9e3c17 100644
--- a/runtime/mirror/field.h
+++ b/runtime/mirror/field.h
@@ -42,6 +42,12 @@
ALWAYS_INLINE uint32_t GetDexFieldIndex() REQUIRES_SHARED(Locks::mutator_lock_) {
return GetField32(OFFSET_OF_OBJECT_MEMBER(Field, dex_field_index_));
}
+ // Public for use by class redefinition code.
+ template<bool kTransactionActive>
+ void SetDexFieldIndex(uint32_t idx) REQUIRES_SHARED(Locks::mutator_lock_) {
+ SetField32<kTransactionActive>(OFFSET_OF_OBJECT_MEMBER(Field, dex_field_index_), idx);
+ }
+
ObjPtr<mirror::Class> GetDeclaringClass() REQUIRES_SHARED(Locks::mutator_lock_);
@@ -107,11 +113,6 @@
}
template<bool kTransactionActive>
- void SetDexFieldIndex(uint32_t idx) REQUIRES_SHARED(Locks::mutator_lock_) {
- SetField32<kTransactionActive>(OFFSET_OF_OBJECT_MEMBER(Field, dex_field_index_), idx);
- }
-
- template<bool kTransactionActive>
void SetOffset(uint32_t offset) REQUIRES_SHARED(Locks::mutator_lock_) {
SetField32<kTransactionActive>(OFFSET_OF_OBJECT_MEMBER(Field, offset_), offset);
}
diff --git a/test/2008-redefine-then-old-reflect-field/expected.txt b/test/2008-redefine-then-old-reflect-field/expected.txt
new file mode 100644
index 0000000..e767888
--- /dev/null
+++ b/test/2008-redefine-then-old-reflect-field/expected.txt
@@ -0,0 +1,2 @@
+PreTransform Field public java.lang.Object art.Test2008$Transform.myField = "bar"
+PostTransform Field public java.lang.Object art.Test2008$Transform.myField = "bar"
diff --git a/test/2008-redefine-then-old-reflect-field/info.txt b/test/2008-redefine-then-old-reflect-field/info.txt
new file mode 100644
index 0000000..08c2799
--- /dev/null
+++ b/test/2008-redefine-then-old-reflect-field/info.txt
@@ -0,0 +1,4 @@
+Tests that j.l.r.Field objects survive across redefinitions
+
+We had a bug where java.lang.reflect.Field objects would be invalid after the class of the Field
+they are referencing is redefined. This tests that the bug is fixed.
diff --git a/test/2008-redefine-then-old-reflect-field/run b/test/2008-redefine-then-old-reflect-field/run
new file mode 100755
index 0000000..c6e62ae
--- /dev/null
+++ b/test/2008-redefine-then-old-reflect-field/run
@@ -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-run "$@" --jvmti
diff --git a/test/2008-redefine-then-old-reflect-field/src/Main.java b/test/2008-redefine-then-old-reflect-field/src/Main.java
new file mode 100644
index 0000000..e51f0c4
--- /dev/null
+++ b/test/2008-redefine-then-old-reflect-field/src/Main.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2019 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.Test2008.run();
+ }
+}
diff --git a/test/2008-redefine-then-old-reflect-field/src/art/Redefinition.java b/test/2008-redefine-then-old-reflect-field/src/art/Redefinition.java
new file mode 120000
index 0000000..81eaf31
--- /dev/null
+++ b/test/2008-redefine-then-old-reflect-field/src/art/Redefinition.java
@@ -0,0 +1 @@
+../../../jvmti-common/Redefinition.java
\ No newline at end of file
diff --git a/test/2008-redefine-then-old-reflect-field/src/art/Test2008.java b/test/2008-redefine-then-old-reflect-field/src/art/Test2008.java
new file mode 100644
index 0000000..c97f29a
--- /dev/null
+++ b/test/2008-redefine-then-old-reflect-field/src/art/Test2008.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.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Base64;
+import java.util.concurrent.CountDownLatch;
+import java.util.function.Supplier;
+import java.util.concurrent.atomic.*;
+import java.lang.reflect.*;
+
+public class Test2008 {
+ public static class Transform {
+ public Transform() { myField = "bar"; }
+ public Object myField;
+ }
+
+ /**
+ * base64 encoded class/dex file for
+ * public static class Transform {
+ * public Transform() { myField = "foo"; };
+ * public Object myField;
+ * }
+ */
+ private static final byte[] DEX_BYTES =
+ Base64.getDecoder()
+ .decode(
+"ZGV4CjAzNQC9mLO3NCcl4Iqwlj+DV0clWONvLK5zDAqAAwAAcAAAAHhWNBIAAAAAAAAAAMgCAAAP" +
+"AAAAcAAAAAYAAACsAAAAAQAAAMQAAAABAAAA0AAAAAIAAADYAAAAAQAAAOgAAAB4AgAACAEAACwB" +
+"AAA0AQAATgEAAF4BAACCAQAAogEAALYBAADFAQAA0AEAANMBAADgAQAA5QEAAO4BAAD0AQAA+wEA" +
+"AAEAAAACAAAAAwAAAAQAAAAFAAAACAAAAAgAAAAFAAAAAAAAAAAABAALAAAAAAAAAAAAAAAEAAAA" +
+"AAAAAAAAAAABAAAABAAAAAAAAAAGAAAAuAIAAJkCAAAAAAAAAgABAAEAAAAoAQAACAAAAHAQAQAB" +
+"ABoACgBbEAAADgAFAA4ABjxpbml0PgAYTGFydC9UZXN0MjAwOCRUcmFuc2Zvcm07AA5MYXJ0L1Rl" +
+"c3QyMDA4OwAiTGRhbHZpay9hbm5vdGF0aW9uL0VuY2xvc2luZ0NsYXNzOwAeTGRhbHZpay9hbm5v" +
+"dGF0aW9uL0lubmVyQ2xhc3M7ABJMamF2YS9sYW5nL09iamVjdDsADVRlc3QyMDA4LmphdmEACVRy" +
+"YW5zZm9ybQABVgALYWNjZXNzRmxhZ3MAA2ZvbwAHbXlGaWVsZAAEbmFtZQAFdmFsdWUAjAF+fkQ4" +
+"eyJjb21waWxhdGlvbi1tb2RlIjoiZGVidWciLCJoYXMtY2hlY2tzdW1zIjpmYWxzZSwibWluLWFw" +
+"aSI6MSwic2hhLTEiOiI2NjA0MGE0MGQzY2JmNDA1MDU0NzQ4YmY1YTllOWYyZjNmZThhMzRiIiwi" +
+"dmVyc2lvbiI6IjIuMC4xMi1kZXYifQACAgENGAECAwIJBAkMFwcAAQEAAAEAgYAEiAIAAAAAAAAA" +
+"AgAAAIoCAACQAgAArAIAAAAAAAAAAAAAAAAAAA8AAAAAAAAAAQAAAAAAAAABAAAADwAAAHAAAAAC" +
+"AAAABgAAAKwAAAADAAAAAQAAAMQAAAAEAAAAAQAAANAAAAAFAAAAAgAAANgAAAAGAAAAAQAAAOgA" +
+"AAABIAAAAQAAAAgBAAADIAAAAQAAACgBAAACIAAADwAAACwBAAAEIAAAAgAAAIoCAAAAIAAAAQAA" +
+"AJkCAAADEAAAAgAAAKgCAAAGIAAAAQAAALgCAAAAEAAAAQAAAMgCAAA=");
+ private static final byte[] CLASS_BYTES =
+ Base64.getDecoder()
+ .decode(
+"yv66vgAAADQAFwoABQAOCAAPCQAEABAHABIHABUBAAdteUZpZWxkAQASTGphdmEvbGFuZy9PYmpl" +
+"Y3Q7AQAGPGluaXQ+AQADKClWAQAEQ29kZQEAD0xpbmVOdW1iZXJUYWJsZQEAClNvdXJjZUZpbGUB" +
+"AA1UZXN0MjAwOC5qYXZhDAAIAAkBAANmb28MAAYABwcAFgEAFmFydC9UZXN0MjAwOCRUcmFuc2Zv" +
+"cm0BAAlUcmFuc2Zvcm0BAAxJbm5lckNsYXNzZXMBABBqYXZhL2xhbmcvT2JqZWN0AQAMYXJ0L1Rl" +
+"c3QyMDA4ACEABAAFAAAAAQABAAYABwAAAAEAAQAIAAkAAQAKAAAAIwACAAEAAAALKrcAASoSArUA" +
+"A7EAAAABAAsAAAAGAAEAAAAFAAIADAAAAAIADQAUAAAACgABAAQAEQATAAk=");
+
+
+ public static void run() throws Exception {
+ Redefinition.setTestConfiguration(Redefinition.Config.COMMON_REDEFINE);
+ doTest();
+ }
+
+ public static void doTest() throws Exception {
+ Transform t = new Transform();
+ Field f = Transform.class.getDeclaredField("myField");
+ System.out.println("PreTransform Field " + f + " = \"" + f.get(t) + "\"");
+ Redefinition.doCommonClassRedefinition(Transform.class, CLASS_BYTES, DEX_BYTES);
+ System.out.println("PostTransform Field " + f + " = \"" + f.get(t) + "\"");
+ }
+}
