From 1d3eebcd6c22d0bc4a271dcfbc7890f87a7fd7fb Mon Sep 17 00:00:00 2001 From: mayankleoboy1 Date: Thu, 10 Sep 2026 15:08:19 +0000 Subject: [PATCH] Bug 661957 - Use PodCopy for same-type POD copyConstruct in Vector. r=sergesanspaille VectorImpl's POD specialization copies appended ranges with a per-element loop, on the grounds that append is usually given small ranges. It is also how JSStringBuilder copies string segments: a samply profile of a 60000-char str.replaceAll spends 57.9% of the call in copyConstruct at roughly 1.4 GB/s, against about 20 GB/s for the equivalent rope flatten. Use PodCopy when T and U are the same type, which is the patch proposed in comment 2 of this bug in 2012. PodCopy already carries the size and platform heuristic measured in bug 933149 and bug 1967062, so ranges under 128 elements on Linux keep the loop they have today. No caller can pass overlapping ranges to PodCopy's new assertion: every moveConstruct site targets a fresh allocation or the opposite of inline and heap storage, and an append whose source lies in the vector's own live range ends at or before the destination, since the source must end by mLength and the destination starts there. Suppress a GCC 10 -Werror=stringop-overflow= false positive in PodCopy's memcpy call. When Vector::convertToHeapStorage inlines through copyConstruct, GCC's value range propagation cannot prove mLength is bounded by the 64-byte heap allocation, and _FORTIFY_SOURCE's __builtin___memcpy_chk reports a spurious overflow. A diagnostic pragma on the memcpy call site is the narrowest fix. Differential Revision: https://phabricator.services.mozilla.com/D323825 --- mfbt/PodOperations.h | 9 +++++++++ mfbt/Vector.h | 16 +++++++--------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/mfbt/PodOperations.h b/mfbt/PodOperations.h index c53572972ff9..f57a9019db86 100644 --- a/mfbt/PodOperations.h +++ b/mfbt/PodOperations.h @@ -116,7 +116,16 @@ static MOZ_ALWAYS_INLINE void PodCopy(T* aDst, const T* aSrc, size_t aNElem) { } #endif + // GCC false-positive: value range propagation cannot prove mLength is bounded + // when Vector::convertToHeapStorage inlines through here (Bug 661957). +#if defined(__GNUC__) && !defined(__clang__) +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wstringop-overflow" +#endif memcpy(aDst, aSrc, aNElem * sizeof(T)); +#if defined(__GNUC__) && !defined(__clang__) +# pragma GCC diagnostic pop +#endif } template diff --git a/mfbt/Vector.h b/mfbt/Vector.h index 7be85736fa6d..a20705a3158c 100644 --- a/mfbt/Vector.h +++ b/mfbt/Vector.h @@ -20,6 +20,7 @@ #include "mozilla/MathAlgorithms.h" #include "mozilla/MemoryReporting.h" #include "mozilla/OperatorNewExtensions.h" +#include "mozilla/PodOperations.h" #include "mozilla/ReentrancyGuard.h" #include "mozilla/Span.h" @@ -271,16 +272,13 @@ struct VectorImpl { template static inline void copyConstruct(T* aDst, const U* aSrcStart, const U* aSrcEnd) { - /* - * See above memset comment. Also, notice that copyConstruct is - * currently templated (T != U), so memcpy won't work without - * requiring T == U. - * - * memcpy(aDst, aSrcStart, sizeof(T) * (aSrcEnd - aSrcStart)); - */ MOZ_ASSERT(aSrcStart <= aSrcEnd); - for (const U* p = aSrcStart; p < aSrcEnd; ++p, ++aDst) { - new_(aDst, *p); + if constexpr (std::is_same_v) { + PodCopy(aDst, aSrcStart, PointerRangeSize(aSrcStart, aSrcEnd)); + } else { + for (const U* p = aSrcStart; p < aSrcEnd; ++p, ++aDst) { + new_(aDst, *p); + } } }