Always parse line and locals table
This removes the early check on parsing the line and local-variable
table. This unblocks the ability to determine the need for these
structures at a later point. In particular to include line tables and
method parameter names regardless of the `-keepattributes` settings.
In particular the latter is needed to support native keep annotations
which do not define properties in the pg configuration.
Fixes: b/260384637
Fixes: b/279555726
Bug: b/232212653
RELNOTES: R8 now encodes line-table mapping information for all
builds (effectively assuming -keepattributes LineNumberTable).
This was already the case for some builds targeting min-api 26
and above and is now the behavior for all builds. This can
cause minor size regressions for builds that used to discard
all line information and thus cannot be retraced. Due to the
compressed debug-info encoding used by R8 any such size
regression will be minor.
Change-Id: Ic0e8673b4e2f267d6eb162eb083267781f3c8ce3
diff --git a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
index 34c1b21..60bcbdc 100644
--- a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
+++ b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
@@ -68,12 +68,9 @@
import com.android.tools.r8.position.MethodPosition;
import com.android.tools.r8.position.TextPosition;
import com.android.tools.r8.position.TextRange;
-import com.android.tools.r8.shaking.ProguardConfiguration;
-import com.android.tools.r8.shaking.ProguardKeepAttributes;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.ExceptionUtils;
import com.android.tools.r8.utils.InternalOptions;
-import com.android.tools.r8.utils.ReachabilitySensitiveValue;
import com.android.tools.r8.utils.Reporter;
import com.android.tools.r8.utils.RetracerForCodePrinting;
import com.android.tools.r8.utils.StringDiagnostic;
@@ -155,12 +152,7 @@
JarApplicationReader application = this.application;
assert context != null;
assert application != null;
- DexProgramClass programOwner = context.owner.asProgramClass();
- ReachabilitySensitiveValue reachabilitySensitive =
- programOwner != null
- ? programOwner.getReachabilitySensitiveValue()
- : ReachabilitySensitiveValue.DISABLED;
- DebugParsingOptions parsingOptions = getParsingOptions(application, reachabilitySensitive);
+ DebugParsingOptions parsingOptions = getParsingOptions(application);
// The ClassCodeVisitor is in charge of setting this.context to null.
try {
parseCode(context, false, parsingOptions);
@@ -1177,34 +1169,13 @@
}
}
- private static DebugParsingOptions getParsingOptions(
- JarApplicationReader application, ReachabilitySensitiveValue reachabilitySensitive) {
+ private static DebugParsingOptions getParsingOptions(JarApplicationReader application) {
// TODO(b/166841731): We should compute our own from the compressed format.
int parsingOptions =
application.options.canUseInputStackMaps()
? ClassReader.EXPAND_FRAMES
: ClassReader.SKIP_FRAMES;
- ProguardConfiguration configuration = application.options.getProguardConfiguration();
- if (configuration == null) {
- return new DebugParsingOptions(true, true, parsingOptions);
- }
- ProguardKeepAttributes keep =
- application.options.getProguardConfiguration().getKeepAttributes();
-
- boolean localsInfo =
- configuration.isKeepParameterNames()
- || keep.localVariableTable
- || keep.localVariableTypeTable
- || reachabilitySensitive.isEnabled();
- boolean lineInfo =
- (keep.lineNumberTable || application.options.canUseNativeDexPcInsteadOfDebugInfo());
- boolean methodParaeters = keep.methodParameters;
-
- if (!localsInfo && !lineInfo && !methodParaeters) {
- parsingOptions |= ClassReader.SKIP_DEBUG;
- }
-
- return new DebugParsingOptions(lineInfo, localsInfo, parsingOptions);
+ return new DebugParsingOptions(true, true, parsingOptions);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardKeepAttributes.java b/src/main/java/com/android/tools/r8/shaking/ProguardKeepAttributes.java
index 19a6946..445e0b0 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardKeepAttributes.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardKeepAttributes.java
@@ -42,7 +42,6 @@
public boolean enclosingMethod = false;
public boolean signature = false;
public boolean exceptions = false;
- public boolean lineNumberTable = false;
public boolean localVariableTable = false;
public boolean localVariableTypeTable = false;
public boolean methodParameters = false;
@@ -119,8 +118,6 @@
sourceDir = update(sourceDir, SOURCE_DIR, patterns);
innerClasses = update(innerClasses, INNER_CLASSES, patterns);
enclosingMethod = update(enclosingMethod, ENCLOSING_METHOD, patterns);
- lineNumberTable = update(lineNumberTable, LINE_NUMBER_TABLE, patterns);
- localVariableTable = update(localVariableTable, LOCAL_VARIABLE_TABLE, patterns);
localVariableTypeTable = update(localVariableTypeTable, LOCAL_VARIABLE_TYPE_TABLE, patterns);
exceptions = update(exceptions, EXCEPTIONS, patterns);
methodParameters = update(methodParameters, METHOD_PARAMETERS, patterns);
@@ -156,16 +153,6 @@
throw new CompilationError("Attribute EnclosingMethod requires InnerClasses attribute. "
+ "Check -keepattributes directive.");
}
- if (forceProguardCompatibility && localVariableTable && !lineNumberTable) {
- // If locals are kept, assume line numbers should be kept too.
- lineNumberTable = true;
- }
- if (localVariableTable && !lineNumberTable) {
- throw new CompilationError(
- "Attribute " + LOCAL_VARIABLE_TABLE
- + " requires " + LINE_NUMBER_TABLE
- + ". Check -keepattributes directive.");
- }
}
@Override
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 d68f123..28bc741 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -317,7 +317,6 @@
assert !proguardConfiguration.isObfuscating();
getProguardConfiguration().getKeepAttributes().sourceFile = true;
getProguardConfiguration().getKeepAttributes().sourceDebugExtension = true;
- getProguardConfiguration().getKeepAttributes().lineNumberTable = true;
getProguardConfiguration().getKeepAttributes().localVariableTable = true;
getProguardConfiguration().getKeepAttributes().localVariableTypeTable = true;
}
diff --git a/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java b/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java
index 5927841..80a7beb 100644
--- a/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java
+++ b/src/test/java/com/android/tools/r8/cf/TryRangeTestRunner.java
@@ -8,10 +8,13 @@
import com.android.tools.r8.CompilationMode;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.cf.code.CfInstruction;
import com.android.tools.r8.cf.code.CfInvoke;
import com.android.tools.r8.cf.code.CfLabel;
import com.android.tools.r8.cf.code.CfLoad;
+import com.android.tools.r8.cf.code.CfPosition;
import com.android.tools.r8.cf.code.CfStackInstruction;
import com.android.tools.r8.graph.AppView;
import com.android.tools.r8.graph.CfCode;
@@ -24,6 +27,10 @@
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import java.util.List;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
/**
* This tests that we produce valid code when having normal-flow with exceptional edges in blocks.
@@ -31,11 +38,19 @@
* instructions that lie on the boundary of the exception table that is generated for a basic block.
* If live-ranges are minimized this could produce VerifyErrors.
*/
+@RunWith(Parameterized.class)
public class TryRangeTestRunner extends TestBase {
+ @Parameters
+ public static TestParametersCollection data() {
+ return TestParameters.builder().withDefaultCfRuntime().build();
+ }
+
+ @Parameter public TestParameters parameters;
+
@Test
public void testRegisterAllocationLimitTrailingRange() throws Exception {
- testForR8(Backend.CF)
+ testForR8(parameters.getBackend())
.addProgramClasses(TryRangeTest.class)
.addKeepMainRule(TryRangeTest.class)
.setMode(CompilationMode.RELEASE)
@@ -43,14 +58,14 @@
.noTreeShaking()
.enableInliningAnnotations()
.addOptionsModification(o -> o.enableLoadStoreOptimization = false)
- .run(TryRangeTest.class)
+ .run(parameters.getRuntime(), TryRangeTest.class)
.assertSuccessWithOutput(StringUtils.lines("10", "7.0"));
}
@Test
public void testRegisterAllocationLimitLeadingRange() throws Exception {
CodeInspector inspector =
- testForR8(Backend.CF)
+ testForR8(parameters.getBackend())
.addProgramClasses(TryRangeTestLimitRange.class)
.addKeepMainRule(TryRangeTestLimitRange.class)
.setMode(CompilationMode.RELEASE)
@@ -62,7 +77,7 @@
o.enableLoadStoreOptimization = false;
o.testing.irModifier = this::processIR;
})
- .run(TryRangeTestLimitRange.class)
+ .run(parameters.getRuntime(), TryRangeTestLimitRange.class)
.assertSuccessWithOutput("")
.inspector();
// Assert that we do not have any register-modifying instructions in the throwing block:
@@ -80,9 +95,11 @@
index++;
}
assert instructions.get(index + 1) instanceof CfLoad;
- assert instructions.get(index + 2) instanceof CfInvoke;
- assert instructions.get(index + 3) == cfCode.getTryCatchRanges().get(0).end;
- assert instructions.get(index + 4) instanceof CfStackInstruction;
+ assert instructions.get(index + 2) instanceof CfLabel;
+ assert instructions.get(index + 3) instanceof CfPosition;
+ assert instructions.get(index + 4) instanceof CfInvoke;
+ assert instructions.get(index + 5) == cfCode.getTryCatchRanges().get(0).end;
+ assert instructions.get(index + 6) instanceof CfStackInstruction;
}
private void processIR(IRCode code, AppView<?> appView) {
diff --git a/src/test/java/com/android/tools/r8/debuginfo/NoKeepLineAttributeTest.java b/src/test/java/com/android/tools/r8/debuginfo/NoKeepLineAttributeTest.java
index 35ae932..002d369 100644
--- a/src/test/java/com/android/tools/r8/debuginfo/NoKeepLineAttributeTest.java
+++ b/src/test/java/com/android/tools/r8/debuginfo/NoKeepLineAttributeTest.java
@@ -47,18 +47,11 @@
List<StackTraceLine> stackTraceLines = stacktrace.getStackTraceLines();
assertEquals(1, stackTraceLines.size());
StackTraceLine stackTraceLine = stackTraceLines.get(0);
- // The frame will always have a line as the VM is reporting the PC.
+ // The frame will always have a line, either real or a PC.
assertTrue(stackTraceLine.hasLineNumber());
- if (parameters.getApiLevel().isLessThan(apiLevelWithPcAsLineNumberSupport())) {
- // If the compile-time API is before native support then no line info is present.
- // The "line" will be the PC and thus small.
- assertTrue(stackTraceLine.lineNumber < 10);
- } else {
- // If the compile-time API is after native support then the compiler will retain and
- // emit the mapping from PC to original line. Here line 50 is to ensure it is not a
- // low PC value.
- assertTrue(stackTraceLine.lineNumber > 50);
- }
+ // The line should always map back to the original line.
+ // Here line 50 is to ensure it is not a low PC value.
+ assertTrue(stackTraceLine.lineNumber > 50);
});
}
diff --git a/src/test/java/com/android/tools/r8/debuginfo/NoKeepSourceFileAttributeTest.java b/src/test/java/com/android/tools/r8/debuginfo/NoKeepSourceFileAttributeTest.java
index 9a68a09..98d9629 100644
--- a/src/test/java/com/android/tools/r8/debuginfo/NoKeepSourceFileAttributeTest.java
+++ b/src/test/java/com/android/tools/r8/debuginfo/NoKeepSourceFileAttributeTest.java
@@ -29,14 +29,6 @@
this.parameters = parameters;
}
- public boolean isRuntimeWithPcAsLineNumberSupport() {
- return parameters.isDexRuntime()
- && parameters
- .getRuntime()
- .maxSupportedApiLevel()
- .isGreaterThanOrEqualTo(apiLevelWithPcAsLineNumberSupport());
- }
-
@Test
public void test() throws Exception {
testForR8(parameters.getBackend())
@@ -50,13 +42,7 @@
List<StackTraceLine> stackTraceLines = stacktrace.getStackTraceLines();
assertEquals(1, stackTraceLines.size());
StackTraceLine stackTraceLine = stackTraceLines.get(0);
- if (!isRuntimeWithPcAsLineNumberSupport()) {
- assertEquals("SourceFile", stackTraceLine.fileName);
- } else {
- // VMs with native PC support and no debug info print "Unknown Source".
- // TODO(b/146565491): This will need a check for new VMs once fixed.
- assertEquals("Unknown Source", stackTraceLine.fileName);
- }
+ assertEquals("SourceFile", stackTraceLine.fileName);
})
.inspectStackTrace(
stacktrace -> {
diff --git a/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaInStacktraceTest.java b/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaInStacktraceTest.java
index 29d8d11..7c2fab0 100644
--- a/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaInStacktraceTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/lambdas/LambdaInStacktraceTest.java
@@ -44,7 +44,6 @@
"main(" + fileName + ")");
private final TestParameters parameters;
- private final boolean isAndroidOOrLater;
private final boolean isDalvik;
@Parameterized.Parameters(name = "{0}")
@@ -55,9 +54,6 @@
public LambdaInStacktraceTest(TestParameters parameters) {
this.parameters = parameters;
isDalvik = parameters.isDexRuntime() && parameters.getDexRuntimeVersion().isDalvik();
- isAndroidOOrLater =
- parameters.isDexRuntime()
- && parameters.getDexRuntimeVersion().isNewerThanOrEqual(Version.V8_1_0);
}
@Test
@@ -102,11 +98,6 @@
.getApiLevel()
.isGreaterThanOrEqualTo(apiLevelWithPcAsLineNumberSupport())) {
return s.contains("(NULL)");
- } else if (isAndroidOOrLater) {
- // On VMs with native support, no line info results in no source file printing.
- // TODO(b/260384637): Create debug info for such methods to avoid this.
- return s.equals("main(NULL)")
- || (!s.startsWith("main") && s.contains("(SourceFile)"));
} else {
return s.contains("(SourceFile)");
}
diff --git a/src/test/java/com/android/tools/r8/naming/IdentityMappingFileTest.java b/src/test/java/com/android/tools/r8/naming/IdentityMappingFileTest.java
index 6bffb78..b9a007e 100644
--- a/src/test/java/com/android/tools/r8/naming/IdentityMappingFileTest.java
+++ b/src/test/java/com/android/tools/r8/naming/IdentityMappingFileTest.java
@@ -18,6 +18,7 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.transformers.ClassFileTransformer.MethodPredicate;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.BooleanBox;
import com.android.tools.r8.utils.FileUtils;
@@ -52,6 +53,7 @@
assertThat(mapping, containsString("# pg_map_id: "));
assertThat(mapping, containsString("# pg_map_hash: SHA-256 "));
// Check the mapping is the identity, e.g., only comments and identity entries are defined.
+ int numberOfIdentityMappings = 0;
for (String line : StringUtils.splitLines(mapping)) {
if (line.startsWith("#")) {
continue;
@@ -61,18 +63,21 @@
String left = parts[0];
String right = parts[1];
if (left.equals(right.substring(0, right.length() - 1))) {
+ numberOfIdentityMappings++;
continue;
}
}
fail("Expected comment or identity, got: " + line);
}
+ // It may be ok to not actually include any identity mapping, but currently we do.
+ assertTrue(numberOfIdentityMappings > 0);
}
@Test
public void testTheTestBuilder() throws Exception {
String mapping =
testForR8(Backend.DEX)
- .addProgramClasses(Main.class)
+ .addProgramClassFileData(getMainWithoutLineTable())
.setMinApi(AndroidApiLevel.B)
.addKeepMainRule(Main.class)
.compile()
@@ -85,7 +90,7 @@
Path mappingPath = temp.newFolder().toPath().resolve("mapping.map");
R8.run(
R8Command.builder()
- .addProgramFiles(ToolHelper.getClassFileForTestClass(Main.class))
+ .addClassProgramData(getMainWithoutLineTable(), Origin.unknown())
.addProguardConfiguration(
ImmutableList.of(keepMainProguardConfiguration(Main.class)), Origin.unknown())
.addLibraryFiles(ToolHelper.getJava8RuntimeJar())
@@ -102,7 +107,7 @@
StringBuilder mappingContent = new StringBuilder();
R8.run(
R8Command.builder()
- .addProgramFiles(ToolHelper.getClassFileForTestClass(Main.class))
+ .addClassProgramData(getMainWithoutLineTable(), Origin.unknown())
.addProguardConfiguration(
ImmutableList.of(keepMainProguardConfiguration(Main.class)), Origin.unknown())
.addLibraryFiles(ToolHelper.getJava8RuntimeJar())
@@ -124,6 +129,10 @@
checkIdentityMappingContent(mappingContent.toString());
}
+ private byte[] getMainWithoutLineTable() throws Exception {
+ return transformer(Main.class).removeLineNumberTable(MethodPredicate.all()).transform();
+ }
+
// Compiling this program with a keep main will result in an identity mapping for the residual
// program. The (identity) mapping should still be created and emitted to the client.
static class Main {
diff --git a/src/test/java/com/android/tools/r8/reachabilitysensitive/ReachabilitySensitiveTest.java b/src/test/java/com/android/tools/r8/reachabilitysensitive/ReachabilitySensitiveTest.java
index 04efdac..e45c4ab 100644
--- a/src/test/java/com/android/tools/r8/reachabilitysensitive/ReachabilitySensitiveTest.java
+++ b/src/test/java/com/android/tools/r8/reachabilitysensitive/ReachabilitySensitiveTest.java
@@ -138,6 +138,7 @@
// as this is a release build.
assertTrue(
(code.getDebugInfo() == null)
+ || code.getDebugInfo().isPcBasedInfo()
|| Arrays.stream(code.getDebugInfo().asEventBasedInfo().events)
.allMatch(event -> !(event instanceof StartLocal)));
}
diff --git a/src/test/java/com/android/tools/r8/regress/b150400371/DebuginfoForInlineFrameRegressionTest.java b/src/test/java/com/android/tools/r8/regress/b150400371/DebuginfoForInlineFrameRegressionTest.java
index b0fdc84..940e463 100644
--- a/src/test/java/com/android/tools/r8/regress/b150400371/DebuginfoForInlineFrameRegressionTest.java
+++ b/src/test/java/com/android/tools/r8/regress/b150400371/DebuginfoForInlineFrameRegressionTest.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.transformers.ClassFileTransformer.MethodPredicate;
import com.android.tools.r8.utils.codeinspector.MethodSubject;
import com.google.common.collect.ImmutableSet;
import it.unimi.dsi.fastutil.ints.IntArraySet;
@@ -35,7 +36,9 @@
@Test
public void testNoLinesForNonInline() throws Exception {
testForR8(parameters.getBackend())
- .addInnerClasses(DebuginfoForInlineFrameRegressionTest.class)
+ .addProgramClassFileData(
+ transformer(InlineInto.class).removeLineNumberTable(MethodPredicate.all()).transform(),
+ transformer(InlineFrom.class).removeLineNumberTable(MethodPredicate.all()).transform())
.addKeepMainRule(InlineInto.class)
.addKeepRules("-keepparameternames")
.setMinApi(parameters)
diff --git a/src/test/java/com/android/tools/r8/shaking/attributes/KeepAttributesTest.java b/src/test/java/com/android/tools/r8/shaking/attributes/KeepAttributesTest.java
index 5314db5..32c535c 100644
--- a/src/test/java/com/android/tools/r8/shaking/attributes/KeepAttributesTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/attributes/KeepAttributesTest.java
@@ -6,7 +6,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.CompilationMode;
@@ -59,7 +58,8 @@
"-keep class ** { *; }"
);
MethodSubject mainMethod = compileRunAndGetMain(keepRules, CompilationMode.RELEASE);
- assertFalse(mainMethod.hasLineNumberTable());
+ // R8 now defaults to retain the line number table regardless of -keepattributes.
+ assertEquals(doesNotHavePcSupport(), mainMethod.hasLineNumberTable());
assertFalse(mainMethod.hasLocalVariableTable());
}
@@ -95,19 +95,12 @@
}
@Test
- public void keepLocalVariableTable() throws IOException, ExecutionException {
+ public void keepLocalVariableTable() throws Exception {
List<String> keepRules = ImmutableList.of(
"-keepattributes " + ProguardKeepAttributes.LOCAL_VARIABLE_TABLE
);
- // Compiling with a keep rule for locals but no line results in an error in R8.
- try {
- compileRunAndGetMain(keepRules, CompilationMode.RELEASE);
- } catch (CompilationFailedException e) {
- assertTrue(e.getCause().getMessage().contains(ProguardKeepAttributes.LOCAL_VARIABLE_TABLE));
- assertTrue(e.getCause().getMessage().contains(ProguardKeepAttributes.LINE_NUMBER_TABLE));
- return;
- }
- fail("Expected error");
+ // Compiling with a keep rule for locals but no line table no longer results in an error in R8.
+ compileRunAndGetMain(keepRules, CompilationMode.RELEASE);
}
private MethodSubject compileRunAndGetMain(List<String> keepRules, CompilationMode mode)
