From a2d6260cd6930fe249c989949ec4362cd5c61294 Mon Sep 17 00:00:00 2001 From: Job Bautista Date: Tue, 14 Mar 2023 22:35:32 +0800 Subject: [PATCH 1/6] Issue #2046 - Follow-up: Fix deprot in builtin/intl Missing a couple of includes. Tag #80 --- js/src/builtin/intl/PluralRules.cpp | 1 + js/src/builtin/intl/SharedIntlData.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/js/src/builtin/intl/PluralRules.cpp b/js/src/builtin/intl/PluralRules.cpp index 78a2f3a847..cc61ff8885 100644 --- a/js/src/builtin/intl/PluralRules.cpp +++ b/js/src/builtin/intl/PluralRules.cpp @@ -6,6 +6,7 @@ /* Implementation of the Intl.PluralRules proposal. */ +#include "builtin/intl/NumberFormat.h" #include "builtin/intl/PluralRules.h" #include "mozilla/Assertions.h" diff --git a/js/src/builtin/intl/SharedIntlData.cpp b/js/src/builtin/intl/SharedIntlData.cpp index 8ae3c17fe2..fbcf134daf 100644 --- a/js/src/builtin/intl/SharedIntlData.cpp +++ b/js/src/builtin/intl/SharedIntlData.cpp @@ -14,6 +14,7 @@ #include #include "jsatom.h" +#include "jscntxt.h" #include "jsstr.h" #include "builtin/intl/CommonFunctions.h" From 3d2a4f4066994958e2a47a8715b7ea420a9351a3 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Tue, 14 Mar 2023 15:28:51 -0500 Subject: [PATCH 2/6] Issue #2160 - Initial support for notarizing during DMG package. Added --with-macbundle-idenity configure option to set a codesign identity. If no identity is set or cross-compiling from Linux no codesigning will be done. Currently doing a full deep bundle v2 sign, instead of limited v1. --- build/moz.configure/old.configure | 1 + old-configure.in | 10 ++++++++++ python/mozbuild/mozpack/dmg.py | 6 ++++++ 3 files changed, 17 insertions(+) diff --git a/build/moz.configure/old.configure b/build/moz.configure/old.configure index 63e59941eb..3186c2c76a 100644 --- a/build/moz.configure/old.configure +++ b/build/moz.configure/old.configure @@ -278,6 +278,7 @@ def old_configure_options(*options): '--with-ios-sdk', '--with-jitreport-granularity', '--with-macbundlename-prefix', + '--with-macbundle-identity', '--with-macos-private-frameworks', '--with-macos-sdk', '--with-nspr-cflags', diff --git a/old-configure.in b/old-configure.in index 5fea6d4c2c..32d03d4104 100644 --- a/old-configure.in +++ b/old-configure.in @@ -4983,6 +4983,16 @@ fi AC_DEFINE_UNQUOTED(MOZ_MACBUNDLE_ID,$MOZ_MACBUNDLE_ID) AC_SUBST(MOZ_MACBUNDLE_ID) +dnl ======================================================== +dnl = Mac bundle codesign identity +dnl ======================================================== +MOZ_ARG_WITH_STRING(macbundle-identity, +[ --with-macbundle-identity=identity + Identity to codesign the Mac application bundle], +[ MOZ_MACBUNDLE_IDENTITY="$withval"]) + +AC_SUBST(MOZ_MACBUNDLE_IDENTITY) + dnl ======================================================== dnl = Child Process Name for IPC dnl ======================================================== diff --git a/python/mozbuild/mozpack/dmg.py b/python/mozbuild/mozpack/dmg.py index 0363022144..ade25aeac3 100644 --- a/python/mozbuild/mozpack/dmg.py +++ b/python/mozbuild/mozpack/dmg.py @@ -102,6 +102,7 @@ def create_dmg(source_directory, output_dmg, volume_name, extra_files): if is_linux: check_tools('DMG_TOOL', 'GENISOIMAGE') with mozfile.TemporaryDirectory() as tmpdir: + import buildconfig stagedir = os.path.join(tmpdir, 'stage') os.mkdir(stagedir) # Copy the app bundle over using rsync @@ -118,4 +119,9 @@ def create_dmg(source_directory, output_dmg, volume_name, extra_files): # Set the folder attributes to use a custom icon set_folder_icon(stagedir) chmod(stagedir) + if not is_linux: + identity = buildconfig.substs['MOZ_MACBUNDLE_IDENTITY'] + if identity != '': + appbundle = os.path.join(stagedir, buildconfig.substs['MOZ_MACBUNDLE_NAME']) + subprocess.check_call(['codesign', '--deep', '-s', identity, appbundle]) create_dmg_from_staged(stagedir, output_dmg, tmpdir, volume_name) From ad8d9639a40db5153851eace93944a6b5135cd2d Mon Sep 17 00:00:00 2001 From: Job Bautista Date: Wed, 8 Mar 2023 18:59:45 +0800 Subject: [PATCH 3/6] Issue #2148 - Don't null out RegExpObject -> RegExpShared pointer on GC. Follow-up for issue #2083. Taken from Mozilla bug 1378736. --- js/src/vm/RegExpObject.cpp | 5 ----- js/src/vm/RegExpObject.h | 6 +++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/js/src/vm/RegExpObject.cpp b/js/src/vm/RegExpObject.cpp index 61baadef81..00d4222272 100644 --- a/js/src/vm/RegExpObject.cpp +++ b/js/src/vm/RegExpObject.cpp @@ -187,11 +187,6 @@ IsMarkingTrace(JSTracer* trc) void RegExpObject::trace(JSTracer* trc) { - // When marking the object normally we have the option of unlinking the - // object from its RegExpShared so that the RegExpShared may be collected. - if (IsMarkingTrace(trc) && !zone()->isPreservingCode()) - sharedRef() = nullptr; - TraceNullableEdge(trc, &sharedRef(), "RegExpObject shared"); } diff --git a/js/src/vm/RegExpObject.h b/js/src/vm/RegExpObject.h index 4f35908087..f4adf76e83 100644 --- a/js/src/vm/RegExpObject.h +++ b/js/src/vm/RegExpObject.h @@ -464,7 +464,7 @@ class RegExpObject : public NativeObject void setShared(RegExpShared& shared) { MOZ_ASSERT(!hasShared()); - sharedRef() = &shared; + sharedRef().init(&shared); } static void trace(JSTracer* trc, JSObject* obj); @@ -490,9 +490,9 @@ class RegExpObject : public NativeObject static MOZ_MUST_USE bool createShared(JSContext* cx, Handle regexp, MutableHandleRegExpShared shared); - ReadBarriered& sharedRef() { + PreBarriered& sharedRef() { auto& ref = NativeObject::privateRef(PRIVATE_SLOT); - return reinterpret_cast&>(ref); + return reinterpret_cast&>(ref); } /* Call setShared in preference to setPrivate. */ From cdd91edd79839107b0ed336a3b9eece3ddcde698 Mon Sep 17 00:00:00 2001 From: Job Bautista Date: Wed, 8 Mar 2023 19:23:13 +0800 Subject: [PATCH 4/6] Issue #2148 - Discard RegExpShared data tables when discarding regexp JIT code. Based on Mozilla bug 1378736's part 2. Should fix the double-free. Follow-up for #2083 --- .../irregexp/NativeRegExpMacroAssembler.cpp | 20 ++++++++++----- js/src/irregexp/NativeRegExpMacroAssembler.h | 8 +++--- js/src/irregexp/RegExpEngine.cpp | 24 +++++++++--------- js/src/irregexp/RegExpEngine.h | 2 +- js/src/irregexp/RegExpMacroAssembler.cpp | 8 +++--- js/src/irregexp/RegExpMacroAssembler.h | 16 ++++-------- js/src/vm/RegExpObject.cpp | 25 ++++++++++++++----- js/src/vm/RegExpObject.h | 8 +++--- 8 files changed, 64 insertions(+), 47 deletions(-) diff --git a/js/src/irregexp/NativeRegExpMacroAssembler.cpp b/js/src/irregexp/NativeRegExpMacroAssembler.cpp index 23d3b73ab1..7787fb07ee 100644 --- a/js/src/irregexp/NativeRegExpMacroAssembler.cpp +++ b/js/src/irregexp/NativeRegExpMacroAssembler.cpp @@ -63,10 +63,11 @@ using namespace js::jit; * The tempN registers are free to use for computations. */ -NativeRegExpMacroAssembler::NativeRegExpMacroAssembler(JSContext* cx, LifoAlloc* alloc, RegExpShared* shared, - JSRuntime* rt, Mode mode, int registers_to_save) - : RegExpMacroAssembler(cx, *alloc, shared, registers_to_save), - cx(cx), runtime(rt), mode_(mode) +NativeRegExpMacroAssembler::NativeRegExpMacroAssembler(LifoAlloc* alloc, JSRuntime* rt, + Mode mode, int registers_to_save, + RegExpShared::JitCodeTables& tables) + : RegExpMacroAssembler(*alloc, registers_to_save), + tables(tables), runtime(rt), mode_(mode) { // Find physical registers for each compiler register. AllocatableGeneralRegisterSet regs(GeneralRegisterSet::All()); @@ -886,11 +887,11 @@ NativeRegExpMacroAssembler::CheckCharacterNotInRange(char16_t from, char16_t to, } void -NativeRegExpMacroAssembler::CheckBitInTable(uint8_t* table, Label* on_bit_set) +NativeRegExpMacroAssembler::CheckBitInTable(RegExpShared::JitCodeTable table, Label* on_bit_set) { JitSpew(SPEW_PREFIX "CheckBitInTable"); - masm.movePtr(ImmPtr(table), temp0); + masm.movePtr(ImmPtr(table.get()), temp0); // kTableMask is currently 127, so we need to mask even if the input is // Latin1. V8 has the same issue. @@ -901,6 +902,13 @@ NativeRegExpMacroAssembler::CheckBitInTable(uint8_t* table, Label* on_bit_set) masm.load8ZeroExtend(BaseIndex(temp0, temp1, TimesOne), temp0); masm.branchTest32(Assembler::NonZero, temp0, temp0, BranchOrBacktrack(on_bit_set)); + + // Transfer ownership of |table| to the |tables| Vector. + { + AutoEnterOOMUnsafeRegion oomUnsafe; + if (!tables.append(Move(table))) + oomUnsafe.crash("RegExp table append"); + } } void diff --git a/js/src/irregexp/NativeRegExpMacroAssembler.h b/js/src/irregexp/NativeRegExpMacroAssembler.h index 78d2e02295..ec22ec7967 100644 --- a/js/src/irregexp/NativeRegExpMacroAssembler.h +++ b/js/src/irregexp/NativeRegExpMacroAssembler.h @@ -87,8 +87,8 @@ class MOZ_STACK_CLASS NativeRegExpMacroAssembler final : public RegExpMacroAssem // Type of input string to generate code for. enum Mode { ASCII = 1, CHAR16 = 2 }; - NativeRegExpMacroAssembler(JSContext* cx, LifoAlloc* alloc, RegExpShared* shared, - JSRuntime* rt, Mode mode, int registers_to_save); + NativeRegExpMacroAssembler(LifoAlloc* alloc, JSRuntime* rt, Mode mode, int registers_to_save, + RegExpShared::JitCodeTables& tables); // Inherited virtual methods. RegExpCode GenerateCode(JSContext* cx, bool match_only); @@ -116,7 +116,7 @@ class MOZ_STACK_CLASS NativeRegExpMacroAssembler final : public RegExpMacroAssem jit::Label* on_in_range); void CheckCharacterNotInRange(char16_t from, char16_t to, jit::Label* on_not_in_range); - void CheckBitInTable(uint8_t* table, jit::Label* on_bit_set); + void CheckBitInTable(RegExpShared::JitCodeTable table, jit::Label* on_bit_set); void CheckPosition(int cp_offset, jit::Label* on_outside_input); void JumpOrBacktrack(jit::Label* to); bool CheckSpecialCharacterClass(char16_t type, jit::Label* on_no_match); @@ -173,8 +173,8 @@ class MOZ_STACK_CLASS NativeRegExpMacroAssembler final : public RegExpMacroAssem private: jit::MacroAssembler masm; + RegExpShared::JitCodeTables& tables; - JSContext* cx; JSRuntime* runtime; Mode mode_; jit::Label entry_label_; diff --git a/js/src/irregexp/RegExpEngine.cpp b/js/src/irregexp/RegExpEngine.cpp index 58757efd9b..75c6f4547a 100644 --- a/js/src/irregexp/RegExpEngine.cpp +++ b/js/src/irregexp/RegExpEngine.cpp @@ -1262,7 +1262,7 @@ RegExpCode irregexp::CompilePattern(JSContext* cx, HandleRegExpShared shared, RegExpCompileData* data, HandleLinearString sample, bool is_global, bool ignore_case, bool is_ascii, bool match_only, bool force_bytecode, bool sticky, - bool unicode) + bool unicode, RegExpShared::JitCodeTables& tables) { if ((data->capture_count + 1) * 2 - 1 > RegExpMacroAssembler::kMaxRegister) { JS_ReportErrorASCII(cx, "regexp too big"); @@ -1350,10 +1350,10 @@ irregexp::CompilePattern(JSContext* cx, HandleRegExpShared shared, RegExpCompile : NativeRegExpMacroAssembler::CHAR16; ctx.emplace(cx, (jit::TempAllocator*) nullptr); - native_assembler.emplace(cx, &alloc, shared, cx->runtime(), mode, (data->capture_count + 1) * 2); + native_assembler.emplace(&alloc, cx->runtime(), mode, (data->capture_count + 1) * 2, tables); assembler = native_assembler.ptr(); } else { - interpreted_assembler.emplace(cx, &alloc, shared, (data->capture_count + 1) * 2); + interpreted_assembler.emplace(&alloc, (data->capture_count + 1) * 2); assembler = interpreted_assembler.ptr(); } @@ -2040,21 +2040,21 @@ BoyerMooreLookahead::EmitSkipInstructions(RegExpMacroAssembler* masm) return true; } - uint8_t* boolean_skip_table; + RegExpShared::JitCodeTable boolean_skip_table; { AutoEnterOOMUnsafeRegion oomUnsafe; - boolean_skip_table = static_cast(js_malloc(kSize)); - if (!boolean_skip_table || !masm->shared->addTable(boolean_skip_table)) + boolean_skip_table.reset(static_cast(js_malloc(kSize))); + if (!boolean_skip_table) oomUnsafe.crash("Table malloc"); } - int skip_distance = GetSkipTable(min_lookahead, max_lookahead, boolean_skip_table); + int skip_distance = GetSkipTable(min_lookahead, max_lookahead, boolean_skip_table.get()); MOZ_ASSERT(skip_distance != 0); jit::Label cont, again; masm->Bind(&again); masm->LoadCurrentCharacter(max_lookahead, &cont, true); - masm->CheckBitInTable(boolean_skip_table, &cont); + masm->CheckBitInTable(Move(boolean_skip_table), &cont); masm->AdvanceCurrentPosition(skip_distance); masm->JumpOrBacktrack(&again); masm->Bind(&cont); @@ -2832,18 +2832,18 @@ EmitUseLookupTable(RegExpMacroAssembler* masm, } // TODO(erikcorry): Cache these. - uint8_t* ba; + RegExpShared::JitCodeTable ba; { AutoEnterOOMUnsafeRegion oomUnsafe; - ba = static_cast(js_malloc(kSize)); - if (!ba || !masm->shared->addTable(ba)) + ba.reset(static_cast(js_malloc(kSize))); + if (!ba) oomUnsafe.crash("Table malloc"); } for (int i = 0; i < kSize; i++) ba[i] = templ[i]; - masm->CheckBitInTable(ba, on_bit_set); + masm->CheckBitInTable(Move(ba), on_bit_set); if (on_bit_clear != fall_through) masm->JumpOrBacktrack(on_bit_clear); } diff --git a/js/src/irregexp/RegExpEngine.h b/js/src/irregexp/RegExpEngine.h index 6b3f6afc2a..42462c7542 100644 --- a/js/src/irregexp/RegExpEngine.h +++ b/js/src/irregexp/RegExpEngine.h @@ -106,7 +106,7 @@ RegExpCode CompilePattern(JSContext* cx, HandleRegExpShared shared, RegExpCompileData* data, HandleLinearString sample, bool is_global, bool ignore_case, bool is_ascii, bool match_only, bool force_bytecode, bool sticky, - bool unicode); + bool unicode, RegExpShared::JitCodeTables& tables); // Note: this may return RegExpRunStatus_Error if an interrupt was requested // while the code was executing. diff --git a/js/src/irregexp/RegExpMacroAssembler.cpp b/js/src/irregexp/RegExpMacroAssembler.cpp index ad61e0fee5..034fe8e0ab 100644 --- a/js/src/irregexp/RegExpMacroAssembler.cpp +++ b/js/src/irregexp/RegExpMacroAssembler.cpp @@ -96,10 +96,9 @@ irregexp::CaseInsensitiveCompareUCStrings(const char16_t* substring1, const char16_t* substring2, size_t byteLength); -InterpretedRegExpMacroAssembler::InterpretedRegExpMacroAssembler(JSContext* cx, LifoAlloc* alloc, - RegExpShared* shared, +InterpretedRegExpMacroAssembler::InterpretedRegExpMacroAssembler(LifoAlloc* alloc, size_t numSavedRegisters) - : RegExpMacroAssembler(cx, *alloc, shared, numSavedRegisters), + : RegExpMacroAssembler(*alloc, numSavedRegisters), pc_(0), advance_current_start_(0), advance_current_offset_(0), @@ -317,7 +316,8 @@ InterpretedRegExpMacroAssembler::CheckCharacterNotInRange(char16_t from, char16_ } void -InterpretedRegExpMacroAssembler::CheckBitInTable(uint8_t* table, jit::Label* on_bit_set) +InterpretedRegExpMacroAssembler::CheckBitInTable(RegExpShared::JitCodeTable table, + jit::Label* on_bit_set) { static const int kBitsPerByte = 8; diff --git a/js/src/irregexp/RegExpMacroAssembler.h b/js/src/irregexp/RegExpMacroAssembler.h index e0499cdfbe..fd055c7f20 100644 --- a/js/src/irregexp/RegExpMacroAssembler.h +++ b/js/src/irregexp/RegExpMacroAssembler.h @@ -40,14 +40,12 @@ namespace irregexp { class MOZ_STACK_CLASS RegExpMacroAssembler { public: - RegExpMacroAssembler(JSContext* cx, LifoAlloc& alloc, RegExpShared* shared, - size_t numSavedRegisters) + RegExpMacroAssembler(LifoAlloc& alloc, size_t numSavedRegisters) : slow_safe_compiler_(false), global_mode_(NOT_GLOBAL), alloc_(alloc), num_registers_(numSavedRegisters), - num_saved_registers_(numSavedRegisters), - shared(cx, shared) + num_saved_registers_(numSavedRegisters) {} enum StackCheckFlag { @@ -137,7 +135,7 @@ class MOZ_STACK_CLASS RegExpMacroAssembler // The current character (modulus the kTableSize) is looked up in the byte // array, and if the found byte is non-zero, we jump to the on_bit_set label. - virtual void CheckBitInTable(uint8_t* table, jit::Label* on_bit_set) = 0; + virtual void CheckBitInTable(RegExpShared::JitCodeTable table, jit::Label* on_bit_set) = 0; // Checks whether the given offset from the current position is before // the end of the string. May overwrite the current character. @@ -213,9 +211,6 @@ class MOZ_STACK_CLASS RegExpMacroAssembler if (num_registers_ <= reg) num_registers_ = reg + 1; } - - public: - RootedRegExpShared shared; }; template @@ -230,8 +225,7 @@ CaseInsensitiveCompareUCStrings(const CharT* substring1, const CharT* substring2 class MOZ_STACK_CLASS InterpretedRegExpMacroAssembler final : public RegExpMacroAssembler { public: - InterpretedRegExpMacroAssembler(JSContext* cx, LifoAlloc* alloc, RegExpShared* shared, - size_t numSavedRegisters); + InterpretedRegExpMacroAssembler(LifoAlloc* alloc, size_t numSavedRegisters); ~InterpretedRegExpMacroAssembler(); // Inherited virtual methods. @@ -258,7 +252,7 @@ class MOZ_STACK_CLASS InterpretedRegExpMacroAssembler final : public RegExpMacro jit::Label* on_in_range); void CheckCharacterNotInRange(char16_t from, char16_t to, jit::Label* on_not_in_range); - void CheckBitInTable(uint8_t* table, jit::Label* on_bit_set); + void CheckBitInTable(RegExpShared::JitCodeTable table, jit::Label* on_bit_set); void JumpOrBacktrack(jit::Label* to); void Fail(); void IfRegisterGE(int reg, int comparand, jit::Label* if_ge); diff --git a/js/src/vm/RegExpObject.cpp b/js/src/vm/RegExpObject.cpp index 00d4222272..66bd9aba36 100644 --- a/js/src/vm/RegExpObject.cpp +++ b/js/src/vm/RegExpObject.cpp @@ -965,6 +965,9 @@ RegExpShared::discardJitCode() { for (auto& comp : compilationArray) comp.jitCode = nullptr; + + // We can also purge the tables used by JIT code. + tables.clearAndFree(); } void @@ -972,8 +975,6 @@ RegExpShared::finalize(FreeOp* fop) { for (auto& comp : compilationArray) js_free(comp.byteCode); - for (size_t i = 0; i < tables.length(); i++) - js_free(tables[i]); tables.~JitCodeTables(); } @@ -1070,6 +1071,7 @@ RegExpShared::compile(JSContext* cx, MutableHandleRegExpShared re, HandleAtom pa } } + JitCodeTables tables; irregexp::RegExpCode code = irregexp::CompilePattern(cx, re, &data, input, false /* global() */, re->ignoreCase(), @@ -1077,7 +1079,8 @@ RegExpShared::compile(JSContext* cx, MutableHandleRegExpShared re, HandleAtom pa mode == MatchOnly, force == ForceByteCode, re->sticky(), - re->unicode()); + re->unicode(), + tables); if (code.empty()) return false; @@ -1085,10 +1088,20 @@ RegExpShared::compile(JSContext* cx, MutableHandleRegExpShared re, HandleAtom pa MOZ_ASSERT_IF(force == ForceByteCode, code.byteCode); RegExpCompilation& compilation = re->compilation(mode, input->hasLatin1Chars()); - if (code.jitCode) + if (code.jitCode) { + // First copy the tables. GC can purge the tables if the RegExpShared + // has no JIT code, so it's important to do this right before setting + // compilation.jitCode (to ensure no purging happens between adding the + // tables and setting the JIT code). + for (size_t i = 0; i < tables.length(); i++) { + if (!re->addTable(Move(tables[i]))) + return false; + } compilation.jitCode = code.jitCode; - else if (code.byteCode) + } else if (code.byteCode) { + MOZ_ASSERT(tables.empty(), "RegExpInterpreter does not use data tables"); compilation.byteCode = code.byteCode; + } return true; } @@ -1244,7 +1257,7 @@ RegExpShared::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) n += tables.sizeOfExcludingThis(mallocSizeOf); for (size_t i = 0; i < tables.length(); i++) - n += mallocSizeOf(tables[i]); + n += mallocSizeOf(tables[i].get()); return n; } diff --git a/js/src/vm/RegExpObject.h b/js/src/vm/RegExpObject.h index f4adf76e83..14ec8509ee 100644 --- a/js/src/vm/RegExpObject.h +++ b/js/src/vm/RegExpObject.h @@ -109,6 +109,9 @@ class RegExpShared : public gc::TenuredCell ForceByteCode }; + using JitCodeTable = UniquePtr; + using JitCodeTables = Vector; + private: friend class RegExpCompartment; friend class RegExpStatics; @@ -148,7 +151,6 @@ class RegExpShared : public gc::TenuredCell } // Tables referenced by JIT code. - using JitCodeTables = Vector; JitCodeTables tables; /* Internal functions. */ @@ -181,8 +183,8 @@ class RegExpShared : public gc::TenuredCell MatchPairs* matches, size_t* endIndex); // Register a table with this RegExpShared, and take ownership. - bool addTable(uint8_t* table) { - return tables.append(table); + bool addTable(JitCodeTable table) { + return tables.append(Move(table)); } /* Accessors */ From 0365f940fe2dadff265a9a716414c556f6e0e817 Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Thu, 9 Mar 2023 14:45:24 +0800 Subject: [PATCH 5/6] Issue #2148 - Make Vector not use AlignedStorage for its inline element storage See Bug 1338374 1/2 --- js/src/jit/RangeAnalysis.h | 8 +- js/src/jsapi-tests/tests.h | 35 ++++-- mfbt/Vector.h | 101 +++++++++++------- security/certverifier/CTLogVerifier.cpp | 11 +- .../certverifier/SignedCertificateTimestamp.h | 3 + 5 files changed, 109 insertions(+), 49 deletions(-) diff --git a/js/src/jit/RangeAnalysis.h b/js/src/jit/RangeAnalysis.h index cc8d5b5481..16fe93e000 100644 --- a/js/src/jit/RangeAnalysis.h +++ b/js/src/jit/RangeAnalysis.h @@ -46,7 +46,8 @@ struct LoopIterationBound : public TempObject // of the loop header. This will use loop invariant terms and header phis. LinearSum currentSum; - LoopIterationBound(MBasicBlock* header, MTest* test, LinearSum boundSum, LinearSum currentSum) + LoopIterationBound(MBasicBlock* header, MTest* test, + const LinearSum& boundSum, const LinearSum& currentSum) : header(header), test(test), boundSum(boundSum), currentSum(currentSum) { @@ -59,7 +60,7 @@ typedef Vector LoopIterationBoundVect struct SymbolicBound : public TempObject { private: - SymbolicBound(LoopIterationBound* loop, LinearSum sum) + SymbolicBound(LoopIterationBound* loop, const LinearSum& sum) : loop(loop), sum(sum) { } @@ -73,7 +74,8 @@ struct SymbolicBound : public TempObject // If nullptr, then 'sum' is always valid. LoopIterationBound* loop; - static SymbolicBound* New(TempAllocator& alloc, LoopIterationBound* loop, LinearSum sum) { + static SymbolicBound* + New(TempAllocator& alloc, LoopIterationBound* loop, const LinearSum& sum) { return new(alloc) SymbolicBound(loop, sum); } diff --git a/js/src/jsapi-tests/tests.h b/js/src/jsapi-tests/tests.h index 2fba9920a7..4d30ba8c85 100644 --- a/js/src/jsapi-tests/tests.h +++ b/js/src/jsapi-tests/tests.h @@ -23,6 +23,7 @@ /* Note: Aborts on OOM. */ class JSAPITestString { js::Vector chars; + public: JSAPITestString() {} explicit JSAPITestString(const char* s) { *this += s; } @@ -32,21 +33,34 @@ class JSAPITestString { const char* end() const { return chars.end(); } size_t length() const { return chars.length(); } - JSAPITestString & operator +=(const char* s) { + JSAPITestString& operator +=(const char* s) { if (!chars.append(s, strlen(s))) abort(); return *this; } - JSAPITestString & operator +=(const JSAPITestString& s) { + JSAPITestString& operator +=(const JSAPITestString& s) { if (!chars.append(s.begin(), s.length())) abort(); return *this; } }; -inline JSAPITestString operator+(JSAPITestString a, const char* b) { return a += b; } -inline JSAPITestString operator+(JSAPITestString a, const JSAPITestString& b) { return a += b; } +inline JSAPITestString +operator+(const JSAPITestString& a, const char* b) +{ + JSAPITestString result = a; + result += b; + return result; +} + +inline JSAPITestString +operator+(const JSAPITestString& a, const JSAPITestString& b) +{ + JSAPITestString result = a; + result += b; + return result; +} class JSAPITest { @@ -205,7 +219,11 @@ class JSAPITest return fail(JSAPITestString("CHECK failed: " #expr), __FILE__, __LINE__); \ } while (false) - bool fail(JSAPITestString msg = JSAPITestString(), const char* filename = "-", int lineno = 0) { + bool fail(const JSAPITestString& msg = JSAPITestString(), + const char* filename = "-", + int lineno = 0) + { + JSAPITestString message = msg; if (JS_IsExceptionPending(cx)) { js::gc::AutoSuppressGC gcoff(cx); JS::RootedValue v(cx); @@ -215,11 +233,12 @@ class JSAPITest if (s) { JSAutoByteString bytes(cx, s); if (!!bytes) - msg += bytes.ptr(); + message += bytes.ptr(); } } - fprintf(stderr, "%s:%d:%.*s\n", filename, lineno, (int) msg.length(), msg.begin()); - msgs += msg; + fprintf(stderr, "%s:%d:%.*s\n", + filename, lineno, int(message.length()), message.begin()); + msgs += message; return false; } diff --git a/mfbt/Vector.h b/mfbt/Vector.h index cda406b30a..b8fc4617bc 100644 --- a/mfbt/Vector.h +++ b/mfbt/Vector.h @@ -277,7 +277,7 @@ struct VectorTesting; template -class Vector final : private AllocPolicy +class MOZ_NON_PARAM Vector final : private AllocPolicy { /* utilities */ @@ -293,36 +293,39 @@ class Vector final : private AllocPolicy /* magic constants */ - static const int kMaxInlineBytes = 1024; - - /* compute constants */ - - /* - * Consider element size to be 1 for buffer sizing if there are 0 inline - * elements. This allows us to compile when the definition of the element - * type is not visible here. +/** + * The maximum space allocated for inline element storage. * - * Explicit specialization is only allowed at namespace scope, so in order - * to keep everything here, we use a dummy template parameter with partial - * specialization. + * We reduce space by what the AllocPolicy base class and prior Vector member + * fields likely consume to attempt to play well with binary size classes. */ - template - struct ElemSize - { - static const size_t value = sizeof(T); - }; - template - struct ElemSize<0, Dummy> - { - static const size_t value = 1; + static constexpr size_t kMaxInlineBytes = + 1024 - + (sizeof(AllocPolicy) + sizeof(T*) + sizeof(size_t) + sizeof(size_t)); + + /** + * The number of T elements of inline capacity built into this Vector. This + * is usually |MinInlineCapacity|, but it may be less (or zero!) for large T. + * + * We use a partially-specialized template (not explicit specialization, which + * is only allowed at namespace scope) to compute this value. The benefit is + * that |sizeof(T)| need not be computed, and |T| doesn't have to be fully + * defined at the time |Vector| appears, if no inline storage is requested. + */ + template + struct ComputeCapacity { + static constexpr size_t value = + tl::Min::value; }; - static const size_t kInlineCapacity = - tl::Min::value>::value; + template + struct ComputeCapacity<0, Dummy> { + static constexpr size_t value = 0; + }; - /* Calculate inline buffer size; avoid 0-sized array. */ - static const size_t kInlineBytes = - tl::Max<1, kInlineCapacity * ElemSize::value>::value; + /** The actual inline capacity in number of elements T. This may be zero! */ + static constexpr size_t kInlineCapacity = + ComputeCapacity::value; /* member data */ @@ -346,8 +349,34 @@ class Vector final : private AllocPolicy size_t mReserved; #endif - /* Memory used for inline storage. */ - AlignedStorage mStorage; + /* + * Memory used for inline storage. We want basically this: + * + * alignas(T) unsigned char storage[kInlineCapacity * sizeof(T)]; + * + * but C++ forbids zero-sized arrays that might result if we did this. We fix + * this by (again) using partial specialization, defining an array only if + * contains at least one element. + */ + template + struct InlineStorage + { + alignas(T) unsigned char mBytes[Capacity * sizeof(T)]; + + // GCC fails due to -Werror=strict-aliasing if |mBytes| is directly cast to + // T*. Indirecting through this function addresses the problem. + void* data() { return mBytes; } + + T* addr() { return static_cast(data()); } + }; + + template + struct InlineStorage<0, Dummy> + { + T* addr() { return nullptr; } + }; + + InlineStorage mStorage; #ifdef DEBUG friend class ReentrancyGuard; @@ -363,7 +392,7 @@ class Vector final : private AllocPolicy T* inlineStorage() { - return static_cast(mStorage.addr()); + return mStorage.addr(); } T* beginNoCheck() const @@ -771,7 +800,7 @@ Vector::Vector(AP aAP) , mEntered(false) #endif { - mBegin = static_cast(mStorage.addr()); + mBegin = inlineStorage(); } /* Move constructor. */ @@ -791,7 +820,7 @@ Vector::Vector(Vector&& aRhs) if (aRhs.usingInlineStorage()) { /* We can't move the buffer over in this case, so copy elements. */ - mBegin = static_cast(mStorage.addr()); + mBegin = inlineStorage(); Impl::moveConstruct(mBegin, aRhs.beginNoCheck(), aRhs.endNoCheck()); /* * Leave aRhs's mLength, mBegin, mCapacity, and mReserved as they are. @@ -803,7 +832,7 @@ Vector::Vector(Vector&& aRhs) * in-line storage. */ mBegin = aRhs.mBegin; - aRhs.mBegin = static_cast(aRhs.mStorage.addr()); + aRhs.mBegin = aRhs.inlineStorage(); aRhs.mCapacity = kInlineCapacity; aRhs.mLength = 0; #ifdef DEBUG @@ -1142,7 +1171,7 @@ Vector::clearAndFree() return; } this->free_(beginNoCheck()); - mBegin = static_cast(mStorage.addr()); + mBegin = inlineStorage(); mCapacity = kInlineCapacity; #ifdef DEBUG mReserved = 0; @@ -1370,7 +1399,7 @@ Vector::extractRawBuffer() } T* ret = mBegin; - mBegin = static_cast(mStorage.addr()); + mBegin = inlineStorage(); mLength = 0; mCapacity = kInlineCapacity; #ifdef DEBUG @@ -1396,7 +1425,7 @@ Vector::extractOrCopyRawBuffer() Impl::moveConstruct(copy, beginNoCheck(), endNoCheck()); Impl::destroy(beginNoCheck(), endNoCheck()); - mBegin = static_cast(mStorage.addr()); + mBegin = inlineStorage(); mLength = 0; mCapacity = kInlineCapacity; #ifdef DEBUG @@ -1424,7 +1453,7 @@ Vector::replaceRawBuffer(T* aP, size_t aLength) * otherwise be acceptable. Maybe this behaviour should be * specifiable with an argument to this function. */ - mBegin = static_cast(mStorage.addr()); + mBegin = inlineStorage(); mLength = aLength; mCapacity = kInlineCapacity; Impl::moveConstruct(mBegin, aP, aP + aLength); diff --git a/security/certverifier/CTLogVerifier.cpp b/security/certverifier/CTLogVerifier.cpp index 202e4b4acd..98e1156dd1 100644 --- a/security/certverifier/CTLogVerifier.cpp +++ b/security/certverifier/CTLogVerifier.cpp @@ -172,10 +172,17 @@ CTLogVerifier::Verify(const LogEntry& entry, if (rv != Success) { return rv; } + + // sct.extensions may be empty. If it is, sctExtensionsInput will remain in + // its default state, which is valid but of length 0. Input sctExtensionsInput; rv = BufferToInput(sct.extensions, sctExtensionsInput); - if (rv != Success) { - return rv; + if (sct.extensions.length() > 0) { + rv = sctExtensionsInput.Init(sct.extensions.begin(), + sct.extensions.length()); + if (rv != Success) { + return rv; + } } Buffer serializedData; diff --git a/security/certverifier/SignedCertificateTimestamp.h b/security/certverifier/SignedCertificateTimestamp.h index 468ffbaded..e70af60cd5 100644 --- a/security/certverifier/SignedCertificateTimestamp.h +++ b/security/certverifier/SignedCertificateTimestamp.h @@ -115,6 +115,9 @@ struct SignedCertificateTimestamp inline pkix::Result BufferToInput(const Buffer& buffer, pkix::Input& input) { + if (buffer.length() == 0) { + return pkix::Result::FATAL_ERROR_LIBRARY_FAILURE; + } return input.Init(buffer.begin(), buffer.length()); } From 481069044d530d1f6e1ab4ef7cf90fe5d2376adf Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Thu, 9 Mar 2023 15:10:02 +0800 Subject: [PATCH 6/6] Issue #2148 - Shrink Vector from (usually) four pointers in size to three when no inline storage is used. See Bug 1338374 --- mfbt/Vector.h | 207 +++++++++++++++++++++++--------------- mfbt/tests/TestVector.cpp | 43 ++++++++ 2 files changed, 168 insertions(+), 82 deletions(-) diff --git a/mfbt/Vector.h b/mfbt/Vector.h index b8fc4617bc..1fcd77242b 100644 --- a/mfbt/Vector.h +++ b/mfbt/Vector.h @@ -146,7 +146,7 @@ struct VectorImpl aV.free_(aV.mBegin); aV.mBegin = newbuf; /* aV.mLength is unchanged. */ - aV.mCapacity = aNewCap; + aV.mTail.mCapacity = aNewCap; return true; } }; @@ -225,28 +225,30 @@ struct VectorImpl { MOZ_ASSERT(!aV.usingInlineStorage()); MOZ_ASSERT(!CapacityHasExcessSpace(aNewCap)); - T* newbuf = aV.template pod_realloc(aV.mBegin, aV.mCapacity, aNewCap); + T* newbuf = + aV.template pod_realloc(aV.mBegin, aV.mTail.mCapacity, aNewCap); if (MOZ_UNLIKELY(!newbuf)) { return false; } aV.mBegin = newbuf; /* aV.mLength is unchanged. */ - aV.mCapacity = aNewCap; + aV.mTail.mCapacity = aNewCap; return true; } static inline void podResizeToFit(Vector& aV) { - if (aV.usingInlineStorage() || aV.mLength == aV.mCapacity) { + if (aV.usingInlineStorage() || aV.mLength == aV.mTail.mCapacity) { return; } - T* newbuf = aV.template pod_realloc(aV.mBegin, aV.mCapacity, aV.mLength); + T* newbuf = + aV.template pod_realloc(aV.mBegin, aV.mTail.mCapacity, aV.mLength); if (MOZ_UNLIKELY(!newbuf)) { return; } aV.mBegin = newbuf; - aV.mCapacity = aV.mLength; + aV.mTail.mCapacity = aV.mLength; } }; @@ -341,42 +343,84 @@ class MOZ_NON_PARAM Vector final : private AllocPolicy /* Number of elements in the vector. */ size_t mLength; - /* Max number of elements storable in the vector without resizing. */ - size_t mCapacity; + /* + * Memory used to store capacity, reserved element count (debug builds only), + * and inline storage. The simple "answer" is: + * + * size_t mCapacity; + * #ifdef DEBUG + * size_t mReserved; + * #endif + * alignas(T) unsigned char mBytes[kInlineCapacity * sizeof(T)]; + * + * but there are complications. First, C++ forbids zero-sized arrays that + * might result. Second, we don't want zero capacity to affect Vector's size + * (even empty classes take up a byte, unless they're base classes). + * + * Yet again, we eliminate the zero-sized array using partial specialization. + * And we eliminate potential size hit by putting capacity/reserved in one + * struct, then putting the array (if any) in a derived struct. If no array + * is needed, the derived struct won't consume extra space. + */ + struct CapacityAndReserved + { + explicit CapacityAndReserved(size_t aCapacity, size_t aReserved) + : mCapacity(aCapacity) +#ifdef DEBUG + , mReserved(aReserved) +#endif + {} + CapacityAndReserved() = default; + + /* Max number of elements storable in the vector without resizing. */ + size_t mCapacity; #ifdef DEBUG - /* Max elements of reserved or used space in this vector. */ - size_t mReserved; + /* Max elements of reserved or used space in this vector. */ + size_t mReserved; #endif + }; + +// Silence warnings about this struct possibly being padded dued to the +// alignas() in it -- there's nothing we can do to avoid it. +#ifdef _MSC_VER +# pragma warning(push) +# pragma warning(disable:4324) +#endif // _MSC_VER - /* - * Memory used for inline storage. We want basically this: - * - * alignas(T) unsigned char storage[kInlineCapacity * sizeof(T)]; - * - * but C++ forbids zero-sized arrays that might result if we did this. We fix - * this by (again) using partial specialization, defining an array only if - * contains at least one element. - */ template - struct InlineStorage + struct CRAndStorage : CapacityAndReserved { + explicit CRAndStorage(size_t aCapacity, size_t aReserved) + : CapacityAndReserved(aCapacity, aReserved) + {} + CRAndStorage() = default; + alignas(T) unsigned char mBytes[Capacity * sizeof(T)]; // GCC fails due to -Werror=strict-aliasing if |mBytes| is directly cast to // T*. Indirecting through this function addresses the problem. void* data() { return mBytes; } - T* addr() { return static_cast(data()); } + T* storage() { return static_cast(data()); } }; template - struct InlineStorage<0, Dummy> + struct CRAndStorage<0, Dummy> : CapacityAndReserved { - T* addr() { return nullptr; } + explicit CRAndStorage(size_t aCapacity, size_t aReserved) + : CapacityAndReserved(aCapacity, aReserved) + {} + CRAndStorage() = default; + + T* storage() { return nullptr; } }; - InlineStorage mStorage; + CRAndStorage mTail; + +#ifdef _MSC_VER +# pragma warning(pop) +#endif // _MSC_VER #ifdef DEBUG friend class ReentrancyGuard; @@ -392,7 +436,7 @@ class MOZ_NON_PARAM Vector final : private AllocPolicy T* inlineStorage() { - return mStorage.addr(); + return mTail.storage(); } T* beginNoCheck() const @@ -420,9 +464,9 @@ class MOZ_NON_PARAM Vector final : private AllocPolicy */ size_t reserved() const { - MOZ_ASSERT(mLength <= mReserved); - MOZ_ASSERT(mReserved <= mCapacity); - return mReserved; + MOZ_ASSERT(mLength <= mTail.mReserved); + MOZ_ASSERT(mTail.mReserved <= mTail.mCapacity); + return mTail.mReserved; } #endif @@ -455,7 +499,7 @@ public: bool empty() const { return mLength == 0; } - size_t capacity() const { return mCapacity; } + size_t capacity() const { return mTail.mCapacity; } T* begin() { @@ -782,10 +826,10 @@ private: /* This does the re-entrancy check plus several other sanity checks. */ #define MOZ_REENTRANCY_GUARD_ET_AL \ ReentrancyGuard g(*this); \ - MOZ_ASSERT_IF(usingInlineStorage(), mCapacity == kInlineCapacity); \ - MOZ_ASSERT(reserved() <= mCapacity); \ + MOZ_ASSERT_IF(usingInlineStorage(), mTail.mCapacity == kInlineCapacity); \ + MOZ_ASSERT(reserved() <= mTail.mCapacity); \ MOZ_ASSERT(mLength <= reserved()); \ - MOZ_ASSERT(mLength <= mCapacity) + MOZ_ASSERT(mLength <= mTail.mCapacity) /* Vector Implementation */ @@ -794,9 +838,8 @@ MOZ_ALWAYS_INLINE Vector::Vector(AP aAP) : AP(aAP) , mLength(0) - , mCapacity(kInlineCapacity) + , mTail(kInlineCapacity, 0) #ifdef DEBUG - , mReserved(0) , mEntered(false) #endif { @@ -813,9 +856,9 @@ Vector::Vector(Vector&& aRhs) #endif { mLength = aRhs.mLength; - mCapacity = aRhs.mCapacity; + mTail.mCapacity = aRhs.mTail.mCapacity; #ifdef DEBUG - mReserved = aRhs.mReserved; + mTail.mReserved = aRhs.mTail.mReserved; #endif if (aRhs.usingInlineStorage()) { @@ -833,10 +876,10 @@ Vector::Vector(Vector&& aRhs) */ mBegin = aRhs.mBegin; aRhs.mBegin = aRhs.inlineStorage(); - aRhs.mCapacity = kInlineCapacity; + aRhs.mTail.mCapacity = kInlineCapacity; aRhs.mLength = 0; #ifdef DEBUG - aRhs.mReserved = 0; + aRhs.mTail.mReserved = 0; #endif } } @@ -900,7 +943,7 @@ Vector::convertToHeapStorage(size_t aNewCap) /* Switch in heap buffer. */ mBegin = newBuf; /* mLength is unchanged. */ - mCapacity = aNewCap; + mTail.mCapacity = aNewCap; return true; } @@ -908,7 +951,7 @@ template MOZ_NEVER_INLINE bool Vector::growStorageBy(size_t aIncr) { - MOZ_ASSERT(mLength + aIncr > mCapacity); + MOZ_ASSERT(mLength + aIncr > mTail.mCapacity); /* * When choosing a new capacity, its size should is as close to 2**N bytes @@ -1000,9 +1043,9 @@ Vector::initCapacity(size_t aRequest) return false; } mBegin = newbuf; - mCapacity = aRequest; + mTail.mCapacity = aRequest; #ifdef DEBUG - mReserved = aRequest; + mTail.mReserved = aRequest; #endif return true; } @@ -1027,7 +1070,7 @@ Vector::maybeCheckSimulatedOOM(size_t aRequestedSize) } #ifdef DEBUG - if (aRequestedSize <= mReserved) { + if (aRequestedSize <= mTail.mReserved) { return true; } #endif @@ -1040,7 +1083,7 @@ inline bool Vector::reserve(size_t aRequest) { MOZ_REENTRANCY_GUARD_ET_AL; - if (aRequest > mCapacity) { + if (aRequest > mTail.mCapacity) { if (MOZ_UNLIKELY(!growStorageBy(aRequest - mLength))) { return false; } @@ -1048,11 +1091,11 @@ Vector::reserve(size_t aRequest) return false; } #ifdef DEBUG - if (aRequest > mReserved) { - mReserved = aRequest; + if (aRequest > mTail.mReserved) { + mTail.mReserved = aRequest; } - MOZ_ASSERT(mLength <= mReserved); - MOZ_ASSERT(mReserved <= mCapacity); + MOZ_ASSERT(mLength <= mTail.mReserved); + MOZ_ASSERT(mTail.mReserved <= mTail.mCapacity); #endif return true; } @@ -1080,20 +1123,20 @@ MOZ_ALWAYS_INLINE bool Vector::growBy(size_t aIncr) { MOZ_REENTRANCY_GUARD_ET_AL; - if (aIncr > mCapacity - mLength) { + if (aIncr > mTail.mCapacity - mLength) { if (MOZ_UNLIKELY(!growStorageBy(aIncr))) { return false; } } else if (!maybeCheckSimulatedOOM(mLength + aIncr)) { return false; } - MOZ_ASSERT(mLength + aIncr <= mCapacity); + MOZ_ASSERT(mLength + aIncr <= mTail.mCapacity); T* newend = endNoCheck() + aIncr; Impl::initialize(endNoCheck(), newend); mLength += aIncr; #ifdef DEBUG - if (mLength > mReserved) { - mReserved = mLength; + if (mLength > mTail.mReserved) { + mTail.mReserved = mLength; } #endif return true; @@ -1104,7 +1147,7 @@ MOZ_ALWAYS_INLINE bool Vector::growByUninitialized(size_t aIncr) { MOZ_REENTRANCY_GUARD_ET_AL; - if (aIncr > mCapacity - mLength) { + if (aIncr > mTail.mCapacity - mLength) { if (MOZ_UNLIKELY(!growStorageBy(aIncr))) { return false; } @@ -1112,8 +1155,8 @@ Vector::growByUninitialized(size_t aIncr) return false; } #ifdef DEBUG - if (mLength + aIncr > mReserved) { - mReserved = mLength + aIncr; + if (mLength + aIncr > mTail.mReserved) { + mTail.mReserved = mLength + aIncr; } #endif infallibleGrowByUninitialized(aIncr); @@ -1172,9 +1215,9 @@ Vector::clearAndFree() } this->free_(beginNoCheck()); mBegin = inlineStorage(); - mCapacity = kInlineCapacity; + mTail.mCapacity = kInlineCapacity; #ifdef DEBUG - mReserved = 0; + mTail.mReserved = 0; #endif } @@ -1191,7 +1234,7 @@ template inline bool Vector::canAppendWithoutRealloc(size_t aNeeded) const { - return mLength + aNeeded <= mCapacity; + return mLength + aNeeded <= mTail.mCapacity; } template @@ -1207,8 +1250,8 @@ template MOZ_ALWAYS_INLINE void Vector::internalAppend(U&& aU) { - MOZ_ASSERT(mLength + 1 <= mReserved); - MOZ_ASSERT(mReserved <= mCapacity); + MOZ_ASSERT(mLength + 1 <= mTail.mReserved); + MOZ_ASSERT(mTail.mReserved <= mTail.mCapacity); Impl::new_(endNoCheck(), Forward(aU)); ++mLength; } @@ -1218,7 +1261,7 @@ MOZ_ALWAYS_INLINE bool Vector::appendN(const T& aT, size_t aNeeded) { MOZ_REENTRANCY_GUARD_ET_AL; - if (mLength + aNeeded > mCapacity) { + if (mLength + aNeeded > mTail.mCapacity) { if (MOZ_UNLIKELY(!growStorageBy(aNeeded))) { return false; } @@ -1226,8 +1269,8 @@ Vector::appendN(const T& aT, size_t aNeeded) return false; } #ifdef DEBUG - if (mLength + aNeeded > mReserved) { - mReserved = mLength + aNeeded; + if (mLength + aNeeded > mTail.mReserved) { + mTail.mReserved = mLength + aNeeded; } #endif internalAppendN(aT, aNeeded); @@ -1238,8 +1281,8 @@ template MOZ_ALWAYS_INLINE void Vector::internalAppendN(const T& aT, size_t aNeeded) { - MOZ_ASSERT(mLength + aNeeded <= mReserved); - MOZ_ASSERT(mReserved <= mCapacity); + MOZ_ASSERT(mLength + aNeeded <= mTail.mReserved); + MOZ_ASSERT(mTail.mReserved <= mTail.mCapacity); Impl::copyConstructN(endNoCheck(), aNeeded, aT); mLength += aNeeded; } @@ -1304,7 +1347,7 @@ Vector::append(const U* aInsBegin, const U* aInsEnd) { MOZ_REENTRANCY_GUARD_ET_AL; size_t aNeeded = PointerRangeSize(aInsBegin, aInsEnd); - if (mLength + aNeeded > mCapacity) { + if (mLength + aNeeded > mTail.mCapacity) { if (MOZ_UNLIKELY(!growStorageBy(aNeeded))) { return false; } @@ -1312,8 +1355,8 @@ Vector::append(const U* aInsBegin, const U* aInsEnd) return false; } #ifdef DEBUG - if (mLength + aNeeded > mReserved) { - mReserved = mLength + aNeeded; + if (mLength + aNeeded > mTail.mReserved) { + mTail.mReserved = mLength + aNeeded; } #endif internalAppend(aInsBegin, aNeeded); @@ -1325,8 +1368,8 @@ template MOZ_ALWAYS_INLINE void Vector::internalAppend(const U* aInsBegin, size_t aInsLength) { - MOZ_ASSERT(mLength + aInsLength <= mReserved); - MOZ_ASSERT(mReserved <= mCapacity); + MOZ_ASSERT(mLength + aInsLength <= mTail.mReserved); + MOZ_ASSERT(mTail.mReserved <= mTail.mCapacity); Impl::copyConstruct(endNoCheck(), aInsBegin, aInsBegin + aInsLength); mLength += aInsLength; } @@ -1337,7 +1380,7 @@ MOZ_ALWAYS_INLINE bool Vector::append(U&& aU) { MOZ_REENTRANCY_GUARD_ET_AL; - if (mLength == mCapacity) { + if (mLength == mTail.mCapacity) { if (MOZ_UNLIKELY(!growStorageBy(1))) { return false; } @@ -1345,8 +1388,8 @@ Vector::append(U&& aU) return false; } #ifdef DEBUG - if (mLength + 1 > mReserved) { - mReserved = mLength + 1; + if (mLength + 1 > mTail.mReserved) { + mTail.mReserved = mLength + 1; } #endif internalAppend(Forward(aU)); @@ -1401,9 +1444,9 @@ Vector::extractRawBuffer() T* ret = mBegin; mBegin = inlineStorage(); mLength = 0; - mCapacity = kInlineCapacity; + mTail.mCapacity = kInlineCapacity; #ifdef DEBUG - mReserved = 0; + mTail.mReserved = 0; #endif return ret; } @@ -1427,9 +1470,9 @@ Vector::extractOrCopyRawBuffer() Impl::destroy(beginNoCheck(), endNoCheck()); mBegin = inlineStorage(); mLength = 0; - mCapacity = kInlineCapacity; + mTail.mCapacity = kInlineCapacity; #ifdef DEBUG - mReserved = 0; + mTail.mReserved = 0; #endif return copy; } @@ -1455,17 +1498,17 @@ Vector::replaceRawBuffer(T* aP, size_t aLength) */ mBegin = inlineStorage(); mLength = aLength; - mCapacity = kInlineCapacity; + mTail.mCapacity = kInlineCapacity; Impl::moveConstruct(mBegin, aP, aP + aLength); Impl::destroy(aP, aP + aLength); this->free_(aP); } else { mBegin = aP; mLength = aLength; - mCapacity = aLength; + mTail.mCapacity = aLength; } #ifdef DEBUG - mReserved = aLength; + mTail.mReserved = aLength; #endif } @@ -1504,9 +1547,9 @@ Vector::swap(Vector& aOther) } Swap(mLength, aOther.mLength); - Swap(mCapacity, aOther.mCapacity); + Swap(mTail.mCapacity, aOther.mTail.mCapacity); #ifdef DEBUG - Swap(mReserved, aOther.mReserved); + Swap(mTail.mReserved, aOther.mTail.mReserved); #endif } diff --git a/mfbt/tests/TestVector.cpp b/mfbt/tests/TestVector.cpp index 24baf8eae7..9aed9e617c 100644 --- a/mfbt/tests/TestVector.cpp +++ b/mfbt/tests/TestVector.cpp @@ -396,6 +396,49 @@ mozilla::detail::VectorTesting::testInsert() MOZ_RELEASE_ASSERT(S::destructCount == 1); } +// Declare but leave (permanently) incomplete. +struct Incomplete; + +// We could even *construct* a Vector if we wanted. But we can't +// destruct it, so it's not worth the trouble. +static_assert(sizeof(Vector) > 0, + "Vector of an incomplete type will compile"); + +// Vector with no inline storage should occupy the absolute minimum space in +// non-debug builds. (Debug adds a laundry list of other constraints, none +// directly relevant to shipping builds, that aren't worth precisely modeling.) +#ifndef DEBUG + +template +struct NoInlineStorageLayout +{ + T* mBegin; + size_t mLength; + struct CRAndStorage { + size_t mCapacity; + } mTail; +}; + +// Only one of these should be necessary, but test a few of them for good +// measure. +static_assert(sizeof(Vector) == sizeof(NoInlineStorageLayout), + "Vector of int without inline storage shouldn't occupy dead " + "space for that absence of storage"); + +static_assert(sizeof(Vector) == sizeof(NoInlineStorageLayout), + "Vector of bool without inline storage shouldn't occupy dead " + "space for that absence of storage"); + +static_assert(sizeof(Vector) == sizeof(NoInlineStorageLayout), + "Vector of S without inline storage shouldn't occupy dead " + "space for that absence of storage"); + +static_assert(sizeof(Vector) == sizeof(NoInlineStorageLayout), + "Vector of an incomplete class without inline storage shouldn't " + "occupy dead space for that absence of storage"); + +#endif // DEBUG + int main() {