Ensure R8 mapping file provide sufficient information for retrace

* Always run line number optimization to collect line numbers and
  inline frame information. For debug mode stick to the identity
  line mapping for now

* Fix line number optimization for the identity mapping

* Skip emitting destination line range of equal to the source
  line range

* Add an initial simple test for using the generated mapping file
  with Proguard retrace

This removed the distinction between line number
optimization "identity" vs "off". Now there is only on and off.

Bug:
Change-Id: Ice1da6262006b6643866f6140570f36e953b72a8
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index ec863ee..46f4876 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -476,21 +476,19 @@
 
       ProguardMapSupplier proguardMapSupplier;
 
-      if (options.lineNumberOptimization != LineNumberOptimization.OFF) {
-        timing.begin("Line number remapping");
-        ClassNameMapper classNameMapper =
-            LineNumberOptimizer.run(
-                application,
-                appView.graphLense(),
-                namingLens,
-                options.lineNumberOptimization == LineNumberOptimization.IDENTITY_MAPPING);
-        timing.end();
-        proguardMapSupplier =
-            ProguardMapSupplier.fromClassNameMapper(classNameMapper, options.minApiLevel);
-      } else {
-        proguardMapSupplier =
-            ProguardMapSupplier.fromNamingLens(namingLens, application, options.minApiLevel);
-      }
+      timing.begin("Line number remapping");
+      // When line number optimization is turned off the identity mapping for line numbers is
+      // used. We still run the line number optimizer to collect line numbers and inline frame
+      // information for the mapping file.
+      ClassNameMapper classNameMapper =
+          LineNumberOptimizer.run(
+              application,
+              appView.graphLense(),
+              namingLens,
+              options.lineNumberOptimization == LineNumberOptimization.OFF);
+      timing.end();
+      proguardMapSupplier =
+          ProguardMapSupplier.fromClassNameMapper(classNameMapper, options.minApiLevel);
 
       // If a method filter is present don't produce output since the application is likely partial.
       if (options.hasMethodsFilter()) {
diff --git a/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java b/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java
index 1264a26..779678d 100644
--- a/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java
+++ b/src/main/java/com/android/tools/r8/graph/DexDebugEvent.java
@@ -47,7 +47,7 @@
       writer.putUleb128(delta);
     }
 
-    AdvancePC(int delta) {
+    public AdvancePC(int delta) {
       this.delta = delta;
     }
 
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java b/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java
index f45a3de..c47a7bd 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java
@@ -401,7 +401,7 @@
         builder.append(minifiedRange).append(':');
       }
       builder.append(signature);
