8028430: JDI: ReferenceType.visibleMethods() return wrong visible methods

Reviewed-by: mchung

jdk/src/share/classes/com/sun/tools/jdi/ArrayTypeImpl.java

@@ -31,6 +31,7 @@ import java.util.List;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.Set;
 
 public class ArrayTypeImpl extends ReferenceTypeImpl
     implements ArrayType
@@ -61,7 +62,8 @@ public class ArrayTypeImpl extends ReferenceTypeImpl
         return findType(componentSignature());
     }
 
-    void addVisibleMethods(Map<String, Method> map) {
+    @Override
+    void addVisibleMethods(Map<String, Method> map, Set<InterfaceType> seenInterfaces) {
         // arrays don't have methods
     }
 

jdk/src/share/classes/com/sun/tools/jdi/ClassTypeImpl.java

@@ -382,7 +382,8 @@ public class ClassTypeImpl extends ReferenceTypeImpl
         }
     }
 
-    void addVisibleMethods(Map<String, Method> methodMap) {
+    @Override
+    void addVisibleMethods(Map<String, Method> methodMap, Set<InterfaceType> seenInterfaces) {
         /*
          * Add methods from
          * parent types first, so that the methods in this class will
@@ -392,12 +393,15 @@ public class ClassTypeImpl extends ReferenceTypeImpl
         Iterator<InterfaceType> iter = interfaces().iterator();
         while (iter.hasNext()) {
             InterfaceTypeImpl interfaze = (InterfaceTypeImpl)iter.next();
-            interfaze.addVisibleMethods(methodMap);
+            if (!seenInterfaces.contains(interfaze)) {
+                interfaze.addVisibleMethods(methodMap, seenInterfaces);
+                seenInterfaces.add(interfaze);
+            }
         }
 
         ClassTypeImpl clazz = (ClassTypeImpl)superclass();
         if (clazz != null) {
-            clazz.addVisibleMethods(methodMap);
+            clazz.addVisibleMethods(methodMap, seenInterfaces);
         }
 
         addToMethodMap(methodMap, methods());

jdk/src/share/classes/com/sun/tools/jdi/InterfaceTypeImpl.java

@@ -32,6 +32,7 @@ import java.util.ArrayList;
 import java.util.Map;
 import java.util.Iterator;
 import java.util.Collections;
+import java.util.Set;
 import java.lang.ref.SoftReference;
 
 public class InterfaceTypeImpl extends ReferenceTypeImpl
@@ -80,7 +81,8 @@ public class InterfaceTypeImpl extends ReferenceTypeImpl
         return implementors;
     }
 
-    void addVisibleMethods(Map<String, Method> methodMap) {
+    @Override
+    void addVisibleMethods(Map<String, Method> methodMap, Set<InterfaceType> seenInterfaces) {
         /*
          * Add methods from
          * parent types first, so that the methods in this class will
@@ -88,7 +90,10 @@ public class InterfaceTypeImpl extends ReferenceTypeImpl
          */
 
         for (InterfaceType interfaze : superinterfaces()) {
-            ((InterfaceTypeImpl)interfaze).addVisibleMethods(methodMap);
+            if (!seenInterfaces.contains(interfaze)) {
+                ((InterfaceTypeImpl)interfaze).addVisibleMethods(methodMap, seenInterfaces);
+                seenInterfaces.add(interfaze);
+            }
         }
 
         addToMethodMap(methodMap, methods());

jdk/src/share/classes/com/sun/tools/jdi/ReferenceTypeImpl.java

@@ -511,7 +511,7 @@ implements ReferenceType {
             methodMap.put(method.name().concat(method.signature()), method);
         }
 
-    abstract void addVisibleMethods(Map<String, Method> methodMap);
+    abstract void addVisibleMethods(Map<String, Method> methodMap, Set<InterfaceType> seenInterfaces);
 
     public List<Method> visibleMethods() {
         /*
@@ -520,7 +520,7 @@ implements ReferenceType {
          * concatenation of name and signature.
          */
         Map<String, Method> map = new HashMap<String, Method>();
-        addVisibleMethods(map);
+        addVisibleMethods(map, new HashSet<InterfaceType>());
 
         /*
          * ... but the hash map destroys order. Methods should be

jdk/test/com/sun/jdi/VisibleMethods.java

@@ -0,0 +1,141 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ *  @test
+ *  @summary Test ReferenceType.visibleMethods
+ *  @bug 8028430
+ *
+ *  @author Staffan Larsen
+ *
+ *  @run build TestScaffold VMConnection TargetListener TargetAdapter
+ *  @run compile -g VisibleMethods.java
+ *  @run main VisibleMethods
+ */
+import com.sun.jdi.Method;
+import com.sun.jdi.ReferenceType;
+import com.sun.jdi.StackFrame;
+import com.sun.jdi.StringReference;
+import com.sun.jdi.ThreadReference;
+import com.sun.jdi.event.BreakpointEvent;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/********** target program **********/
+
+interface Super {
+    public void m(Object o); // This method should not be visible in AC
+    public void m(String s); // This method should not be visible in AC
+}
+
+interface One extends Super {
+    public void m(Object o);
+    public void m1(); // Either this method or Two.m1 should be visible in AC
+}
+
+interface Two extends Super {
+    public void m(String s);
+    public void m1(); // Either this method or One.m1 should be visible in AC
+}
+
+abstract class AC implements One, Two {
+}
+
+class CC extends AC {
+    public void m(Object o) {
+    }
+    public void m(String s) {
+    }
+    public void m1() {
+    }
+    public static void main(String[] args) {
+        System.out.println("Goodbye from VisibleMethods!");
+    }
+}
+
+/********** test program **********/
+
+public class VisibleMethods extends TestScaffold {
+    ReferenceType targetClass;
+    ThreadReference mainThread;
+
+    VisibleMethods(String args[]) {
+        super(args);
+    }
+
+    public static void main(String[] args) throws Exception {
+        new VisibleMethods(args).startTests();
+    }
+
+    /********** test core **********/
+
+    protected void runTests()
+        throws Exception
+    {
+        /*
+         * Run to String.<init>
+         */
+        startToMain("CC");
+
+        ReferenceType ac = findReferenceType("AC");
+        List<String> visible = ac.visibleMethods().
+                stream().
+                map(Method::toString).
+                collect(Collectors.toList());
+
+        System.out.println("visibleMethods(): " + visible);
+
+        verifyContains(visible, 1, "Two.m(java.lang.String)");
+        verifyContains(visible, 1, "One.m(java.lang.Object)");
+        verifyContains(visible, 0, "Super.m(java.lang.Object)");
+        verifyContains(visible, 0, "Super.m(java.lang.String)");
+        verifyContains(visible, 1, "Two.m1()", "One.m1()");
+
+        /*
+         * resume the target listening for events
+         */
+        listenUntilVMDisconnect();
+    }
+
+    private void verifyContains(List<String> methods, int matches,
+            String... sigs) throws Exception {
+        if (countMatches(methods, sigs) != matches) {
+            throw new Exception("visibleMethods() should have contained "
+                    + matches + " entry/entries from " + Arrays.toString(sigs));
+        }
+    }
+
+    private int countMatches(List<String> list1, String[] list2) {
+        int count = 0;
+        for (String s1 : list1) {
+            for (String s2 : list2) {
+                if (s1.equals(s2)) {
+                    count++;
+                }
+            }
+        }
+        return count;
+    }
+}