Clean realloc() related bits in QString/QBA and Q*ArrayOps

Fixed misleading naming of "slowReallocatePath". It's no longer "slow",
it's downright dangerous now to reallocate under certain conditions

Added several asserts which should've been there already as our code
would run into a UB/crash anyhow - let's at least get extra checks
that are closer to the trouble causing places

Bring back the (slightly modified) code-cleaning changes from
504972f838

Change-Id: Ie1358aebc619062d3991a78049e366dc0e8c267e
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
This commit is contained in:
Andrei Golubev 2020-11-10 19:43:52 +01:00
parent 7549d18054
commit 405305069f
4 changed files with 16 additions and 13 deletions

View File

@ -1708,12 +1708,11 @@ void QByteArray::reallocData(qsizetype alloc, QArrayData::AllocationOption optio
return;
}
// there's a case of slow reallocate path where we need to memmove the data
// before a call to ::realloc(), meaning that there's an extra "heavy"
// operation. just prefer ::malloc() branch in this case
const bool slowReallocatePath = d.freeSpaceAtBegin() > 0;
// don't use reallocate path when reducing capacity and there's free space
// at the beginning: might shift data pointer outside of allocated space
const bool cannotUseReallocate = d.freeSpaceAtBegin() > 0;
if (d->needsDetach() || slowReallocatePath) {
if (d->needsDetach() || cannotUseReallocate) {
DataPointer dd(Data::allocate(alloc, option), qMin(alloc, d.size));
if (dd.size > 0)
::memcpy(dd.data(), d.data(), dd.size);

View File

@ -2504,12 +2504,11 @@ void QString::reallocData(qsizetype alloc, QArrayData::AllocationOption option)
return;
}
// there's a case of slow reallocate path where we need to memmove the data
// before a call to ::realloc(), meaning that there's an extra "heavy"
// operation. just prefer ::malloc() branch in this case
const bool slowReallocatePath = d.freeSpaceAtBegin() > 0;
// don't use reallocate path when reducing capacity and there's free space
// at the beginning: might shift data pointer outside of allocated space
const bool cannotUseReallocate = d.freeSpaceAtBegin() > 0;
if (d->needsDetach() || slowReallocatePath) {
if (d->needsDetach() || cannotUseReallocate) {
DataPointer dd(Data::allocate(alloc, option), qMin(alloc, d.size));
if (dd.size > 0)
::memcpy(dd.data(), d.data(), dd.size * sizeof(QChar));

View File

@ -233,10 +233,13 @@ QArrayData::reallocateUnaligned(QArrayData *data, void *dataPointer,
{
Q_ASSERT(!data || !data->isShared());
qsizetype headerSize = sizeof(QArrayData);
const qsizetype headerSize = sizeof(QArrayData);
qsizetype allocSize = calculateBlockSize(capacity, objectSize, headerSize, option);
qptrdiff offset = dataPointer ? reinterpret_cast<char *>(dataPointer) - reinterpret_cast<char *>(data) : headerSize;
const qptrdiff offset = dataPointer
? reinterpret_cast<char *>(dataPointer) - reinterpret_cast<char *>(data)
: headerSize;
Q_ASSERT(offset > 0);
Q_ASSERT(offset <= allocSize); // equals when all free space is at the beginning
allocSize = reserveExtraBytes(allocSize);
if (Q_UNLIKELY(allocSize < 0)) // handle overflow. cannot reallocate reliably
@ -244,7 +247,7 @@ QArrayData::reallocateUnaligned(QArrayData *data, void *dataPointer,
QArrayData *header = static_cast<QArrayData *>(::realloc(data, size_t(allocSize)));
if (header) {
header->alloc = uint(capacity);
header->alloc = capacity;
dataPointer = reinterpret_cast<char *>(header) + offset;
} else {
dataPointer = nullptr;

View File

@ -479,6 +479,7 @@ public:
void reallocate(qsizetype alloc, QArrayData::AllocationOption option)
{
auto pair = Data::reallocateUnaligned(this->d, this->ptr, alloc, option);
Q_ASSERT(pair.first != nullptr);
this->d = pair.first;
this->ptr = pair.second;
}
@ -1132,6 +1133,7 @@ public:
void reallocate(qsizetype alloc, QArrayData::AllocationOption option)
{
auto pair = Data::reallocateUnaligned(this->d, this->ptr, alloc, option);
Q_ASSERT(pair.first != nullptr);
this->d = pair.first;
this->ptr = pair.second;
}