Reviewers: Erik Corry,
Message:
Also ports changes from https://chromiumcodereview.appspot.com/9625003
(r10957)
to X64 and ARM.
Description:
Make SubStringStub more robust wrt unsafe arguments.
BUG=
TEST=test-strings/RobustSubStringStub
Please review this at http://codereview.chromium.org/9969196/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/arm/code-stubs-arm.cc
M src/ia32/code-stubs-ia32.cc
M src/x64/code-stubs-x64.cc
M test/cctest/test-strings.cc
Index: src/arm/code-stubs-arm.cc
diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc
index
f772db9be2078f0ecaa24a633e1af438be6dbe9c..2f14d191b0598d9f27be0eab01fdb90493a76be9
100644
--- a/src/arm/code-stubs-arm.cc
+++ b/src/arm/code-stubs-arm.cc
@@ -5873,36 +5873,12 @@ void SubStringStub::Generate(MacroAssembler* masm) {
// r2: result string length
__ ldr(r4, FieldMemOperand(r0, String::kLengthOffset));
__ cmp(r2, Operand(r4, ASR, 1));
+ // Return original string.
__ b(eq, &return_r0);
+ // Longer than original string's length or negative: unsafe arguments.
+ __ b(hi, &runtime);
+ // Shorter than original string's length: an actual substring.
- Label result_longer_than_two;
- // Check for special case of two character ASCII string, in which case
- // we do a lookup in the symbol table first.
- __ cmp(r2, Operand(2));
- __ b(gt, &result_longer_than_two);
- __ b(lt, &runtime);
-
- __ JumpIfInstanceTypeIsNotSequentialAscii(r1, r1, &runtime);
-
- // Get the two characters forming the sub string.
- __ add(r0, r0, Operand(r3));
- __ ldrb(r3, FieldMemOperand(r0, SeqAsciiString::kHeaderSize));
- __ ldrb(r4, FieldMemOperand(r0, SeqAsciiString::kHeaderSize + 1));
-
- // Try to lookup two character string in symbol table.
- Label make_two_character_string;
- StringHelper::GenerateTwoCharacterSymbolTableProbe(
- masm, r3, r4, r1, r5, r6, r7, r9, &make_two_character_string);
- __ jmp(&return_r0);
-
- // r2: result string length.
- // r3: two characters combined into halfword in little endian byte order.
- __ bind(&make_two_character_string);
- __ AllocateAsciiString(r0, r2, r4, r5, r9, &runtime);
- __ strh(r3, FieldMemOperand(r0, SeqAsciiString::kHeaderSize));
- __ jmp(&return_r0);
-
- __ bind(&result_longer_than_two);
// Deal with different string types: update the index if necessary
// and put the underlying string into r5.
// r0: original string
Index: src/ia32/code-stubs-ia32.cc
diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index
2287b63775c9c1927ae796689f28eb42e84a179d..471114cbe4f219e04cb4b921de79326f6d3bebff
100644
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -6162,7 +6162,11 @@ void SubStringStub::Generate(MacroAssembler* masm) {
__ sub(ecx, edx);
__ cmp(ecx, FieldOperand(eax, String::kLengthOffset));
Label not_original_string;
- __ j(not_equal, ¬_original_string, Label::kNear);
+ // Shorter than original string's length: an actual substring.
+ __ j(below, ¬_original_string, Label::kNear);
+ // Longer than original string's length or negative: unsafe arguments.
+ __ j(above, &runtime);
+ // Return original string.
Counters* counters = masm->isolate()->counters();
__ IncrementCounter(counters->sub_string_native(), 1);
__ ret(3 * kPointerSize);
Index: src/x64/code-stubs-x64.cc
diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc
index
284503977128c86ff92776424c47f3267be931c7..fb2e79d233c5214d5a6a605a1b507afbdb4ca712
100644
--- a/src/x64/code-stubs-x64.cc
+++ b/src/x64/code-stubs-x64.cc
@@ -5112,7 +5112,6 @@ void SubStringStub::Generate(MacroAssembler* masm) {
// rax: string
// rbx: instance type
// Calculate length of sub string using the smi values.
- Label result_longer_than_two;
__ movq(rcx, Operand(rsp, kToOffset));
__ movq(rdx, Operand(rsp, kFromOffset));
__ JumpUnlessBothNonNegativeSmi(rcx, rdx, &runtime);
@@ -5120,48 +5119,16 @@ void SubStringStub::Generate(MacroAssembler* masm) {
__ SmiSub(rcx, rcx, rdx); // Overflow doesn't happen.
__ cmpq(FieldOperand(rax, String::kLengthOffset), rcx);
Label not_original_string;
- __ j(not_equal, ¬_original_string, Label::kNear);
+ // Shorter than original string's length: an actual substring.
+ __ j(below, ¬_original_string, Label::kNear);
+ // Longer than original string's length or negative: unsafe arguments.
+ __ j(above, &runtime);
+ // Return original string.
Counters* counters = masm->isolate()->counters();
__ IncrementCounter(counters->sub_string_native(), 1);
__ ret(kArgumentsSize);
__ bind(¬_original_string);
- // Special handling of sub-strings of length 1 and 2. One character
strings
- // are handled in the runtime system (looked up in the single character
- // cache). Two character strings are looked for in the symbol cache.
- __ SmiToInteger32(rcx, rcx);
- __ cmpl(rcx, Immediate(2));
- __ j(greater, &result_longer_than_two);
- __ j(less, &runtime);
-
- // Sub string of length 2 requested.
- // rax: string
- // rbx: instance type
- // rcx: sub string length (value is 2)
- // rdx: from index (smi)
- __ JumpIfInstanceTypeIsNotSequentialAscii(rbx, rbx, &runtime);
-
- // Get the two characters forming the sub string.
- __ SmiToInteger32(rdx, rdx); // From index is no longer smi.
- __ movzxbq(rbx, FieldOperand(rax, rdx, times_1,
SeqAsciiString::kHeaderSize));
- __ movzxbq(rdi,
- FieldOperand(rax, rdx, times_1, SeqAsciiString::kHeaderSize +
1));
-
- // Try to lookup two character string in symbol table.
- Label make_two_character_string;
- StringHelper::GenerateTwoCharacterSymbolTableProbe(
- masm, rbx, rdi, r9, r11, r14, r15, &make_two_character_string);
- __ IncrementCounter(counters->sub_string_native(), 1);
- __ ret(3 * kPointerSize);
-
- __ bind(&make_two_character_string);
- // Set up registers for allocating the two character string.
- __ movzxwq(rbx, FieldOperand(rax, rdx, times_1,
SeqAsciiString::kHeaderSize));
- __ AllocateAsciiString(rax, rcx, r11, r14, r15, &runtime);
- __ movw(FieldOperand(rax, SeqAsciiString::kHeaderSize), rbx);
- __ IncrementCounter(counters->sub_string_native(), 1);
- __ ret(3 * kPointerSize);
- __ bind(&result_longer_than_two);
// rax: string
// rbx: instance type
// rcx: sub string length
Index: test/cctest/test-strings.cc
diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc
index
e2a179f9324d2375da2ee5a78ac9378f83086aee..8c4442c432246f1e0f3e8a1306b8bfbe0a4875aa
100644
--- a/test/cctest/test-strings.cc
+++ b/test/cctest/test-strings.cc
@@ -620,3 +620,16 @@ TEST(AsciiArrayJoin) {
CHECK(result.IsEmpty());
CHECK(context->HasOutOfMemoryException());
}
+
+
+TEST(RobustSubStringStub) {
+ // This tests whether the SubStringStub can handle unsafe arguments.
+ // If not recognized, those unsafe arguments lead to out-of-bounds reads.
+ FLAG_allow_natives_syntax = true;
+ InitializeVM();
+ HandleScope scope;
+ v8::Local<v8::Value> result;
+ Handle<String> string;
+ CompileRun("%_SubString('abcdef', 0, 10000)");
+ CompileRun("%_SubString('abcdef', 5, 2)");
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
