Fix a compiler bug related to a catch-less try-finally statement.
Ensure a dead basic block produced in this case is properly
removed.
Change-Id: I7c88e26aaa6c6378892f7c7c299494fa42312db2
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index 7584f1b..ba4dccf 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -30,6 +30,36 @@
VisitBlockForBackEdges(entry_block_, visited, &visiting);
}
+static void RemoveAsUser(HInstruction* instruction) {
+ for (size_t i = 0; i < instruction->InputCount(); i++) {
+ instruction->InputAt(i)->RemoveUser(instruction, i);
+ }
+
+ HEnvironment* environment = instruction->GetEnvironment();
+ if (environment != nullptr) {
+ for (size_t i = 0, e = environment->Size(); i < e; ++i) {
+ HInstruction* vreg = environment->GetInstructionAt(i);
+ if (vreg != nullptr) {
+ vreg->RemoveEnvironmentUser(environment, i);
+ }
+ }
+ }
+}
+
+void HGraph::RemoveInstructionsAsUsersFromDeadBlocks(const ArenaBitVector& visited) const {
+ for (size_t i = 0; i < blocks_.Size(); ++i) {
+ if (!visited.IsBitSet(i)) {
+ HBasicBlock* block = blocks_.Get(i);
+ for (HInstructionIterator it(block->GetPhis()); !it.Done(); it.Advance()) {
+ RemoveAsUser(it.Current());
+ }
+ for (HInstructionIterator it(block->GetInstructions()); !it.Done(); it.Advance()) {
+ RemoveAsUser(it.Current());
+ }
+ }
+ }
+}
+
void HGraph::RemoveDeadBlocks(const ArenaBitVector& visited) const {
for (size_t i = 0; i < blocks_.Size(); ++i) {
if (!visited.IsBitSet(i)) {
@@ -72,16 +102,21 @@
// (1) Find the back edges in the graph doing a DFS traversal.
FindBackEdges(&visited);
- // (2) Remove blocks not visited during the initial DFS.
- // Step (3) requires dead blocks to be removed from the
+ // (2) Remove instructions and phis from blocks not visited during
+ // the initial DFS as users from other instructions, so that
+ // users can be safely removed before uses later.
+ RemoveInstructionsAsUsersFromDeadBlocks(visited);
+
+ // (3) Remove blocks not visited during the initial DFS.
+ // Step (4) requires dead blocks to be removed from the
// predecessors list of live blocks.
RemoveDeadBlocks(visited);
- // (3) Simplify the CFG now, so that we don't need to recompute
+ // (4) Simplify the CFG now, so that we don't need to recompute
// dominators and the reverse post order.
SimplifyCFG();
- // (4) Compute the immediate dominator of each block. We visit
+ // (5) Compute the immediate dominator of each block. We visit
// the successors of a block only when all its forward branches
// have been processed.
GrowableArray<size_t> visits(arena_, blocks_.Size());
@@ -391,19 +426,7 @@
instruction->SetBlock(nullptr);
instruction_list->RemoveInstruction(instruction);
- for (size_t i = 0; i < instruction->InputCount(); i++) {
- instruction->InputAt(i)->RemoveUser(instruction, i);
- }
-
- HEnvironment* environment = instruction->GetEnvironment();
- if (environment != nullptr) {
- for (size_t i = 0, e = environment->Size(); i < e; ++i) {
- HInstruction* vreg = environment->GetInstructionAt(i);
- if (vreg != nullptr) {
- vreg->RemoveEnvironmentUser(environment, i);
- }
- }
- }
+ RemoveAsUser(instruction);
}
void HBasicBlock::RemoveInstruction(HInstruction* instruction) {
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 9d0b4a9..0054d25 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -173,6 +173,7 @@
void VisitBlockForBackEdges(HBasicBlock* block,
ArenaBitVector* visited,
ArenaBitVector* visiting);
+ void RemoveInstructionsAsUsersFromDeadBlocks(const ArenaBitVector& visited) const;
void RemoveDeadBlocks(const ArenaBitVector& visited) const;
ArenaAllocator* const arena_;
diff --git a/test/435-try-finally-without-catch/expected.txt b/test/435-try-finally-without-catch/expected.txt
new file mode 100644
index 0000000..8a67802
--- /dev/null
+++ b/test/435-try-finally-without-catch/expected.txt
@@ -0,0 +1,3 @@
+In finally
+In finally
+In finally
diff --git a/test/435-try-finally-without-catch/info.txt b/test/435-try-finally-without-catch/info.txt
new file mode 100644
index 0000000..46217c5
--- /dev/null
+++ b/test/435-try-finally-without-catch/info.txt
@@ -0,0 +1,26 @@
+Exercise a method containing a `try' statement with several
+instructions with a `finally' clause but without any `catch' block,
+enclosed in a loop.
+
+When dx processes an integer division (or modulo) enclosing a `try'
+block and whose result is assigned to a local value, it is smart
+enough not to emit a `div-int' (or `rem-int') instruction when the
+divisor is non-null, as it wouldn't be used. However, dx is not
+that clever regarding exception handling: if the divisor is known to
+be non-null at compile-time (as is the case in this test), it will
+still emit a block with the exception catching and rethrowing
+mechanism, even if it is not used.
+
+This used to be a problem for a `try' block followed by a `finally'
+clause but with no `catch' block: in that case, the generated Dex code
+item would list zero catch block for this method (see
+art::CodeItem::tries_size_) and the optimizing compiler would have no
+clue that it contains a `try' statement, which it cannot optimize
+(yet). With no hint that this method might contain one (or several)
+special block(s) related to `catch'-less `try' statement(s), the
+optimizing compiler considered this (these) as dead block(s) and
+improperly tried to remove its (their) instructions, sometimes
+removing instructions used by others instructions, thus triggering
+assertions. The optimizing compiler was thus adjusted to remove these
+instructions in a proper fashion, by removing them as users first, and
+then by suppressing them for good.
diff --git a/test/435-try-finally-without-catch/src/Main.java b/test/435-try-finally-without-catch/src/Main.java
new file mode 100644
index 0000000..3c29ce8
--- /dev/null
+++ b/test/435-try-finally-without-catch/src/Main.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main {
+
+ public static void main(String[] args){
+ foo();
+ }
+
+ // Reduced test case inspired by constantPropagationTest() from
+ // test/083-compiler-regressions.
+ static void foo() {
+ int a = 0;
+ int b = 1;
+
+ for (int i = 0; i < 3; i++) {
+ try {
+ a = 1;
+ // Would throw an ArithmeticException if b were null (hence
+ // the enclosing `try' statement).
+ int c = a % b;
+ }
+ finally {
+ System.out.println("In finally");
+ }
+ }
+ }
+}
