Fix various places that do not compute max stack
Bug: 181185068
Change-Id: Ic8697018f87f24ec97b7d082e006428399e2b7c7
diff --git a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInitializerSynthesizedCode.java b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInitializerSynthesizedCode.java
index c919e63..d78d74d 100644
--- a/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInitializerSynthesizedCode.java
+++ b/src/main/java/com/android/tools/r8/horizontalclassmerging/ClassInitializerSynthesizedCode.java
@@ -59,11 +59,14 @@
}
public CfCode synthesizeCode(DexType originalHolder) {
+ // Building the instructions will adjust maxStack and maxLocals. Build it here before invoking
+ // the CfCode constructor to ensure that the value passed in is the updated values.
+ List<CfInstruction> instructions = buildInstructions();
return new CfCode(
originalHolder,
maxStack,
maxLocals,
- buildInstructions(),
+ instructions,
Collections.emptyList(),
Collections.emptyList());
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java b/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
index 7d07a7d..5cd32f2 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/BackportedMethodRewriter.java
@@ -78,7 +78,8 @@
CfInvoke invoke = instruction.asInvoke();
MethodProvider methodProvider = getMethodProviderOrNull(invoke.getMethod());
return methodProvider != null
- ? methodProvider.rewriteInvoke(invoke, appView, eventConsumer, methodProcessingContext)
+ ? methodProvider.rewriteInvoke(
+ invoke, appView, eventConsumer, methodProcessingContext, localStackAllocator)
: null;
}
@@ -1364,7 +1365,8 @@
CfInvoke invoke,
AppView<?> appView,
BackportedMethodDesugaringEventConsumer eventConsumer,
- MethodProcessingContext methodProcessingContext);
+ MethodProcessingContext methodProcessingContext,
+ LocalStackAllocator localStackAllocator);
}
private static final class InvokeRewriter extends MethodProvider {
@@ -1381,8 +1383,9 @@
CfInvoke invoke,
AppView<?> appView,
BackportedMethodDesugaringEventConsumer eventConsumer,
- MethodProcessingContext methodProcessingContext) {
- return rewriter.rewrite(invoke, appView.dexItemFactory());
+ MethodProcessingContext methodProcessingContext,
+ LocalStackAllocator localStackAllocator) {
+ return rewriter.rewrite(invoke, appView.dexItemFactory(), localStackAllocator);
}
}
@@ -1406,7 +1409,8 @@
CfInvoke invoke,
AppView<?> appView,
BackportedMethodDesugaringEventConsumer eventConsumer,
- MethodProcessingContext methodProcessingContext) {
+ MethodProcessingContext methodProcessingContext,
+ LocalStackAllocator localStackAllocator) {
ProgramMethod method = getSyntheticMethod(appView, methodProcessingContext);
eventConsumer.acceptBackportedMethod(method, methodProcessingContext.getMethodContext());
return ImmutableList.of(new CfInvoke(Opcodes.INVOKESTATIC, method.getReference(), false));
@@ -1465,7 +1469,8 @@
CfInstruction rewriteSingle(CfInvoke invoke, DexItemFactory factory);
// Convenience wrapper since most rewrites are to a single instruction.
- default Collection<CfInstruction> rewrite(CfInvoke invoke, DexItemFactory factory) {
+ default Collection<CfInstruction> rewrite(
+ CfInvoke invoke, DexItemFactory factory, LocalStackAllocator localStackAllocator) {
return ImmutableList.of(rewriteSingle(invoke, factory));
}
}
@@ -1478,6 +1483,7 @@
}
@Override
- public abstract Collection<CfInstruction> rewrite(CfInvoke invoke, DexItemFactory factory);
+ public abstract Collection<CfInstruction> rewrite(
+ CfInvoke invoke, DexItemFactory factory, LocalStackAllocator localStackAllocator);
}
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/LambdaMainMethodSourceCode.java b/src/main/java/com/android/tools/r8/ir/desugar/LambdaMainMethodSourceCode.java
index cbab081..6975252 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/LambdaMainMethodSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/LambdaMainMethodSourceCode.java
@@ -361,9 +361,18 @@
Builder<CfInstruction> instructions,
DexItemFactory factory) {
internalAdjustType(fromType, toType, returnType, instructions, factory);
+ if (fromType == toType) {
+ return ValueType.fromDexType(fromType).requiredRegisters();
+ }
+ // Account for the potential unboxing of a wide type.
+ DexType fromTypeAsPrimitive = factory.getPrimitiveFromBoxed(fromType);
return Math.max(
ValueType.fromDexType(fromType).requiredRegisters(),
- ValueType.fromDexType(toType).requiredRegisters());
+ Math.max(
+ fromTypeAsPrimitive == null
+ ? 0
+ : ValueType.fromDexType(fromTypeAsPrimitive).requiredRegisters(),
+ ValueType.fromDexType(toType).requiredRegisters()));
}
private static void internalAdjustType(
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/NonEmptyCfInstructionDesugaringCollection.java b/src/main/java/com/android/tools/r8/ir/desugar/NonEmptyCfInstructionDesugaringCollection.java
index 1865244..3a7a195 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/NonEmptyCfInstructionDesugaringCollection.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/NonEmptyCfInstructionDesugaringCollection.java
@@ -133,12 +133,14 @@
method,
methodProcessingContext);
if (replacement != null) {
- // Record if we increased the max number of locals for the method, and reset the
- // next temporary locals register.
+ // Record if we increased the max number of locals and stack height for the method,
+ // and reset the next temporary locals register.
maxLocalsForCode.setMax(maxLocalsForInstruction.getAndSet(cfCode.getMaxLocals()));
+ maxStackForCode.setMax(maxStackForInstruction.getAndSet(cfCode.getMaxStack()));
} else {
// The next temporary locals register should be unchanged.
assert maxLocalsForInstruction.get() == cfCode.getMaxLocals();
+ assert maxStackForInstruction.get() == cfCode.getMaxStack();
}
return replacement;
},
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/backports/NumericMethodRewrites.java b/src/main/java/com/android/tools/r8/ir/desugar/backports/NumericMethodRewrites.java
index 51949c9..8b928aa 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/backports/NumericMethodRewrites.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/backports/NumericMethodRewrites.java
@@ -8,6 +8,7 @@
import com.android.tools.r8.ir.code.NumericType;
import com.android.tools.r8.ir.desugar.BackportedMethodRewriter.FullMethodInvokeRewriter;
import com.android.tools.r8.ir.desugar.BackportedMethodRewriter.MethodInvokeRewriter;
+import com.android.tools.r8.ir.desugar.LocalStackAllocator;
import java.util.Collection;
import java.util.Collections;
import org.objectweb.asm.Opcodes;
@@ -34,7 +35,8 @@
public static MethodInvokeRewriter rewriteAsIdentity() {
return new FullMethodInvokeRewriter() {
@Override
- public Collection<CfInstruction> rewrite(CfInvoke invoke, DexItemFactory factory) {
+ public Collection<CfInstruction> rewrite(
+ CfInvoke invoke, DexItemFactory factory, LocalStackAllocator localStackAllocator) {
// The invoke consumes the stack value and pushes another assumed to be the same.
return Collections.emptyList();
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/backports/ObjectsMethodRewrites.java b/src/main/java/com/android/tools/r8/ir/desugar/backports/ObjectsMethodRewrites.java
index f8ab4a7..20998db 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/backports/ObjectsMethodRewrites.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/backports/ObjectsMethodRewrites.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.desugar.BackportedMethodRewriter.FullMethodInvokeRewriter;
import com.android.tools.r8.ir.desugar.BackportedMethodRewriter.MethodInvokeRewriter;
+import com.android.tools.r8.ir.desugar.LocalStackAllocator;
import com.google.common.collect.ImmutableList;
import java.util.Collection;
import org.objectweb.asm.Opcodes;
@@ -32,8 +33,10 @@
return new FullMethodInvokeRewriter() {
@Override
- public Collection<CfInstruction> rewrite(CfInvoke invoke, DexItemFactory factory) {
+ public Collection<CfInstruction> rewrite(
+ CfInvoke invoke, DexItemFactory factory, LocalStackAllocator localStackAllocator) {
// requireNonNull returns the operand, so dup top-of-stack, do getClass and pop the class.
+ localStackAllocator.allocateLocalStack(1);
return ImmutableList.of(
new CfStackInstruction(Opcode.Dup),
new CfInvoke(Opcodes.INVOKEVIRTUAL, factory.objectMembers.getClass, false),
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/FieldAccessorBuilder.java b/src/main/java/com/android/tools/r8/ir/synthetic/FieldAccessorBuilder.java
index b02544d..1f3c6d0 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/FieldAccessorBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/FieldAccessorBuilder.java
@@ -89,6 +89,7 @@
// Load the argument.
ValueType fieldType = ValueType.fromDexType(field.getType());
instructions.add(new CfLoad(fieldType, maxLocals));
+ maxStack += fieldType.requiredRegisters();
maxLocals += fieldType.requiredRegisters();
}
@@ -102,6 +103,7 @@
instructions.add(new CfReturnVoid());
} else {
ValueType fieldType = ValueType.fromDexType(field.getType());
+ maxStack = Math.max(fieldType.requiredRegisters(), maxStack);
instructions.add(new CfReturn(fieldType));
}
diff --git a/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodBuilder.java b/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodBuilder.java
index 84acf70..0c6b359 100644
--- a/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/synthetic/ForwardMethodBuilder.java
@@ -182,6 +182,13 @@
maybeInsertArgumentCast(i, parameter, instructions);
}
instructions.add(new CfInvoke(getInvokeOpcode(), targetMethod, isInterface));
+ if (!targetMethod.getReturnType().isVoidType()) {
+ // If the return type is not void, it will push a value on the stack. We subtract the
+ // arguments pushed by the invoke to see if bumping the stack height is necessary.
+ maxStack =
+ Math.max(
+ maxStack, ValueType.fromDexType(targetMethod.getReturnType()).requiredRegisters());
+ }
if (isSourceReturnVoid()) {
assert !isConstructorDelegate;
instructions.add(new CfReturnVoid());
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 f2587df..c9ee155 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1039,7 +1039,7 @@
}
// Currently the filter is simple string equality on the qualified name.
String qualifiedName = method.qualifiedName();
- return methodsFilter.indexOf(qualifiedName) >= 0;
+ return methodsFilter.contains(qualifiedName);
}
public boolean methodMatchesLogArgumentsFilter(DexEncodedMethod method) {
@@ -1049,7 +1049,7 @@
}
// Currently the filter is simple string equality on the qualified name.
String qualifiedName = method.qualifiedName();
- return logArgumentsFilter.indexOf(qualifiedName) >= 0;
+ return logArgumentsFilter.contains(qualifiedName);
}
public enum PackageObfuscationMode {
diff --git a/src/test/java/com/android/tools/r8/shaking/InstantiatedLambdaReceiverTest.java b/src/test/java/com/android/tools/r8/shaking/InstantiatedLambdaReceiverTest.java
index 16d9503..0a1f760 100644
--- a/src/test/java/com/android/tools/r8/shaking/InstantiatedLambdaReceiverTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/InstantiatedLambdaReceiverTest.java
@@ -7,7 +7,8 @@
import static org.junit.Assume.assumeTrue;
import com.android.tools.r8.TestBase;
-import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -16,33 +17,34 @@
@RunWith(Parameterized.class)
public class InstantiatedLambdaReceiverTest extends TestBase {
- private Backend backend;
-
private static final String expectedOutput = "In C.m()";
+ private final TestParameters parameters;
- @Parameters(name = "Backend: {0}")
- public static Backend[] data() {
- return ToolHelper.getBackends();
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withDexRuntimes().withAllApiLevels().build();
}
- public InstantiatedLambdaReceiverTest(Backend backend) {
- this.backend = backend;
+ public InstantiatedLambdaReceiverTest(TestParameters parameters) {
+ this.parameters = parameters;
}
@Test
public void jvmTest() throws Exception {
- assumeTrue(
- "JVM test independent of Art version - only run when testing on latest",
- ToolHelper.getDexVm().getVersion().isLatest());
- testForJvm().addTestClasspath().run(TestClass.class).assertSuccessWithOutput(expectedOutput);
+ assumeTrue(parameters.isCfRuntime());
+ testForJvm()
+ .addTestClasspath()
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccessWithOutput(expectedOutput);
}
@Test
- public void dexTest() throws Exception {
- testForR8(backend)
+ public void r8Test() throws Exception {
+ testForR8(parameters.getBackend())
.addInnerClasses(InstantiatedLambdaReceiverTest.class)
.addKeepMainRule(TestClass.class)
- .run(TestClass.class)
+ .setMinApi(parameters.getApiLevel())
+ .run(parameters.getRuntime(), TestClass.class)
.assertSuccessWithOutput(expectedOutput);
}
