Fix bug in proguard deobfuscation of file names.

Configure proguard to preserve stack traces in the test-dump so we can
properly test ahat's deobfuscation of stack traces.

Fix a bug in proguard deobfuscation of file names. Specifically, we
should always compute the file name based on the class associated with a
stack frame, regardless of whether or not the method has been
obfuscated.

Restructure test-dump program to test more cases of stack frame
deobfuscation.

Test: m ahat-test, with expanded proguard deobufscation test.
Change-Id: Idbb564147c6aea6b9c2091612ecf17c01070d5ac
diff --git a/tools/ahat/etc/test-dump.pro b/tools/ahat/etc/test-dump.pro
index 284e4b8..296d411 100644
--- a/tools/ahat/etc/test-dump.pro
+++ b/tools/ahat/etc/test-dump.pro
@@ -3,3 +3,12 @@
   public static void main(java.lang.String[]);
 }
 
+-keep public class SuperDumpedStuff {
+  public void allocateObjectAtUnObfSuperSite();
+}
+
+# Produce useful obfuscated stack traces so we can test useful deobfuscation
+# of stack traces.
+-renamesourcefileattribute SourceFile
+-keepattributes SourceFile,LineNumberTable
+
diff --git a/tools/ahat/src/main/com/android/ahat/proguard/ProguardMap.java b/tools/ahat/src/main/com/android/ahat/proguard/ProguardMap.java
index 131bbf3..32bb209 100644
--- a/tools/ahat/src/main/com/android/ahat/proguard/ProguardMap.java
+++ b/tools/ahat/src/main/com/android/ahat/proguard/ProguardMap.java
@@ -88,12 +88,10 @@
       String key = obfuscatedMethodName + clearSignature;
       FrameData frame = mFrames.get(key);
       if (frame == null) {
-        return new Frame(obfuscatedMethodName, clearSignature,
-            obfuscatedFilename, obfuscatedLine);
+        frame = new FrameData(obfuscatedMethodName, 0);
       }
       return new Frame(frame.clearMethodName, clearSignature,
-          getFileName(clearClassName, frame.clearMethodName),
-          obfuscatedLine - frame.lineDelta);
+          getFileName(clearClassName), obfuscatedLine - frame.lineDelta);
     }
   }
 
@@ -313,15 +311,10 @@
     return builder.toString();
   }
 
