Reviewers: Jakob,
Message:
The problem was cause by object_ being changed before bailing out to
runtime.
Therefore a slice of external string is perceived by the runtime function
as an
external string, in which case the slice offset is ignored.
Haven't actually tested the MIPS version as it is broken.
PTaL.
Description:
Fixing issue 1757 (string slices of external strings).
BUG=v8:1757
TEST=regress-1757.js
Please review this at http://codereview.chromium.org/8217011/
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/mips/code-stubs-mips.cc
M src/x64/code-stubs-x64.cc
A test/mjsunit/regress/regress-1757.js
Index: src/arm/code-stubs-arm.cc
diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc
index
e2a313372e6be6e4501a46aa5740182b911676c7..effa6080e9a7e419a1eec16c29eecb48318276d7
100644
--- a/src/arm/code-stubs-arm.cc
+++ b/src/arm/code-stubs-arm.cc
@@ -5088,23 +5088,26 @@ void
StringCharCodeAtGenerator::GenerateFast(MacroAssembler* masm) {
__ cmp(result_, Operand(ip));
__ b(ne, &call_runtime_);
// Get the first of the two strings and load its instance type.
- __ ldr(object_, FieldMemOperand(object_, ConsString::kFirstOffset));
+ __ ldr(result_, FieldMemOperand(object_, ConsString::kFirstOffset));
__ jmp(&assure_seq_string);
// SlicedString, unpack and add offset.
__ bind(&sliced_string);
__ ldr(result_, FieldMemOperand(object_, SlicedString::kOffsetOffset));
__ add(scratch_, scratch_, result_);
- __ ldr(object_, FieldMemOperand(object_, SlicedString::kParentOffset));
+ __ ldr(result_, FieldMemOperand(object_, SlicedString::kParentOffset));
// Assure that we are dealing with a sequential string. Go to runtime if
not.
__ bind(&assure_seq_string);
- __ ldr(result_, FieldMemOperand(object_, HeapObject::kMapOffset));
+ __ ldr(result_, FieldMemOperand(result_, HeapObject::kMapOffset));
__ ldrb(result_, FieldMemOperand(result_, Map::kInstanceTypeOffset));
// Check that parent is not an external string. Go to runtime otherwise.
STATIC_ASSERT(kSeqStringTag == 0);
__ tst(result_, Operand(kStringRepresentationMask));
__ b(ne, &call_runtime_);
+ // Actually fetch the parent string if it is confirmed to be sequential.
+ STATIC_ASSERT(SlicedString::kParentOffset == ConsString::kFirstOffset);
+ __ ldr(object_, FieldMemOperand(object_, SlicedString::kParentOffset));
// Check for 1-byte or 2-byte string.
__ bind(&flat_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
c20df4d10690f2264f4f43f875b2768f70403835..8a3b897a842426f0d32edd1454451fc27fea98e3
100644
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -5119,22 +5119,24 @@ void
StringCharCodeAtGenerator::GenerateFast(MacroAssembler* masm) {
Immediate(masm->isolate()->factory()->empty_string()));
__ j(not_equal, &call_runtime_);
// Get the first of the two strings and load its instance type.
- __ mov(object_, FieldOperand(object_, ConsString::kFirstOffset));
+ __ mov(result_, FieldOperand(object_, ConsString::kFirstOffset));
__ jmp(&assure_seq_string, Label::kNear);
// SlicedString, unpack and add offset.
__ bind(&sliced_string);
__ add(scratch_, FieldOperand(object_, SlicedString::kOffsetOffset));
- __ mov(object_, FieldOperand(object_, SlicedString::kParentOffset));
+ __ mov(result_, FieldOperand(object_, SlicedString::kParentOffset));
// Assure that we are dealing with a sequential string. Go to runtime if
not.
__ bind(&assure_seq_string);
- __ mov(result_, FieldOperand(object_, HeapObject::kMapOffset));
+ __ mov(result_, FieldOperand(result_, HeapObject::kMapOffset));
__ movzx_b(result_, FieldOperand(result_, Map::kInstanceTypeOffset));
STATIC_ASSERT(kSeqStringTag == 0);
__ test(result_, Immediate(kStringRepresentationMask));
__ j(not_zero, &call_runtime_);
- __ jmp(&flat_string, Label::kNear);
+ // Actually fetch the parent string if it is confirmed to be sequential.
+ STATIC_ASSERT(SlicedString::kParentOffset == ConsString::kFirstOffset);
+ __ mov(object_, FieldOperand(object_, SlicedString::kParentOffset));
// Check for 1-byte or 2-byte string.
__ bind(&flat_string);
Index: src/mips/code-stubs-mips.cc
diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc
index
b5ecc68ee0d4e51a98fa7f034ff76be40ffcd66a..a0c6d5055981b215eb0f2ba17aca0b6a70dcdc88
100644
--- a/src/mips/code-stubs-mips.cc
+++ b/src/mips/code-stubs-mips.cc
@@ -5130,24 +5130,27 @@ void
StringCharCodeAtGenerator::GenerateFast(MacroAssembler* masm) {
__ Branch(&call_runtime_, ne, result_, Operand(t0));
// Get the first of the two strings and load its instance type.
- __ lw(object_, FieldMemOperand(object_, ConsString::kFirstOffset));
+ __ lw(result_, FieldMemOperand(object_, ConsString::kFirstOffset));
__ jmp(&assure_seq_string);
// SlicedString, unpack and add offset.
__ bind(&sliced_string);
__ lw(result_, FieldMemOperand(object_, SlicedString::kOffsetOffset));
__ addu(scratch_, scratch_, result_);
- __ lw(object_, FieldMemOperand(object_, SlicedString::kParentOffset));
+ __ lw(result_, FieldMemOperand(object_, SlicedString::kParentOffset));
// Assure that we are dealing with a sequential string. Go to runtime if
not.
__ bind(&assure_seq_string);
- __ lw(result_, FieldMemOperand(object_, HeapObject::kMapOffset));
+ __ lw(result_, FieldMemOperand(result_, HeapObject::kMapOffset));
__ lbu(result_, FieldMemOperand(result_, Map::kInstanceTypeOffset));
// Check that parent is not an external string. Go to runtime otherwise.
STATIC_ASSERT(kSeqStringTag == 0);
__ And(t0, result_, Operand(kStringRepresentationMask));
__ Branch(&call_runtime_, ne, t0, Operand(zero_reg));
+ // Actually fetch the parent string if it is confirmed to be sequential.
+ STATIC_ASSERT(SlicedString::kParentOffset == ConsString::kFirstOffset);
+ __ lw(object_, FieldMemOperand(object_, SlicedString::kParentOffset));
// Check for 1-byte or 2-byte string.
__ bind(&flat_string);
Index: src/x64/code-stubs-x64.cc
diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc
index
b04e1ff59834cbe2261e260dbbe279a6a958d256..3fc4e15e0560bd972527b731670e77e0a126fe81
100644
--- a/src/x64/code-stubs-x64.cc
+++ b/src/x64/code-stubs-x64.cc
@@ -4084,22 +4084,22 @@ void
StringCharCodeAtGenerator::GenerateFast(MacroAssembler* masm) {
Heap::kEmptyStringRootIndex);
__ j(not_equal, &call_runtime_);
// Get the first of the two strings and load its instance type.
- __ movq(object_, FieldOperand(object_, ConsString::kFirstOffset));
+ __ movq(kScratchRegister, FieldOperand(object_,
ConsString::kFirstOffset));
__ jmp(&assure_seq_string, Label::kNear);
// SlicedString, unpack and add offset.
__ bind(&sliced_string);
__ addq(scratch_, FieldOperand(object_, SlicedString::kOffsetOffset));
- __ movq(object_, FieldOperand(object_, SlicedString::kParentOffset));
+ __ movq(kScratchRegister, FieldOperand(object_,
SlicedString::kParentOffset));
__ bind(&assure_seq_string);
- __ movq(result_, FieldOperand(object_, HeapObject::kMapOffset));
+ __ movq(result_, FieldOperand(kScratchRegister, HeapObject::kMapOffset));
__ movzxbl(result_, FieldOperand(result_, Map::kInstanceTypeOffset));
// If the first cons component is also non-flat, then go to runtime.
STATIC_ASSERT(kSeqStringTag == 0);
__ testb(result_, Immediate(kStringRepresentationMask));
__ j(not_zero, &call_runtime_);
- __ jmp(&flat_string);
+ __ movq(object_, kScratchRegister);
// Check for 1-byte or 2-byte string.
__ bind(&flat_string);
Index: test/mjsunit/regress/regress-1757.js
diff --git a/test/mjsunit/regress/regress-1757.js
b/test/mjsunit/regress/regress-1757.js
new file mode 100644
index
0000000000000000000000000000000000000000..f7a5516cac99ee106234e35e1517d9cf57995685
--- /dev/null
+++ b/test/mjsunit/regress/regress-1757.js
@@ -0,0 +1,32 @@
+// Copyright 2011 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --string-slices --expose-externalize-string
+
+var a = "abcdefghijklmnopqrstuvqxy"+"z";
+externalizeString(a, true);
+assertEquals('b', a.substring(1).charAt(0));
\ No newline at end of file
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
