Finally fix crash in inplace-modified data-constructed images
Avoid all inplace modification of images using external data
buffers. Since the QImage methods are documented to create a
(modified) copy, there is afterwards no API requirement on the
lifetime of the data buffer.
This patch supersedes 509bc7e59c
Task-number: QTBUG-53721
Change-Id: I3ccc01619eb61d8630104449394e0b76df0af695
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
bb10
parent
c165cbaef2
commit
c2b7841843
|
|
@ -3115,6 +3115,8 @@ void QImage::mirrored_inplace(bool horizontal, bool vertical)
|
|||
return;
|
||||
|
||||
detach();
|
||||
if (!d->own_data)
|
||||
*this = copy();
|
||||
|
||||
do_mirror(d, d, horizontal, vertical);
|
||||
}
|
||||
|
|
@ -3261,6 +3263,8 @@ void QImage::rgbSwapped_inplace()
|
|||
return;
|
||||
|
||||
detach();
|
||||
if (!d->own_data)
|
||||
*this = copy();
|
||||
|
||||
switch (d->format) {
|
||||
case Format_Invalid:
|
||||
|
|
@ -4745,7 +4749,7 @@ bool QImageData::convertInPlace(QImage::Format newFormat, Qt::ImageConversionFla
|
|||
return true;
|
||||
|
||||
// No in-place conversion if we have to detach
|
||||
if (ref.load() > 1 || ro_data)
|
||||
if (ref.load() > 1 || !own_data)
|
||||
return false;
|
||||
|
||||
InPlace_Image_Converter converter = qimage_inplace_converter_map[format][newFormat];
|
||||
|
|
|
|||
|
|
@ -763,8 +763,8 @@ static bool convert_A2RGB30_PM_to_ARGB_inplace(QImageData *data, Qt::ImageConver
|
|||
static bool convert_indexed8_to_ARGB_PM_inplace(QImageData *data, Qt::ImageConversionFlags)
|
||||
{
|
||||
Q_ASSERT(data->format == QImage::Format_Indexed8);
|
||||
if (!data->own_data)
|
||||
return false;
|
||||
Q_ASSERT(data->own_data);
|
||||
|
||||
const int depth = 32;
|
||||
|
||||
const int dst_bytes_per_line = ((data->width * depth + 31) >> 5) << 2;
|
||||
|
|
@ -817,8 +817,8 @@ static bool convert_indexed8_to_ARGB_PM_inplace(QImageData *data, Qt::ImageConve
|
|||
static bool convert_indexed8_to_ARGB_inplace(QImageData *data, Qt::ImageConversionFlags)
|
||||
{
|
||||
Q_ASSERT(data->format == QImage::Format_Indexed8);
|
||||
if (!data->own_data)
|
||||
return false;
|
||||
Q_ASSERT(data->own_data);
|
||||
|
||||
const int depth = 32;
|
||||
|
||||
const int dst_bytes_per_line = ((data->width * depth + 31) >> 5) << 2;
|
||||
|
|
@ -868,8 +868,7 @@ static bool convert_indexed8_to_ARGB_inplace(QImageData *data, Qt::ImageConversi
|
|||
static bool convert_indexed8_to_RGB_inplace(QImageData *data, Qt::ImageConversionFlags flags)
|
||||
{
|
||||
Q_ASSERT(data->format == QImage::Format_Indexed8);
|
||||
if (!data->own_data)
|
||||
return false;
|
||||
Q_ASSERT(data->own_data);
|
||||
|
||||
if (data->has_alpha_clut) {
|
||||
for (int i = 0; i < data->colortable.size(); ++i)
|
||||
|
|
@ -886,8 +885,8 @@ static bool convert_indexed8_to_RGB_inplace(QImageData *data, Qt::ImageConversio
|
|||
static bool convert_indexed8_to_RGB16_inplace(QImageData *data, Qt::ImageConversionFlags)
|
||||
{
|
||||
Q_ASSERT(data->format == QImage::Format_Indexed8);
|
||||
if (!data->own_data)
|
||||
return false;
|
||||
Q_ASSERT(data->own_data);
|
||||
|
||||
const int depth = 16;
|
||||
|
||||
const int dst_bytes_per_line = ((data->width * depth + 31) >> 5) << 2;
|
||||
|
|
@ -943,8 +942,8 @@ static bool convert_indexed8_to_RGB16_inplace(QImageData *data, Qt::ImageConvers
|
|||
static bool convert_RGB_to_RGB16_inplace(QImageData *data, Qt::ImageConversionFlags)
|
||||
{
|
||||
Q_ASSERT(data->format == QImage::Format_RGB32);
|
||||
if (!data->own_data)
|
||||
return false;
|
||||
Q_ASSERT(data->own_data);
|
||||
|
||||
const int depth = 16;
|
||||
|
||||
const int dst_bytes_per_line = ((data->width * depth + 31) >> 5) << 2;
|
||||
|
|
|
|||
|
|
@ -2447,6 +2447,35 @@ void tst_QImage::inplaceRgbSwapped()
|
|||
}
|
||||
|
||||
QCOMPARE(imageSwapped.constScanLine(0), orginalPtr);
|
||||
|
||||
for (int rw = 0; rw <= 1; rw++) {
|
||||
// Test attempted inplace conversion of images created on existing buffer
|
||||
uchar *volatileData = 0;
|
||||
QImage orig = imageSwapped;
|
||||
QImage dataSwapped;
|
||||
{
|
||||
QVERIFY(!orig.isNull());
|
||||
volatileData = new uchar[orig.byteCount()];
|
||||
memcpy(volatileData, orig.constBits(), orig.byteCount());
|
||||
|
||||
QImage dataImage;
|
||||
if (rw)
|
||||
dataImage = QImage(volatileData, orig.width(), orig.height(), orig.format());
|
||||
else
|
||||
dataImage = QImage((const uchar *)volatileData, orig.width(), orig.height(), orig.format());
|
||||
|
||||
if (orig.colorCount())
|
||||
dataImage.setColorTable(orig.colorTable());
|
||||
|
||||
dataSwapped = std::move(dataImage).rgbSwapped();
|
||||
QVERIFY(!dataSwapped.isNull());
|
||||
delete[] volatileData;
|
||||
}
|
||||
|
||||
QVERIFY2(dataSwapped.constBits() != volatileData, rw ? "non-const" : "const");
|
||||
QCOMPARE(dataSwapped, orig.rgbSwapped());
|
||||
}
|
||||
|
||||
#endif
|
||||
}
|
||||
|
||||
|
|
@ -2529,6 +2558,35 @@ void tst_QImage::inplaceMirrored()
|
|||
}
|
||||
}
|
||||
QCOMPARE(imageMirrored.constScanLine(0), originalPtr);
|
||||
|
||||
for (int rw = 0; rw <= 1; rw++) {
|
||||
// Test attempted inplace conversion of images created on existing buffer
|
||||
uchar *volatileData = 0;
|
||||
QImage orig = imageMirrored;
|
||||
QImage dataSwapped;
|
||||
{
|
||||
QVERIFY(!orig.isNull());
|
||||
volatileData = new uchar[orig.byteCount()];
|
||||
memcpy(volatileData, orig.constBits(), orig.byteCount());
|
||||
|
||||
QImage dataImage;
|
||||
if (rw)
|
||||
dataImage = QImage(volatileData, orig.width(), orig.height(), orig.format());
|
||||
else
|
||||
dataImage = QImage((const uchar *)volatileData, orig.width(), orig.height(), orig.format());
|
||||
|
||||
if (orig.colorCount())
|
||||
dataImage.setColorTable(orig.colorTable());
|
||||
|
||||
dataSwapped = std::move(dataImage).mirrored(swap_horizontal, swap_vertical);
|
||||
QVERIFY(!dataSwapped.isNull());
|
||||
delete[] volatileData;
|
||||
}
|
||||
|
||||
QVERIFY2(dataSwapped.constBits() != volatileData, rw ? "non-const" : "const");
|
||||
QCOMPARE(dataSwapped, orig.mirrored(swap_horizontal, swap_vertical));
|
||||
}
|
||||
|
||||
#endif
|
||||
}
|
||||
|
||||
|
|
@ -2680,16 +2738,24 @@ void tst_QImage::inplaceRgbConversion()
|
|||
static const quint32 readOnlyData[] = { 0xff0102ffU, 0xff0506ffU, 0xff0910ffU, 0xff1314ffU };
|
||||
quint32 readWriteData[] = { 0xff0102ffU, 0xff0506ffU, 0xff0910ffU, 0xff1314ffU };
|
||||
|
||||
QImage roImage((const uchar *)readOnlyData, 2, 2, format);
|
||||
QImage roInplaceConverted = std::move(roImage).convertToFormat(dest_format);
|
||||
QImage roInplaceConverted;
|
||||
QImage rwInplaceConverted;
|
||||
|
||||
QImage rwImage((uchar *)readWriteData, 2, 2, format);
|
||||
QImage rwInplaceConverted = std::move(rwImage).convertToFormat(dest_format);
|
||||
{
|
||||
QImage roImage((const uchar *)readOnlyData, 2, 2, format);
|
||||
roInplaceConverted = std::move(roImage).convertToFormat(dest_format);
|
||||
|
||||
QImage rwImage((uchar *)readWriteData, 2, 2, format);
|
||||
rwInplaceConverted = std::move(rwImage).convertToFormat(dest_format);
|
||||
}
|
||||
|
||||
QImage roImage2((const uchar *)readOnlyData, 2, 2, format);
|
||||
QImage normalConverted = roImage2.convertToFormat(dest_format);
|
||||
|
||||
QVERIFY(roInplaceConverted.constBits() != (const uchar *)readOnlyData);
|
||||
QCOMPARE(normalConverted, roInplaceConverted);
|
||||
|
||||
QVERIFY(rwInplaceConverted.constBits() != (const uchar *)readWriteData);
|
||||
QCOMPARE(normalConverted, rwInplaceConverted);
|
||||
}
|
||||
#endif
|
||||
|
|
|
|||
Loading…
Reference in New Issue