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<char,32>::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
This commit is contained in:
committed by
mayankleoboy1@gmail.com
parent
fe8a160b95
commit
1d3eebcd6c
@@ -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 <typename T>
|
||||
|
||||
+7
-9
@@ -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<T, N, AP, true> {
|
||||
template <typename U>
|
||||
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<T, U>) {
|
||||
PodCopy(aDst, aSrcStart, PointerRangeSize(aSrcStart, aSrcEnd));
|
||||
} else {
|
||||
for (const U* p = aSrcStart; p < aSrcEnd; ++p, ++aDst) {
|
||||
new_(aDst, *p);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user