Compensate in compiler for verifier shortcomings.
The verifier does not differentiate zero and null, so a move-object
of zero can be used as a non-object later on.
Change the compiler to ignore the object conversion when the input
is zero or a phi (which might just hold zeros). The type propagation
will then do proper inferencing of the types.
Also remove some stalled comments in ssa_builder.cc.
bug:31313170
test: dex2oat b31313170.apk
test: run-test 800
test: m test-art-host-run-test
Change-Id: I579d667415a7decf8ff2c2238dae4c13eec5d0e0
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index 5a6a212..f7dc237 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -1815,7 +1815,20 @@
case Instruction::MOVE_OBJECT:
case Instruction::MOVE_OBJECT_16:
case Instruction::MOVE_OBJECT_FROM16: {
- HInstruction* value = LoadLocal(instruction.VRegB(), Primitive::kPrimNot);
+ // The verifier has no notion of a null type, so a move-object of constant 0
+ // will lead to the same constant 0 in the destination register. To mimic
+ // this behavior, we just pretend we haven't seen a type change (int to reference)
+ // for the 0 constant and phis. We rely on our type propagation to eventually get the
+ // types correct.
+ uint32_t reg_number = instruction.VRegB();
+ HInstruction* value = (*current_locals_)[reg_number];
+ if (value->IsIntConstant()) {
+ DCHECK_EQ(value->AsIntConstant()->GetValue(), 0);
+ } else if (value->IsPhi()) {
+ DCHECK(value->GetType() == Primitive::kPrimInt || value->GetType() == Primitive::kPrimNot);
+ } else {
+ value = LoadLocal(reg_number, Primitive::kPrimNot);
+ }
UpdateLocal(instruction.VRegA(), value);
break;
}
diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc
index f7dc112..03807ba 100644
--- a/compiler/optimizing/ssa_builder.cc
+++ b/compiler/optimizing/ssa_builder.cc
@@ -163,18 +163,12 @@
// Replace inputs of `phi` to match its type. Return false if conflict is identified.
bool SsaBuilder::TypeInputsOfPhi(HPhi* phi, ArenaVector<HPhi*>* worklist) {
Primitive::Type common_type = phi->GetType();
- if (common_type == Primitive::kPrimVoid || Primitive::IsIntegralType(common_type)) {
- // Phi either contains only other untyped phis (common_type == kPrimVoid),
- // or `common_type` is integral and we do not need to retype ambiguous inputs
- // because they are always constructed with the integral type candidate.
+ if (Primitive::IsIntegralType(common_type)) {
+ // We do not need to retype ambiguous inputs because they are always constructed
+ // with the integral type candidate.
if (kIsDebugBuild) {
for (HInstruction* input : phi->GetInputs()) {
- if (common_type == Primitive::kPrimVoid) {
- DCHECK(input->IsPhi() && input->GetType() == Primitive::kPrimVoid);
- } else {
- DCHECK((input->IsPhi() && input->GetType() == Primitive::kPrimVoid) ||
- HPhi::ToPhiType(input->GetType()) == common_type);
- }
+ DCHECK(HPhi::ToPhiType(input->GetType()) == common_type);
}
}
// Inputs did not need to be replaced, hence no conflict. Report success.
diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index 3bb3725..b8324e5 100644
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -70,4 +70,5 @@
b/29778499 (1)
b/29778499 (2)
b/30458218
+b/31313170
Done!
diff --git a/test/800-smali/smali/b_31313170.smali b/test/800-smali/smali/b_31313170.smali
new file mode 100644
index 0000000..327942a
--- /dev/null
+++ b/test/800-smali/smali/b_31313170.smali
@@ -0,0 +1,22 @@
+.class public LB31313170;
+.super Ljava/lang/Object;
+
+
+.method public constructor <init>()V
+.registers 1
+ invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+ return-void
+.end method
+
+.method public static run()I
+.registers 4
+ const/4 v0, 0
+ const/4 v1, 1
+ sget v2, LB31313170;->a:I
+ if-nez v2, :exit
+ move-object v1, v0
+ :exit
+ return v1
+.end method
+
+.field static public a:I
diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 34f2580..8d39f09 100644
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -181,6 +181,7 @@
testCases.add(new TestCase("b/29778499 (2)", "B29778499_2", "run", null,
new IncompatibleClassChangeError(), null));
testCases.add(new TestCase("b/30458218", "B30458218", "run", null, null, null));
+ testCases.add(new TestCase("b/31313170", "B31313170", "run", null, null, 0));
}
public void runTests() {
@@ -228,7 +229,7 @@
tc.testName);
} else if (tc.expectedReturn == null && retValue != null) {
errorReturn = new IllegalStateException("Expected a null result in test " +
- tc.testName);
+ tc.testName + " got " + retValue);
} else if (tc.expectedReturn != null &&
(retValue == null || !tc.expectedReturn.equals(retValue))) {
errorReturn = new IllegalStateException("Expected return " +