From 47c4a5b4ef7218eaa6fad404bff9ae62fe2aabb0 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Tue, 17 Apr 2012 10:49:15 +0000 Subject: [PATCH] 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 | 32 ++++------------------------ src/ia32/code-stubs-ia32.cc | 6 +++++- src/x64/code-stubs-x64.cc | 44 ++++++-------------------------------- test/cctest/test-strings.cc | 52 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 67 deletions(-) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index f772db9..2f14d19 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 diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 2287b63..471114c 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); diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 2845039..ce9067c 100644 --- a/src/x64/code-stubs-x64.cc +++ b/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, ¬_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 diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc index e2a179f..d86886f 100644 --- a/test/cctest/test-strings.cc +++ b/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 result; + Handle 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);"); +} -- 2.7.4