Fix null pointer check elimination for catch entries.
Remove the special treatment of catch blocks for null
pointer check elimination and class initialization check
elimination. In both cases this can help optimizing
previously missed cases. In the null check case, this
avoids incorrect optimization as exposed by the new test.
Bug: 16230771
(cherry picked from 0a810d2eab27cd097ebd09a44f0ce83aa608285b)
Change-Id: I0764f47fa0aacfa89904a82e9528177b3ad67e31
diff --git a/compiler/dex/mir_optimization.cc b/compiler/dex/mir_optimization.cc
index 869c48f..f69eea7 100644
--- a/compiler/dex/mir_optimization.cc
+++ b/compiler/dex/mir_optimization.cc
@@ -737,11 +737,9 @@
ArenaBitVector* ssa_regs_to_check = temp_bit_vector_;
if (do_nce) {
/*
- * Set initial state. Be conservative with catch
- * blocks and start with no assumptions about null check
- * status (except for "this").
+ * Set initial state. Catch blocks don't need any special treatment.
*/
- if ((bb->block_type == kEntryBlock) | bb->catch_entry) {
+ if (bb->block_type == kEntryBlock) {
ssa_regs_to_check->ClearAllBits();
// Assume all ins are objects.
for (uint16_t in_reg = cu_->num_dalvik_registers - cu_->num_ins;
@@ -1047,12 +1045,11 @@
}
/*
- * Set initial state. Be conservative with catch
- * blocks and start with no assumptions about class init check status.
+ * Set initial state. Catch blocks don't need any special treatment.
*/
ArenaBitVector* classes_to_check = temp_bit_vector_;
DCHECK(classes_to_check != nullptr);
- if ((bb->block_type == kEntryBlock) | bb->catch_entry) {
+ if (bb->block_type == kEntryBlock) {
classes_to_check->SetInitialBits(temp_bit_vector_size_);
} else if (bb->predecessors->Size() == 1) {
BasicBlock* pred_bb = GetBasicBlock(bb->predecessors->Get(0));
diff --git a/compiler/dex/mir_optimization_test.cc b/compiler/dex/mir_optimization_test.cc
index 8c70b5c..4a0cf5c 100644
--- a/compiler/dex/mir_optimization_test.cc
+++ b/compiler/dex/mir_optimization_test.cc
@@ -373,30 +373,47 @@
static const SFieldDef sfields[] = {
{ 0u, 1u, 0u, 0u },
{ 1u, 1u, 1u, 1u },
+ { 2u, 1u, 2u, 2u },
+ { 3u, 1u, 3u, 3u },
};
static const BBDef bbs[] = {
DEF_BB(kNullBlock, DEF_SUCC0(), DEF_PRED0()),
DEF_BB(kEntryBlock, DEF_SUCC1(3), DEF_PRED0()),
- DEF_BB(kExitBlock, DEF_SUCC0(), DEF_PRED1(5)),
- DEF_BB(kDalvikByteCode, DEF_SUCC2(5, 4), DEF_PRED1(1)),
- DEF_BB(kDalvikByteCode, DEF_SUCC1(5), DEF_PRED1(3)), // Catch handler.
- DEF_BB(kDalvikByteCode, DEF_SUCC1(2), DEF_PRED2(3, 4)),
+ DEF_BB(kExitBlock, DEF_SUCC0(), DEF_PRED1(6)),
+ DEF_BB(kDalvikByteCode, DEF_SUCC1(4), DEF_PRED1(1)), // The top.
+ DEF_BB(kDalvikByteCode, DEF_SUCC1(6), DEF_PRED1(3)), // The throwing insn.
+ DEF_BB(kDalvikByteCode, DEF_SUCC1(6), DEF_PRED1(3)), // Catch handler.
+ DEF_BB(kDalvikByteCode, DEF_SUCC1(2), DEF_PRED2(4, 5)), // The merged block.
};
static const MIRDef mirs[] = {
- DEF_MIR(Instruction::SGET, 3u, 0u),
- DEF_MIR(Instruction::SGET, 3u, 1u),
- DEF_MIR(Instruction::SGET, 4u, 1u),
- DEF_MIR(Instruction::SGET, 5u, 0u), // Not eliminated.
- DEF_MIR(Instruction::SGET, 5u, 1u), // Eliminated.
+ DEF_MIR(Instruction::SGET, 3u, 0u), // Before the exception edge.
+ DEF_MIR(Instruction::SGET, 3u, 1u), // Before the exception edge.
+ DEF_MIR(Instruction::SGET, 4u, 2u), // After the exception edge.
+ DEF_MIR(Instruction::SGET, 4u, 3u), // After the exception edge.
+ DEF_MIR(Instruction::SGET, 5u, 0u), // In catch handler; class init check eliminated.
+ DEF_MIR(Instruction::SGET, 5u, 2u), // In catch handler; class init check not eliminated.
+ DEF_MIR(Instruction::SGET, 6u, 0u), // Class init check eliminated.
+ DEF_MIR(Instruction::SGET, 6u, 1u), // Class init check eliminated.
+ DEF_MIR(Instruction::SGET, 6u, 2u), // Class init check eliminated.
+ DEF_MIR(Instruction::SGET, 6u, 3u), // Class init check not eliminated.
};
static const bool expected_ignore_clinit_check[] = {
- false, false, false, false, true
+ false, false, false, false, true, false, true, true, true, false
};
PrepareSFields(sfields);
PrepareBasicBlocks(bbs);
- BasicBlock* catch_handler = cu_.mir_graph->GetBasicBlock(4u);
+ BasicBlock* catch_handler = cu_.mir_graph->GetBasicBlock(5u);
catch_handler->catch_entry = true;
+ // Add successor block info to the check block.
+ BasicBlock* check_bb = cu_.mir_graph->GetBasicBlock(3u);
+ check_bb->successor_block_list_type = kCatch;
+ check_bb->successor_blocks = new (&cu_.arena) GrowableArray<SuccessorBlockInfo*>(
+ &cu_.arena, 2, kGrowableArraySuccessorBlocks);
+ SuccessorBlockInfo* successor_block_info = reinterpret_cast<SuccessorBlockInfo*>
+ (cu_.arena.Alloc(sizeof(SuccessorBlockInfo), kArenaAllocSuccessor));
+ successor_block_info->block = catch_handler->id;
+ check_bb->successor_blocks->Insert(successor_block_info);
PrepareMIRs(mirs);
PerformClassInitCheckElimination();
ASSERT_EQ(arraysize(expected_ignore_clinit_check), mir_count_);
diff --git a/test/083-compiler-regressions/expected.txt b/test/083-compiler-regressions/expected.txt
index 10406c7..9f57dbd 100644
--- a/test/083-compiler-regressions/expected.txt
+++ b/test/083-compiler-regressions/expected.txt
@@ -14,6 +14,7 @@
false
b13679511Test finishing
b16177324TestWrapper caught NPE as expected.
+b16230771TestWrapper caught NPE as expected.
largeFrame passes
largeFrameFloat passes
mulBy1Test passes
diff --git a/test/083-compiler-regressions/src/Main.java b/test/083-compiler-regressions/src/Main.java
index 18bc674..748b0de 100644
--- a/test/083-compiler-regressions/src/Main.java
+++ b/test/083-compiler-regressions/src/Main.java
@@ -36,6 +36,7 @@
b5884080Test();
b13679511Test();
b16177324TestWrapper();
+ b16230771TestWrapper();
largeFrameTest();
largeFrameTestFloat();
mulBy1Test();
@@ -927,6 +928,28 @@
System.out.println("Unexpectedly retrieved all values: " + v1 + ", " + v2 + ", " + v3);
}
+ static void b16230771TestWrapper() {
+ try {
+ b16230771Test();
+ } catch (NullPointerException expected) {
+ System.out.println("b16230771TestWrapper caught NPE as expected.");
+ }
+ }
+
+ static void b16230771Test() {
+ Integer[] array = { null };
+ for (Integer i : array) {
+ try {
+ int value = i; // Null check on unboxing should fail.
+ System.out.println("Unexpectedly retrieved value " + value);
+ } catch (NullPointerException e) {
+ int value = i; // Null check on unboxing should fail.
+ // The bug was a missing null check, so this would actually cause SIGSEGV.
+ System.out.println("Unexpectedly retrieved value " + value + " in NPE catch handler");
+ }
+ }
+ }
+
static double TooManyArgs(
long l00,
long l01,
