tst_QTreeView: Fix UB (invalid downcast, member call)

As reported by UBSan:

  tst_qtreeview.cpp:2187:36: runtime error: downcast of address 0x7ffc15749f20 which does not point to an object of type 'PublicView'
    0x7ffc15749f20: note: object is of type 'QTreeView'

Fix by making the test a friend of QTreeView (and, for
Clang, of QAbstractItemView) instead.

Change-Id: I5b748696ab441a91058f4d45a18bd5ed75a6e560
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
This commit is contained in:
Marc Mutz 2016-07-28 19:51:53 +03:00
parent 08f38d2214
commit 5dc83974f7
3 changed files with 32 additions and 70 deletions

View File

@ -40,6 +40,7 @@
#include <QtWidgets/qabstractitemdelegate.h> #include <QtWidgets/qabstractitemdelegate.h>
class tst_QAbstractItemView; class tst_QAbstractItemView;
class tst_QTreeView;
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
@ -362,6 +363,7 @@ private:
#endif #endif
friend class ::tst_QAbstractItemView; friend class ::tst_QAbstractItemView;
friend class ::tst_QTreeView;
friend class QTreeViewPrivate; // needed to compile with MSVC friend class QTreeViewPrivate; // needed to compile with MSVC
friend class QListModeViewBase; friend class QListModeViewBase;
friend class QListViewPrivate; friend class QListViewPrivate;

View File

@ -36,6 +36,8 @@
#include <QtWidgets/qabstractitemview.h> #include <QtWidgets/qabstractitemview.h>
class tst_QTreeView;
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
@ -213,6 +215,7 @@ protected:
void currentChanged(const QModelIndex &current, const QModelIndex &previous) Q_DECL_OVERRIDE; void currentChanged(const QModelIndex &current, const QModelIndex &previous) Q_DECL_OVERRIDE;
private: private:
friend class ::tst_QTreeView;
friend class QAccessibleTable; friend class QAccessibleTable;
friend class QAccessibleTree; friend class QAccessibleTree;
friend class QAccessibleTableCell; friend class QAccessibleTableCell;

View File

