ART: Prevent float inference back to SSA in-regs.
.method public static getInt(I)I
.registers 2
const/4 v0, 0x0
if-ne v0, v0, :after
float-to-int v0, v0
:exit
add-int/2addr v0, v1
return v0
:after
move v1, v0
goto :exit
.end method
In this code sample, v1 is the single parameter to this method. In one
of the phi-nodes inserted between :exit and add-int/2addr, v1's two
incoming SSA regs are:
- the initial def of v1 as a parameter
- the v1 def'd at move v1, v0.
During type inference, because the 2nd def is a float (because of the
earlier float-to-int v0, v0) this will change the type of the 1st def to a
float as well, which is incorrect since the first parameter is known to be
non-float.
This fix checks during phi-node type-inference if an SSA reg that is the
initial def of a parameter vreg is about to be set as float when it was
not previously, and skips the inference if so.
In this case, when using a hard-float ABI, having the in-reg v1 set as
float causes FlushIns() to read the argument to the method from an FP reg,
when the argument will be passed in a core reg by any caller.
Also included is a smali test for this bug: compare difference between
./run-test --64 800
./run-test --64 --interpreter 800
when the vreg_analysis patch has not been applied.
(Requires 64-bit because 32-bit ARM currently does not use hard-float.)
getInt(I)I should return its argument, but it returns an incorrect
value.
Change-Id: I1d4b5be6a931fe853279e89dd820820f29823da1
Signed-off-by: Stephen Kyle <stephen.kyle@arm.com>
diff --git a/compiler/dex/vreg_analysis.cc b/compiler/dex/vreg_analysis.cc
index bdfab13..f6c7d52 100644
--- a/compiler/dex/vreg_analysis.cc
+++ b/compiler/dex/vreg_analysis.cc
@@ -378,7 +378,20 @@
changed |= SetWide(defs[1]);
changed |= SetHigh(defs[1]);
}
+
+ bool has_ins = (GetNumOfInVRs() > 0);
+
for (int i = 0; i < ssa_rep->num_uses; i++) {
+ if (has_ins && IsInVReg(uses[i])) {
+ // NB: The SSA name for the first def of an in-reg will be the same as
+ // the reg's actual name.
+ if (!reg_location_[uses[i]].fp && defined_fp) {
+ // If we were about to infer that this first def of an in-reg is a float
+ // when it wasn't previously (because float/int is set during SSA initialization),
+ // do not allow this to happen.
+ continue;
+ }
+ }
changed |= SetFp(uses[i], defined_fp);
changed |= SetCore(uses[i], defined_core);
changed |= SetRef(uses[i], defined_ref);
diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index 468e7a6..4002fbf 100644
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -1,2 +1,3 @@
b/17790197
+FloatBadArgReg
Done!
diff --git a/test/800-smali/smali/FloatBadArgReg.smali b/test/800-smali/smali/FloatBadArgReg.smali
new file mode 100644
index 0000000..719ba09
--- /dev/null
+++ b/test/800-smali/smali/FloatBadArgReg.smali
@@ -0,0 +1,16 @@
+.class public LFloatBadArgReg;
+
+.super Ljava/lang/Object;
+
+.method public static getInt(I)I
+ .registers 2
+ const/4 v0, 0x0
+ if-ne v0, v0, :after
+ float-to-int v0, v0
+ :exit
+ add-int/2addr v0, v1
+ return v0
+ :after
+ move v1, v0
+ goto :exit
+.end method
diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 0ef3a9d..c86470c 100644
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -49,6 +49,8 @@
testCases = new LinkedList<TestCase>();
testCases.add(new TestCase("b/17790197", "B17790197", "getInt", null, null, 100));
+ testCases.add(new TestCase("FloatBadArgReg", "FloatBadArgReg", "getInt",
+ new Object[]{100}, null, 100));
}
public void runTests() {