-      if (originalRange != null) {
+      if (originalRange != null && !minifiedRange.equals(originalRange)) {
         builder.append(":").append(originalRange);
       }
       builder.append(" -> ").append(renamedName);
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 828a463..e6fd878 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -41,8 +41,7 @@
 
   public enum LineNumberOptimization {
     OFF,
-    ON,
-    IDENTITY_MAPPING
+    ON
   }
 
   public final DexItemFactory itemFactory;
diff --git a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
index 66490b2..e81d920 100644
--- a/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
+++ b/src/main/java/com/android/tools/r8/utils/LineNumberOptimizer.java
@@ -10,6 +10,16 @@
 import com.android.tools.r8.graph.DexApplication;
 import com.android.tools.r8.graph.DexCode;
 import com.android.tools.r8.graph.DexDebugEvent;
+import com.android.tools.r8.graph.DexDebugEvent.AdvanceLine;
+import com.android.tools.r8.graph.DexDebugEvent.AdvancePC;
+import com.android.tools.r8.graph.DexDebugEvent.Default;
+import com.android.tools.r8.graph.DexDebugEvent.EndLocal;
+import com.android.tools.r8.graph.DexDebugEvent.RestartLocal;
+import com.android.tools.r8.graph.DexDebugEvent.SetEpilogueBegin;
+import com.android.tools.r8.graph.DexDebugEvent.SetFile;
+import com.android.tools.r8.graph.DexDebugEvent.SetInlineFrame;
+import com.android.tools.r8.graph.DexDebugEvent.SetPrologueEnd;
+import com.android.tools.r8.graph.DexDebugEvent.StartLocal;
 import com.android.tools.r8.graph.DexDebugEventBuilder;
 import com.android.tools.r8.graph.DexDebugEventVisitor;
 import com.android.tools.r8.graph.DexDebugInfo;
@@ -41,86 +51,6 @@
 
 public class LineNumberOptimizer {
 
-  // EventFilter is a visitor for DebugEvents, splits events into two sinks:
-  // - Forwards non-positional events unchanged into a BypassedEventReceiver
-  // - Forwards positional events, accumulated into DexDebugPositionStates, into
-  //   positionEventReceiver.
-  private static class EventFilter implements DexDebugEventVisitor {
-    private final BypassedEventReceiver bypassedEventReceiver;
-    private final PositionEventReceiver positionEventReceiver;
-
-    private interface BypassedEventReceiver {
-      void receiveBypassedEvent(DexDebugEvent event);
-    }
-
-    private interface PositionEventReceiver {
-      void receivePositionEvent(DexDebugPositionState positionState);
-    }
-
-    private final DexDebugPositionState positionState;
-
-    private EventFilter(
-        int startLine,
-        DexMethod method,
-        BypassedEventReceiver bypassedEventReceiver,
-        PositionEventReceiver positionEventReceiver) {
-      positionState = new DexDebugPositionState(startLine, method);
-      this.bypassedEventReceiver = bypassedEventReceiver;
-      this.positionEventReceiver = positionEventReceiver;
-    }
-
-    @Override
-    public void visit(DexDebugEvent.SetPrologueEnd event) {
-      bypassedEventReceiver.receiveBypassedEvent(event);
-    }
-
-    @Override
-    public void visit(DexDebugEvent.SetEpilogueBegin event) {
-      bypassedEventReceiver.receiveBypassedEvent(event);
-    }
-
-    @Override
-    public void visit(DexDebugEvent.StartLocal event) {
-      bypassedEventReceiver.receiveBypassedEvent(event);
-    }
-
-    @Override
-    public void visit(DexDebugEvent.EndLocal event) {
-      bypassedEventReceiver.receiveBypassedEvent(event);
-    }
-
-    @Override
-    public void visit(DexDebugEvent.RestartLocal event) {
-      bypassedEventReceiver.receiveBypassedEvent(event);
-    }
-
-    @Override
-    public void visit(DexDebugEvent.AdvancePC advancePC) {
-      positionState.visit(advancePC);
-    }
-
-    @Override
-    public void visit(DexDebugEvent.AdvanceLine advanceLine) {
-      positionState.visit(advanceLine);
-    }
-
-    @Override
-    public void visit(DexDebugEvent.SetInlineFrame setInlineFrame) {
-      positionState.visit(setInlineFrame);
-    }
-
-    @Override
-    public void visit(DexDebugEvent.Default defaultEvent) {
-      positionState.visit(defaultEvent);
-      positionEventReceiver.receivePositionEvent(positionState);
-    }
-
-    @Override
-    public void visit(DexDebugEvent.SetFile setFile) {
-      positionState.visit(setFile);
-    }
-  }
-
   // PositionRemapper is a stateful function which takes a position (represented by a
   // DexDebugPositionState) and returns a remapped Position.
   private interface PositionRemapper {
@@ -165,6 +95,11 @@
       this.processedEvents = processedEvents;
     }
 
+    private void emitAdvancePc(int pc) {
+      processedEvents.add(new AdvancePC(pc - previousPc));
+      previousPc = pc;
+    }
+
     private void emitPositionEvents(int currentPc, Position currentPosition) {
       if (previousPosition == null) {
         startLine = currentPosition.line;
@@ -255,7 +190,6 @@
 
         for (DexEncodedMethod method : methods) {
           List<MappedPosition> mappedPositions = new ArrayList<>();
-
           Code code = method.getCode();
           if (code != null) {
             if (code.isDexCode() && doesContainPositions(code.asDexCode())) {
@@ -469,36 +403,96 @@
     DexDebugInfo debugInfo = dexCode.getDebugInfo();
     List<DexDebugEvent> processedEvents = new ArrayList<>();
 
-    // Our pipeline will be:
-    // [debugInfo.events] -> eventFilter -> positionRemapper -> positionEventEmitter ->
-    // [processedEvents]
     PositionEventEmitter positionEventEmitter =
         new PositionEventEmitter(application.dexItemFactory, method.method, processedEvents);
 
-    EventFilter eventFilter =
-        new EventFilter(
-            debugInfo.startLine,
-            method.method,
-            processedEvents::add,
-            positionState -> {
-              int currentLine = positionState.getCurrentLine();
-              assert currentLine >= 0;
-              Position position =
-                  positionRemapper.createRemappedPosition(
-                      positionState.getCurrentLine(),
-                      positionState.getCurrentFile(),
-                      positionState.getCurrentMethod(),
-                      positionState.getCurrentCallerPosition());
-              mappedPositions.add(
-                  new MappedPosition(
-                      positionState.getCurrentMethod(),
-                      currentLine,
-                      positionState.getCurrentCallerPosition(),
-                      position.line));
-              positionEventEmitter.emitPositionEvents(positionState.getCurrentPc(), position);
-            });
+    // Debug event visitor to map line numbers.
+    // TODO(117268618): Cleanup the duplicate pc tracking.
+    DexDebugEventVisitor visitor =
+        new DexDebugEventVisitor() {
+          DexDebugPositionState state =
+              new DexDebugPositionState(debugInfo.startLine, method.method);
+          int currentPc = 0;
+
+          private void flushPc() {
+            if (currentPc != state.getCurrentPc()) {
+              positionEventEmitter.emitAdvancePc(state.getCurrentPc());
+              currentPc = state.getCurrentPc();
+            }
+          }
+
+          @Override
+          public void visit(AdvancePC advancePC) {
+            state.visit(advancePC);
+          }
+
+          @Override
+          public void visit(AdvanceLine advanceLine) {
+            state.visit(advanceLine);
+          }
+
+          @Override
+          public void visit(SetInlineFrame setInlineFrame) {
+            state.visit(setInlineFrame);
+          }
+
+          @Override
+          public void visit(Default defaultEvent) {
+            state.visit(defaultEvent);
+            int currentLine = state.getCurrentLine();
+            assert currentLine >= 0;
+            Position position =
+                positionRemapper.createRemappedPosition(
+                    state.getCurrentLine(),
+                    state.getCurrentFile(),
+                    state.getCurrentMethod(),
+                    state.getCurrentCallerPosition());
+            mappedPositions.add(
+                new MappedPosition(
+                    state.getCurrentMethod(),
+                    currentLine,
+                    state.getCurrentCallerPosition(),
+                    position.line));
+            positionEventEmitter.emitPositionEvents(state.getCurrentPc(), position);
+            currentPc = state.getCurrentPc();
+          }
+
+          @Override
+          public void visit(SetFile setFile) {
+            processedEvents.add(setFile);
+          }
+
+          @Override
+          public void visit(SetPrologueEnd setPrologueEnd) {
+            processedEvents.add(setPrologueEnd);
+          }
+
+          @Override
+          public void visit(SetEpilogueBegin setEpilogueBegin) {
+            processedEvents.add(setEpilogueBegin);
+          }
+
+          @Override
+          public void visit(StartLocal startLocal) {
+            flushPc();
+            processedEvents.add(startLocal);
+          }
+
+          @Override
+          public void visit(EndLocal endLocal) {
+            flushPc();
+            processedEvents.add(endLocal);
+          }
+
+          @Override
+          public void visit(RestartLocal restartLocal) {
+            flushPc();
+            processedEvents.add(restartLocal);
+          }
+        };
+
     for (DexDebugEvent event : debugInfo.events) {
-      event.accept(eventFilter);
+      event.accept(visitor);
     }
 
     DexDebugInfo optimizedDebugInfo =
diff --git a/src/test/java/com/android/tools/r8/ToolHelper.java b/src/test/java/com/android/tools/r8/ToolHelper.java
index 6dde9ed..0382a08 100644
--- a/src/test/java/com/android/tools/r8/ToolHelper.java
+++ b/src/test/java/com/android/tools/r8/ToolHelper.java
@@ -105,6 +105,9 @@
   private static final String PROGUARD6_0_1 = "third_party/proguard/proguard6.0.1/bin/proguard";
   private static final String PROGUARD = PROGUARD5_2_1;
 
+  private static final String RETRACE6_0_1 = "third_party/proguard/proguard6.0.1/bin/retrace";
+  private static final String RETRACE = RETRACE6_0_1;
+
   public static final Path R8_JAR = Paths.get(LIBS_DIR, "r8.jar");
 
   public enum DexVm {
@@ -524,6 +527,13 @@
     return PROGUARD6_0_1 + ".sh";
   }
 
+  private static String getRetraceScript() {
+    if (isWindows()) {
+      return RETRACE + ".bat";
+    }
+    return RETRACE + ".sh";
+  }
+
   private static Path getDxExecutablePath() {
     String toolsDir = toolsDir();
     String executableName = toolsDir.equals("windows") ? "dx.bat" : "dx";
@@ -1498,6 +1508,23 @@
     return runProguard(getProguard6Script(), inJar, outJar, configs, map);
   }
 
+  public static ProcessResult runRetraceRaw(Path map, Path stackTrace) throws IOException {
+    List<String> command = new ArrayList<>();
+    command.add(getRetraceScript());
+    command.add(map.toString());
+    command.add(stackTrace.toString());
+    ProcessBuilder builder = new ProcessBuilder(command);
+    return ToolHelper.runProcess(builder);
+  }
+
+  public static String runRetrace(Path map, Path stackTrace) throws IOException {
+    ProcessResult result = runRetraceRaw(map, stackTrace);
+    if (result.exitCode != 0) {
+      fail("Retrace failed, exit code " + result.exitCode + ", stderr:\n" + result.stderr);
+    }
+    return result.stdout;
+  }
+
   public static class ProcessResult {
 
     public final int exitCode;
diff --git a/src/test/java/com/android/tools/r8/debug/LineNumberOptimizationTest.java b/src/test/java/com/android/tools/r8/debug/LineNumberOptimizationTest.java
index 616256e..78e9a56 100644
--- a/src/test/java/com/android/tools/r8/debug/LineNumberOptimizationTest.java
+++ b/src/test/java/com/android/tools/r8/debug/LineNumberOptimizationTest.java
@@ -96,12 +96,6 @@
   }
 
   @Test
-  public void testIdentityCompilation() throws Throwable {
-    // Compilation will fail if the identity translation does.
-    makeConfig(LineNumberOptimization.IDENTITY_MAPPING, true, false, runtimeKind);
-  }
-
-  @Test
   public void testNotOptimized() throws Throwable {
     testRelease(
         makeConfig(LineNumberOptimization.OFF, false, false, runtimeKind), ORIGINAL_LINE_NUMBERS);
diff --git a/src/test/java/com/android/tools/r8/naming/retrace/RetraceTest.java b/src/test/java/com/android/tools/r8/naming/retrace/RetraceTest.java
new file mode 100644
index 0000000..7ef1c3f
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/naming/retrace/RetraceTest.java
@@ -0,0 +1,135 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.naming.retrace;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.FileUtils;
+import com.android.tools.r8.utils.StringUtils;
+import com.google.common.collect.ImmutableList;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+import java.util.function.BiConsumer;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class RetraceTest extends TestBase {
+  private Backend backend;
+  private CompilationMode mode;
+
+  @Parameters(name = "Backend: {0}, mode: {1}")
+  public static Collection<Object[]> data() {
+    List<Object[]> parameters = new ArrayList<>();
+    for (Backend backend : Backend.values()) {
+      for (CompilationMode mode : CompilationMode.values()) {
+        parameters.add(new Object[] {backend, mode});
+      }
+    }
+    return parameters;
+  }
+
+  public RetraceTest(Backend backend, CompilationMode mode) {
+    this.backend = backend;
+    this.mode = mode;
+  }
+
+  private List<String> retrace(String map, List<String> stackTrace) throws IOException {
+    Path t = temp.newFolder().toPath();
+    Path mapFile = t.resolve("map");
+    Path stackTraceFile = t.resolve("stackTrace");
+    FileUtils.writeTextFile(mapFile, map);
+    FileUtils.writeTextFile(stackTraceFile, stackTrace);
+    return StringUtils.splitLines(ToolHelper.runRetrace(mapFile, stackTraceFile));
+  }
+
+  private List<String> extractStackTrace(ProcessResult result) {
+    ImmutableList.Builder<String> builder = ImmutableList.builder();
+    List<String> stderr = StringUtils.splitLines(result.stderr);
+    Iterator<String> iterator = stderr.iterator();
+    while (iterator.hasNext()) {
+      String line = iterator.next();
+      if (line.startsWith("Exception in thread \"main\"")) {
+        break;
+      }
+    }
+    iterator.forEachRemaining(builder::add);
+    return builder.build();
+  }
+
+  public void runTest(Class<?> mainClass, BiConsumer<List<String>, List<String>> checker)
+      throws Exception {
+    StringBuilder proguardMapBuilder = new StringBuilder();
+    AndroidApp output =
+        ToolHelper.runR8(
+            R8Command.builder()
+                .setMode(mode)
+                .addClassProgramData(ToolHelper.getClassAsBytes(mainClass), Origin.unknown())
+                .addProguardConfiguration(
+                    ImmutableList.of(keepMainProguardConfiguration(mainClass)), Origin.unknown())
+                .setProgramConsumer(emptyConsumer(backend))
+                .setProguardMapConsumer((string, ignore) -> proguardMapBuilder.append(string))
+                .build());
+
+    ProcessResult result = runOnVMRaw(output, mainClass, backend);
+    List<String> stackTrace = extractStackTrace(result);
+    List<String> retracesStackTrace = retrace(proguardMapBuilder.toString(), stackTrace);
+    checker.accept(stackTrace, retracesStackTrace);
+  }
+
+  @Test
+  public void test() throws Exception {
+    runTest(
+        Main.class,
+        (List<String> stackTrace, List<String> retracesStackTrace) -> {
+          assertEquals(
+              mode == CompilationMode.RELEASE, stackTrace.size() != retracesStackTrace.size());
+          if (mode == CompilationMode.DEBUG) {
+            assertThat(stackTrace.get(0), not(containsString("method2")));
+            assertThat(stackTrace.get(1), not(containsString("method1")));
+            assertThat(stackTrace.get(2), containsString("main"));
+          }
+          assertEquals(3, retracesStackTrace.size());
+          assertThat(retracesStackTrace.get(0), containsString("method2"));
+          assertThat(retracesStackTrace.get(1), containsString("method1"));
+          assertThat(retracesStackTrace.get(2), containsString("main"));
+        });
+  }
+}
+
+class Main {
+  public static void method2(int i) {
+    System.out.println("In method2");
+    throw null;
+  }
+
+  public static void method1(String s) {
+    System.out.println("In method1");
+    for (int i = 0; i < 10; i++) {
+      method2(Integer.parseInt(s));
+    }
+  }
+
+  public static void main(String[] args) {
+    System.out.println("In main");
+    method1("1");
+  }
+}