From d53e38777f7bdb8c2afdc4d9d6056f71edda799a Mon Sep 17 00:00:00 2001 From: "mstarzinger@chromium.org" Date: Thu, 28 Nov 2013 09:29:57 +0000 Subject: [PATCH] Fix missing bounds check in n-arguments Array constructor. LOG=N R=mvstanton@chromium.org BUG=v8:3027 TEST=mjsunit/regress/regress-3027 Review URL: https://codereview.chromium.org/92103003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@18116 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/code-stubs-hydrogen.cc | 18 +++++++++++----- test/mjsunit/regress/regress-3027.js | 42 ++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 test/mjsunit/regress/regress-3027.js diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index 894c3d3..f717d75 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -742,6 +742,15 @@ HValue* CodeStubGraphBuilderBase::BuildArraySingleArgumentConstructor( HValue* CodeStubGraphBuilderBase::BuildArrayNArgumentsConstructor( JSArrayBuilder* array_builder, ElementsKind kind) { + // Insert a bounds check because the number of arguments might exceed + // the kInitialMaxFastElementArray limit. This cannot happen for code + // that was parsed, but calling via Array.apply(thisArg, [...]) might + // trigger it. + HValue* length = GetArgumentsLength(); + HConstant* max_alloc_length = + Add(JSObject::kInitialMaxFastElementArray); + HValue* checked_length = Add(length, max_alloc_length); + // We need to fill with the hole if it's a smi array in the multi-argument // case because we might have to bail out while copying arguments into // the array because they aren't compatible with a smi array. @@ -750,12 +759,11 @@ HValue* CodeStubGraphBuilderBase::BuildArrayNArgumentsConstructor( // // TODO(mvstanton): consider an instruction to memset fill the array // with zero in this case instead. - HValue* length = GetArgumentsLength(); JSArrayBuilder::FillMode fill_mode = IsFastSmiElementsKind(kind) ? JSArrayBuilder::FILL_WITH_HOLE : JSArrayBuilder::DONT_FILL_WITH_HOLE; - HValue* new_object = array_builder->AllocateArray(length, - length, + HValue* new_object = array_builder->AllocateArray(checked_length, + checked_length, fill_mode); HValue* elements = array_builder->GetElementsLocation(); ASSERT(elements != NULL); @@ -765,10 +773,10 @@ HValue* CodeStubGraphBuilderBase::BuildArrayNArgumentsConstructor( context(), LoopBuilder::kPostIncrement); HValue* start = graph()->GetConstant0(); - HValue* key = builder.BeginBody(start, length, Token::LT); + HValue* key = builder.BeginBody(start, checked_length, Token::LT); HInstruction* argument_elements = Add(false); HInstruction* argument = Add( - argument_elements, length, key); + argument_elements, checked_length, key); Add(elements, key, argument, kind); builder.EndBody(); diff --git a/test/mjsunit/regress/regress-3027.js b/test/mjsunit/regress/regress-3027.js new file mode 100644 index 0000000..c7ebd53 --- /dev/null +++ b/test/mjsunit/regress/regress-3027.js @@ -0,0 +1,42 @@ +// 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. + +// Test to exceed the Heap::MaxRegularSpaceAllocationSize with an array +// constructor call taking many arguments. + +function boom() { + var args = []; + for (var i = 0; i < 125000; i++) { + args.push(i); + } + return Array.apply(Array, args); +} + +var array = boom(); + +assertEquals(125000, array.length); +assertEquals(124999, array[124999]); -- 2.7.4