From 637d207e397c13c09a8dcbd718ee85bce2548e90 Mon Sep 17 00:00:00 2001 From: Frank Osterfeld Date: Thu, 5 Aug 2010 13:42:58 +0200 Subject: QDom: Do not crash on "text" "a:" is not a valid tagname. The function creating the element node notices that and returns 0, but the parser ignores it and continues, and then crashes later when processing the "text". This patch aborts the parsing immediately when creating the element node failed and fixes the crash. Merge-request: 2431 Reviewed-by: Olivier Goffart --- src/xml/dom/qdom.cpp | 6 ++++-- tests/auto/qdom/tst_qdom.cpp | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/xml/dom/qdom.cpp b/src/xml/dom/qdom.cpp index 1267e7e..662c796 100644 --- a/src/xml/dom/qdom.cpp +++ b/src/xml/dom/qdom.cpp @@ -7418,8 +7418,10 @@ bool QDomHandler::startElement(const QString& nsURI, const QString&, const QStri n = doc->createElement(qName); } - if (n) - n->setLocation(locator->lineNumber(), locator->columnNumber()); + if (!n) + return false; + + n->setLocation(locator->lineNumber(), locator->columnNumber()); node->appendChild(n); node = n; diff --git a/tests/auto/qdom/tst_qdom.cpp b/tests/auto/qdom/tst_qdom.cpp index 0f6cdaa..8bf7620 100644 --- a/tests/auto/qdom/tst_qdom.cpp +++ b/tests/auto/qdom/tst_qdom.cpp @@ -1776,6 +1776,7 @@ void tst_QDom::crashInSetContent() const QDomImplementation::setInvalidDataPolicy(QDomImplementation::ReturnNullNode); QDomDocument docImport; + QCOMPARE(docImport.setContent(QLatin1String("text"), true), false); QVERIFY(docImport.setContent(QLatin1String(""))); } -- cgit v0.12 From 15505360dc8e8b8f1343bba6fd3e5a9c95718d30 Mon Sep 17 00:00:00 2001 From: Benjamin Poulain Date: Thu, 5 Aug 2010 13:43:19 +0200 Subject: Replace the SSE prologues by a macro Replace the code of the SSE prologue by a macro to avoid copying the prologue everywhere. Reviewed-by: Andreas Kling --- src/corelib/tools/qsimd_p.h | 4 +++- src/gui/painting/qdrawhelper_sse2.cpp | 14 ++++---------- src/gui/painting/qdrawingprimitive_sse2_p.h | 8 ++------ 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/corelib/tools/qsimd_p.h b/src/corelib/tools/qsimd_p.h index 5ff0f97..a3148fb 100644 --- a/src/corelib/tools/qsimd_p.h +++ b/src/corelib/tools/qsimd_p.h @@ -150,7 +150,9 @@ enum CPUFeatures { Q_CORE_EXPORT uint qDetectCPUFeatures(); -Q_CORE_EXPORT uint qDetectCPUFeatures(); + +#define ALIGNMENT_PROLOGUE_16BYTES(ptr, i, length) \ + for (; i < static_cast(qMin(static_cast(length), ((4 - ((reinterpret_cast(ptr) >> 2) & 0x3)) & 0x3))); ++i) QT_END_NAMESPACE diff --git a/src/gui/painting/qdrawhelper_sse2.cpp b/src/gui/painting/qdrawhelper_sse2.cpp index 7ab9eda..22c0384 100644 --- a/src/gui/painting/qdrawhelper_sse2.cpp +++ b/src/gui/painting/qdrawhelper_sse2.cpp @@ -112,9 +112,7 @@ void qt_blend_rgb32_on_rgb32_sse2(uchar *destPixels, int dbpl, int x = 0; // First, align dest to 16 bytes: - const int offsetToAlignOn16Bytes = (4 - ((reinterpret_cast(dst) >> 2) & 0x3)) & 0x3; - const int prologLength = qMin(w, offsetToAlignOn16Bytes); - for (; x < prologLength; ++x) { + ALIGNMENT_PROLOGUE_16BYTES(dst, x, w) { quint32 s = src[x]; s = BYTE_MUL(s, const_alpha); dst[x] = INTERPOLATE_PIXEL_255(src[x], const_alpha, dst[x], one_minus_const_alpha); @@ -182,12 +180,10 @@ inline int comp_func_Plus_one_pixel(uint d, const uint s) void QT_FASTCALL comp_func_Plus_sse2(uint *dst, const uint *src, int length, uint const_alpha) { int x = 0; - const int offsetToAlignOn16Bytes = (4 - ((reinterpret_cast(dst) >> 2) & 0x3)) & 0x3; - const int prologLength = qMin(length, offsetToAlignOn16Bytes); if (const_alpha == 255) { // 1) Prologue: align destination on 16 bytes - for (; x < prologLength; ++x) + ALIGNMENT_PROLOGUE_16BYTES(dst, x, length) dst[x] = comp_func_Plus_one_pixel(dst[x], src[x]); // 2) composition with SSE2 @@ -208,7 +204,7 @@ void QT_FASTCALL comp_func_Plus_sse2(uint *dst, const uint *src, int length, uin const __m128i oneMinusConstAlpha = _mm_set1_epi16(one_minus_const_alpha); // 1) Prologue: align destination on 16 bytes - for (; x < prologLength; ++x) + ALIGNMENT_PROLOGUE_16BYTES(dst, x, length) dst[x] = comp_func_Plus_one_pixel_const_alpha(dst[x], src[x], const_alpha, one_minus_const_alpha); const __m128i half = _mm_set1_epi16(0x80); @@ -239,9 +235,7 @@ void QT_FASTCALL comp_func_Source_sse2(uint *dst, const uint *src, int length, u int x = 0; // 1) prologue, align on 16 bytes - const int offsetToAlignOn16Bytes = (4 - ((reinterpret_cast(dst) >> 2) & 0x3)) & 0x3; - const int prologLength = qMin(length, offsetToAlignOn16Bytes); - for (; x < prologLength; ++x) + ALIGNMENT_PROLOGUE_16BYTES(dst, x, length) dst[x] = INTERPOLATE_PIXEL_255(src[x], const_alpha, dst[x], ialpha); // 2) interpolate pixels with SSE2 diff --git a/src/gui/painting/qdrawingprimitive_sse2_p.h b/src/gui/painting/qdrawingprimitive_sse2_p.h index 18355c2..d8f6bf5 100644 --- a/src/gui/painting/qdrawingprimitive_sse2_p.h +++ b/src/gui/painting/qdrawingprimitive_sse2_p.h @@ -143,9 +143,7 @@ QT_BEGIN_NAMESPACE int x = 0; \ \ /* First, get dst aligned. */ \ - const int offsetToAlignOn16Bytes = (4 - ((reinterpret_cast(dst) >> 2) & 0x3)) & 0x3;\ - const int prologLength = qMin(length, offsetToAlignOn16Bytes);\ - for (; x < prologLength; ++x) { \ + ALIGNMENT_PROLOGUE_16BYTES(dst, x, length) { \ uint s = src[x]; \ if (s >= 0xff000000) \ dst[x] = s; \ @@ -202,9 +200,7 @@ QT_BEGIN_NAMESPACE { \ int x = 0; \ \ - const int offsetToAlignOn16Bytes = (4 - ((reinterpret_cast(dst) >> 2) & 0x3)) & 0x3;\ - const int prologLength = qMin(length, offsetToAlignOn16Bytes);\ - for (; x < prologLength; ++x) { \ + ALIGNMENT_PROLOGUE_16BYTES(dst, x, length) { \ quint32 s = src[x]; \ if (s != 0) { \ s = BYTE_MUL(s, const_alpha); \ -- cgit v0.12 From 05bb249c2ad3ee15eb205a806f8546c105683096 Mon Sep 17 00:00:00 2001 From: Jiang Jiang Date: Thu, 5 Aug 2010 11:21:03 +0200 Subject: Fix QTextEngine overflow caused by extremely long text Internally, QTextEngine use int to calculate most of the layout data required. If a string longer than 2^24 is passed into either QTextEngine or classes using it (QStackTextEngine, QPainter, QFontMetrics, etc.), overflow will happen because the memory size required to allocate for layout will become too large for int to handle. This patch will prevent these cases and add error handling mechanism for relevant code. Task-number: QT-3658 Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/gui/text/qtextengine.cpp | 87 ++++++++++++++++++++++++++++++++------------ src/gui/text/qtextengine_p.h | 14 +++++-- src/gui/text/qtextlayout.cpp | 19 +++++++--- 3 files changed, 87 insertions(+), 33 deletions(-) diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp index 439f2a4..5670e29 100644 --- a/src/gui/text/qtextengine.cpp +++ b/src/gui/text/qtextengine.cpp @@ -923,6 +923,13 @@ void QTextEngine::shapeText(int item) const si.width += glyphs.advances_x[i]; } +static inline bool hasCaseChange(const QScriptItem &si) +{ + return si.analysis.flags == QScriptAnalysis::SmallCaps || + si.analysis.flags == QScriptAnalysis::Uppercase || + si.analysis.flags == QScriptAnalysis::Lowercase; +} + #if defined(Q_WS_WINCE) //TODO // set the glyph attributes heuristically. Assumes a 1 to 1 relationship between chars and glyphs // and no reordering. @@ -1050,14 +1057,15 @@ void QTextEngine::shapeTextWithCE(int item) const if (option.useDesignMetrics()) flags |= DesignMetrics; - attributes(); // pre-initialize char attributes + // pre-initialize char attributes + if (! attributes()) + return; const int len = length(item); int num_glyphs = length(item); const QChar *str = layoutData->string.unicode() + si.position; ushort upperCased[256]; - if (si.analysis.flags == QScriptAnalysis::SmallCaps || si.analysis.flags == QScriptAnalysis::Uppercase - || si.analysis.flags == QScriptAnalysis::Lowercase) { + if (hasCaseChange(si)) { ushort *uc = upperCased; if (len > 256) uc = new ushort[len]; @@ -1071,7 +1079,14 @@ void QTextEngine::shapeTextWithCE(int item) const } while (true) { - ensureSpace(num_glyphs); + if (! ensureSpace(num_glyphs)) { + // If str is converted to uppercase/lowercase form with a new buffer, + // we need to delete that buffer before return for error + const ushort *uc = reinterpret_cast(str); + if (hasCaseChange(si) && uc != upperCased) + delete [] uc; + return; + } num_glyphs = layoutData->glyphLayout.numGlyphs - layoutData->used; QGlyphLayout g = availableGlyphs(&si); @@ -1092,9 +1107,7 @@ void QTextEngine::shapeTextWithCE(int item) const layoutData->used += si.num_glyphs; const ushort *uc = reinterpret_cast(str); - if ((si.analysis.flags == QScriptAnalysis::SmallCaps || si.analysis.flags == QScriptAnalysis::Uppercase - || si.analysis.flags == QScriptAnalysis::Lowercase) - && uc != upperCased) + if (hasCaseChange(si) && uc != upperCased) delete [] uc; } #endif @@ -1133,8 +1146,7 @@ void QTextEngine::shapeTextWithHarfbuzz(int item) const entire_shaper_item.item.bidiLevel = si.analysis.bidiLevel; HB_UChar16 upperCased[256]; // XXX what about making this 4096, so we don't have to extend it ever. - if (si.analysis.flags == QScriptAnalysis::SmallCaps || si.analysis.flags == QScriptAnalysis::Uppercase - || si.analysis.flags == QScriptAnalysis::Lowercase) { + if (hasCaseChange(si)) { HB_UChar16 *uc = upperCased; if (entire_shaper_item.item.length > 256) uc = new HB_UChar16[entire_shaper_item.item.length]; @@ -1156,17 +1168,24 @@ void QTextEngine::shapeTextWithHarfbuzz(int item) const entire_shaper_item.shaperFlags |= HB_ShaperFlag_UseDesignMetrics; entire_shaper_item.num_glyphs = qMax(layoutData->glyphLayout.numGlyphs - layoutData->used, int(entire_shaper_item.item.length)); - ensureSpace(entire_shaper_item.num_glyphs); + if (! ensureSpace(entire_shaper_item.num_glyphs)) { + if (hasCaseChange(si)) + delete [] const_cast(entire_shaper_item.string); + return; + } QGlyphLayout initialGlyphs = availableGlyphs(&si).mid(0, entire_shaper_item.num_glyphs); if (!stringToGlyphs(&entire_shaper_item, &initialGlyphs, font)) { - ensureSpace(entire_shaper_item.num_glyphs); + if (! ensureSpace(entire_shaper_item.num_glyphs)) { + if (hasCaseChange(si)) + delete [] const_cast(entire_shaper_item.string); + return; + } initialGlyphs = availableGlyphs(&si).mid(0, entire_shaper_item.num_glyphs); if (!stringToGlyphs(&entire_shaper_item, &initialGlyphs, font)) { // ############ if this happens there's a bug in the fontengine - if ((si.analysis.flags == QScriptAnalysis::SmallCaps || si.analysis.flags == QScriptAnalysis::Uppercase - || si.analysis.flags == QScriptAnalysis::Lowercase) && entire_shaper_item.string != upperCased) + if (hasCaseChange(si) && entire_shaper_item.string != upperCased) delete [] const_cast(entire_shaper_item.string); return; } @@ -1231,7 +1250,11 @@ void QTextEngine::shapeTextWithHarfbuzz(int item) const remaining_glyphs -= shaper_item.initialGlyphCount; do { - ensureSpace(glyph_pos + shaper_item.num_glyphs + remaining_glyphs); + if (! ensureSpace(glyph_pos + shaper_item.num_glyphs + remaining_glyphs)) { + if (hasCaseChange(si)) + delete [] const_cast(entire_shaper_item.string); + return; + } const QGlyphLayout g = availableGlyphs(&si).mid(glyph_pos); moveGlyphData(g.mid(shaper_item.num_glyphs), g.mid(shaper_item.initialGlyphCount), remaining_glyphs); @@ -1271,8 +1294,7 @@ void QTextEngine::shapeTextWithHarfbuzz(int item) const layoutData->used += si.num_glyphs; - if ((si.analysis.flags == QScriptAnalysis::SmallCaps || si.analysis.flags == QScriptAnalysis::Uppercase) - && entire_shaper_item.string != upperCased) + if (hasCaseChange(si) && entire_shaper_item.string != upperCased) delete [] const_cast(entire_shaper_item.string); } @@ -1317,7 +1339,8 @@ const HB_CharAttributes *QTextEngine::attributes() const return (HB_CharAttributes *) layoutData->memory; itemize(); - ensureSpace(layoutData->string.length()); + if (! ensureSpace(layoutData->string.length())) + return NULL; QVarLengthArray hbScriptItems(layoutData->items.size()); @@ -1864,7 +1887,10 @@ void QTextEngine::justify(const QScriptLine &line) // don't include trailing white spaces when doing justification int line_length = line.length; - const HB_CharAttributes *a = attributes()+line.from; + const HB_CharAttributes *a = attributes(); + if (! a) + return; + a += line.from; while (line_length && a[line_length-1].whiteSpace) --line_length; // subtract one char more, as we can't justfy after the last character @@ -2045,7 +2071,7 @@ QTextEngine::LayoutData::LayoutData() memory_on_stack = false; used = 0; hasBidi = false; - inLayout = false; + layoutState = LayoutEmpty; haveCharAttributes = false; logClustersPtr = 0; available_glyphs = 0; @@ -2079,7 +2105,7 @@ QTextEngine::LayoutData::LayoutData(const QString &str, void **stack_memory, int } used = 0; hasBidi = false; - inLayout = false; + layoutState = LayoutEmpty; haveCharAttributes = false; } @@ -2090,12 +2116,12 @@ QTextEngine::LayoutData::~LayoutData() memory = 0; } -void QTextEngine::LayoutData::reallocate(int totalGlyphs) +bool QTextEngine::LayoutData::reallocate(int totalGlyphs) { Q_ASSERT(totalGlyphs >= glyphLayout.numGlyphs); if (memory_on_stack && available_glyphs >= totalGlyphs) { glyphLayout.grow(glyphLayout.data(), totalGlyphs); - return; + return true; } int space_charAttributes = sizeof(HB_CharAttributes)*string.length()/sizeof(void*) + 1; @@ -2103,7 +2129,14 @@ void QTextEngine::LayoutData::reallocate(int totalGlyphs) int space_glyphs = QGlyphLayout::spaceNeededForGlyphLayout(totalGlyphs)/sizeof(void*) + 2; int newAllocated = space_charAttributes + space_glyphs + space_logClusters; - Q_ASSERT(newAllocated >= allocated); + // These values can be negative if the length of string/glyphs causes overflow, + // we can't layout such a long string all at once, so return false here to + // indicate there is a failure + if (space_charAttributes < 0 || space_logClusters < 0 || space_glyphs < 0 || newAllocated < allocated) { + layoutState = LayoutFailed; + return false; + } + void **newMem = memory; newMem = (void **)::realloc(memory_on_stack ? 0 : memory, newAllocated*sizeof(void *)); Q_CHECK_PTR(newMem); @@ -2124,6 +2157,7 @@ void QTextEngine::LayoutData::reallocate(int totalGlyphs) glyphLayout.grow(reinterpret_cast(m), totalGlyphs); allocated = newAllocated; + return true; } // grow to the new size, copying the existing data to the new layout @@ -2155,7 +2189,7 @@ void QTextEngine::freeMemory() } else { layoutData->used = 0; layoutData->hasBidi = false; - layoutData->inLayout = false; + layoutData->layoutState = LayoutEmpty; layoutData->haveCharAttributes = false; } for (int i = 0; i < lines.size(); ++i) { @@ -2314,6 +2348,9 @@ QString QTextEngine::elidedText(Qt::TextElideMode mode, const QFixed &width, int shape(i); HB_CharAttributes *attributes = const_cast(this->attributes()); + if (!attributes) + return QString(); + unsigned short *logClusters = this->logClusters(&si); QGlyphLayout glyphs = shapedGlyphs(&si); @@ -2385,6 +2422,8 @@ QString QTextEngine::elidedText(Qt::TextElideMode mode, const QFixed &width, int return QString(); const HB_CharAttributes *attributes = this->attributes(); + if (!attributes) + return QString(); if (mode == Qt::ElideRight) { QFixed currentWidth; diff --git a/src/gui/text/qtextengine_p.h b/src/gui/text/qtextengine_p.h index e623fa5..4cbe81f 100644 --- a/src/gui/text/qtextengine_p.h +++ b/src/gui/text/qtextengine_p.h @@ -416,6 +416,11 @@ class QTextFormatCollection; class Q_GUI_EXPORT QTextEngine { public: + enum LayoutState { + LayoutEmpty, + InLayout, + LayoutFailed, + }; struct LayoutData { LayoutData(const QString &str, void **stack_memory, int mem_size); LayoutData(); @@ -428,11 +433,11 @@ public: QGlyphLayout glyphLayout; mutable int used; uint hasBidi : 1; - uint inLayout : 1; + uint layoutState : 2; uint memory_on_stack : 1; bool haveCharAttributes; QString string; - void reallocate(int totalGlyphs); + bool reallocate(int totalGlyphs); }; QTextEngine(LayoutData *data); @@ -520,9 +525,10 @@ public: return layoutData->glyphLayout.mid(si->glyph_data_offset, si->num_glyphs); } - inline void ensureSpace(int nGlyphs) const { + inline bool ensureSpace(int nGlyphs) const { if (layoutData->glyphLayout.numGlyphs - layoutData->used < nGlyphs) - layoutData->reallocate((((layoutData->used + nGlyphs)*3/2 + 15) >> 4) << 4); + return layoutData->reallocate((((layoutData->used + nGlyphs)*3/2 + 15) >> 4) << 4); + return true; } void freeMemory(); diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp index 674064e..5a11c87 100644 --- a/src/gui/text/qtextlayout.cpp +++ b/src/gui/text/qtextlayout.cpp @@ -74,6 +74,8 @@ static inline QFixed leadingSpaceWidth(QTextEngine *eng, const QScriptLine &line int pos = line.length; const HB_CharAttributes *attributes = eng->attributes(); + if (!attributes) + return QFixed(); while (pos > 0 && attributes[line.from + pos - 1].whiteSpace) --pos; return eng->width(line.from + pos, line.length - pos); @@ -601,7 +603,7 @@ bool QTextLayout::cacheEnabled() const void QTextLayout::beginLayout() { #ifndef QT_NO_DEBUG - if (d->layoutData && d->layoutData->inLayout) { + if (d->layoutData && d->layoutData->layoutState == QTextEngine::InLayout) { qWarning("QTextLayout::beginLayout: Called while already doing layout"); return; } @@ -609,7 +611,7 @@ void QTextLayout::beginLayout() d->invalidate(); d->clearLineData(); d->itemize(); - d->layoutData->inLayout = true; + d->layoutData->layoutState = QTextEngine::InLayout; } /*! @@ -618,7 +620,7 @@ void QTextLayout::beginLayout() void QTextLayout::endLayout() { #ifndef QT_NO_DEBUG - if (!d->layoutData || !d->layoutData->inLayout) { + if (!d->layoutData || d->layoutData->layoutState == QTextEngine::LayoutEmpty) { qWarning("QTextLayout::endLayout: Called without beginLayout()"); return; } @@ -627,7 +629,7 @@ void QTextLayout::endLayout() if (l && d->lines.at(l-1).length < 0) { QTextLine(l-1, d).setNumColumns(INT_MAX); } - d->layoutData->inLayout = false; + d->layoutData->layoutState = QTextEngine::LayoutEmpty; if (!d->cacheGlyphs) d->freeMemory(); } @@ -757,11 +759,14 @@ bool QTextLayout::isValidCursorPosition(int pos) const QTextLine QTextLayout::createLine() { #ifndef QT_NO_DEBUG - if (!d->layoutData || !d->layoutData->inLayout) { + if (!d->layoutData || d->layoutData->layoutState == QTextEngine::LayoutEmpty) { qWarning("QTextLayout::createLine: Called without layouting"); return QTextLine(); } #endif + if (d->layoutData->layoutState == QTextEngine::LayoutFailed) + return QTextLine(); + int l = d->lines.size(); if (l && d->lines.at(l-1).length < 0) { QTextLine(l-1, d).setNumColumns(INT_MAX); @@ -1801,6 +1806,8 @@ void QTextLine::layout_helper(int maxGlyphs) Qt::Alignment alignment = eng->option.alignment(); const HB_CharAttributes *attributes = eng->attributes(); + if (!attributes) + return; lbh.currentPosition = line.from; int end = 0; lbh.logClusters = eng->layoutData->logClustersPtr; @@ -1814,6 +1821,8 @@ void QTextLine::layout_helper(int maxGlyphs) if (!current.num_glyphs) { eng->shape(item); attributes = eng->attributes(); + if (!attributes) + return; lbh.logClusters = eng->layoutData->logClustersPtr; } lbh.currentPosition = qMax(line.from, current.position); -- cgit v0.12 From a49f2700c6b5c0f5851f9cfab38b5554e9530577 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 5 Aug 2010 17:22:38 +0200 Subject: QStyleSheet documentation: QMenu's tear-off is styled with ::tearoff Task-number: QTBUG-12553 Reviewed-by: Carlos Duclos --- doc/src/widgets-and-layouts/stylesheet.qdoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/widgets-and-layouts/stylesheet.qdoc b/doc/src/widgets-and-layouts/stylesheet.qdoc index 1390376..977f641 100644 --- a/doc/src/widgets-and-layouts/stylesheet.qdoc +++ b/doc/src/widgets-and-layouts/stylesheet.qdoc @@ -925,7 +925,7 @@ The scroller is styled using the \l{#scroller-sub}{::scroller}. - The tear-off is styled using the \l{#tear-off-sub}{::tear-off}. + The tear-off is styled using the \l{#tearoff-sub}{::tearoff}. See \l{Qt Style Sheets Examples#Customizing QMenu}{Customizing QMenu} for an example. @@ -3337,7 +3337,7 @@ \o The tear indicator of a QTabBar. \row - \o \c ::tear-off \target tear-off-sub + \o \c ::tearoff \target tearoff-sub \o The tear-off indicator of a QMenu. \row -- cgit v0.12