From f10b31fc1be0697f65b13ee7ad966f0085f7ed00 Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Wed, 18 Mar 2020 10:56:53 +0100 Subject: ANGLE: Fix resizing of windows (Take 2) Task-number: QTBUG-62475 Change-Id: I0ea17e7875906508941ae64bb396a4236928b0f9 Reviewed-by: Miguel Costa Reviewed-by: Friedemann Kleint --- .../libANGLE/renderer/d3d/d3d11/SwapChain11.cpp | 2 +- ...0017-ANGLE-Fix-resizing-of-windows-Take-2.patch | 27 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 src/angle/patches/0017-ANGLE-Fix-resizing-of-windows-Take-2.patch diff --git a/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp b/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp index e8f13b388f..9ece77ecbc 100644 --- a/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp +++ b/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp @@ -845,7 +845,7 @@ EGLint SwapChain11::copyOffscreenToBackbuffer(const gl::Context *context, stateManager->setRenderTarget(mBackBufferRTView.get(), nullptr); // Set the viewport - stateManager->setSimpleViewport(mWidth, mHeight); + stateManager->setSimpleViewport(width, height); // Apply textures stateManager->setSimplePixelTextureAndSampler(mOffscreenSRView, mPassThroughSampler); diff --git a/src/angle/patches/0017-ANGLE-Fix-resizing-of-windows-Take-2.patch b/src/angle/patches/0017-ANGLE-Fix-resizing-of-windows-Take-2.patch new file mode 100644 index 0000000000..abab74b192 --- /dev/null +++ b/src/angle/patches/0017-ANGLE-Fix-resizing-of-windows-Take-2.patch @@ -0,0 +1,27 @@ +From 029d42d1049dcde7950c11fb9adf07c07a8c4c02 Mon Sep 17 00:00:00 2001 +From: Oliver Wolff +Date: Wed, 18 Mar 2020 10:56:53 +0100 +Subject: [PATCH] ANGLE: Fix resizing of windows (Take 2) + +Task-number: QTBUG-62475 +Change-Id: I0ea17e7875906508941ae64bb396a4236928b0f9 +--- + .../angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp b/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp +index e8f13b388f..9ece77ecbc 100644 +--- a/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp ++++ b/src/3rdparty/angle/src/libANGLE/renderer/d3d/d3d11/SwapChain11.cpp +@@ -845,7 +845,7 @@ EGLint SwapChain11::copyOffscreenToBackbuffer(const gl::Context *context, + stateManager->setRenderTarget(mBackBufferRTView.get(), nullptr); + + // Set the viewport +- stateManager->setSimpleViewport(mWidth, mHeight); ++ stateManager->setSimpleViewport(width, height); + + // Apply textures + stateManager->setSimplePixelTextureAndSampler(mOffscreenSRView, mPassThroughSampler); +-- +2.20.1.windows.1 + -- cgit v1.2.3 From 02fa39ed22e8ca5889639661b531f1653c6388f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Tue, 24 Mar 2020 10:08:49 +0100 Subject: macOS: Flush sublayers via separate IOSurface backingstores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flushing sublayers via QImage copies of the root IOSurface was causing performance regressions due to the constant allocations of new images each frame. We now re-use the QCALayerBackingStore implementation for sublayers, which gives a dynamic swap-chain. We're still paying the CPU cost of the copy from the root backingstore to the layered backingstores, as well as the memory cost, but at least improves the situation. We do not try to be smart and paint directly into the sublayers, as that would leave the root backingstore stale, potentially causing glitches when views are repositioned. Investigating this is left for future work. Fixes: QTBUG-82986 Change-Id: I758a3d8e1e40e2ed4fe6bc590a4a5a988d87a3a7 Reviewed-by: Morten Johan Sørvig Reviewed-by: Tor Arne Vestbø (cherry picked from commit ce2d68ebe1aefeae78ff2fd8ec5ff7e20790ef69) --- src/plugins/platforms/cocoa/qcocoabackingstore.h | 10 ++- src/plugins/platforms/cocoa/qcocoabackingstore.mm | 83 ++++++++++++++++------- 2 files changed, 68 insertions(+), 25 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoabackingstore.h b/src/plugins/platforms/cocoa/qcocoabackingstore.h index b57deacb57..3d9dfd8359 100644 --- a/src/plugins/platforms/cocoa/qcocoabackingstore.h +++ b/src/plugins/platforms/cocoa/qcocoabackingstore.h @@ -47,6 +47,8 @@ #include #include "qiosurfacegraphicsbuffer.h" +#include + QT_BEGIN_NAMESPACE class QCocoaBackingStore : public QRasterBackingStore @@ -71,8 +73,9 @@ private: void redrawRoundedBottomCorners(CGRect) const; }; -class QCALayerBackingStore : public QCocoaBackingStore +class QCALayerBackingStore : public QObject, public QCocoaBackingStore { + Q_OBJECT public: QCALayerBackingStore(QWindow *window); ~QCALayerBackingStore(); @@ -119,6 +122,11 @@ private: QMacNotificationObserver m_backingPropertiesObserver; std::list> m_buffers; + + void flushSubWindow(QWindow *window); + std::unordered_map> m_subWindowBackingstores; + void windowDestroyed(QObject *object); + bool m_clearSurfaceOnPaint = true; }; QT_END_NAMESPACE diff --git a/src/plugins/platforms/cocoa/qcocoabackingstore.mm b/src/plugins/platforms/cocoa/qcocoabackingstore.mm index cb019c3775..2b4c71f279 100644 --- a/src/plugins/platforms/cocoa/qcocoabackingstore.mm +++ b/src/plugins/platforms/cocoa/qcocoabackingstore.mm @@ -382,7 +382,7 @@ void QCALayerBackingStore::beginPaint(const QRegion ®ion) // Although undocumented, QBackingStore::beginPaint expects the painted region // to be cleared before use if the window has a surface format with an alpha. // Fresh IOSurfaces are already cleared, so we don't need to clear those. - if (!bufferWasRecreated && window()->format().hasAlpha()) { + if (m_clearSurfaceOnPaint && !bufferWasRecreated && window()->format().hasAlpha()) { qCDebug(lcQpaBackingStore) << "Clearing" << region << "before use"; QPainter painter(m_buffers.back()->asImage()); painter.setCompositionMode(QPainter::CompositionMode_Source); @@ -511,9 +511,13 @@ void QCALayerBackingStore::flush(QWindow *flushedWindow, const QRegion ®ion, if (!prepareForFlush()) return; + if (flushedWindow != window()) { + flushSubWindow(flushedWindow); + return; + } + QMacAutoReleasePool pool; - NSView *backingStoreView = static_cast(window()->handle())->view(); NSView *flushedView = static_cast(flushedWindow->handle())->view(); // If the backingstore is just flushed, without being painted to first, then we may @@ -548,7 +552,7 @@ void QCALayerBackingStore::flush(QWindow *flushedWindow, const QRegion ®ion, // are committed as part of a display-cycle instead of on the next runloop pass. This // means CA won't try to throttle us if we flush too fast, and we'll coalesce our flush // with other pending view and layer updates. - backingStoreView.window.viewsNeedDisplay = YES; + flushedView.window.viewsNeedDisplay = YES; if (window()->format().swapBehavior() == QSurfaceFormat::SingleBuffer) { // The private API [CALayer reloadValueForKeyPath:@"contents"] would be preferable, @@ -556,28 +560,10 @@ void QCALayerBackingStore::flush(QWindow *flushedWindow, const QRegion ®ion, flushedView.layer.contents = nil; } - if (flushedView == backingStoreView) { - qCInfo(lcQpaBackingStore) << "Flushing" << backBufferSurface - << "to" << flushedView.layer << "of" << flushedView; - flushedView.layer.contents = backBufferSurface; - } else { - auto subviewRect = [flushedView convertRect:flushedView.bounds toView:backingStoreView]; - auto scale = flushedView.layer.contentsScale; - subviewRect = CGRectApplyAffineTransform(subviewRect, CGAffineTransformMakeScale(scale, scale)); - - // We make a copy of the image data up front, which means we don't - // need to mark the IOSurface as being in use. FIXME: Investigate - // if there's a cheaper way to get sub-image data to a layer. - m_buffers.back()->lock(QPlatformGraphicsBuffer::SWReadAccess); - QImage subImage = m_buffers.back()->asImage()->copy(QRectF::fromCGRect(subviewRect).toRect()); - m_buffers.back()->unlock(); + qCInfo(lcQpaBackingStore) << "Flushing" << backBufferSurface + << "to" << flushedView.layer << "of" << flushedView; - qCInfo(lcQpaBackingStore) << "Flushing" << subImage - << "to" << flushedView.layer << "of subview" << flushedView; - QCFType cgImage = CGImageCreateCopyWithColorSpace( - QCFType(subImage.toCGImage()), colorSpace()); - flushedView.layer.contents = (__bridge id)static_cast(cgImage); - } + flushedView.layer.contents = backBufferSurface; // Since we may receive multiple flushes before a new frame is started, we do not // swap any buffers just yet. Instead we check in the next beginPaint if the layer's @@ -589,6 +575,53 @@ void QCALayerBackingStore::flush(QWindow *flushedWindow, const QRegion ®ion, // the window server. } +void QCALayerBackingStore::flushSubWindow(QWindow *subWindow) +{ + qCInfo(lcQpaBackingStore) << "Flushing sub-window" << subWindow + << "via its own backingstore"; + + auto &subWindowBackingStore = m_subWindowBackingstores[subWindow]; + if (!subWindowBackingStore) { + subWindowBackingStore.reset(new QCALayerBackingStore(subWindow)); + QObject::connect(subWindow, &QObject::destroyed, this, &QCALayerBackingStore::windowDestroyed); + subWindowBackingStore->m_clearSurfaceOnPaint = false; + } + + auto subWindowSize = subWindow->size(); + static const auto kNoStaticContents = QRegion(); + subWindowBackingStore->resize(subWindowSize, kNoStaticContents); + + auto subWindowLocalRect = QRect(QPoint(), subWindowSize); + subWindowBackingStore->beginPaint(subWindowLocalRect); + + QPainter painter(subWindowBackingStore->m_buffers.back()->asImage()); + painter.setCompositionMode(QPainter::CompositionMode_Source); + + NSView *backingStoreView = static_cast(window()->handle())->view(); + NSView *flushedView = static_cast(subWindow->handle())->view(); + auto subviewRect = [flushedView convertRect:flushedView.bounds toView:backingStoreView]; + auto scale = flushedView.layer.contentsScale; + subviewRect = CGRectApplyAffineTransform(subviewRect, CGAffineTransformMakeScale(scale, scale)); + + m_buffers.back()->lock(QPlatformGraphicsBuffer::SWReadAccess); + const QImage *backingStoreImage = m_buffers.back()->asImage(); + painter.drawImage(subWindowLocalRect, *backingStoreImage, QRectF::fromCGRect(subviewRect)); + m_buffers.back()->unlock(); + + painter.end(); + subWindowBackingStore->endPaint(); + subWindowBackingStore->flush(subWindow, subWindowLocalRect, QPoint()); + + qCInfo(lcQpaBackingStore) << "Done flushing sub-window" << subWindow; +} + +void QCALayerBackingStore::windowDestroyed(QObject *object) +{ + auto *window = static_cast(object); + qCInfo(lcQpaBackingStore) << "Removing backingstore for sub-window" << window; + m_subWindowBackingstores.erase(window); +} + #ifndef QT_NO_OPENGL void QCALayerBackingStore::composeAndFlush(QWindow *window, const QRegion ®ion, const QPoint &offset, QPlatformTextureList *textures, bool translucentBackground) @@ -722,4 +755,6 @@ QImage *QCALayerBackingStore::GraphicsBuffer::asImage() return &m_image; } +#include "moc_qcocoabackingstore.cpp" + QT_END_NAMESPACE -- cgit v1.2.3 From b7da66132bdd196c4f0b0e0fdf53f9e3b9a8bdaf Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sat, 7 Mar 2020 07:31:42 -0800 Subject: QCborValue: create a wrapper to set the QCborStreamReader error state The next commit will need to do so from outside QCborContainerPrivate, where QCborStreamReader::d can't be accessed (private). Change-Id: Iaa63461109844e978376fffd15fa0f6f04081bf2 Reviewed-by: Lars Knoll --- src/corelib/serialization/qcborvalue.cpp | 17 +++++++++++------ src/corelib/serialization/qcborvalue_p.h | 1 + 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index 4052bfa22e..5d4dc6ad5e 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -833,8 +833,6 @@ static QCborValue::Type convertToExtendedType(QCborContainerPrivate *d) return QCborValue::Tag; } -// in qcborstream.cpp -extern void qt_cbor_stream_set_error(QCborStreamReaderPrivate *d, QCborError error); static void writeDoubleToCbor(QCborStreamWriter &writer, double d, QCborValue::EncodingOptions opt) { @@ -1472,6 +1470,13 @@ static QCborValue taggedValueFromCbor(QCborStreamReader &reader) return QCborContainerPrivate::makeValue(type, -1, d); } +// in qcborstream.cpp +extern void qt_cbor_stream_set_error(QCborStreamReaderPrivate *d, QCborError error); +inline void QCborContainerPrivate::setErrorInReader(QCborStreamReader &reader, QCborError error) +{ + qt_cbor_stream_set_error(reader.d.data(), error); +} + void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) { auto addByteData_local = [this](QByteArray::size_type len) -> qint64 { @@ -1516,7 +1521,7 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) return; // error if (len != rawlen) { // truncation - qt_cbor_stream_set_error(reader.d.data(), { QCborError::DataTooLarge }); + setErrorInReader(reader, { QCborError::DataTooLarge }); return; } @@ -1526,7 +1531,7 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) e.value = addByteData_local(len); if (e.value < 0) { // overflow - qt_cbor_stream_set_error(reader.d.data(), { QCborError::DataTooLarge }); + setErrorInReader(reader, { QCborError::DataTooLarge }); return; } } @@ -1540,7 +1545,7 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) auto utf8result = QUtf8::isValidUtf8(dataPtr() + data.size() - len, len); if (!utf8result.isValidUtf8) { r.status = QCborStreamReader::Error; - qt_cbor_stream_set_error(reader.d.data(), { QCborError::InvalidUtf8String }); + setErrorInReader(reader, { QCborError::InvalidUtf8String }); break; } isAscii = isAscii && utf8result.isValidAscii; @@ -1564,7 +1569,7 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) // error r.status = QCborStreamReader::Error; - qt_cbor_stream_set_error(reader.d.data(), { QCborError::DataTooLarge }); + setErrorInReader(reader, { QCborError::DataTooLarge }); } if (r.status == QCborStreamReader::Error) { diff --git a/src/corelib/serialization/qcborvalue_p.h b/src/corelib/serialization/qcborvalue_p.h index a74ac2ba10..6d41586594 100644 --- a/src/corelib/serialization/qcborvalue_p.h +++ b/src/corelib/serialization/qcborvalue_p.h @@ -408,6 +408,7 @@ public: void decodeValueFromCbor(QCborStreamReader &reader); void decodeFromCbor(QCborStreamReader &reader); void decodeStringFromCbor(QCborStreamReader &reader); + static inline void setErrorInReader(QCborStreamReader &reader, QCborError error); }; QT_END_NAMESPACE -- cgit v1.2.3 From 02d595946faa7a21f6aa4109227f7e90db20ae7a Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sat, 7 Mar 2020 07:38:51 -0800 Subject: QCborValue::fromCbor: Apply a recursion limit to decoding A simple 16k file can produce deep enough recursion in Qt to cause stack overflow. So prevent that. I tested 4096 recursions just fine on my Linux system (8 MB stack), but decided 1024 was sufficient, as this code will also be run on embedded systems that could have smaller stacks. [ChangeLog][QtCore][QCborValue] fromCbor() now limits decoding to at most 1024 nested maps, arrays, and tags to prevent stack overflows. This should be sufficient for most uses of CBOR. An API to limit further or to relax the limit will be provided in 5.15. Meanwhile, if decoding more is required, QCborStreamReader can be used (note that each level of map and array allocates memory). Change-Id: Iaa63461109844e978376fffd15fa0fbefbf607a2 Reviewed-by: Lars Knoll --- src/corelib/serialization/qcborvalue.cpp | 39 +++++++++++----- src/corelib/serialization/qcborvalue_p.h | 4 +- .../serialization/qcborvalue/tst_qcborvalue.cpp | 54 ++++++++++++++++++++++ 3 files changed, 84 insertions(+), 13 deletions(-) diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index 5d4dc6ad5e..23b4a15c0c 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -1438,23 +1438,33 @@ static Element decodeBasicValueFromCbor(QCborStreamReader &reader) return e; } -static inline QCborContainerPrivate *createContainerFromCbor(QCborStreamReader &reader) +static inline QCborContainerPrivate *createContainerFromCbor(QCborStreamReader &reader, int remainingRecursionDepth) { + if (Q_UNLIKELY(remainingRecursionDepth == 0)) { + QCborContainerPrivate::setErrorInReader(reader, { QCborError::NestingTooDeep }); + return nullptr; + } + auto d = new QCborContainerPrivate; d->ref.storeRelaxed(1); - d->decodeFromCbor(reader); + d->decodeContainerFromCbor(reader, remainingRecursionDepth - 1); return d; } -static QCborValue taggedValueFromCbor(QCborStreamReader &reader) +static QCborValue taggedValueFromCbor(QCborStreamReader &reader, int remainingRecursionDepth) { + if (Q_UNLIKELY(remainingRecursionDepth == 0)) { + QCborContainerPrivate::setErrorInReader(reader, { QCborError::NestingTooDeep }); + return QCborValue::Invalid; + } + auto d = new QCborContainerPrivate; d->append(reader.toTag()); reader.next(); if (reader.lastError() == QCborError::NoError) { // decode tagged value - d->decodeValueFromCbor(reader); + d->decodeValueFromCbor(reader, remainingRecursionDepth - 1); } QCborValue::Type type; @@ -1595,9 +1605,10 @@ void QCborContainerPrivate::decodeStringFromCbor(QCborStreamReader &reader) elements.append(e); } -void QCborContainerPrivate::decodeValueFromCbor(QCborStreamReader &reader) +void QCborContainerPrivate::decodeValueFromCbor(QCborStreamReader &reader, int remainingRecursionDepth) { - switch (reader.type()) { + QCborStreamReader::Type t = reader.type(); + switch (t) { case QCborStreamReader::UnsignedInteger: case QCborStreamReader::NegativeInteger: case QCborStreamReader::SimpleType: @@ -1614,15 +1625,19 @@ void QCborContainerPrivate::decodeValueFromCbor(QCborStreamReader &reader) case QCborStreamReader::Array: case QCborStreamReader::Map: + return append(makeValue(t == QCborStreamReader::Array ? QCborValue::Array : QCborValue::Map, -1, + createContainerFromCbor(reader, remainingRecursionDepth), + MoveContainer)); + case QCborStreamReader::Tag: - return append(QCborValue::fromCbor(reader)); + return append(taggedValueFromCbor(reader, remainingRecursionDepth)); case QCborStreamReader::Invalid: return; // probably a decode error } } -void QCborContainerPrivate::decodeFromCbor(QCborStreamReader &reader) +void QCborContainerPrivate::decodeContainerFromCbor(QCborStreamReader &reader, int remainingRecursionDepth) { int mapShift = reader.isMap() ? 1 : 0; if (reader.isLengthKnown()) { @@ -1640,7 +1655,7 @@ void QCborContainerPrivate::decodeFromCbor(QCborStreamReader &reader) return; while (reader.hasNext() && reader.lastError() == QCborError::NoError) - decodeValueFromCbor(reader); + decodeValueFromCbor(reader, remainingRecursionDepth); if (reader.lastError() == QCborError::NoError) reader.leaveContainer(); @@ -2340,6 +2355,8 @@ QCborValueRef QCborValue::operator[](qint64 key) return { container, index }; } +enum { MaximumRecursionDepth = 1024 }; + /*! Decodes one item from the CBOR stream found in \a reader and returns the equivalent representation. This function is recursive: if the item is a map @@ -2400,12 +2417,12 @@ QCborValue QCborValue::fromCbor(QCborStreamReader &reader) case QCborStreamReader::Map: result.n = -1; result.t = reader.isArray() ? Array : Map; - result.container = createContainerFromCbor(reader); + result.container = createContainerFromCbor(reader, MaximumRecursionDepth); break; // tag case QCborStreamReader::Tag: - result = taggedValueFromCbor(reader); + result = taggedValueFromCbor(reader, MaximumRecursionDepth); break; } diff --git a/src/corelib/serialization/qcborvalue_p.h b/src/corelib/serialization/qcborvalue_p.h index 6d41586594..2358626541 100644 --- a/src/corelib/serialization/qcborvalue_p.h +++ b/src/corelib/serialization/qcborvalue_p.h @@ -405,8 +405,8 @@ public: elements.remove(idx); } - void decodeValueFromCbor(QCborStreamReader &reader); - void decodeFromCbor(QCborStreamReader &reader); + void decodeValueFromCbor(QCborStreamReader &reader, int remainiingStackDepth); + void decodeContainerFromCbor(QCborStreamReader &reader, int remainingStackDepth); void decodeStringFromCbor(QCborStreamReader &reader); static inline void setErrorInReader(QCborStreamReader &reader, QCborError error); }; diff --git a/tests/auto/corelib/serialization/qcborvalue/tst_qcborvalue.cpp b/tests/auto/corelib/serialization/qcborvalue/tst_qcborvalue.cpp index d5a9012f9f..49bb9cc144 100644 --- a/tests/auto/corelib/serialization/qcborvalue/tst_qcborvalue.cpp +++ b/tests/auto/corelib/serialization/qcborvalue/tst_qcborvalue.cpp @@ -102,6 +102,8 @@ private slots: void fromCborStreamReaderIODevice(); void validation_data(); void validation(); + void recursionLimit_data(); + void recursionLimit(); void toDiagnosticNotation_data(); void toDiagnosticNotation(); @@ -1720,6 +1722,58 @@ void tst_QCborValue::validation() } } +void tst_QCborValue::recursionLimit_data() +{ + constexpr int RecursionAttempts = 4096; + QTest::addColumn("data"); + QByteArray arrays(RecursionAttempts, char(0x81)); + QByteArray _arrays(RecursionAttempts, char(0x9f)); + QByteArray maps(RecursionAttempts, char(0xa1)); + QByteArray _maps(RecursionAttempts, char(0xbf)); + QByteArray tags(RecursionAttempts, char(0xc0)); + + QTest::newRow("array-nesting-too-deep") << arrays; + QTest::newRow("_array-nesting-too-deep") << _arrays; + QTest::newRow("map-nesting-too-deep") << maps; + QTest::newRow("_map-nesting-too-deep") << _maps; + QTest::newRow("tag-nesting-too-deep") << tags; + + QByteArray mixed(5 * RecursionAttempts, Qt::Uninitialized); + char *ptr = mixed.data(); + for (int i = 0; i < RecursionAttempts; ++i) { + quint8 type = qBound(quint8(QCborStreamReader::Array), quint8(i & 0x80), quint8(QCborStreamReader::Tag)); + quint8 additional_info = i & 0x1f; + if (additional_info == 0x1f) + (void)additional_info; // leave it + else if (additional_info > 0x1a) + additional_info = 0x1a; + else if (additional_info < 1) + additional_info = 1; + + *ptr++ = type | additional_info; + if (additional_info == 0x18) { + *ptr++ = uchar(i); + } else if (additional_info == 0x19) { + qToBigEndian(ushort(i), ptr); + ptr += 2; + } else if (additional_info == 0x1a) { + qToBigEndian(uint(i), ptr); + ptr += 4; + } + } + + QTest::newRow("mixed-nesting-too-deep") << mixed; +} + +void tst_QCborValue::recursionLimit() +{ + QFETCH(QByteArray, data); + + QCborParserError error; + QCborValue decoded = QCborValue::fromCbor(data, &error); + QCOMPARE(error.error, QCborError::NestingTooDeep); +} + void tst_QCborValue::toDiagnosticNotation_data() { QTest::addColumn("v"); -- cgit v1.2.3 From f581b041199ba7c825fbffd9110727360d1f2668 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Fri, 20 Mar 2020 18:43:09 -0300 Subject: QCborValue: apply a simple optimization to avoid unnecessary allocations If the map or array is known to be empty, we don't need to allocate a QCborContainerPrivate. Change-Id: Ief61acdfbe4d4b5ba1f0fffd15fe212b6a6e77c3 Reviewed-by: Edward Welbourne Reviewed-by: Lars Knoll --- src/corelib/serialization/qcborvalue.cpp | 56 +++++++++++++++++--------------- src/corelib/serialization/qcborvalue_p.h | 1 - 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/corelib/serialization/qcborvalue.cpp b/src/corelib/serialization/qcborvalue.cpp index 23b4a15c0c..f4ceba3836 100644 --- a/src/corelib/serialization/qcborvalue.cpp +++ b/src/corelib/serialization/qcborvalue.cpp @@ -1445,9 +1445,35 @@ static inline QCborContainerPrivate *createContainerFromCbor(QCborStreamReader & return nullptr; } - auto d = new QCborContainerPrivate; - d->ref.storeRelaxed(1); - d->decodeContainerFromCbor(reader, remainingRecursionDepth - 1); + QCborContainerPrivate *d = nullptr; + int mapShift = reader.isMap() ? 1 : 0; + if (reader.isLengthKnown()) { + quint64 len = reader.length(); + + // Clamp allocation to 1M elements (avoids crashing due to corrupt + // stream or loss of precision when converting from quint64 to + // QVector::size_type). + len = qMin(len, quint64(1024 * 1024 - 1)); + if (len) { + d = new QCborContainerPrivate; + d->ref.storeRelaxed(1); + d->elements.reserve(qsizetype(len) << mapShift); + } + } else { + d = new QCborContainerPrivate; + d->ref.storeRelaxed(1); + } + + reader.enterContainer(); + if (reader.lastError() != QCborError::NoError) + return d; + + while (reader.hasNext() && reader.lastError() == QCborError::NoError) + d->decodeValueFromCbor(reader, remainingRecursionDepth - 1); + + if (reader.lastError() == QCborError::NoError) + reader.leaveContainer(); + return d; } @@ -1637,30 +1663,6 @@ void QCborContainerPrivate::decodeValueFromCbor(QCborStreamReader &reader, int r } } -void QCborContainerPrivate::decodeContainerFromCbor(QCborStreamReader &reader, int remainingRecursionDepth) -{ - int mapShift = reader.isMap() ? 1 : 0; - if (reader.isLengthKnown()) { - quint64 len = reader.length(); - - // Clamp allocation to 1M elements (avoids crashing due to corrupt - // stream or loss of precision when converting from quint64 to - // QVector::size_type). - len = qMin(len, quint64(1024 * 1024 - 1)); - elements.reserve(qsizetype(len) << mapShift); - } - - reader.enterContainer(); - if (reader.lastError() != QCborError::NoError) - return; - - while (reader.hasNext() && reader.lastError() == QCborError::NoError) - decodeValueFromCbor(reader, remainingRecursionDepth); - - if (reader.lastError() == QCborError::NoError) - reader.leaveContainer(); -} - /*! Creates a QCborValue with byte array value \a ba. The value can later be retrieved using toByteArray(). diff --git a/src/corelib/serialization/qcborvalue_p.h b/src/corelib/serialization/qcborvalue_p.h index 2358626541..041a20e746 100644 --- a/src/corelib/serialization/qcborvalue_p.h +++ b/src/corelib/serialization/qcborvalue_p.h @@ -406,7 +406,6 @@ public: } void decodeValueFromCbor(QCborStreamReader &reader, int remainiingStackDepth); - void decodeContainerFromCbor(QCborStreamReader &reader, int remainingStackDepth); void decodeStringFromCbor(QCborStreamReader &reader); static inline void setErrorInReader(QCborStreamReader &reader, QCborError error); }; -- cgit v1.2.3