Merge "ART: Fix VISIT_OBJECTS and ABORT for FollowReferences"

am: f62940eab1

Change-Id: Ifecc04ef4212576771d15fd00dc876a568435145

3 files changed, 62 insertions, 7 deletions

diff --git a/runtime/openjdkjvmti/ti_heap.cc b/runtime/openjdkjvmti/ti_heap.cc
index c2495e3a6b..49d9aca46e 100644
--- a/runtime/openjdkjvmti/ti_heap.cc
+++ b/runtime/openjdkjvmti/ti_heap.cc
@@ -882,12 +882,17 @@ class FollowReferencesHelper FINAL {
     void AddRoot(art::mirror::Object* root_obj, const art::RootInfo& info)
         REQUIRES_SHARED(art::Locks::mutator_lock_)
         REQUIRES(!*tag_table_->GetAllowDisallowLock()) {
+      if (stop_reports_) {
+        return;
+      }
+      bool add_to_worklist = ReportRoot(root_obj, info);
       // We use visited_ to mark roots already so we do not need another set.
       if (visited_->find(root_obj) == visited_->end()) {
         visited_->insert(root_obj);
-        worklist_->push_back(root_obj);
+        if (add_to_worklist) {
+          worklist_->push_back(root_obj);
+        }
       }
-      ReportRoot(root_obj, info);
     }
 
     // Remove NO_THREAD_SAFETY_ANALYSIS once ASSERT_CAPABILITY works correctly.
@@ -993,7 +998,7 @@ class FollowReferencesHelper FINAL {
       UNREACHABLE();
     }
 
-    void ReportRoot(art::mirror::Object* root_obj, const art::RootInfo& info)
+    bool ReportRoot(art::mirror::Object* root_obj, const art::RootInfo& info)
         REQUIRES_SHARED(art::Locks::mutator_lock_)
         REQUIRES(!*tag_table_->GetAllowDisallowLock()) {
       jvmtiHeapReferenceInfo ref_info;
@@ -1002,6 +1007,7 @@ class FollowReferencesHelper FINAL {
       if ((result & JVMTI_VISIT_ABORT) != 0) {
         stop_reports_ = true;
       }
+      return (result & JVMTI_VISIT_OBJECTS) != 0;
     }
 
    private:
diff --git a/test/913-heaps/expected.txt b/test/913-heaps/expected.txt
index fc2761e800..2a183ee06b 100644
--- a/test/913-heaps/expected.txt
+++ b/test/913-heaps/expected.txt
@@ -79,6 +79,37 @@ root@root --(thread)--> 3000@0 [size=132, length=-1]
 5@1002 --(field@9)--> 1@1000 [size=16, length=-1]
 6@1000 --(class)--> 1000@0 [size=123, length=-1]
 ---
+root@root --(thread)--> 3000@0 [size=132, length=-1]
+---
+3@1001 --(class)--> 1001@0 [size=123, length=-1]
+---
+root@root --(thread)--> 3000@0 [size=132, length=-1]
+---
+3@1001 --(class)--> 1001@0 [size=123, length=-1]
+---
+root@root --(stack-local[id=1,tag=3000,depth=2,method=doFollowReferencesTestNonRoot,vreg=13,location= 32])--> 1@1000 [size=16, length=-1]
+root@root --(stack-local[id=1,tag=3000,depth=3,method=doFollowReferencesTest,vreg=1,location= 28])--> 3000@0 [size=132, length=-1]
+root@root --(thread)--> 3000@0 [size=132, length=-1]
+0@0 --(array-element@0)--> 1@1000 [size=16, length=-1]
+---
+1001@0 --(superclass)--> 1000@0 [size=123, length=-1]
+3@1001 --(class)--> 1001@0 [size=123, length=-1]
+3@1001 --(field@4)--> 4@1000 [size=16, length=-1]
+3@1001 --(field@5)--> 5@1002 [size=32, length=-1]
+---
+root@root --(jni-global)--> 1@1000 [size=16, length=-1]
+root@root --(jni-local[id=1,tag=3000,depth=0,method=followReferences])--> 1@1000 [size=16, length=-1]
+root@root --(stack-local[id=1,tag=3000,depth=1,method=doFollowReferencesTestImpl,vreg=13,location= 10])--> 1@1000 [size=16, length=-1]
+root@root --(stack-local[id=1,tag=3000,depth=1,method=doFollowReferencesTestImpl,vreg=5,location= 10])--> 1@1000 [size=16, length=-1]
+root@root --(stack-local[id=1,tag=3000,depth=2,method=doFollowReferencesTestRoot,vreg=4,location= 19])--> 1@1000 [size=16, length=-1]
+root@root --(thread)--> 1@1000 [size=16, length=-1]
+root@root --(thread)--> 3000@0 [size=132, length=-1]
+---
+1001@0 --(superclass)--> 1000@0 [size=123, length=-1]
+3@1001 --(class)--> 1001@0 [size=123, length=-1]
+3@1001 --(field@4)--> 4@1000 [size=16, length=-1]
+3@1001 --(field@5)--> 5@1002 [size=32, length=-1]
+---
 [1@0 (32, 'HelloWorld'), 2@0 (16, '')]
 2
 3
diff --git a/test/913-heaps/src/Main.java b/test/913-heaps/src/Main.java
index 66f68834a1..10778ff3dd 100644
--- a/test/913-heaps/src/Main.java
+++ b/test/913-heaps/src/Main.java
@@ -28,6 +28,16 @@ public class Main {
     Runtime.getRuntime().gc();
     Runtime.getRuntime().gc();
 
+    new TestConfig(null, 0, 1, -1).doFollowReferencesTest();
+
+    Runtime.getRuntime().gc();
+    Runtime.getRuntime().gc();
+
+    new TestConfig(null, 0, Integer.MAX_VALUE, 1).doFollowReferencesTest();
+
+    Runtime.getRuntime().gc();
+    Runtime.getRuntime().gc();
+
     doStringTest();
 
     Runtime.getRuntime().gc();
@@ -202,6 +212,8 @@ public class Main {
   private static class TestConfig {
     private Class<?> klass = null;
     private int heapFilter = 0;
+    private int stopAfter = Integer.MAX_VALUE;
+    private int followSet = -1;
 
     public TestConfig() {
     }
@@ -209,6 +221,12 @@ public class Main {
       this.klass = klass;
       this.heapFilter = heapFilter;
     }
+    public TestConfig(Class<?> klass, int heapFilter, int stopAfter, int followSet) {
+      this.klass = klass;
+      this.heapFilter = heapFilter;
+      this.stopAfter = stopAfter;
+      this.followSet = followSet;
+    }
 
     public void doFollowReferencesTest() throws Exception {
       // Force GCs to clean up dirt.
@@ -241,8 +259,8 @@ public class Main {
       tmpStorage.add(a);
       v.add("0@0", "1@1000");  // tmpStorage[0] --(array-element)--> a.
 
-      doFollowReferencesTestImpl(null, Integer.MAX_VALUE, -1, null, v, null);
-      doFollowReferencesTestImpl(a.foo2, Integer.MAX_VALUE, -1, null, v, "3@1001");
+      doFollowReferencesTestImpl(null, stopAfter, followSet, null, v, null);
+      doFollowReferencesTestImpl(a.foo2, stopAfter, followSet, null, v, "3@1001");
 
       tmpStorage.clear();
     }
@@ -252,8 +270,8 @@ public class Main {
       tagClasses(v);
       A a = createTree(v);
 
-      doFollowReferencesTestImpl(null, Integer.MAX_VALUE, -1, a, v, null);
-      doFollowReferencesTestImpl(a.foo2, Integer.MAX_VALUE, -1, a, v, "3@1001");
+      doFollowReferencesTestImpl(null, stopAfter, followSet, a, v, null);
+      doFollowReferencesTestImpl(a.foo2, stopAfter, followSet, a, v, "3@1001");
     }
 
     private void doFollowReferencesTestImpl(A root, int stopAfter, int followSet,