Extend catch-all range for monitors to include exceptional monitor-exit
Fixes: b/348785664
Change-Id: I39cc17d08b97274fcd0324511b2780154eacad9d
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 48241f6..982ef6a 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,6 +3,8 @@
// 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;
@@ -59,12 +61,26 @@
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 boolean currentlyGeneratingMethodSynchronization = false;
+ private GeneratedMethodSynchronizationBlock currentlyGeneratingMethodSynchronization;
private Monitor monitorEnter = null;
private static class TryHandlerList {
@@ -129,7 +145,7 @@
}
if (needsGeneratedMethodSynchronization && !seenCatchAll) {
guards.add(factory.throwableType);
- offsets.add(EXCEPTIONAL_SYNC_EXIT_OFFSET);
+ offsets.add(EXCEPTIONAL_SYNC_EXIT_MONITOR_EXIT_OFFSET);
}
return new TryHandlerList(startOffset, endOffset, guards, offsets);
}
@@ -239,6 +255,18 @@
&& 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();
}
@@ -272,7 +300,6 @@
@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()) {
@@ -393,16 +420,30 @@
}
private boolean isCurrentlyGeneratingMethodSynchronization() {
- return currentlyGeneratingMethodSynchronization;
+ return currentlyGeneratingMethodSynchronization != GeneratedMethodSynchronizationBlock.NONE;
}
- private boolean isExceptionalExitForMethodSynchronization(int instructionIndex) {
- return instructionIndex == EXCEPTIONAL_SYNC_EXIT_OFFSET;
+ 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 void buildMethodEnterSynchronization(IRBuilder builder) {
assert needsGeneratedMethodSynchronization;
- currentlyGeneratingMethodSynchronization = true;
+ setCurrentlyGeneratingMethodSynchronization(GeneratedMethodSynchronizationBlock.METHOD_ENTER);
DexType type = method.getHolderType();
int monitorRegister;
if (getMethod().isStatic()) {
@@ -414,24 +455,30 @@
}
// Build the monitor enter and save it for when generating exits later.
monitorEnter = builder.addMonitor(MonitorType.ENTER, monitorRegister);
- currentlyGeneratingMethodSynchronization = false;
+ unsetCurrentlyGeneratingMethodSynchronization();
}
- private void buildExceptionalExitMethodSynchronization(IRBuilder builder) {
+ private void buildExceptionalExitForMethodSynchronization(
+ IRBuilder builder, int instructionIndex) {
assert needsGeneratedMethodSynchronization;
- 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;
+ 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();
}
@Override
public void buildPostlude(IRBuilder builder) {
if (needsGeneratedMethodSynchronization) {
- currentlyGeneratingMethodSynchronization = true;
- builder.add(new Monitor(MonitorType.EXIT, monitorEnter.inValues().get(0)));
- currentlyGeneratingMethodSynchronization = false;
+ setCurrentlyGeneratingMethodSynchronization(GeneratedMethodSynchronizationBlock.METHOD_EXIT);
+ builder.add(new Monitor(MonitorType.EXIT, monitorEnter.object()));
+ unsetCurrentlyGeneratingMethodSynchronization();
}
}
@@ -484,7 +531,7 @@
public void buildInstruction(
IRBuilder builder, int instructionIndex, boolean firstBlockInstruction) {
if (isExceptionalExitForMethodSynchronization(instructionIndex)) {
- buildExceptionalExitMethodSynchronization(builder);
+ buildExceptionalExitForMethodSynchronization(builder, instructionIndex);
return;
}
CfInstruction instruction = code.getInstructions().get(instructionIndex);
@@ -507,7 +554,7 @@
if (instruction.canThrow()) {
Snapshot exceptionTransfer =
- state.getSnapshot().exceptionTransfer(builder.appView.dexItemFactory().throwableType);
+ state.getSnapshot().exceptionTransfer(appView.dexItemFactory().throwableType);
for (int target : currentBlockInfo.exceptionalSuccessors) {
recordStateForTarget(target, exceptionTransfer);
}
@@ -820,12 +867,19 @@
if (inPrelude) {
return null;
}
+ TryHandlerList tryHandlers;
if (isCurrentlyGeneratingMethodSynchronization()) {
- return null;
+ 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());
}
- TryHandlerList tryHandlers =
- getTryHandlers(
- instructionOffset(currentInstructionIndex), builder.appView.dexItemFactory());
if (tryHandlers.isEmpty()) {
return null;
}
@@ -857,7 +911,7 @@
@Override
public Position getCanonicalDebugPositionAtOffset(int offset) {
- if (offset == EXCEPTIONAL_SYNC_EXIT_OFFSET) {
+ if (isExceptionalExitForMethodSynchronization(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 8d03811..2c84747 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,6 +14,9 @@
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;
@@ -156,6 +159,14 @@
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:
@@ -378,6 +389,8 @@
@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;
@@ -631,6 +644,9 @@
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)) {
@@ -2470,14 +2486,24 @@
// 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 (offset >= 0 && !isOffsetProcessed(offset)) {
+ if (!isOffsetProcessed(offset)) {
traceBlocksWorklist.add(offset);
}
return ensureBlockWithoutEnqueuing(offset);
}
private boolean isOffsetProcessed(int offset) {
- return isIndexProcessed(source.instructionIndex(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;
+ }
}
private boolean isIndexProcessed(int index) {
@@ -2498,6 +2524,34 @@
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<>();
@@ -2732,4 +2786,3 @@
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 90de297..316af73 100644
--- a/src/test/java/com/android/tools/r8/workaround/ExpectedToBeWithinCatchAllAfterInliningWithSoftVerificationErrorTest.java
+++ b/src/test/java/com/android/tools/r8/workaround/ExpectedToBeWithinCatchAllAfterInliningWithSoftVerificationErrorTest.java
@@ -6,7 +6,6 @@
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;
@@ -37,11 +36,7 @@
.setMinApi(parameters)
.compile()
.run(parameters.getRuntime(), Main.class)
- .applyIf(
- parameters.isCfRuntime()
- || !parameters.getDexRuntimeVersion().isEqualTo(Version.V5_1_1),
- runResult -> runResult.assertSuccessWithOutputLines("Hello, world!"),
- runResult -> runResult.assertFailureWithErrorThatThrows(VerifyError.class));
+ .assertSuccessWithOutputLines("Hello, world!");
}
static class Main {
