Revision: 11349
Author: [email protected]
Date: Tue Apr 17 03:49:15 2012
Log: Make SubStringStub more robust wrt unsafe arguments.
BUG=
TEST=test-strings/RobustSubStringStub
Review URL: https://chromiumcodereview.appspot.com/9969196
http://code.google.com/p/v8/source/detail?r=11349
Modified:
/branches/bleeding_edge/src/arm/code-stubs-arm.cc
/branches/bleeding_edge/src/ia32/code-stubs-ia32.cc
/branches/bleeding_edge/src/x64/code-stubs-x64.cc
/branches/bleeding_edge/test/cctest/test-strings.cc
=======================================
--- /branches/bleeding_edge/src/arm/code-stubs-arm.cc Mon Mar 12 06:56:56
2012
+++ /branches/bleeding_edge/src/arm/code-stubs-arm.cc Tue Apr 17 03:49:15
2012
@@ -5873,36 +5873,12 @@
// r2: result string length
__ ldr(r4, FieldMemOperand(r0, String::kLengthOffset));
__ cmp(r2, Operand(r4, ASR, 1));
+ // Return original string.
__ b(eq, &return_r0);
-
- 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);
+ // Longer than original string's length or negative: unsafe arguments.
+ __ b(hi, &runtime);
+ // Shorter than original string's length: an actual substring.
+
// Deal with different string types: update the index if necessary
// and put the underlying string into r5.
// r0: original string
=======================================
--- /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Tue Apr 10 06:39:28
2012
+++ /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Tue Apr 17 03:49:15
2012
@@ -6162,7 +6162,11 @@
__ 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);
=======================================
--- /branches/bleeding_edge/src/x64/code-stubs-x64.cc Mon Mar 12 06:56:56
2012
+++ /branches/bleeding_edge/src/x64/code-stubs-x64.cc Tue Apr 17 03:49:15
2012
@@ -5112,56 +5112,24 @@
// 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);
__ SmiSub(rcx, rcx, rdx); // Overflow doesn't happen.
- __ cmpq(FieldOperand(rax, String::kLengthOffset), rcx);
+ __ cmpq(rcx, FieldOperand(rax, 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(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
=======================================
--- /branches/bleeding_edge/test/cctest/test-strings.cc Wed Apr 4 07:37:07
2012
+++ /branches/bleeding_edge/test/cctest/test-strings.cc Tue Apr 17 03:49:15
2012
@@ -620,3 +620,55 @@
CHECK(result.IsEmpty());
CHECK(context->HasOutOfMemoryException());
}
+
+
+static void CheckException(const char* source) {
+ // An empty handle is returned upon exception.
+ CHECK(CompileRun(source).IsEmpty());
+}
+
+
+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("var short = 'abcdef';");
+
+ // Invalid indices.
+ CheckException("%_SubString(short, 0, 10000);");
+ CheckException("%_SubString(short, -1234, 5);");
+ CheckException("%_SubString(short, 5, 2);");
+ // Special HeapNumbers.
+ CheckException("%_SubString(short, 1, Infinity);");
+ CheckException("%_SubString(short, NaN, 5);");
+ // String arguments.
+ CheckException("%_SubString(short, '2', '5');");
+ // Ordinary HeapNumbers can be handled (in runtime).
+ result = CompileRun("%_SubString(short, Math.sqrt(4), 5.1);");
+ string = v8::Utils::OpenHandle(v8::String::Cast(*result));
+ CHECK_EQ("cde", *(string->ToCString()));
+
+ CompileRun("var long = 'abcdefghijklmnopqrstuvwxyz';");
+ // Invalid indices.
+ CheckException("%_SubString(long, 0, 10000);");
+ CheckException("%_SubString(long, -1234, 17);");
+ CheckException("%_SubString(long, 17, 2);");
+ // Special HeapNumbers.
+ CheckException("%_SubString(long, 1, Infinity);");
+ CheckException("%_SubString(long, NaN, 17);");
+ // String arguments.
+ CheckException("%_SubString(long, '2', '17');");
+ // Ordinary HeapNumbers within bounds can be handled (in runtime).
+ result = CompileRun("%_SubString(long, Math.sqrt(4), 17.1);");
+ string = v8::Utils::OpenHandle(v8::String::Cast(*result));
+ CHECK_EQ("cdefghijklmnopq", *(string->ToCString()));
+
+ // Test that out-of-bounds substring of a slice fails when the indices
+ // would have been valid for the underlying string.
+ CompileRun("var slice = long.slice(1, 15);");
+ CheckException("%_SubString(slice, 0, 17);");
+}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