-  // Return a file name for the given clear class name and method.
-  private static String getFileName(String clearClass, String method) {
-    int dot = method.lastIndexOf('.');
-    if (dot != -1) {
-      clearClass = method.substring(0, dot);
-    }
-
+  // Return a file name for the given clear class name.
+  private static String getFileName(String clearClass) {
     String filename = clearClass;
-    dot = filename.lastIndexOf('.');
+    int dot = filename.lastIndexOf('.');
     if (dot != -1) {
       filename = filename.substring(dot + 1);
     }
diff --git a/tools/ahat/src/test-dump/DumpedStuff.java b/tools/ahat/src/test-dump/DumpedStuff.java
new file mode 100644
index 0000000..98ead07
--- /dev/null
+++ b/tools/ahat/src/test-dump/DumpedStuff.java
@@ -0,0 +1,160 @@
+/*
+ * Copyright (C) 2015 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.lang.ref.PhantomReference;
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.SoftReference;
+import java.lang.ref.WeakReference;
+import libcore.util.NativeAllocationRegistry;
+
+// We take a heap dump that includes a single instance of this
+// DumpedStuff class. Objects stored as fields in this class can be easily
+// found in the hprof dump by searching for the instance of the DumpedStuff
+// class and reading the desired field.
+public class DumpedStuff extends SuperDumpedStuff {
+  private void allocateObjectAtKnownSite() {
+    objectAllocatedAtKnownSite = new Object();
+    allocateObjectAtKnownSubSite();
+    allocateObjectAtObfSuperSite();
+    allocateObjectAtUnObfSuperSite();
+    allocateObjectAtOverriddenSite();
+  }
+
+  private void allocateObjectAtKnownSubSite() {
+    objectAllocatedAtKnownSubSite = new Object();
+  }
+
+  public void allocateObjectAtOverriddenSite() {
+    objectAllocatedAtOverriddenSite = new Object();
+  }
+
+  DumpedStuff(boolean baseline) {
+    allocateObjectAtKnownSite();
+
+    int n = baseline ? 400000 : 1000000;
+    bigArray = new byte[n];
+    for (int i = 0; i < n; i++) {
+      bigArray[i] = (byte)((i * i) & 0xFF);
+    }
+
+    // 0x12345, 50000, and 0xABCDABCD are arbitrary values.
+    NativeAllocationRegistry registry = new NativeAllocationRegistry(
+        Main.class.getClassLoader(), 0x12345, 50000);
+    registry.registerNativeAllocation(anObject, 0xABCDABCD);
+
+    {
+      Object object = new Object();
+      aLongStrongPathToSamplePathObject = new Reference(new Reference(new Reference(object)));
+      aShortWeakPathToSamplePathObject = new WeakReference(new Reference(object));
+    }
+
+    addedObject = baseline ? null : new AddedObject();
+    removedObject = baseline ? new RemovedObject() : null;
+    modifiedObject = new ModifiedObject();
+    modifiedObject.value = baseline ? 5 : 8;
+    modifiedObject.modifiedRefField = baseline ? "A1" : "A2";
+    modifiedObject.unmodifiedRefField = "B";
+    modifiedStaticField = baseline ? "C1" : "C2";
+    modifiedArray = baseline ? new int[]{0, 1, 2, 3} : new int[]{3, 1, 2, 0};
+
+    // Deep matching dominator trees shouldn't smash the stack when we try
+    // to diff them. Make some deep dominator trees to help test it.
+    for (int i = 0; i < 10000; i++) {
+      StackSmasher smasher = new StackSmasher();
+      smasher.child = stackSmasher;
+      stackSmasher = smasher;
+
+      if (!baseline) {
+        smasher = new StackSmasher();
+        smasher.child = stackSmasherAdded;
+        stackSmasherAdded = smasher;
+      }
+    }
+
+    gcPathArray[2].right.left = gcPathArray[2].left.right;
+  }
+
+  public static class ObjectTree {
+    public ObjectTree left;
+    public ObjectTree right;
+
+    public ObjectTree(ObjectTree left, ObjectTree right) {
+      this.left = left;
+      this.right = right;
+    }
+  }
+
+  public static class AddedObject {
+  }
+
+  public static class RemovedObject {
+  }
+
+  public static class UnchangedObject {
+  }
+
+  public static class ModifiedObject {
+    public int value;
+    public String modifiedRefField;
+    public String unmodifiedRefField;
+  }
+
+  public static class StackSmasher {
+    public StackSmasher child;
+  }
+
+  public static class Reference {
+    public Object referent;
+
+    public Reference(Object referent) {
+      this.referent = referent;
+    }
+  }
+
+  public String basicString = "hello, world";
+  public String nonAscii = "Sigma (Ʃ) is not ASCII";
+  public String embeddedZero = "embedded\0...";  // Non-ASCII for string compression purposes.
+  public char[] charArray = "char thing".toCharArray();
+  public String nullString = null;
+  public Object anObject = new Object();
+  public Reference aReference = new Reference(anObject);
+  public ReferenceQueue<Object> referenceQueue = new ReferenceQueue<Object>();
+  public PhantomReference aPhantomReference = new PhantomReference(anObject, referenceQueue);
+  public WeakReference aWeakReference = new WeakReference(anObject, referenceQueue);
+  public WeakReference aNullReferentReference = new WeakReference(null, referenceQueue);
+  public SoftReference aSoftReference = new SoftReference(new Object());
+  public byte[] bigArray;
+  public ObjectTree[] gcPathArray = new ObjectTree[]{null, null,
+    new ObjectTree(
+        new ObjectTree(null, new ObjectTree(null, null)),
+        new ObjectTree(null, null)),
+    null};
+  public Reference aLongStrongPathToSamplePathObject;
+  public WeakReference aShortWeakPathToSamplePathObject;
+  public WeakReference aWeakRefToGcRoot = new WeakReference(Main.class);
+  public SoftReference aWeakChain = new SoftReference(new Reference(new Reference(new Object())));
+  public Object[] basicStringRef;
+  public AddedObject addedObject;
+  public UnchangedObject unchangedObject = new UnchangedObject();
+  public RemovedObject removedObject;
+  public ModifiedObject modifiedObject;
+  public StackSmasher stackSmasher;
+  public StackSmasher stackSmasherAdded;
+  public static String modifiedStaticField;
+  public int[] modifiedArray;
+  public Object objectAllocatedAtKnownSite;
+  public Object objectAllocatedAtKnownSubSite;
+}
diff --git a/tools/ahat/src/test-dump/Main.java b/tools/ahat/src/test-dump/Main.java
index 079be7d..de36748 100644
--- a/tools/ahat/src/test-dump/Main.java
+++ b/tools/ahat/src/test-dump/Main.java
@@ -16,11 +16,6 @@
 
 import dalvik.system.VMDebug;
 import java.io.IOException;
-import java.lang.ref.PhantomReference;
-import java.lang.ref.ReferenceQueue;
-import java.lang.ref.SoftReference;
-import java.lang.ref.WeakReference;
-import libcore.util.NativeAllocationRegistry;
 import org.apache.harmony.dalvik.ddmc.DdmVmInternal;
 
 /**
@@ -31,138 +26,6 @@
   // collected before we take the heap dump.
   public static DumpedStuff stuff;
 
-  public static class ObjectTree {
-    public ObjectTree left;
-    public ObjectTree right;
-
-    public ObjectTree(ObjectTree left, ObjectTree right) {
-      this.left = left;
-      this.right = right;
-    }
-  }
-
-  public static class AddedObject {
-  }
-
-  public static class RemovedObject {
-  }
-
-  public static class UnchangedObject {
-  }
-
-  public static class ModifiedObject {
-    public int value;
-    public String modifiedRefField;
-    public String unmodifiedRefField;
-  }
-
-  public static class StackSmasher {
-    public StackSmasher child;
-  }
-
-  public static class Reference {
-    public Object referent;
-
-    public Reference(Object referent) {
-      this.referent = referent;
-    }
-  }
-
-  // We will take a heap dump that includes a single instance of this
-  // DumpedStuff class. Objects stored as fields in this class can be easily
-  // found in the hprof dump by searching for the instance of the DumpedStuff
-  // class and reading the desired field.
-  public static class DumpedStuff {
-    public String basicString = "hello, world";
-    public String nonAscii = "Sigma (Ʃ) is not ASCII";
-    public String embeddedZero = "embedded\0...";  // Non-ASCII for string compression purposes.
-    public char[] charArray = "char thing".toCharArray();
-    public String nullString = null;
-    public Object anObject = new Object();
-    public Reference aReference = new Reference(anObject);
-    public ReferenceQueue<Object> referenceQueue = new ReferenceQueue<Object>();
-    public PhantomReference aPhantomReference = new PhantomReference(anObject, referenceQueue);
-    public WeakReference aWeakReference = new WeakReference(anObject, referenceQueue);
-    public WeakReference aNullReferentReference = new WeakReference(null, referenceQueue);
-    public SoftReference aSoftReference = new SoftReference(new Object());
-    public byte[] bigArray;
-    public ObjectTree[] gcPathArray = new ObjectTree[]{null, null,
-      new ObjectTree(
-          new ObjectTree(null, new ObjectTree(null, null)),
-          new ObjectTree(null, null)),
-      null};
-    public Reference aLongStrongPathToSamplePathObject;
-    public WeakReference aShortWeakPathToSamplePathObject;
-    public WeakReference aWeakRefToGcRoot = new WeakReference(Main.class);
-    public SoftReference aWeakChain = new SoftReference(new Reference(new Reference(new Object())));
-    public Object[] basicStringRef;
-    public AddedObject addedObject;
-    public UnchangedObject unchangedObject = new UnchangedObject();
-    public RemovedObject removedObject;
-    public ModifiedObject modifiedObject;
-    public StackSmasher stackSmasher;
-    public StackSmasher stackSmasherAdded;
-    public static String modifiedStaticField;
-    public int[] modifiedArray;
-    public Object objectAllocatedAtKnownSite1;
-    public Object objectAllocatedAtKnownSite2;
-
-    private void allocateObjectAtKnownSite1() {
-      objectAllocatedAtKnownSite1 = new Object();
-      allocateObjectAtKnownSite2();
-    }
-
-    private void allocateObjectAtKnownSite2() {
-      objectAllocatedAtKnownSite2 = new Object();
-    }
-
-    DumpedStuff(boolean baseline) {
-      int n = baseline ? 400000 : 1000000;
-      bigArray = new byte[n];
-      for (int i = 0; i < n; i++) {
-        bigArray[i] = (byte)((i * i) & 0xFF);
-      }
-
-      // 0x12345, 50000, and 0xABCDABCD are arbitrary values.
-      NativeAllocationRegistry registry = new NativeAllocationRegistry(
-          Main.class.getClassLoader(), 0x12345, 50000);
-      registry.registerNativeAllocation(anObject, 0xABCDABCD);
-
-      {
-        Object object = new Object();
-        aLongStrongPathToSamplePathObject = new Reference(new Reference(new Reference(object)));
-        aShortWeakPathToSamplePathObject = new WeakReference(new Reference(object));
-      }
-
-      addedObject = baseline ? null : new AddedObject();
-      removedObject = baseline ? new RemovedObject() : null;
-      modifiedObject = new ModifiedObject();
-      modifiedObject.value = baseline ? 5 : 8;
-      modifiedObject.modifiedRefField = baseline ? "A1" : "A2";
-      modifiedObject.unmodifiedRefField = "B";
-      modifiedStaticField = baseline ? "C1" : "C2";
-      modifiedArray = baseline ? new int[]{0, 1, 2, 3} : new int[]{3, 1, 2, 0};
-
-      allocateObjectAtKnownSite1();
-
-      // Deep matching dominator trees shouldn't smash the stack when we try
-      // to diff them. Make some deep dominator trees to help test it.
-      for (int i = 0; i < 10000; i++) {
-        StackSmasher smasher = new StackSmasher();
-        smasher.child = stackSmasher;
-        stackSmasher = smasher;
-
-        if (!baseline) {
-          smasher = new StackSmasher();
-          smasher.child = stackSmasherAdded;
-          stackSmasherAdded = smasher;
-        }
-      }
-
-      gcPathArray[2].right.left = gcPathArray[2].left.right;
-    }
-  }
-
   public static void main(String[] args) throws IOException {
     if (args.length < 1) {
       System.err.println("no output file specified");
diff --git a/tools/ahat/src/test-dump/SuperDumpedStuff.java b/tools/ahat/src/test-dump/SuperDumpedStuff.java
new file mode 100644
index 0000000..5ec62ae
--- /dev/null
+++ b/tools/ahat/src/test-dump/SuperDumpedStuff.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2017 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.
+ */
+
+// A super class for DumpedStuff to test deobfuscation of methods inherited
+// from the super class.
+public class SuperDumpedStuff {
+
+  public void allocateObjectAtObfSuperSite() {
+    objectAllocatedAtObfSuperSite = new Object();
+  }
+
+  public void allocateObjectAtUnObfSuperSite() {
+    objectAllocatedAtUnObfSuperSite = new Object();
+  }
+
+  public void allocateObjectAtOverriddenSite() {
+    objectAllocatedAtOverriddenSite = new Object();
+  }
+
+  public Object objectAllocatedAtObfSuperSite;
+  public Object objectAllocatedAtUnObfSuperSite;
+  public Object objectAllocatedAtOverriddenSite;
+}
diff --git a/tools/ahat/src/test/com/android/ahat/ProguardMapTest.java b/tools/ahat/src/test/com/android/ahat/ProguardMapTest.java
index ad40f45..02976b5 100644
--- a/tools/ahat/src/test/com/android/ahat/ProguardMapTest.java
+++ b/tools/ahat/src/test/com/android/ahat/ProguardMapTest.java
@@ -45,7 +45,6 @@
     + "    64:66:class.with.only.Fields methodWithObfRes() -> n\n"
     + "    80:80:void lineObfuscatedMethod():8:8 -> o\n"
     + "    90:90:void lineObfuscatedMethod2():9 -> p\n"
-    + "    120:121:void method.from.a.Superclass.supermethod() -> q\n"
     ;
 
   @Test
@@ -160,12 +159,10 @@
     assertEquals("Methods.java", frame.filename);
     assertEquals(13, frame.line);
 
-    frame = map.getFrame("class.with.Methods", "q", "()V", "SourceFile.java", 120);
-    // TODO: Should this be "supermethod", instead of
-    // "method.from.a.Superclass.supermethod"?
-    assertEquals("method.from.a.Superclass.supermethod", frame.method);
-    assertEquals("()V", frame.signature);
-    assertEquals("Superclass.java", frame.filename);
-    assertEquals(120, frame.line);
+    // Some methods may not have been obfuscated. We should still be able
+    // to compute the filename properly.
+    frame = map.getFrame("class.with.Methods", "unObfuscatedMethodName",
+        "()V", "SourceFile.java", 0);
+    assertEquals("Methods.java", frame.filename);
   }
 }
