Make SubStringStub more robust wrt unsafe arguments.

BUG=
TEST=test-strings/RobustSubStringStub

Review URL: https://chromiumcodereview.appspot.com/9969196

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11349 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

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

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, &not_original_string, Label::kNear);
+  // Shorter than original string's length: an actual substring.
+  __ j(below, &not_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);

src/x64/code-stubs-x64.cc

@@ -5112,56 +5112,24 @@ 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);
 
   __ 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, &not_original_string, Label::kNear);
+  // Shorter than original string's length: an actual substring.
+  __ j(below, &not_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(&not_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

test/cctest/test-strings.cc

@@ -620,3 +620,55 @@ TEST(AsciiArrayJoin) {
   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);");
+}