From e0a245bcec744e44b17ae187428136a9f1e1bdaa Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Wed, 26 Nov 2014 16:01:19 -0500 Subject: [PATCH] Bug 1105261 - Revert fresh vectors to not prereserving their inline allocation space, because the guaranteed extent of that space is an implementation detail. r=nbp --- js/src/asmjs/AsmJSLink.cpp | 4 +- js/src/asmjs/AsmJSValidate.cpp | 10 +++- js/src/frontend/TokenStream.cpp | 2 +- js/src/jsreflect.cpp | 4 +- mfbt/Vector.h | 31 ++++++++++--- mfbt/tests/TestVector.cpp | 82 +++++++++++++++++++++++++++++++++ mfbt/tests/moz.build | 1 + 7 files changed, 121 insertions(+), 13 deletions(-) create mode 100644 mfbt/tests/TestVector.cpp diff --git a/js/src/asmjs/AsmJSLink.cpp b/js/src/asmjs/AsmJSLink.cpp index 74c51ce0157..008792b88a6 100644 --- a/js/src/asmjs/AsmJSLink.cpp +++ b/js/src/asmjs/AsmJSLink.cpp @@ -827,7 +827,9 @@ HandleDynamicLinkFailure(JSContext *cx, CallArgs args, AsmJSModule &module, Hand return false; AutoNameVector formals(cx); - formals.reserve(3); + if (!formals.reserve(3)) + return false; + if (module.globalArgumentName()) formals.infallibleAppend(module.globalArgumentName()); if (module.importArgumentName()) diff --git a/js/src/asmjs/AsmJSValidate.cpp b/js/src/asmjs/AsmJSValidate.cpp index e0f19f4fd40..9ce290c1298 100644 --- a/js/src/asmjs/AsmJSValidate.cpp +++ b/js/src/asmjs/AsmJSValidate.cpp @@ -8328,7 +8328,8 @@ GenerateFFIInterpExit(ModuleCompiler &m, const ModuleCompiler::ExitDescriptor &e MIRType_Int32, // argc MIRType_Pointer }; // argv MIRTypeVector invokeArgTypes(m.cx()); - invokeArgTypes.infallibleAppend(typeArray, ArrayLength(typeArray)); + if (!invokeArgTypes.append(typeArray, ArrayLength(typeArray))) + return false; // At the point of the call, the stack layout shall be (sp grows to the left): // | stack args | padding | Value argv[] | padding | retaddr | caller stack args | @@ -8445,7 +8446,9 @@ GenerateFFIIonExit(ModuleCompiler &m, const ModuleCompiler::ExitDescriptor &exit // | stack args | padding | Value argv[1] | ... // The padding between args and argv ensures that argv is aligned. MIRTypeVector coerceArgTypes(m.cx()); - coerceArgTypes.infallibleAppend(MIRType_Pointer); // argv + if (!coerceArgTypes.append(MIRType_Pointer)) // argv + return false; + unsigned offsetToCoerceArgv = AlignBytes(StackArgBytes(coerceArgTypes), sizeof(double)); unsigned totalCoerceBytes = offsetToCoerceArgv + sizeof(Value) + MaybeSavedGlobalReg; unsigned coerceFrameSize = StackDecrementForCall(masm, AsmJSStackAlignment, totalCoerceBytes); @@ -8705,6 +8708,9 @@ GenerateBuiltinThunk(ModuleCompiler &m, AsmJSExit::BuiltinKind builtin) MOZ_ASSERT(masm.framePushed() == 0); MIRTypeVector argTypes(m.cx()); + if (!argTypes.reserve(2)) + return false; + switch (builtin) { case AsmJSExit::Builtin_ToInt32: argTypes.infallibleAppend(MIRType_Int32); diff --git a/js/src/frontend/TokenStream.cpp b/js/src/frontend/TokenStream.cpp index 2372366c853..894a03f50ee 100644 --- a/js/src/frontend/TokenStream.cpp +++ b/js/src/frontend/TokenStream.cpp @@ -151,7 +151,7 @@ TokenStream::SourceCoords::SourceCoords(ExclusiveContext *cx, uint32_t ln) // appends cannot fail because |lineStartOffsets_| has statically-allocated // elements. MOZ_ASSERT(lineStartOffsets_.capacity() >= 2); - (void)lineStartOffsets_.reserve(2); + MOZ_ALWAYS_TRUE(lineStartOffsets_.reserve(2)); lineStartOffsets_.infallibleAppend(0); lineStartOffsets_.infallibleAppend(maxPtr); } diff --git a/js/src/jsreflect.cpp b/js/src/jsreflect.cpp index 8adb48363b9..6522ae7e634 100644 --- a/js/src/jsreflect.cpp +++ b/js/src/jsreflect.cpp @@ -2112,7 +2112,7 @@ ASTSerializer::importDeclaration(ParseNode *pn, MutableHandleValue dst) MOZ_ASSERT(pn->pn_right->isKind(PNK_STRING)); NodeVector elts(cx); - if (!elts.reserve(pn->pn_count)) + if (!elts.reserve(pn->pn_left->pn_count)) return false; for (ParseNode *next = pn->pn_left->pn_head; next; next = next->pn_next) { @@ -2151,7 +2151,7 @@ ASTSerializer::exportDeclaration(ParseNode *pn, MutableHandleValue dst) ParseNode *kid = pn->isKind(PNK_EXPORT) ? pn->pn_kid : pn->pn_left; switch (ParseNodeKind kind = kid->getKind()) { case PNK_EXPORT_SPEC_LIST: - if (!elts.reserve(pn->pn_count)) + if (!elts.reserve(pn->pn_left->pn_count)) return false; for (ParseNode *next = pn->pn_left->pn_head; next; next = next->pn_next) { diff --git a/mfbt/Vector.h b/mfbt/Vector.h index a258227a7e7..d35340c469b 100644 --- a/mfbt/Vector.h +++ b/mfbt/Vector.h @@ -215,6 +215,10 @@ struct VectorImpl } }; +// A struct for TestVector.cpp to access private internal fields. +// DO NOT DEFINE IN YOUR OWN CODE. +struct VectorTesting; + } // namespace detail /* @@ -233,6 +237,8 @@ class VectorBase : private AllocPolicy typedef detail::VectorImpl Impl; friend struct detail::VectorImpl; + friend struct detail::VectorTesting; + bool growStorageBy(size_t aIncr); bool convertToHeapStorage(size_t aNewCap); @@ -327,10 +333,17 @@ class VectorBase : private AllocPolicy } #ifdef DEBUG + /** + * The amount of explicitly allocated space in this vector that is immediately + * available to be filled by appending additional elements. This value is + * always greater than or equal to |length()| -- the vector's actual elements + * are implicitly reserved. This value is always less than or equal to + * |capacity()|. It may be explicitly increased using the |reserve()| method. + */ size_t reserved() const { - MOZ_ASSERT(mReserved <= mCapacity); MOZ_ASSERT(mLength <= mReserved); + MOZ_ASSERT(mReserved <= mCapacity); return mReserved; } #endif @@ -450,8 +463,12 @@ public: bool initCapacity(size_t aRequest); /** - * If reserve(length() + N) succeeds, the N next appends are guaranteed to - * succeed. + * If reserve(aRequest) succeeds and |aRequest >= length()|, then appending + * |aRequest - length()| elements, in any sequence of append/appendAll calls, + * is guaranteed to succeed. + * + * A request to reserve an amount less than the current length does not affect + * reserved space. */ bool reserve(size_t aRequest); @@ -622,7 +639,7 @@ VectorBase::VectorBase(AP aAP) , mLength(0) , mCapacity(kInlineCapacity) #ifdef DEBUG - , mReserved(kInlineCapacity) + , mReserved(0) , mEntered(false) #endif { @@ -662,7 +679,7 @@ VectorBase::VectorBase(TV&& aRhs) aRhs.mCapacity = kInlineCapacity; aRhs.mLength = 0; #ifdef DEBUG - aRhs.mReserved = kInlineCapacity; + aRhs.mReserved = 0; #endif } } @@ -941,7 +958,7 @@ VectorBase::clearAndFree() mBegin = static_cast(mStorage.addr()); mCapacity = kInlineCapacity; #ifdef DEBUG - mReserved = kInlineCapacity; + mReserved = 0; #endif } @@ -1155,7 +1172,7 @@ VectorBase::extractRawBuffer() mLength = 0; mCapacity = kInlineCapacity; #ifdef DEBUG - mReserved = kInlineCapacity; + mReserved = 0; #endif } return ret; diff --git a/mfbt/tests/TestVector.cpp b/mfbt/tests/TestVector.cpp new file mode 100644 index 00000000000..dff589e3f16 --- /dev/null +++ b/mfbt/tests/TestVector.cpp @@ -0,0 +1,82 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "mozilla/Move.h" +#include "mozilla/Vector.h" + +using mozilla::detail::VectorTesting; +using mozilla::Move; +using mozilla::Vector; + +struct mozilla::detail::VectorTesting +{ + static void testReserved(); +}; + +void +mozilla::detail::VectorTesting::testReserved() +{ +#ifdef DEBUG + Vector bv; + MOZ_RELEASE_ASSERT(bv.reserved() == 0); + + MOZ_RELEASE_ASSERT(bv.append(true)); + MOZ_RELEASE_ASSERT(bv.reserved() == 1); + + Vector otherbv; + MOZ_RELEASE_ASSERT(otherbv.append(false)); + MOZ_RELEASE_ASSERT(otherbv.append(true)); + MOZ_RELEASE_ASSERT(bv.appendAll(otherbv)); + MOZ_RELEASE_ASSERT(bv.reserved() == 3); + + MOZ_RELEASE_ASSERT(bv.reserve(5)); + MOZ_RELEASE_ASSERT(bv.reserved() == 5); + + MOZ_RELEASE_ASSERT(bv.reserve(1)); + MOZ_RELEASE_ASSERT(bv.reserved() == 5); + + Vector bv2(Move(bv)); + MOZ_RELEASE_ASSERT(bv.reserved() == 0); + MOZ_RELEASE_ASSERT(bv2.reserved() == 5); + + bv2.clearAndFree(); + MOZ_RELEASE_ASSERT(bv2.reserved() == 0); + + Vector iv; + MOZ_RELEASE_ASSERT(iv.reserved() == 0); + + MOZ_RELEASE_ASSERT(iv.append(17)); + MOZ_RELEASE_ASSERT(iv.reserved() == 1); + + Vector otheriv; + MOZ_RELEASE_ASSERT(otheriv.append(42)); + MOZ_RELEASE_ASSERT(otheriv.append(37)); + MOZ_RELEASE_ASSERT(iv.appendAll(otheriv)); + MOZ_RELEASE_ASSERT(iv.reserved() == 3); + + MOZ_RELEASE_ASSERT(iv.reserve(5)); + MOZ_RELEASE_ASSERT(iv.reserved() == 5); + + MOZ_RELEASE_ASSERT(iv.reserve(1)); + MOZ_RELEASE_ASSERT(iv.reserved() == 5); + + MOZ_RELEASE_ASSERT(iv.reserve(55)); + MOZ_RELEASE_ASSERT(iv.reserved() == 55); + + Vector iv2(Move(iv)); + MOZ_RELEASE_ASSERT(iv.reserved() == 0); + MOZ_RELEASE_ASSERT(iv2.reserved() == 55); + + iv2.clearAndFree(); + MOZ_RELEASE_ASSERT(iv2.reserved() == 0); +#endif +} + +int +main() +{ + VectorTesting::testReserved(); +} diff --git a/mfbt/tests/moz.build b/mfbt/tests/moz.build index 27ed9497d11..fb089729dfb 100644 --- a/mfbt/tests/moz.build +++ b/mfbt/tests/moz.build @@ -30,6 +30,7 @@ CppUnitTests([ 'TestTypedEnum', 'TestTypeTraits', 'TestUniquePtr', + 'TestVector', 'TestWeakPtr', ])