From 371d6f6a9863e5f7ed55b066e2abceb85a2b57a9 Mon Sep 17 00:00:00 2001 From: "mvstanton@chromium.org" Date: Tue, 28 Jan 2014 11:53:11 +0000 Subject: [PATCH] We shouldn't throw under FLAG_debug_code, rather abort. Throwing under FLAG_debug_code confuses the rest of our infrastructure which expects a safe point at the site of call into the runtime for throw. We were doing that to make a clusterfuzz test happy, but the better solution is to assert/abort under debug_code, and prevent clusterfuzz from fuzzing on internal APIs that crash on incorrect values. We'll need to alter the fuzzer to turn off fuzzing for: string-natives.js lithium/SeqStringSetChar.js regress/regress-seqstrsetchar-ex3.js regress/regress-seqstrsetchar-ex1.js regress/regress-crbug-320922.js So as to prevent the fuzzer from running %_OneByteSeqStringSetChar() and %_TwoByteSeqStringSetChar(). BUG= R=hpayer@chromium.org, machenbach@chromium.org Review URL: https://codereview.chromium.org/139903005 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18878 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/full-codegen-arm.cc | 8 +-- src/arm/macro-assembler-arm.cc | 10 ++-- src/ia32/full-codegen-ia32.cc | 8 +-- src/ia32/macro-assembler-ia32.cc | 11 ++-- src/mips/full-codegen-mips.cc | 8 +-- src/mips/macro-assembler-mips.cc | 10 ++-- src/x64/full-codegen-x64.cc | 8 +-- src/x64/macro-assembler-x64.cc | 8 +-- test/mjsunit/mjsunit.status | 3 - test/mjsunit/regress/regress-320948.js | 81 -------------------------- 10 files changed, 35 insertions(+), 120 deletions(-) delete mode 100644 test/mjsunit/regress/regress-320948.js diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 2c9b2756ed..3d4f8ed8a6 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -3458,9 +3458,9 @@ void FullCodeGenerator::EmitOneByteSeqStringSetChar(CallRuntime* expr) { if (FLAG_debug_code) { __ SmiTst(value); - __ ThrowIf(ne, kNonSmiValue); + __ Check(eq, kNonSmiValue); __ SmiTst(index); - __ ThrowIf(ne, kNonSmiIndex); + __ Check(eq, kNonSmiIndex); __ SmiUntag(index, index); static const uint32_t one_byte_seq_type = kSeqStringTag | kOneByteStringTag; __ EmitSeqStringSetCharCheck(string, index, value, one_byte_seq_type); @@ -3491,9 +3491,9 @@ void FullCodeGenerator::EmitTwoByteSeqStringSetChar(CallRuntime* expr) { if (FLAG_debug_code) { __ SmiTst(value); - __ ThrowIf(ne, kNonSmiValue); + __ Check(eq, kNonSmiValue); __ SmiTst(index); - __ ThrowIf(ne, kNonSmiIndex); + __ Check(eq, kNonSmiIndex); __ SmiUntag(index, index); static const uint32_t two_byte_seq_type = kSeqStringTag | kTwoByteStringTag; __ EmitSeqStringSetCharCheck(string, index, value, two_byte_seq_type); diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc index cc7283b8e3..a83a96aabf 100644 --- a/src/arm/macro-assembler-arm.cc +++ b/src/arm/macro-assembler-arm.cc @@ -3433,14 +3433,14 @@ void MacroAssembler::EmitSeqStringSetCharCheck(Register string, uint32_t encoding_mask) { Label is_object; SmiTst(string); - ThrowIf(eq, kNonObject); + Check(ne, kNonObject); ldr(ip, FieldMemOperand(string, HeapObject::kMapOffset)); ldrb(ip, FieldMemOperand(ip, Map::kInstanceTypeOffset)); and_(ip, ip, Operand(kStringRepresentationMask | kStringEncodingMask)); cmp(ip, Operand(encoding_mask)); - ThrowIf(ne, kUnexpectedStringType); + Check(eq, kUnexpectedStringType); // The index is assumed to be untagged coming in, tag it to compare with the // string length without using a temp register, it is restored at the end of @@ -3449,15 +3449,15 @@ void MacroAssembler::EmitSeqStringSetCharCheck(Register string, TrySmiTag(index, index, &index_tag_bad); b(&index_tag_ok); bind(&index_tag_bad); - Throw(kIndexIsTooLarge); + Abort(kIndexIsTooLarge); bind(&index_tag_ok); ldr(ip, FieldMemOperand(string, String::kLengthOffset)); cmp(index, ip); - ThrowIf(ge, kIndexIsTooLarge); + Check(lt, kIndexIsTooLarge); cmp(index, Operand(Smi::FromInt(0))); - ThrowIf(lt, kIndexIsNegative); + Check(ge, kIndexIsNegative); SmiUntag(index, index); } diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 08686e37c9..ed11ce9c59 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -3411,9 +3411,9 @@ void FullCodeGenerator::EmitOneByteSeqStringSetChar(CallRuntime* expr) { if (FLAG_debug_code) { __ test(value, Immediate(kSmiTagMask)); - __ ThrowIf(not_zero, kNonSmiValue); + __ Check(zero, kNonSmiValue); __ test(index, Immediate(kSmiTagMask)); - __ ThrowIf(not_zero, kNonSmiValue); + __ Check(zero, kNonSmiValue); } __ SmiUntag(value); @@ -3446,9 +3446,9 @@ void FullCodeGenerator::EmitTwoByteSeqStringSetChar(CallRuntime* expr) { if (FLAG_debug_code) { __ test(value, Immediate(kSmiTagMask)); - __ ThrowIf(not_zero, kNonSmiValue); + __ Check(zero, kNonSmiValue); __ test(index, Immediate(kSmiTagMask)); - __ ThrowIf(not_zero, kNonSmiValue); + __ Check(zero, kNonSmiValue); __ SmiUntag(index); static const uint32_t two_byte_seq_type = kSeqStringTag | kTwoByteStringTag; __ EmitSeqStringSetCharCheck(string, index, value, two_byte_seq_type); diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc index 25598cef55..faf768e11d 100644 --- a/src/ia32/macro-assembler-ia32.cc +++ b/src/ia32/macro-assembler-ia32.cc @@ -3223,7 +3223,7 @@ void MacroAssembler::EmitSeqStringSetCharCheck(Register string, uint32_t encoding_mask) { Label is_object; JumpIfNotSmi(string, &is_object, Label::kNear); - Throw(kNonObject); + Abort(kNonObject); bind(&is_object); push(value); @@ -3233,20 +3233,19 @@ void MacroAssembler::EmitSeqStringSetCharCheck(Register string, and_(value, Immediate(kStringRepresentationMask | kStringEncodingMask)); cmp(value, Immediate(encoding_mask)); pop(value); - ThrowIf(not_equal, kUnexpectedStringType); + Check(equal, kUnexpectedStringType); // The index is assumed to be untagged coming in, tag it to compare with the // string length without using a temp register, it is restored at the end of // this function. SmiTag(index); - // Can't use overflow here directly, compiler can't seem to disambiguate. - ThrowIf(NegateCondition(no_overflow), kIndexIsTooLarge); + Check(no_overflow, kIndexIsTooLarge); cmp(index, FieldOperand(string, String::kLengthOffset)); - ThrowIf(greater_equal, kIndexIsTooLarge); + Check(less, kIndexIsTooLarge); cmp(index, Immediate(Smi::FromInt(0))); - ThrowIf(less, kIndexIsNegative); + Check(greater_equal, kIndexIsNegative); // Restore the index SmiUntag(index); diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index 972210a9c1..0fbcc46b4a 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -3500,9 +3500,9 @@ void FullCodeGenerator::EmitOneByteSeqStringSetChar(CallRuntime* expr) { if (FLAG_debug_code) { __ SmiTst(value, at); - __ ThrowIf(ne, kNonSmiValue, at, Operand(zero_reg)); + __ Check(eq, kNonSmiValue, at, Operand(zero_reg)); __ SmiTst(index, at); - __ ThrowIf(ne, kNonSmiIndex, at, Operand(zero_reg)); + __ Check(eq, kNonSmiIndex, at, Operand(zero_reg)); __ SmiUntag(index, index); static const uint32_t one_byte_seq_type = kSeqStringTag | kOneByteStringTag; Register scratch = t5; @@ -3537,9 +3537,9 @@ void FullCodeGenerator::EmitTwoByteSeqStringSetChar(CallRuntime* expr) { if (FLAG_debug_code) { __ SmiTst(value, at); - __ ThrowIf(ne, kNonSmiValue, at, Operand(zero_reg)); + __ Check(eq, kNonSmiValue, at, Operand(zero_reg)); __ SmiTst(index, at); - __ ThrowIf(ne, kNonSmiIndex, at, Operand(zero_reg)); + __ Check(eq, kNonSmiIndex, at, Operand(zero_reg)); __ SmiUntag(index, index); static const uint32_t two_byte_seq_type = kSeqStringTag | kTwoByteStringTag; Register scratch = t5; diff --git a/src/mips/macro-assembler-mips.cc b/src/mips/macro-assembler-mips.cc index 517344857f..fe077458ea 100644 --- a/src/mips/macro-assembler-mips.cc +++ b/src/mips/macro-assembler-mips.cc @@ -5050,14 +5050,14 @@ void MacroAssembler::EmitSeqStringSetCharCheck(Register string, uint32_t encoding_mask) { Label is_object; SmiTst(string, at); - ThrowIf(eq, kNonObject, at, Operand(zero_reg)); + Check(ne, kNonObject, at, Operand(zero_reg)); lw(at, FieldMemOperand(string, HeapObject::kMapOffset)); lbu(at, FieldMemOperand(at, Map::kInstanceTypeOffset)); andi(at, at, kStringRepresentationMask | kStringEncodingMask); li(scratch, Operand(encoding_mask)); - ThrowIf(ne, kUnexpectedStringType, at, Operand(scratch)); + Check(eq, kUnexpectedStringType, at, Operand(scratch)); // The index is assumed to be untagged coming in, tag it to compare with the // string length without using a temp register, it is restored at the end of @@ -5066,14 +5066,14 @@ void MacroAssembler::EmitSeqStringSetCharCheck(Register string, TrySmiTag(index, scratch, &index_tag_bad); Branch(&index_tag_ok); bind(&index_tag_bad); - Throw(kIndexIsTooLarge); + Abort(kIndexIsTooLarge); bind(&index_tag_ok); lw(at, FieldMemOperand(string, String::kLengthOffset)); - ThrowIf(ge, kIndexIsTooLarge, index, Operand(at)); + Check(lt, kIndexIsTooLarge, index, Operand(at)); ASSERT(Smi::FromInt(0) == 0); - ThrowIf(lt, kIndexIsNegative, index, Operand(zero_reg)); + Check(ge, kIndexIsNegative, index, Operand(zero_reg)); SmiUntag(index, index); } diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index c4aedd4b68..078c6df204 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -3392,8 +3392,8 @@ void FullCodeGenerator::EmitOneByteSeqStringSetChar(CallRuntime* expr) { __ pop(index); if (FLAG_debug_code) { - __ ThrowIf(NegateCondition(__ CheckSmi(value)), kNonSmiValue); - __ ThrowIf(NegateCondition(__ CheckSmi(index)), kNonSmiValue); + __ Check(__ CheckSmi(value), kNonSmiValue); + __ Check(__ CheckSmi(index), kNonSmiValue); } __ SmiToInteger32(value, value); @@ -3425,8 +3425,8 @@ void FullCodeGenerator::EmitTwoByteSeqStringSetChar(CallRuntime* expr) { __ pop(index); if (FLAG_debug_code) { - __ ThrowIf(NegateCondition(__ CheckSmi(value)), kNonSmiValue); - __ ThrowIf(NegateCondition(__ CheckSmi(index)), kNonSmiValue); + __ Check(__ CheckSmi(value), kNonSmiValue); + __ Check(__ CheckSmi(index), kNonSmiValue); } __ SmiToInteger32(value, value); diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc index c2e6ba0fd2..4c19fced69 100644 --- a/src/x64/macro-assembler-x64.cc +++ b/src/x64/macro-assembler-x64.cc @@ -4605,7 +4605,7 @@ void MacroAssembler::EmitSeqStringSetCharCheck(Register string, uint32_t encoding_mask) { Label is_object; JumpIfNotSmi(string, &is_object); - Throw(kNonObject); + Abort(kNonObject); bind(&is_object); push(value); @@ -4615,17 +4615,17 @@ void MacroAssembler::EmitSeqStringSetCharCheck(Register string, andb(value, Immediate(kStringRepresentationMask | kStringEncodingMask)); cmpq(value, Immediate(encoding_mask)); pop(value); - ThrowIf(not_equal, kUnexpectedStringType); + Check(equal, kUnexpectedStringType); // The index is assumed to be untagged coming in, tag it to compare with the // string length without using a temp register, it is restored at the end of // this function. Integer32ToSmi(index, index); SmiCompare(index, FieldOperand(string, String::kLengthOffset)); - ThrowIf(greater_equal, kIndexIsTooLarge); + Check(less, kIndexIsTooLarge); SmiCompare(index, Smi::FromInt(0)); - ThrowIf(less, kIndexIsNegative); + Check(greater_equal, kIndexIsNegative); // Restore the index SmiToInteger32(index, index); diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index f9e6fe15d8..572291a63f 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -35,9 +35,6 @@ # BUG(v8:2921). 'debug-step-4-in-frame': [PASS, FAIL, SLOW], - # TODO(mvstanton): Investigate. - 'regress/regress-320948': [SKIP], - ############################################################################## # Fails. 'regress/regress-1119': [FAIL], diff --git a/test/mjsunit/regress/regress-320948.js b/test/mjsunit/regress/regress-320948.js deleted file mode 100644 index 734031c35c..0000000000 --- a/test/mjsunit/regress/regress-320948.js +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright 2013 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: --allow-natives-syntax --debug-code - -var one_byte = %NewString(10, true); -var two_byte = %NewString(10, false); - -function foo1(s, arg1, arg2) { - return %_OneByteSeqStringSetChar(s, arg1, arg2) -} -foo1(one_byte, 0, 0); -assertThrows("{ foo1(4, 0, 0); }"); -assertThrows("{ foo1(one_byte, new Object(), 0); }"); -assertThrows("{ foo1(one_byte, 0, new Object()); }"); -assertThrows("{ foo1(one_byte, 100000, 100; }"); -assertThrows("{ foo1(one_byte, -1, 100; }"); - -function bar1(s, arg1, arg2) { - return %_OneByteSeqStringSetChar(s, arg1, arg2) -} - -bar1(one_byte, 0, 0); -bar1(one_byte, 0, 0); -bar1(one_byte, 0, 0); -%OptimizeFunctionOnNextCall(bar1); -bar1(one_byte, 0, 0); -assertThrows("{ bar1(4, 0, 0); }"); -assertThrows("{ bar1(one_byte, new Object(), 0); }"); -assertThrows("{ bar1(one_byte, 0, new Object()); }"); -assertThrows("{ bar1(one_byte, 100000, 100; }"); -assertThrows("{ bar1(one_byte, -1, 100; }"); - -function foo2(s, arg1, arg2) { - return %_TwoByteSeqStringSetChar(s, arg1, arg2) -} -foo2(two_byte, 0, 0); -assertThrows("{ foo2(4, 0, 0); }"); -assertThrows("{ foo2(two_byte, new Object(), 0); }"); -assertThrows("{ foo2(two_byte, 0, new Object()); }"); -assertThrows("{ foo2(two_byte, 100000, 100; }"); -assertThrows("{ foo2(two_byte, -1, 100; }"); - -function bar2(s, arg1, arg2) { - return %_TwoByteSeqStringSetChar(s, arg1, arg2) -} - -bar2(two_byte, 0, 0); -bar2(two_byte, 0, 0); -bar2(two_byte, 0, 0); -%OptimizeFunctionOnNextCall(bar2); -bar2(two_byte, 0, 0); -assertThrows("{ bar2(4, 0, 0); }"); -assertThrows("{ bar2(two_byte, new Object(), 0); }"); -assertThrows("{ bar2(two_byte, 0, new Object()); }"); -assertThrows("{ bar2(two_byte, 100000, 100; }"); -assertThrows("{ bar2(two_byte, -1, 100; }");