String.IndexOf method handles negative start index value in incorrect way
The standard implementation of String.IndexOf converts the negative value of
the start index to 0 and searching will start from the beginning of the string.
But current implementation may start searching from the incorrect memory
offset, that can lead to sigsegv or return incorrect result.
This patch adds the handler for cases when fromIndex is negative.
Change-Id: I3ac86290712789559eaf5e46bef0006872395bfa
Signed-off-by: Alexei Zavjalov <alexei.zavjalov@intel.com>
diff --git a/compiler/dex/quick/x86/target_x86.cc b/compiler/dex/quick/x86/target_x86.cc
index dcc5d9b..5a8ad7a 100644
--- a/compiler/dex/quick/x86/target_x86.cc
+++ b/compiler/dex/quick/x86/target_x86.cc
@@ -1064,6 +1064,7 @@
LoadWordDisp(rs_rDX, count_offset, rs_rCX);
LIR *length_compare = nullptr;
int start_value = 0;
+ bool is_index_on_stack = false;
if (zero_based) {
// We have to handle an empty string. Use special instruction JECXZ.
length_compare = NewLIR0(kX86Jecxz8);
@@ -1084,14 +1085,32 @@
// Runtime start index.
rl_start = UpdateLoc(rl_start);
if (rl_start.location == kLocPhysReg) {
+ // Handle "start index < 0" case.
+ OpRegReg(kOpXor, rs_rBX, rs_rBX);
+ OpRegReg(kOpCmp, rl_start.reg, rs_rBX);
+ OpCondRegReg(kOpCmov, kCondLt, rl_start.reg, rs_rBX);
+
+ // The length of the string should be greater than the start index.
length_compare = OpCmpBranch(kCondLe, rs_rCX, rl_start.reg, nullptr);
OpRegReg(kOpSub, rs_rCX, rl_start.reg);
+ if (rl_start.reg == rs_rDI) {
+ // The special case. We will use EDI further, so lets put start index to stack.
+ NewLIR1(kX86Push32R, rDI);
+ is_index_on_stack = true;
+ }
} else {
- // Compare to memory to avoid a register load. Handle pushed EDI.
+ // Load the start index from stack, remembering that we pushed EDI.
int displacement = SRegOffset(rl_start.s_reg_low) + sizeof(uint32_t);
- OpRegMem(kOpCmp, rs_rCX, rs_rX86_SP, displacement);
- length_compare = NewLIR2(kX86Jcc8, 0, kX86CondLe);
- OpRegMem(kOpSub, rs_rCX, rs_rX86_SP, displacement);
+ LoadWordDisp(rs_rX86_SP, displacement, rs_rBX);
+ OpRegReg(kOpXor, rs_rDI, rs_rDI);
+ OpRegReg(kOpCmp, rs_rBX, rs_rDI);
+ OpCondRegReg(kOpCmov, kCondLt, rs_rBX, rs_rDI);
+
+ length_compare = OpCmpBranch(kCondLe, rs_rCX, rs_rBX, nullptr);
+ OpRegReg(kOpSub, rs_rCX, rs_rBX);
+ // Put the start index to stack.
+ NewLIR1(kX86Push32R, rBX);
+ is_index_on_stack = true;
}
}
}
@@ -1113,21 +1132,12 @@
NewLIR3(kX86Lea32RM, rDI, rBX, 2 * start_value);
}
} else {
- if (rl_start.location == kLocPhysReg) {
- if (rl_start.reg.GetReg() == rDI) {
- // We have a slight problem here. We are already using RDI!
- // Grab the value from the stack.
- LoadWordDisp(rs_rX86_SP, 0, rs_rDX);
- OpLea(rs_rDI, rs_rBX, rs_rDX, 1, 0);
- } else {
- OpLea(rs_rDI, rs_rBX, rl_start.reg, 1, 0);
- }
- } else {
- OpRegCopy(rs_rDI, rs_rBX);
- // Load the start index from stack, remembering that we pushed EDI.
- int displacement = SRegOffset(rl_start.s_reg_low) + sizeof(uint32_t);
- LoadWordDisp(rs_rX86_SP, displacement, rs_rDX);
+ if (is_index_on_stack == true) {
+ // Load the start index from stack.
+ NewLIR1(kX86Pop32R, rDX);
OpLea(rs_rDI, rs_rBX, rs_rDX, 1, 0);
+ } else {
+ OpLea(rs_rDI, rs_rBX, rl_start.reg, 1, 0);
}
}
diff --git a/test/082-inline-execute/src/Main.java b/test/082-inline-execute/src/Main.java
index 86a03ab..55ecf69 100644
--- a/test/082-inline-execute/src/Main.java
+++ b/test/082-inline-execute/src/Main.java
@@ -97,6 +97,7 @@
}
static int start;
+ private static int[] negIndex = { -100000 };
public static void test_String_indexOf() {
String str0 = "";
String str1 = "/";
@@ -125,6 +126,7 @@
Assert.assertEquals(str0.indexOf('a',0), -1);
Assert.assertEquals(str0.indexOf('a',-1), -1);
Assert.assertEquals(str1.indexOf('/',++start), -1);
+ Assert.assertEquals(str1.indexOf('a',negIndex[0]), -1);
Assert.assertEquals(str3.indexOf('a',0), 0);
Assert.assertEquals(str3.indexOf('a',1), -1);
Assert.assertEquals(str3.indexOf('a',1234), -1);