Handle potential <clinit>() correctly in LVN.
Bug: 16177324
Change-Id: I727ab6ce9aa9a608fe570cf391a6b732a12a8655
diff --git a/compiler/dex/local_value_numbering.cc b/compiler/dex/local_value_numbering.cc
index 6259496..69a94a5 100644
--- a/compiler/dex/local_value_numbering.cc
+++ b/compiler/dex/local_value_numbering.cc
@@ -356,7 +356,11 @@
}
uint16_t LocalValueNumbering::HandleSGet(MIR* mir, uint16_t opcode) {
- const MirFieldInfo& field_info = cu_->mir_graph->GetSFieldLoweringInfo(mir);
+ const MirSFieldLoweringInfo& field_info = cu_->mir_graph->GetSFieldLoweringInfo(mir);
+ if (!field_info.IsInitialized() && (mir->optimization_flags & MIR_IGNORE_CLINIT_CHECK) == 0) {
+ // Class initialization can call arbitrary functions, we need to wipe aliasing values.
+ HandleInvokeOrClInit(mir);
+ }
uint16_t res;
if (!field_info.IsResolved() || field_info.IsVolatile()) {
// Volatile fields always get a new memory version; field id is irrelevant.
@@ -381,8 +385,12 @@
}
void LocalValueNumbering::HandleSPut(MIR* mir, uint16_t opcode) {
+ const MirSFieldLoweringInfo& field_info = cu_->mir_graph->GetSFieldLoweringInfo(mir);
+ if (!field_info.IsInitialized() && (mir->optimization_flags & MIR_IGNORE_CLINIT_CHECK) == 0) {
+ // Class initialization can call arbitrary functions, we need to wipe aliasing values.
+ HandleInvokeOrClInit(mir);
+ }
uint16_t type = opcode - Instruction::SPUT;
- const MirFieldInfo& field_info = cu_->mir_graph->GetSFieldLoweringInfo(mir);
if (!field_info.IsResolved()) {
// Unresolved fields always alias with everything of the same type.
// Use mir->offset as modifier; without elaborate inlining, it will be unique.
@@ -405,6 +413,15 @@
}
}
+void LocalValueNumbering::HandleInvokeOrClInit(MIR* mir) {
+ // Use mir->offset as modifier; without elaborate inlining, it will be unique.
+ global_memory_version_ = LookupValue(kInvokeMemoryVersionBumpOp, 0u, 0u, mir->offset);
+ // All fields of escaped references need to be treated as potentially modified.
+ non_aliasing_ifields_.clear();
+ // Array elements may also have been modified via escaped array refs.
+ escaped_array_refs_.clear();
+}
+
uint16_t LocalValueNumbering::GetValueNumber(MIR* mir) {
uint16_t res = kNoValue;
uint16_t opcode = mir->dalvikInsn.opcode;
@@ -475,17 +492,12 @@
case Instruction::INVOKE_STATIC:
case Instruction::INVOKE_STATIC_RANGE:
if ((mir->optimization_flags & MIR_INLINED) == 0) {
- // Use mir->offset as modifier; without elaborate inlining, it will be unique.
- global_memory_version_ = LookupValue(kInvokeMemoryVersionBumpOp, 0u, 0u, mir->offset);
// Make ref args aliasing.
for (size_t i = 0u, count = mir->ssa_rep->num_uses; i != count; ++i) {
uint16_t reg = GetOperandValue(mir->ssa_rep->uses[i]);
non_aliasing_refs_.erase(reg);
}
- // All fields of escaped references need to be treated as potentially modified.
- non_aliasing_ifields_.clear();
- // Array elements may also have been modified via escaped array refs.
- escaped_array_refs_.clear();
+ HandleInvokeOrClInit(mir);
}
break;
diff --git a/compiler/dex/local_value_numbering.h b/compiler/dex/local_value_numbering.h
index 2a815be..45700df 100644
--- a/compiler/dex/local_value_numbering.h
+++ b/compiler/dex/local_value_numbering.h
@@ -269,6 +269,7 @@
void HandleIPut(MIR* mir, uint16_t opcode);
uint16_t HandleSGet(MIR* mir, uint16_t opcode);
void HandleSPut(MIR* mir, uint16_t opcode);
+ void HandleInvokeOrClInit(MIR* mir);
CompilationUnit* const cu_;
diff --git a/compiler/dex/local_value_numbering_test.cc b/compiler/dex/local_value_numbering_test.cc
index efc4fc8..a2e3f3d 100644
--- a/compiler/dex/local_value_numbering_test.cc
+++ b/compiler/dex/local_value_numbering_test.cc
@@ -113,11 +113,13 @@
for (size_t i = 0u; i != count; ++i) {
const SFieldDef* def = &defs[i];
MirSFieldLoweringInfo field_info(def->field_idx);
+ // Mark even unresolved fields as initialized.
+ field_info.flags_ = MirSFieldLoweringInfo::kFlagIsStatic |
+ MirSFieldLoweringInfo::kFlagIsInitialized;
if (def->declaring_dex_file != 0u) {
field_info.declaring_dex_file_ = reinterpret_cast<const DexFile*>(def->declaring_dex_file);
field_info.declaring_field_idx_ = def->declaring_field_idx;
- field_info.flags_ = MirSFieldLoweringInfo::kFlagIsStatic |
- (def->is_volatile ? MirSFieldLoweringInfo::kFlagIsVolatile : 0u);
+ field_info.flags_ |= (def->is_volatile ? MirSFieldLoweringInfo::kFlagIsVolatile : 0u);
}
cu_.mir_graph->sfield_lowering_infos_.Insert(field_info);
}
@@ -168,6 +170,12 @@
DoPrepareMIRs(defs, count);
}
+ void MakeSFieldUninitialized(uint32_t sfield_index) {
+ CHECK_LT(sfield_index, cu_.mir_graph->sfield_lowering_infos_.Size());
+ cu_.mir_graph->sfield_lowering_infos_.GetRawStorage()[sfield_index].flags_ &=
+ ~MirSFieldLoweringInfo::kFlagIsInitialized;
+ }
+
void PerformLVN() {
value_names_.resize(mir_count_);
for (size_t i = 0; i != mir_count_; ++i) {
@@ -597,4 +605,25 @@
}
}
+TEST_F(LocalValueNumberingTest, ClInitOnSget) {
+ static const SFieldDef sfields[] = {
+ { 0u, 1u, 0u, false },
+ { 1u, 2u, 1u, false },
+ };
+ static const MIRDef mirs[] = {
+ DEF_SGET(Instruction::SGET_OBJECT, 0u, 0u),
+ DEF_AGET(Instruction::AGET, 1u, 0u, 100u),
+ DEF_SGET(Instruction::SGET_OBJECT, 2u, 1u),
+ DEF_SGET(Instruction::SGET_OBJECT, 3u, 0u),
+ DEF_AGET(Instruction::AGET, 4u, 3u, 100u),
+ };
+
+ PrepareSFields(sfields);
+ MakeSFieldUninitialized(1u);
+ PrepareMIRs(mirs);
+ PerformLVN();
+ ASSERT_EQ(value_names_.size(), 5u);
+ EXPECT_NE(value_names_[0], value_names_[3]);
+}
+
} // namespace art
diff --git a/compiler/dex/mir_optimization.cc b/compiler/dex/mir_optimization.cc
index 4b2bc4a..6566e03 100644
--- a/compiler/dex/mir_optimization.cc
+++ b/compiler/dex/mir_optimization.cc
@@ -998,11 +998,14 @@
mir->dalvikInsn.opcode <= Instruction::SPUT_SHORT) {
const MirSFieldLoweringInfo& field_info = GetSFieldLoweringInfo(mir);
uint16_t index = 0xffffu;
- if (field_info.IsResolved() && !field_info.IsInitialized()) {
+ if (!field_info.IsInitialized()) {
DCHECK_LT(class_to_index_map.size(), 0xffffu);
MapEntry entry = {
- field_info.DeclaringDexFile(),
- field_info.DeclaringClassIndex(),
+ // Treat unresolved fields as if each had its own class.
+ field_info.IsResolved() ? field_info.DeclaringDexFile()
+ : nullptr,
+ field_info.IsResolved() ? field_info.DeclaringClassIndex()
+ : field_info.FieldIndex(),
static_cast<uint16_t>(class_to_index_map.size())
};
index = class_to_index_map.insert(entry).first->index;
diff --git a/compiler/dex/mir_optimization_test.cc b/compiler/dex/mir_optimization_test.cc
index 9b2e798..8c70b5c 100644
--- a/compiler/dex/mir_optimization_test.cc
+++ b/compiler/dex/mir_optimization_test.cc
@@ -248,7 +248,7 @@
DEF_MIR(Instruction::SGET, 3u, 4u),
};
static const bool expected_ignore_clinit_check[] = {
- false, false, false, false, false, true, true, true, false, false, true
+ false, false, false, false, true, true, true, true, true, false, true
};
PrepareSFields(sfields);
@@ -312,7 +312,7 @@
DEF_MIR(Instruction::SPUT, 6u, 9u), // Eliminated (with sfield[8] in block #4).
};
static const bool expected_ignore_clinit_check[] = {
- false, false, // Unresolved: sfield[10], method[2]
+ false, true, // Unresolved: sfield[10], method[2]
false, true, // sfield[0]
false, false, // sfield[1]
false, true, // sfield[2]
diff --git a/test/083-compiler-regressions/expected.txt b/test/083-compiler-regressions/expected.txt
index 7576b02..10406c7 100644
--- a/test/083-compiler-regressions/expected.txt
+++ b/test/083-compiler-regressions/expected.txt
@@ -13,6 +13,7 @@
1
false
b13679511Test finishing
+b16177324TestWrapper 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 6a12ca9..0f7527c 100644
--- a/test/083-compiler-regressions/src/Main.java
+++ b/test/083-compiler-regressions/src/Main.java
@@ -35,6 +35,7 @@
b2487514Test();
b5884080Test();
b13679511Test();
+ b16177324TestWrapper();
largeFrameTest();
largeFrameTestFloat();
mulBy1Test();
@@ -908,6 +909,24 @@
System.out.println("b13679511Test finishing");
}
+ static void b16177324TestWrapper() {
+ try {
+ b16177324Test();
+ } catch (NullPointerException expected) {
+ System.out.println("b16177324TestWrapper caught NPE as expected.");
+ }
+ }
+
+ static void b16177324Test() {
+ // We need this to be a single BasicBlock. Putting it into a try block would cause it to
+ // be split at each insn that can throw. So we do the try-catch in a wrapper function.
+ int v1 = B16177324Values.values[0]; // Null-check on array element access.
+ int v2 = B16177324ValuesKiller.values[0]; // clinit<>() sets B16177324Values.values to null.
+ int v3 = B16177324Values.values[0]; // Should throw NPE.
+ // If the null-check for v3 was eliminated we should fail with SIGSEGV.
+ System.out.println("Unexpectedly retrieved all values: " + v1 + ", " + v2 + ", " + v3);
+ }
+
static double TooManyArgs(
long l00,
long l01,
@@ -9743,3 +9762,14 @@
}
}
}
+
+class B16177324Values {
+ public static int values[] = { 42 };
+}
+
+class B16177324ValuesKiller {
+ public static int values[] = { 1234 };
+ static {
+ B16177324Values.values = null;
+ }
+}
