commit ad0777d89df7eb21d7d2001f9743882d10de3f5c
author Roland Levillain <rpl@google.com> Mon Feb 12 20:00:18 2018 +0000
committer Roland Levillain <rpl@google.com> Thu Feb 15 18:51:24 2018 +0000
tree 15bf8fbfd1a3534972195d4ada9d92f1d0c98903
parent db8d9091bbab41060584ab80882b60df20337da7

Visit proxy methods reference arguments when visiting Quick frames roots.

The arguments of a proxy method, stored in the proxy method's stack
frame, need to be visited as GC roots. This is especially important
in the case of a moving GC, where these reference arguments may be
moved like any object. Previously, we would only visit the target
(`this` argument) of proxy methods when visiting Quick frames roots.

Test: art/test/testrunner/testrunner.py --gcstress -t 999-proxy-method-arguments
Test: m test-art-host
Test: m test-art-target
Bug: 73149739
Bug: 70216372
Bug: 67679263
Change-Id: Ieacc966ab1038935600f2193c14e6ca01e88602e

runtime/thread.cc

diff --git a/runtime/thread.cc b/runtime/thread.cc
index 2ee7f9d..1de7b20 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -3436,7 +3436,7 @@
   return object != nullptr && object->GetLockOwnerThreadId() == GetThreadId();
 }
 
-extern "C" StackReference<mirror::Object>* artQuickGetProxyThisObjectReference(ArtMethod** sp)
+extern std::vector<StackReference<mirror::Object>*> GetProxyReferenceArguments(ArtMethod** sp)
     REQUIRES_SHARED(Locks::mutator_lock_);
 
 // RootVisitor parameters are: (const Object* obj, size_t vreg, const StackVisitor* visitor).
@@ -3482,7 +3482,7 @@
       }
     }
     // Mark lock count map required for structured locking checks.
-    shadow_frame->GetLockCountData().VisitMonitors(visitor_, -1, this);
+    shadow_frame->GetLockCountData().VisitMonitors(visitor_, /* vreg */ -1, this);
   }
 
  private:
@@ -3520,7 +3520,7 @@
         }
       }
       mirror::Object* new_ref = klass.Ptr();
-      visitor_(&new_ref, -1, this);
+      visitor_(&new_ref, /* vreg */ -1, this);
       if (new_ref != klass) {
         method->CASDeclaringClass(klass.Ptr(), new_ref->AsClass());
       }
@@ -3583,17 +3583,20 @@
           }
         }
       }
-    } else if (!m->IsStatic() && !m->IsRuntimeMethod() && m->IsProxyMethod()) {
-      // If this is a non-static proxy method, visit its target (`this` object).
+    } else if (!m->IsRuntimeMethod() && m->IsProxyMethod()) {
+      // If this is a proxy method, visit its reference arguments.
+      DCHECK(!m->IsStatic());
       DCHECK(!m->IsNative());
-      StackReference<mirror::Object>* ref_addr =
-          artQuickGetProxyThisObjectReference(cur_quick_frame);
-      mirror::Object* ref = ref_addr->AsMirrorPtr();
-      if (ref != nullptr) {
-        mirror::Object* new_ref = ref;
-        visitor_(&new_ref, -1, this);
-        if (ref != new_ref) {
-          ref_addr->Assign(new_ref);
+      std::vector<StackReference<mirror::Object>*> ref_addrs =
+          GetProxyReferenceArguments(cur_quick_frame);
+      for (StackReference<mirror::Object>* ref_addr : ref_addrs) {
+        mirror::Object* ref = ref_addr->AsMirrorPtr();
+        if (ref != nullptr) {
+          mirror::Object* new_ref = ref;
+          visitor_(&new_ref, /* vreg */ -1, this);
+          if (ref != new_ref) {
+            ref_addr->Assign(new_ref);
+          }
         }
       }
     }