@ -36,7 +36,7 @@
#include <QtTest/QtTest> #include <QtTest/QtTest>
#include <QtGui/QtGui> #include <QtGui/QtGui>
#include <QtWidgets/QtWidgets> #include <QtWidgets/QtWidgets>
#include <private/qabstractitemview_p.h> #include <private/qtreeview_p.h>
#ifndef QT_NO_DRAGANDDROP #ifndef QT_NO_DRAGANDDROP
Q_DECLARE_METATYPE(QAbstractItemView::DragDropMode) Q_DECLARE_METATYPE(QAbstractItemView::DragDropMode)
@ -62,49 +62,6 @@ static void initStandardTreeModel(QStandardItemModel *model)
model->insertRow(2, item); model->insertRow(2, item);
} }
class tst_QTreeView;
struct PublicView : public QTreeView
{
friend class tst_QTreeView;
inline void executeDelayedItemsLayout()
{ QTreeView::executeDelayedItemsLayout(); }
enum PublicCursorAction {
MoveUp = QAbstractItemView::MoveUp,
MoveDown = QAbstractItemView::MoveDown,
MoveLeft = QAbstractItemView::MoveLeft,
MoveRight = QAbstractItemView::MoveRight,
MoveHome = QAbstractItemView::MoveHome,
MoveEnd = QAbstractItemView::MoveEnd,
MovePageUp = QAbstractItemView::MovePageUp,
MovePageDown = QAbstractItemView::MovePageDown,
MoveNext = QAbstractItemView::MoveNext,
MovePrevious = QAbstractItemView::MovePrevious
};
// enum PublicCursorAction and moveCursor() are protected in QTreeView.
inline QModelIndex doMoveCursor(PublicCursorAction ca, Qt::KeyboardModifiers kbm)
{ return QTreeView::moveCursor((CursorAction)ca, kbm); }
inline void setSelection(const QRect &rect, QItemSelectionModel::SelectionFlags command)
{
QTreeView::setSelection(rect, command);
}
inline int state()
{
return QTreeView::state();
}
inline int rowHeight(QModelIndex idx) { return QTreeView::rowHeight(idx); }
inline int indexRowSizeHint(const QModelIndex &index) const { return QTreeView::indexRowSizeHint(index); }
inline QModelIndexList selectedIndexes() const { return QTreeView::selectedIndexes(); }
inline QStyleOptionViewItem viewOptions() const { return QTreeView::viewOptions(); }
inline int sizeHintForColumn(int column) const { return QTreeView::sizeHintForColumn(column); }
QAbstractItemViewPrivate* aiv_priv() { return static_cast<QAbstractItemViewPrivate*>(d_ptr.data()); }
};
// Make a widget frameless to prevent size constraints of title bars // Make a widget frameless to prevent size constraints of title bars
// from interfering (Windows). // from interfering (Windows).
static inline void setFrameless(QWidget *w) static inline void setFrameless(QWidget *w)
@ -1783,7 +1740,7 @@ void tst_QTreeView::moveCursor()
QFETCH(bool, scrollPerPixel); QFETCH(bool, scrollPerPixel);
QtTestModel model(8, 6); QtTestModel model(8, 6);
PublicView view; QTreeView view;
view.setUniformRowHeights(uniformRowHeights); view.setUniformRowHeights(uniformRowHeights);
view.setModel(&model); view.setModel(&model);
view.setRowHidden(0, QModelIndex(), true); view.setRowHidden(0, QModelIndex(), true);
@ -1802,7 +1759,7 @@ void tst_QTreeView::moveCursor()
QCOMPARE(view.currentIndex(), expected); QCOMPARE(view.currentIndex(), expected);
//then pressing down should go to the next line //then pressing down should go to the next line
QModelIndex actual = view.doMoveCursor(PublicView::MoveDown, Qt::NoModifier); QModelIndex actual = view.moveCursor(QTreeView::MoveDown, Qt::NoModifier);
expected = model.index(2, 1, QModelIndex()); expected = model.index(2, 1, QModelIndex());
QCOMPARE(actual, expected); QCOMPARE(actual, expected);
@ -1811,7 +1768,7 @@ void tst_QTreeView::moveCursor()
// PageUp was broken with uniform row heights turned on // PageUp was broken with uniform row heights turned on
view.setCurrentIndex(model.index(1, 0)); view.setCurrentIndex(model.index(1, 0));
actual = view.doMoveCursor(PublicView::MovePageUp, Qt::NoModifier); actual = view.moveCursor(QTreeView::MovePageUp, Qt::NoModifier);
expected = model.index(0, 0, QModelIndex()); expected = model.index(0, 0, QModelIndex());
QCOMPARE(actual, expected); QCOMPARE(actual, expected);
@ -1910,7 +1867,7 @@ void tst_QTreeView::setSelection()
QtTestModel model(10, 5); QtTestModel model(10, 5);
model.levels = 1; model.levels = 1;
model.setDecorationsEnabled(true); model.setDecorationsEnabled(true);
PublicView view; QTreeView view;
view.resize(400, 300); view.resize(400, 300);
view.show(); view.show();
view.setRootIsDecorated(false); view.setRootIsDecorated(false);
@ -2097,7 +2054,7 @@ void tst_QTreeView::rowsAboutToBeRemoved()
} }
} }
PublicView view; QTreeView view;
view.setModel(&model); view.setModel(&model);
view.show(); view.show();
QModelIndex index = model.index(0,0, QModelIndex()); QModelIndex index = model.index(0,0, QModelIndex());
@ -2111,7 +2068,7 @@ void tst_QTreeView::rowsAboutToBeRemoved()
QSignalSpy spy1(&model, SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int))); QSignalSpy spy1(&model, SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)));
model.removeRows(1,1); model.removeRows(1,1);
QCOMPARE(view.state(), 0); QCOMPARE(int(view.state()), 0);
// Should not be 5 (or any other number for that sake :) // Should not be 5 (or any other number for that sake :)
QCOMPARE(spy1.count(), 1); QCOMPARE(spy1.count(), 1);
@ -2214,7 +2171,7 @@ void tst_QTreeView::rowsAboutToBeRemoved_move()
view.resize(600,800); view.resize(600,800);
view.show(); view.show();
view.doItemsLayout(); view.doItemsLayout();
static_cast<PublicView *>(&view)->executeDelayedItemsLayout(); static_cast<QTreeView *>(&view)->executeDelayedItemsLayout();
parent = indexThatWantsToLiveButWillDieDieITellYou.parent(); parent = indexThatWantsToLiveButWillDieDieITellYou.parent();
QCOMPARE(view.isExpanded(indexThatWantsToLiveButWillDieDieITellYou), true); QCOMPARE(view.isExpanded(indexThatWantsToLiveButWillDieDieITellYou), true);
QCOMPARE(parent.isValid(), true); QCOMPARE(parent.isValid(), true);
@ -2320,7 +2277,7 @@ void tst_QTreeView::spanningItems()
{ {
QtTestModel model; QtTestModel model;
model.rows = model.cols = 10; model.rows = model.cols = 10;
PublicView view; QTreeView view;
view.setModel(&model); view.setModel(&model);
view.show(); view.show();
@ -2459,7 +2416,7 @@ void tst_QTreeView::selectionWithHiddenItems()
void tst_QTreeView::selectAll() void tst_QTreeView::selectAll()
{ {
QStandardItemModel model(4,4); QStandardItemModel model(4,4);
PublicView view2; QTreeView view2;
view2.setModel(&model); view2.setModel(&model);
view2.setSelectionMode(QAbstractItemView::ExtendedSelection); view2.setSelectionMode(QAbstractItemView::ExtendedSelection);
view2.selectAll(); // Should work with an empty model view2.selectAll(); // Should work with an empty model
@ -2468,13 +2425,13 @@ void tst_QTreeView::selectAll()
for (int i = 0; i < model.rowCount(); ++i) for (int i = 0; i < model.rowCount(); ++i)
model.setData(model.index(i,0), QString("row %1").arg(i)); model.setData(model.index(i,0), QString("row %1").arg(i));
PublicView view; QTreeView view;
view.setModel(&model); view.setModel(&model);
int selectedCount = view.selectedIndexes().count(); int selectedCount = view.selectedIndexes().count();
view.selectAll(); view.selectAll();
QCOMPARE(view.selectedIndexes().count(), selectedCount); QCOMPARE(view.selectedIndexes().count(), selectedCount);
PublicView view3; QTreeView view3;
view3.setModel(&model); view3.setModel(&model);
view3.setSelectionMode(QAbstractItemView::NoSelection); view3.setSelectionMode(QAbstractItemView::NoSelection);
view3.selectAll(); view3.selectAll();
@ -2836,7 +2793,7 @@ void tst_QTreeView::evilModel()
{ {
QFETCH(bool, visible); QFETCH(bool, visible);
// init // init
PublicView view; QTreeView view;
EvilModel model; EvilModel model;
view.setModel(&model); view.setModel(&model);
view.setVisible(visible); view.setVisible(visible);
@ -2894,7 +2851,7 @@ void tst_QTreeView::evilModel()
view.setSelection(rect, QItemSelectionModel::Select); view.setSelection(rect, QItemSelectionModel::Select);
model.change(); model.change();
view.doMoveCursor(PublicView::MoveDown, Qt::NoModifier); view.moveCursor(QTreeView::MoveDown, Qt::NoModifier);
model.change(); model.change();
view.resizeColumnToContents(1); view.resizeColumnToContents(1);
@ -3005,7 +2962,7 @@ void tst_QTreeView::evilModel()
void tst_QTreeView::indexRowSizeHint() void tst_QTreeView::indexRowSizeHint()
{ {
QStandardItemModel model(10, 1); QStandardItemModel model(10, 1);
PublicView view; QTreeView view;
view.setModel(&model); view.setModel(&model);
@ -3051,7 +3008,7 @@ void tst_QTreeView::renderToPixmap_data()
void tst_QTreeView::renderToPixmap() void tst_QTreeView::renderToPixmap()
{ {
QFETCH(int, row); QFETCH(int, row);
PublicView view; QTreeView view;
QStandardItemModel model; QStandardItemModel model;
model.appendRow(new QStandardItem("Spanning")); model.appendRow(new QStandardItem("Spanning"));
@ -3067,7 +3024,7 @@ void tst_QTreeView::renderToPixmap()
// We select the index at row=1 for coverage. // We select the index at row=1 for coverage.
QItemSelection sel(model.index(row,0), model.index(row,1)); QItemSelection sel(model.index(row,0), model.index(row,1));
QRect rect; QRect rect;
view.aiv_priv()->renderToPixmap(sel.indexes(), &rect); view.d_func()->renderToPixmap(sel.indexes(), &rect);
} }
#endif #endif
} }
@ -3129,7 +3086,7 @@ void tst_QTreeView::styleOptionViewItem()
bool allCollapsed; bool allCollapsed;
}; };
PublicView view; QTreeView view;
QStandardItemModel model; QStandardItemModel model;
view.setModel(&model); view.setModel(&model);
MyDelegate delegate; MyDelegate delegate;
@ -3185,7 +3142,7 @@ void tst_QTreeView::styleOptionViewItem()
delegate.count = 0; delegate.count = 0;
QItemSelection sel(model.index(0,0), model.index(0,modelColumns-1)); QItemSelection sel(model.index(0,0), model.index(0,modelColumns-1));
QRect rect; QRect rect;
view.aiv_priv()->renderToPixmap(sel.indexes(), &rect); view.d_func()->renderToPixmap(sel.indexes(), &rect);
if (delegate.count != visibleColumns) { if (delegate.count != visibleColumns) {
qDebug() << rect << view.rect() << view.isVisible(); qDebug() << rect << view.rect() << view.isVisible();
} }
@ -3213,7 +3170,7 @@ void tst_QTreeView::styleOptionViewItem()
delegate.count = 0; delegate.count = 0;
QItemSelection sel(model.index(0,0), model.index(0,modelColumns-1)); QItemSelection sel(model.index(0,0), model.index(0,modelColumns-1));
QRect rect; QRect rect;
view.aiv_priv()->renderToPixmap(sel.indexes(), &rect); view.d_func()->renderToPixmap(sel.indexes(), &rect);
if (delegate.count != visibleColumns) { if (delegate.count != visibleColumns) {
qDebug() << rect << view.rect() << view.isVisible(); qDebug() << rect << view.rect() << view.isVisible();
} }
@ -4168,7 +4125,7 @@ void tst_QTreeView::taskQTBUG_13567_removeLastItemRegression()
// Note: define QT_BUILD_INTERNAL to run this test // Note: define QT_BUILD_INTERNAL to run this test
void tst_QTreeView::taskQTBUG_25333_adjustViewOptionsForIndex() void tst_QTreeView::taskQTBUG_25333_adjustViewOptionsForIndex()
{ {
PublicView view; QTreeView view;
QStandardItemModel model; QStandardItemModel model;
QStandardItem *item1 = new QStandardItem("Item1"); QStandardItem *item1 = new QStandardItem("Item1");
QStandardItem *item2 = new QStandardItem("Item2"); QStandardItem *item2 = new QStandardItem("Item2");
@ -4194,9 +4151,9 @@ void tst_QTreeView::taskQTBUG_25333_adjustViewOptionsForIndex()
{ {
QStyleOptionViewItem option; QStyleOptionViewItem option;
view.aiv_priv()->adjustViewOptionsForIndex(&option, model.indexFromItem(item1)); view.d_func()->adjustViewOptionsForIndex(&option, model.indexFromItem(item1));
view.aiv_priv()->adjustViewOptionsForIndex(&option, model.indexFromItem(item3)); view.d_func()->adjustViewOptionsForIndex(&option, model.indexFromItem(item3));
} }
#endif #endif
@ -4302,7 +4259,7 @@ void tst_QTreeView::quickExpandCollapse()
//this unit tests makes sure the state after the animation is restored correctly //this unit tests makes sure the state after the animation is restored correctly
//after starting a 2nd animation while the first one was still on-going //after starting a 2nd animation while the first one was still on-going
//this tests that the stateBeforeAnimation is not set to AnimatingState //this tests that the stateBeforeAnimation is not set to AnimatingState
PublicView tree; QTreeView tree;
tree.setAnimated(true); tree.setAnimated(true);
QStandardItemModel model; QStandardItemModel model;
QStandardItem *root = new QStandardItem("root"); QStandardItem *root = new QStandardItem("root");
@ -4316,13 +4273,13 @@ void tst_QTreeView::quickExpandCollapse()
tree.show(); tree.show();
QTest::qWaitForWindowExposed(&tree); QTest::qWaitForWindowExposed(&tree);
int initialState = tree.state(); const QAbstractItemView::State initialState = tree.state();
tree.expand(rootIndex); tree.expand(rootIndex);
QCOMPARE(tree.state(), (int)PublicView::AnimatingState); QCOMPARE(tree.state(), QTreeView::AnimatingState);
tree.collapse(rootIndex); tree.collapse(rootIndex);
QCOMPARE(tree.state(), (int)PublicView::AnimatingState); QCOMPARE(tree.state(), QTreeView::AnimatingState);
QTest::qWait(500); //the animation lasts for 250ms max so 500 should be enough QTest::qWait(500); //the animation lasts for 250ms max so 500 should be enough