diff --git a/tools/ahat/src/test/com/android/ahat/SiteTest.java b/tools/ahat/src/test/com/android/ahat/SiteTest.java
index dc0fe08..0443d7f 100644
--- a/tools/ahat/src/test/com/android/ahat/SiteTest.java
+++ b/tools/ahat/src/test/com/android/ahat/SiteTest.java
@@ -23,6 +23,7 @@
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
 
 public class SiteTest {
@@ -31,36 +32,54 @@
     TestDump dump = TestDump.getTestDump();
     AhatSnapshot snapshot = dump.getAhatSnapshot();
 
-    AhatInstance obj1 = dump.getDumpedAhatInstance("objectAllocatedAtKnownSite1");
-    AhatInstance obj2 = dump.getDumpedAhatInstance("objectAllocatedAtKnownSite2");
-    Site s2 = obj2.getSite();
-    Site s1b = s2.getParent();
-    Site s1a = obj1.getSite();
-    Site s = s1a.getParent();
+    AhatInstance oKnownSite = dump.getDumpedAhatInstance("objectAllocatedAtKnownSite");
+    Site sKnownSite = oKnownSite.getSite();
+    assertEquals("DumpedStuff.java", sKnownSite.getFilename());
+    assertEquals("allocateObjectAtKnownSite", sKnownSite.getMethodName());
+    assertEquals(29, sKnownSite.getLineNumber());
+    assertSame(sKnownSite, snapshot.getSite(sKnownSite.getId()));
 
-    // TODO: The following commented out assertion fails due to a problem with
-    //       proguard deobfuscation. That bug should be fixed.
-    // assertEquals("Main.java", s.getFilename());
+    AhatInstance oKnownSubSite = dump.getDumpedAhatInstance("objectAllocatedAtKnownSubSite");
+    Site sKnownSubSite = oKnownSubSite.getSite();
+    assertEquals("DumpedStuff.java", sKnownSubSite.getFilename());
+    assertEquals("allocateObjectAtKnownSubSite", sKnownSubSite.getMethodName());
+    assertEquals(37, sKnownSubSite.getLineNumber());
+    assertSame(sKnownSubSite, snapshot.getSite(sKnownSubSite.getId()));
 
-    assertEquals("Main.java", s1a.getFilename());
-    assertEquals("Main.java", s1b.getFilename());
-    assertEquals("Main.java", s2.getFilename());
+    Site sKnownSubSiteParent = sKnownSubSite.getParent();
+    assertEquals("DumpedStuff.java", sKnownSubSiteParent.getFilename());
+    assertEquals("allocateObjectAtKnownSite", sKnownSubSiteParent.getMethodName());
+    assertEquals(30, sKnownSubSiteParent.getLineNumber());
+    assertSame(sKnownSubSiteParent, snapshot.getSite(sKnownSubSiteParent.getId()));
 
-    assertEquals("allocateObjectAtKnownSite1", s1a.getMethodName());
-    assertEquals("allocateObjectAtKnownSite1", s1b.getMethodName());
-    assertEquals("allocateObjectAtKnownSite2", s2.getMethodName());
+    assertNotSame(sKnownSite, sKnownSubSiteParent);
+    assertSame(sKnownSite.getParent(), sKnownSubSiteParent.getParent());
 
-    // TODO: The following commented out assertion fails due to a problem with
-    //       stack frame line numbers - we don't get different line numbers
-    //       for the different sites, so they are indistiguishable. The
-    //       problem with line numbers should be understood and fixed.
-    // assertNotSame(s1a, s1b);
+    Site sKnownSiteParent = sKnownSite.getParent();
+    assertEquals("DumpedStuff.java", sKnownSiteParent.getFilename());
+    assertEquals("<init>", sKnownSiteParent.getMethodName());
+    assertEquals(45, sKnownSiteParent.getLineNumber());
+    assertSame(sKnownSiteParent, snapshot.getSite(sKnownSiteParent.getId()));
 
-    assertSame(s1a.getParent(), s1b.getParent());
+    AhatInstance oObfSuperSite = dump.getDumpedAhatInstance("objectAllocatedAtObfSuperSite");
+    Site sObfSuperSite = oObfSuperSite.getSite();
+    assertEquals("SuperDumpedStuff.java", sObfSuperSite.getFilename());
+    assertEquals("allocateObjectAtObfSuperSite", sObfSuperSite.getMethodName());
+    assertEquals(22, sObfSuperSite.getLineNumber());
+    assertSame(sObfSuperSite, snapshot.getSite(sObfSuperSite.getId()));
 
-    assertSame(s, snapshot.getSite(s.getId()));
-    assertSame(s1a, snapshot.getSite(s1a.getId()));
-    assertSame(s1b, snapshot.getSite(s1b.getId()));
-    assertSame(s2, snapshot.getSite(s2.getId()));
+    AhatInstance oUnObfSuperSite = dump.getDumpedAhatInstance("objectAllocatedAtUnObfSuperSite");
+    Site sUnObfSuperSite = oUnObfSuperSite.getSite();
+    assertEquals("SuperDumpedStuff.java", sUnObfSuperSite.getFilename());
+    assertEquals("allocateObjectAtUnObfSuperSite", sUnObfSuperSite.getMethodName());
+    assertEquals(26, sUnObfSuperSite.getLineNumber());
+    assertSame(sUnObfSuperSite, snapshot.getSite(sUnObfSuperSite.getId()));
+
+    AhatInstance oOverriddenSite = dump.getDumpedAhatInstance("objectAllocatedAtOverriddenSite");
+    Site sOverriddenSite = oOverriddenSite.getSite();
+    assertEquals("DumpedStuff.java", sOverriddenSite.getFilename());
+    assertEquals("allocateObjectAtOverriddenSite", sOverriddenSite.getMethodName());
+    assertEquals(41, sOverriddenSite.getLineNumber());
+    assertSame(sOverriddenSite, snapshot.getSite(sOverriddenSite.getId()));
   }
 }