Revert "Extend catch-all range for monitors to include exceptional monitor-exit"
This reverts commit 01285a255ff7849dfbe3f8fce6ec9d66fd0e55e9.
Reason for revert: AssertionError building :test:assembleR8LibWithRelocatedDeps
Change-Id: I274357ca8dca53617a49c3828dc11fbe53310bcc
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
index 982ef6a..48241f6 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CfSourceCode.java
@@ -3,8 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.conversion;
-import static com.android.tools.r8.ir.conversion.IRBuilder.EXCEPTIONAL_SYNC_EXIT_MONITOR_EXIT_OFFSET;
-import static com.android.tools.r8.ir.conversion.IRBuilder.EXCEPTIONAL_SYNC_EXIT_THROW_OFFSET;
import static it.unimi.dsi.fastutil.ints.Int2ReferenceSortedMaps.emptyMap;
import com.android.tools.r8.cf.code.CfFrame;
@@ -61,26 +59,12 @@
public class CfSourceCode implements SourceCode {
- private enum GeneratedMethodSynchronizationBlock {
- METHOD_ENTER,
- METHOD_EXIT,
- EXCEPTIONAL_MONITOR_EXIT,
- EXCEPTIONAL_THROW,
- NONE;
-
- static GeneratedMethodSynchronizationBlock fromOffset(int offset) {
- assert isExceptionalExitForMethodSynchronization(offset);
- return isExceptionalMonitorExitForMethodSynchronization(offset)
- ? EXCEPTIONAL_MONITOR_EXIT
- : EXCEPTIONAL_THROW;
- }
- }
-
private BlockInfo currentBlockInfo;
private boolean hasExitingInstruction = false;
+ private static final int EXCEPTIONAL_SYNC_EXIT_OFFSET = -2;
private final boolean needsGeneratedMethodSynchronization;
- private GeneratedMethodSynchronizationBlock currentlyGeneratingMethodSynchronization;
+ private boolean currentlyGeneratingMethodSynchronization = false;
private Monitor monitorEnter = null;
private static class TryHandlerList {
@@ -145,7 +129,7 @@
}
if (needsGeneratedMethodSynchronization && !seenCatchAll) {
guards.add(factory.throwableType);
- offsets.add(EXCEPTIONAL_SYNC_EXIT_MONITOR_EXIT_OFFSET);
+ offsets.add(EXCEPTIONAL_SYNC_EXIT_OFFSET);
}
return new TryHandlerList(startOffset, endOffset, guards, offsets);
}
@@ -255,18 +239,6 @@
&& getMethod().isSynchronized();
}
- void setCurrentlyGeneratingMethodSynchronization(
- GeneratedMethodSynchronizationBlock currentlyGeneratingMethodSynchronization) {
- assert this.currentlyGeneratingMethodSynchronization
- == GeneratedMethodSynchronizationBlock.NONE;
- this.currentlyGeneratingMethodSynchronization = currentlyGeneratingMethodSynchronization;
- }
-
- void unsetCurrentlyGeneratingMethodSynchronization() {
- assert currentlyGeneratingMethodSynchronization != GeneratedMethodSynchronizationBlock.NONE;
- currentlyGeneratingMethodSynchronization = GeneratedMethodSynchronizationBlock.NONE;
- }
-
private DexEncodedMethod getMethod() {
return method.getDefinition();
}
@@ -300,6 +272,7 @@
@Override
public int traceInstruction(int instructionIndex, IRBuilder builder) {
CfInstruction instruction = code.getInstructions().get(instructionIndex);
+ AppView<?> appView = builder.appView;
assert appView.options().isGeneratingClassFiles()
== internalOutputMode.isGeneratingClassFiles();
if (instruction.canThrow()) {
@@ -420,30 +393,16 @@
}
private boolean isCurrentlyGeneratingMethodSynchronization() {
- return currentlyGeneratingMethodSynchronization != GeneratedMethodSynchronizationBlock.NONE;
+ return currentlyGeneratingMethodSynchronization;
}
- private boolean isCurrentlyGeneratingExceptionalMonitorExitForMethodSynchronization() {
- return currentlyGeneratingMethodSynchronization
- == GeneratedMethodSynchronizationBlock.EXCEPTIONAL_MONITOR_EXIT;
- }
-
- public static boolean isExceptionalMonitorExitForMethodSynchronization(int instructionIndex) {
- return instructionIndex == EXCEPTIONAL_SYNC_EXIT_MONITOR_EXIT_OFFSET;
- }
-
- public static boolean isExceptionalThrowForMethodSynchronization(int instructionIndex) {
- return instructionIndex == EXCEPTIONAL_SYNC_EXIT_THROW_OFFSET;
- }
-
- public static boolean isExceptionalExitForMethodSynchronization(int instructionIndex) {
- return isExceptionalMonitorExitForMethodSynchronization(instructionIndex)
- || isExceptionalThrowForMethodSynchronization(instructionIndex);
+ private boolean isExceptionalExitForMethodSynchronization(int instructionIndex) {
+ return instructionIndex == EXCEPTIONAL_SYNC_EXIT_OFFSET;
}
private void buildMethodEnterSynchronization(IRBuilder builder) {
assert needsGeneratedMethodSynchronization;
- setCurrentlyGeneratingMethodSynchronization(GeneratedMethodSynchronizationBlock.METHOD_ENTER);
+ currentlyGeneratingMethodSynchronization = true;
DexType type = method.getHolderType();
int monitorRegister;
if (getMethod().isStatic()) {
@@ -455,30 +414,24 @@
}
// Build the monitor enter and save it for when generating exits later.
monitorEnter = builder.addMonitor(MonitorType.ENTER, monitorRegister);
- unsetCurrentlyGeneratingMethodSynchronization();
+ currentlyGeneratingMethodSynchronization = false;
}
- private void buildExceptionalExitForMethodSynchronization(
- IRBuilder builder, int instructionIndex) {
+ private void buildExceptionalExitMethodSynchronization(IRBuilder builder) {
assert needsGeneratedMethodSynchronization;
- setCurrentlyGeneratingMethodSynchronization(
- GeneratedMethodSynchronizationBlock.fromOffset(instructionIndex));
- state.setPosition(getCanonicalDebugPositionAtOffset(instructionIndex));
- if (isExceptionalMonitorExitForMethodSynchronization(instructionIndex)) {
- builder.add(new Monitor(MonitorType.EXIT, monitorEnter.object()));
- builder.addGoto(EXCEPTIONAL_SYNC_EXIT_THROW_OFFSET);
- } else {
- builder.addThrow(getMoveExceptionRegister(0));
- }
- unsetCurrentlyGeneratingMethodSynchronization();
+ currentlyGeneratingMethodSynchronization = true;
+ state.setPosition(getCanonicalDebugPositionAtOffset(EXCEPTIONAL_SYNC_EXIT_OFFSET));
+ builder.add(new Monitor(MonitorType.EXIT, monitorEnter.inValues().get(0)));
+ builder.addThrow(getMoveExceptionRegister(0));
+ currentlyGeneratingMethodSynchronization = false;
}
@Override
public void buildPostlude(IRBuilder builder) {
if (needsGeneratedMethodSynchronization) {
- setCurrentlyGeneratingMethodSynchronization(GeneratedMethodSynchronizationBlock.METHOD_EXIT);
- builder.add(new Monitor(MonitorType.EXIT, monitorEnter.object()));
- unsetCurrentlyGeneratingMethodSynchronization();
+ currentlyGeneratingMethodSynchronization = true;
+ builder.add(new Monitor(MonitorType.EXIT, monitorEnter.inValues().get(0)));
+ currentlyGeneratingMethodSynchronization = false;
}
}
@@ -531,7 +484,7 @@
public void buildInstruction(
IRBuilder builder, int instructionIndex, boolean firstBlockInstruction) {
if (isExceptionalExitForMethodSynchronization(instructionIndex)) {
- buildExceptionalExitForMethodSynchronization(builder, instructionIndex);
+ buildExceptionalExitMethodSynchronization(builder);
return;
}
CfInstruction instruction = code.getInstructions().get(instructionIndex);
@@ -554,7 +507,7 @@
if (instruction.canThrow()) {
Snapshot exceptionTransfer =
- state.getSnapshot().exceptionTransfer(appView.dexItemFactory().throwableType);
+ state.getSnapshot().exceptionTransfer(builder.appView.dexItemFactory().throwableType);
for (int target : currentBlockInfo.exceptionalSuccessors) {
recordStateForTarget(target, exceptionTransfer);
}
@@ -867,19 +820,12 @@
if (inPrelude) {
return null;
}
- TryHandlerList tryHandlers;
if (isCurrentlyGeneratingMethodSynchronization()) {
- if (!isCurrentlyGeneratingExceptionalMonitorExitForMethodSynchronization()) {
- return null;
- }
- // Ensure that the exceptional monitor-exit instruction is guarded by a catch-all handler that
- // repeats the monitor-exit.
- tryHandlers =
- getTryHandlers(EXCEPTIONAL_SYNC_EXIT_MONITOR_EXIT_OFFSET, appView.dexItemFactory());
- } else {
- tryHandlers =
- getTryHandlers(instructionOffset(currentInstructionIndex), appView.dexItemFactory());
+ return null;
}
+ TryHandlerList tryHandlers =
+ getTryHandlers(
+ instructionOffset(currentInstructionIndex), builder.appView.dexItemFactory());
if (tryHandlers.isEmpty()) {
return null;
}
@@ -911,7 +857,7 @@
@Override
public Position getCanonicalDebugPositionAtOffset(int offset) {
- if (isExceptionalExitForMethodSynchronization(offset)) {
+ if (offset == EXCEPTIONAL_SYNC_EXIT_OFFSET) {
return canonicalPositions.getExceptionalExitPosition(
appView.options().debug,
() ->
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
index 2c84747..8d03811 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
@@ -14,9 +14,6 @@
import static com.android.tools.r8.ir.analysis.type.TypeElement.getNull;
import static com.android.tools.r8.ir.analysis.type.TypeElement.getSingle;
import static com.android.tools.r8.ir.analysis.type.TypeElement.getWide;
-import static com.android.tools.r8.ir.conversion.CfSourceCode.isExceptionalExitForMethodSynchronization;
-import static com.android.tools.r8.ir.conversion.CfSourceCode.isExceptionalMonitorExitForMethodSynchronization;
-import static com.android.tools.r8.ir.conversion.CfSourceCode.isExceptionalThrowForMethodSynchronization;
import com.android.tools.r8.cf.CfVersion;
import com.android.tools.r8.errors.CompilationError;
@@ -159,14 +156,6 @@
public static final int INITIAL_BLOCK_OFFSET = -1;
- // The (synthetic) offset of the monitor-exit instruction on the exceptional exit, when desugaring
- // declared synchronized methods.
- public static final int EXCEPTIONAL_SYNC_EXIT_MONITOR_EXIT_OFFSET = -2;
-
- // The (synthetic) offset of the throw instruction that follows the monitor-exit instruction on
- // the exceptional exit, when desugaring declared synchronized methods.
- public static final int EXCEPTIONAL_SYNC_EXIT_THROW_OFFSET = -3;
-
private static TypeElement fromMemberType(MemberType type) {
switch (type) {
case BOOLEAN_OR_BYTE:
@@ -389,8 +378,6 @@
@SuppressWarnings("JdkObsolete")
private final Queue<Integer> traceBlocksWorklist = new LinkedList<>();
- private boolean processedExceptionalMonitorExitForMethodSynchronization;
-
// Bitmap to ensure we don't process an instruction more than once.
private boolean[] processedInstructions = null;
@@ -644,9 +631,6 @@
traceBlocksWorklist.add(0);
while (!traceBlocksWorklist.isEmpty()) {
int startOfBlockOffset = traceBlocksWorklist.remove();
- if (handleExceptionalExitForMethodSynchronization(startOfBlockOffset)) {
- continue;
- }
int startOfBlockIndex = source.instructionIndex(startOfBlockOffset);
// Check that the block has not been processed after being added.
if (isIndexProcessed(startOfBlockIndex)) {
@@ -2486,24 +2470,14 @@
// Ensure there is a block starting at offset and add it to the work-list if it needs processing.
private BlockInfo ensureBlock(int offset) {
// We don't enqueue negative targets (these are special blocks, eg, an argument prelude).
- if (!isOffsetProcessed(offset)) {
+ if (offset >= 0 && !isOffsetProcessed(offset)) {
traceBlocksWorklist.add(offset);
}
return ensureBlockWithoutEnqueuing(offset);
}
private boolean isOffsetProcessed(int offset) {
- if (offset >= 0) {
- return isIndexProcessed(source.instructionIndex(offset));
- }
- if (isExceptionalMonitorExitForMethodSynchronization(offset)) {
- return isExceptionalMonitorExitForMethodSynchronizationProcessed();
- } else {
- // We never need to process this block since it has no successors and its predecessor is set
- // up when processing the monitor-exit block.
- assert isExceptionalThrowForMethodSynchronization(offset);
- return true;
- }
+ return isIndexProcessed(source.instructionIndex(offset));
}
private boolean isIndexProcessed(int index) {
@@ -2524,34 +2498,6 @@
processedSubroutineInstructions.add(index);
}
- private boolean handleExceptionalExitForMethodSynchronization(int startOfBlockOffset) {
- if (!isExceptionalExitForMethodSynchronization(startOfBlockOffset)) {
- return false;
- }
- // We never need to process the throw block. We therefore treat it as always processed, meaning
- // it should never be traced here.
- assert isExceptionalMonitorExitForMethodSynchronization(startOfBlockOffset);
- if (markExceptionalMonitorExitForMethodSynchronizationProcessed()) {
- ensureNormalSuccessorBlock(
- EXCEPTIONAL_SYNC_EXIT_MONITOR_EXIT_OFFSET, EXCEPTIONAL_SYNC_EXIT_THROW_OFFSET);
- ensureExceptionalSuccessorBlock(
- EXCEPTIONAL_SYNC_EXIT_MONITOR_EXIT_OFFSET, EXCEPTIONAL_SYNC_EXIT_MONITOR_EXIT_OFFSET);
- }
- return true;
- }
-
- private boolean isExceptionalMonitorExitForMethodSynchronizationProcessed() {
- return processedExceptionalMonitorExitForMethodSynchronization;
- }
-
- private boolean markExceptionalMonitorExitForMethodSynchronizationProcessed() {
- if (isExceptionalMonitorExitForMethodSynchronizationProcessed()) {
- return false;
- }
- processedExceptionalMonitorExitForMethodSynchronization = true;
- return true;
- }
-
private void ensureSubroutineProcessedInstructions() {
if (processedSubroutineInstructions == null) {
processedSubroutineInstructions = new HashSet<>();
@@ -2786,3 +2732,4 @@
return builder.toString();
}
}
+
diff --git a/src/test/java/com/android/tools/r8/workaround/ExpectedToBeWithinCatchAllAfterInliningWithSoftVerificationErrorTest.java b/src/test/java/com/android/tools/r8/workaround/ExpectedToBeWithinCatchAllAfterInliningWithSoftVerificationErrorTest.java
index 316af73..90de297 100644
--- a/src/test/java/com/android/tools/r8/workaround/ExpectedToBeWithinCatchAllAfterInliningWithSoftVerificationErrorTest.java
+++ b/src/test/java/com/android/tools/r8/workaround/ExpectedToBeWithinCatchAllAfterInliningWithSoftVerificationErrorTest.java
@@ -6,6 +6,7 @@
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ToolHelper.DexVm.Version;
import java.util.Objects;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -36,7 +37,11 @@
.setMinApi(parameters)
.compile()
.run(parameters.getRuntime(), Main.class)
- .assertSuccessWithOutputLines("Hello, world!");
+ .applyIf(
+ parameters.isCfRuntime()
+ || !parameters.getDexRuntimeVersion().isEqualTo(Version.V5_1_1),
+ runResult -> runResult.assertSuccessWithOutputLines("Hello, world!"),
+ runResult -> runResult.assertFailureWithErrorThatThrows(VerifyError.class));
}
static class Main {
