pqAnimationTimeWidget: Fix undefined behavior
Fix a problem that recently cropped up with an old state file, which causes a crash on some user systems (but on my system I had to build with -fsanitize=address in order to detect the problem).
This code change looks to me to introduce undefined behavior: If the break statement is never hit, then idx is equal to the length of the vector, and in that case we should not use it to index the vector.
Click to expand log
Here's the output from the sanitizer run when I opened the old state file:
$ ./bin/paraview
=================================================================
==1772250==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6070005a84a0 at pc 0x7f1bb12e0129 bp 0x7fffa486cd00 sp 0x7fffa486ccf0
READ of size 8 at 0x6070005a84a0 thread T0
#0 0x7f1bb12e0128 in pqAnimationTimeWidget::pqInternals::render(pqAnimationTimeWidget*) /data/scott/projects/ParaView/pvSrc/Qt/Components/pqAnimationTimeWidget.cxx:238
#1 0x7f1bb12e39ec in pqAnimationTimeWidget::updateCurrentTime(double) /data/scott/projects/ParaView/pvSrc/Qt/Components/pqAnimationTimeWidget.cxx:319
#2 0x7f1bb1192ff6 in pqAnimationTimeWidget::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (/data/scott/projects/ParaView/build_san/bin/../lib/libpqComponents-pv5.12.so.1+0x1f97ff6)
#3 0x7f1b3c5a6e18 in QMetaObject::activate(QObject*, int, int, void**) kernel/qobject.cpp:3804
#4 0x7f1bad0de8ef in pqAnimationScene::animationTime(double) /data/scott/projects/ParaView/build_san/Qt/Core/pqCore_autogen/EWIEGA46WW/moc_pqAnimationScene.cpp:401
#5 0x7f1bad1be548 in pqAnimationScene::onAnimationTimePropertyChanged() /data/scott/projects/ParaView/pvSrc/Qt/Core/pqAnimationScene.cxx:495
#6 0x7f1bad0da234 in pqAnimationScene::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) /data/scott/projects/ParaView/build_san/Qt/Core/pqCore_autogen/EWIEGA46WW/moc_pqAnimationScene.cpp:171
#7 0x7f1b3c5a6e18 in QMetaObject::activate(QObject*, int, int, void**) kernel/qobject.cpp:3804
#8 0x7f1ba9a0aee2 in vtkQtConnection::EmitExecute(vtkObject*, unsigned long, void*, void*, vtkCommand*) /data/scott/projects/ParaView/build_san/VTK/GUISupport/Qt/GUISupportQt_autogen/EWIEGA46WW/moc_vtkQtConnection.cpp:144
#9 0x7f1ba9baab19 in vtkQtConnection::Execute(vtkObject*, unsigned long, void*) /data/scott/projects/ParaView/pvSrc/VTK/GUISupport/Qt/vtkQtConnection.cxx:64
#10 0x7f1ba9baa531 in vtkQtConnection::DoCallback(vtkObject*, unsigned long, void*, void*) /data/scott/projects/ParaView/pvSrc/VTK/GUISupport/Qt/vtkQtConnection.cxx:56
#11 0x7f1b0b3340e6 in vtkCallbackCommand::Execute(vtkObject*, unsigned long, void*) /data/scott/projects/ParaView/pvSrc/VTK/Common/Core/vtkCallbackCommand.cxx:30
#12 0x7f1b0ba197c8 in vtkSubjectHelper::InvokeEvent(unsigned long, void*, vtkObject*) /data/scott/projects/ParaView/pvSrc/VTK/Common/Core/vtkObject.cxx:638
#13 0x7f1b0ba1c6d0 in vtkObject::InvokeEvent(unsigned long, void*) /data/scott/projects/ParaView/pvSrc/VTK/Common/Core/vtkObject.cxx:807
#14 0x7f1b0ba1d671 in vtkObject::Modified() /data/scott/projects/ParaView/pvSrc/VTK/Common/Core/vtkObject.cxx:872
#15 0x7f1ba5ff00b8 in vtkSMProperty::Modified() /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMProperty.h:488
#16 0x7f1ba603e359 in vtkSMVectorPropertyTemplate::SetElements(double const*, unsigned int) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMVectorPropertyTemplate.h:280
#17 0x7f1ba6041d2a in vtkSMVectorPropertyTemplate::LoadStateValues(vtkPVXMLElement*) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMVectorPropertyTemplate.h:395
#18 0x7f1ba60263aa in vtkSMDoubleVectorProperty::LoadState(vtkPVXMLElement*, vtkSMProxyLocator*) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMDoubleVectorProperty.cxx:297
#19 0x7f1ba6499635 in vtkSMProxy::LoadXMLState(vtkPVXMLElement*, vtkSMProxyLocator*) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMProxy.cxx:2152
#20 0x7f1ac47f2642 in vtkSMAnimationSceneProxy::LoadXMLState(vtkPVXMLElement*, vtkSMProxyLocator*) /data/scott/projects/ParaView/pvSrc/Remoting/Animation/vtkSMAnimationSceneProxy.cxx:253
#21 0x7f1ba5f86f55 in vtkSMDeserializerXML::LoadProxyState(vtkPVXMLElement*, vtkSMProxy*, vtkSMProxyLocator*) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMDeserializerXML.cxx:76
#22 0x7f1ba5f85692 in vtkSMDeserializerXML::NewProxy(unsigned int, vtkSMProxyLocator*) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMDeserializerXML.cxx:46
#23 0x7f1ba655cbac in vtkSMProxyLocator::NewProxy(unsigned int) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMProxyLocator.cxx:99
#24 0x7f1ba655c157 in vtkSMProxyLocator::LocateProxy(unsigned int) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMProxyLocator.cxx:76
#25 0x7f1ba67cfce8 in vtkSMStateLoader::HandleProxyCollection(vtkPVXMLElement*) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMStateLoader.cxx:422
#26 0x7f1ba67dc5dd in vtkSMStateLoader::LoadStateInternal(vtkPVXMLElement*) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMStateLoader.cxx:658
#27 0x7f1ba67d6ed7 in vtkSMStateLoader::LoadState(vtkPVXMLElement*, bool) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMStateLoader.cxx:535
#28 0x7f1ba66c7379 in vtkSMSessionProxyManager::LoadXMLState(vtkPVXMLElement*, vtkSMStateLoader*, bool) /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMSessionProxyManager.cxx:1233
#29 0x7f1ba616c995 in vtkSMLoadStateOptionsProxy::Load() /data/scott/projects/ParaView/pvSrc/Remoting/ServerManager/vtkSMLoadStateOptionsProxy.cxx:659
#30 0x7f1bb752a935 in pqLoadStateReaction::loadState(QString const&, bool, pqServer*, unsigned int) /data/scott/projects/ParaView/pvSrc/Qt/ApplicationComponents/pqLoadStateReaction.cxx:87
#31 0x7f1bb752be8a in pqLoadStateReaction::loadState() /data/scott/projects/ParaView/pvSrc/Qt/ApplicationComponents/pqLoadStateReaction.cxx:132
#32 0x7f1bb6f41e1c in pqLoadStateReaction::onTriggered() (/data/scott/projects/ParaView/build_san/bin/../lib/libpqApplicationComponents-pv5.12.so.1+0x1b6be1c)
#33 0x7f1bb6ef5938 in pqReaction::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) /data/scott/projects/ParaView/build_san/Qt/ApplicationComponents/pqApplicationComponents_autogen/EWIEGA46WW/moc_pqReaction.cpp:78
#34 0x7f1b3c5a6e18 in QMetaObject::activate(QObject*, int, int, void**) kernel/qobject.cpp:3804
#35 0x7f1bb4ce0911 in QAction::triggered(bool) .moc/moc_qaction.cpp:380
#36 0x7f1bb4ce2d2f in QAction::activate(QAction::ActionEvent) kernel/qaction.cpp:1166
#37 0x7f1bb4e4ca29 in QMenuPrivate::activateCausedStack(QVector > const&, QAction*, QAction::ActionEvent, bool) widgets/qmenu.cpp:1356
#38 0x7f1bb4e53cd5 in QMenuPrivate::activateAction(QAction*, QAction::ActionEvent, bool) widgets/qmenu.cpp:1433
#39 0x7f1bb4e54bdf in QMenu::mouseReleaseEvent(QMouseEvent*) widgets/qmenu.cpp:2950
#40 0x7f1bb4d2471f in QWidget::event(QEvent*) kernel/qwidget.cpp:9346
#41 0x7f1bb4e570fa in QMenu::event(QEvent*) widgets/qmenu.cpp:3072
#42 0x7f1bb4ce691b in QApplicationPrivate::notify_helper(QObject*, QEvent*) kernel/qapplication.cpp:3650
#43 0x7f1bb4cee6ee in QApplication::notify(QObject*, QEvent*) kernel/qapplication.cpp:3110
#44 0x7f1b3c57b327 in QCoreApplication::notifyInternal2(QObject*, QEvent*) kernel/qcoreapplication.cpp:1088
#45 0x7f1bb4ced019 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer&, bool, bool) kernel/qapplication.cpp:2648
#46 0x7f1bb4d3e4ff in QWidgetWindow::handleMouseEvent(QMouseEvent*) kernel/qwidgetwindow.cpp:570
#47 0x7f1bb4d40cc2 in QWidgetWindow::event(QEvent*) kernel/qwidgetwindow.cpp:293
#48 0x7f1bb4ce691b in QApplicationPrivate::notify_helper(QObject*, QEvent*) kernel/qapplication.cpp:3650
#49 0x7f1bb4ceda3f in QApplication::notify(QObject*, QEvent*) kernel/qapplication.cpp:3396
#50 0x7f1b3c57b327 in QCoreApplication::notifyInternal2(QObject*, QEvent*) kernel/qcoreapplication.cpp:1088
#51 0x7f1baab46649 in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) kernel/qguiapplication.cpp:2163
#52 0x7f1baab47b94 in QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*) kernel/qguiapplication.cpp:1898
#53 0x7f1baab250aa in QWindowSystemInterface::sendWindowSystemEvents(QFlags) kernel/qwindowsysteminterface.cpp:1151
#54 0x7f199caed1c9 in xcbSourceDispatch /home/qt/work/qt/qtbase/src/plugins/platforms/xcb/qxcbeventdispatcher.cpp:105
#55 0x7f1a0601517c in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5217c)
#56 0x7f1a060153ff (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x523ff)
#57 0x7f1a060154a2 in g_main_context_iteration (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x524a2)
#58 0x7f1b3c5d264e in QEventDispatcherGlib::processEvents(QFlags) kernel/qeventdispatcher_glib.cpp:422
#59 0x7f1b3c579d09 in QEventLoop::exec(QFlags) kernel/qeventloop.cpp:225
#60 0x7f1b3c58262f in QCoreApplication::exec() kernel/qcoreapplication.cpp:1389
#61 0x557c0056c438 in main /data/scott/projects/ParaView/build_san/Clients/ParaView/paraview_main.cxx:134
#62 0x7f1add224082 in __libc_start_main ../csu/libc-start.c:308
#63 0x557c005320cd in _start (/data/scott/projects/ParaView/build_san/bin/paraview+0x770cd)
0x6070005a84a0 is located 0 bytes to the right of 80-byte region [0x6070005a8450,0x6070005a84a0)
allocated by thread T0 here:
#0 0x7f1bb9f61587 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cc:104
#1 0x7f1bb711c82a in __gnu_cxx::new_allocator::allocate(unsigned long, void const*) /usr/include/c++/9/ext/new_allocator.h:114
#2 0x7f1bb7117c1f in std::allocator_traits >::allocate(std::allocator&, unsigned long) /usr/include/c++/9/bits/alloc_traits.h:443
#3 0x7f1bb7111345 in std::_Vector_base >::_M_allocate(unsigned long) /usr/include/c++/9/bits/stl_vector.h:343
#4 0x7f1bb77bafdd in std::vector >::_M_default_append(unsigned long) /usr/include/c++/9/bits/vector.tcc:635
#5 0x7f1bb77b4e1e in std::vector >::resize(unsigned long) /usr/include/c++/9/bits/stl_vector.h:937
#6 0x7f1bb12db61d in updateSequenceTimeSteps /data/scott/projects/ParaView/pvSrc/Qt/Components/pqAnimationTimeWidget.cxx:103
#7 0x7f1bb12e6b3a in pqAnimationTimeWidget::setNumberOfFrames(int) /data/scott/projects/ParaView/pvSrc/Qt/Components/pqAnimationTimeWidget.cxx:458
#8 0x7f1bb11942b4 in pqAnimationTimeWidget::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (/data/scott/projects/ParaView/build_san/bin/../lib/libpqComponents-pv5.12.so.1+0x1f992b4)
#9 0x7f1bb119507d in pqAnimationTimeWidget::qt_metacall(QMetaObject::Call, int, void**) (/data/scott/projects/ParaView/build_san/bin/../lib/libpqComponents-pv5.12.so.1+0x1f9a07d)
#10 0x7f1b3c58a10f in QMetaProperty::write(QObject*, QVariant const&) const kernel/qmetaobject.cpp:3251
#11 0x7f1b00000001 (/data/scott/projects/ParaView/build_san/bin/../lib/libvtkCommonCore-pv5.12.so.1+0x217e8001)
SUMMARY: AddressSanitizer: heap-buffer-overflow /data/scott/projects/ParaView/pvSrc/Qt/Components/pqAnimationTimeWidget.cxx:238 in pqAnimationTimeWidget::pqInternals::render(pqAnimationTimeWidget*)
Shadow bytes around the buggy address:
0x0c0e800ad040: fa fa fa fa 00 00 00 00 00 00 00 00 00 fa fa fa
0x0c0e800ad050: fa fa 00 00 00 00 00 00 00 00 00 00 fa fa fa fa
0x0c0e800ad060: 00 00 00 00 00 00 00 00 00 fa fa fa fa fa 00 00
0x0c0e800ad070: 00 00 00 00 00 00 00 00 fa fa fa fa fd fd fd fd
0x0c0e800ad080: fd fd fd fd fd fa fa fa fa fa 00 00 00 00 00 00
=>0x0c0e800ad090: 00 00 00 00[fa]fa fa fa 00 00 00 00 00 00 00 00
0x0c0e800ad0a0: 00 fa fa fa fa fa 00 00 00 00 00 00 00 00 00 fa
0x0c0e800ad0b0: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 fa fa
0x0c0e800ad0c0: fa fa 00 00 00 00 00 00 00 00 00 fa fa fa fa fa
0x0c0e800ad0d0: 00 00 00 00 00 00 00 00 00 fa fa fa fa fa 00 00
0x0c0e800ad0e0: 00 00 00 00 00 00 04 fa fa fa fa fa 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==1772250==ABORTINGI'm not sure this is the correct approach to solve the problem, but it does fix my sanitizer build.
Edited by Scott Wittenburg