Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Segfault when editing pose in moveit_setup_assistant #2340

Merged
merged 1 commit into from
Oct 2, 2020

Conversation

tylerjw
Copy link
Member

@tylerjw tylerjw commented Oct 2, 2020

Description

When the item() method is called it can return a nullptr if the row/column doesn't exist. Here is the documentation. The way to reproduce this before was to create a pose, then click edit on that pose, and then click Save. This would result in the stack trace below. The basics of it are that row is set to -1 by clearContents and that causes the callback previewClicked to be called because the row value changed. -1 is not a valid row index so item() returns nullptr and then we dereference it.

My fix just checks for nullptr before dereferencing and performing the rest of the logic in previewClicked. I tested this manually and was not able to reproduce the segfault.

Thread 1 "moveit_setup_as" received signal SIGSEGV, Segmentation fault.
moveit_setup_assistant::RobotPosesWidget::previewClicked (this=0x5555556be2f0, row=-1)
    at /home/tyler/workspace/ws_moveit_noetic/src/moveit/moveit_setup_assistant/src/widgets/robot_poses_widget.cpp:324
324	  const std::string& name = data_table_->item(row, 0)->text().toStdString();


(gdb) info stack
#0  moveit_setup_assistant::RobotPosesWidget::previewClicked(int, int, int, int) (this=0x5555556be2f0, row=-1)
    at /home/tyler/workspace/ws_moveit_noetic/src/moveit/moveit_setup_assistant/src/widgets/robot_poses_widget.cpp:324
#1  0x00007ffff7f04fae in moveit_setup_assistant::RobotPosesWidget::qt_static_metacall(QObject*, QMetaObject::Call, int, void**)
    (_o=<optimized out>, _c=<optimized out>, _id=<optimized out>, _a=<optimized out>)
    at /home/tyler/workspace/ws_moveit_noetic/build/moveit_setup_assistant/moveit_setup_assistant_widgets_autogen/RZ4CM6YYJ6/moc_robot_poses_widget.cpp:115
#2  0x00007ffff72ea300 in QMetaObject::activate(QObject*, int, int, void**) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#3  0x00007ffff7bf506d in QTableWidget::currentCellChanged(int, int, int, int) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#4  0x00007ffff72ea300 in QMetaObject::activate(QObject*, int, int, void**) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#5  0x00007ffff727a017 in QItemSelectionModel::currentChanged(QModelIndex const&, QModelIndex const&) ()
    at /lib/x86_64-linux-gnu/libQt5Core.so.5
#6  0x00007ffff727a1bc in QItemSelectionModel::clearCurrentIndex() () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#7  0x00007ffff7bf6d07 in QTableWidget::clearContents() () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#8  0x00007ffff7f42647 in moveit_setup_assistant::RobotPosesWidget::loadDataTable() (this=0x5555556be2f0)
    at /home/tyler/workspace/ws_moveit_noetic/src/moveit/moveit_setup_assistant/src/widgets/robot_poses_widget.cpp:737
#9  0x00007ffff7f47f3d in moveit_setup_assistant::RobotPosesWidget::doneEditing() (this=0x5555556be2f0)
    at /home/tyler/workspace/ws_moveit_noetic/src/moveit/moveit_setup_assistant/src/widgets/robot_poses_widget.cpp:708
#10 0x00007ffff7f04fae in moveit_setup_assistant::RobotPosesWidget::qt_static_metacall(QObject*, QMetaObject::Call, int, void**)
    (_o=<optimized out>, _c=<optimized out>, _id=<optimized out>, _a=<optimized out>)
    at /home/tyler/workspace/ws_moveit_noetic/build/moveit_setup_assistant/moveit_setup_assistant_widgets_autogen/RZ4CM6YYJ6/moc_robot_poses_widget.cpp:115
#11 0x00007ffff72ea300 in QMetaObject::activate(QObject*, int, int, void**) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#12 0x00007ffff7a20806 in QAbstractButton::clicked(bool) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#13 0x00007ffff7a20a2e in  () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#14 0x00007ffff7a21e73 in  () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#15 0x00007ffff7a22035 in QAbstractButton::mouseReleaseEvent(QMouseEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#16 0x00007ffff796e2b6 in QWidget::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#17 0x00007ffff792ba66 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#18 0x00007ffff7935343 in QApplication::notify(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#19 0x00007ffff72be93a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#20 0x00007ffff7934457 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool, bool) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#21 0x00007ffff798a35d in  () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#22 0x00007ffff798d1ec in  () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#23 0x00007ffff792ba66 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#24 0x00007ffff79350f0 in QApplication::notify(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#25 0x00007ffff72be93a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#26 0x00007ffff60b97d3 in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) ()
    at /lib/x86_64-linux-gnu/libQt5Gui.so.5
#27 0x00007ffff60bb10b in QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*) ()
    at /lib/x86_64-linux-gnu/libQt5Gui.so.5
#28 0x00007ffff609535b in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /lib/x86_64-linux-gnu/libQt5Gui.so.5
#29 0x00007fffec1f732e in  () at /lib/x86_64-linux-gnu/libQt5XcbQpa.so.5
#30 0x00007ffff575cfbd in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#31 0x00007ffff575d240 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#32 0x00007ffff575d2e3 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#33 0x00007ffff7316565 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /lib/x86_64-linux-gnu/libQt5Core.so.5
#34 0x00007ffff72bd4db in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#35 0x00007ffff72c5246 in QCoreApplication::exec() () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#36 0x000055555555e154 in main(int, char**) (argc=<optimized out>, argv=0x7fffffffce58)
    at /home/tyler/workspace/ws_moveit_noetic/src/moveit/moveit_setup_assistant/src/setup_assistant_main.cpp:107

Checklist

@tylerjw tylerjw added bug simple improvements This issue can likely be resolved in less than a day labels Oct 2, 2020
@tylerjw tylerjw requested a review from rhaschke as a code owner October 2, 2020 19:01
Copy link
Contributor

@felixvd felixvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed this issue before but didn't take the time to look into it. This solves it. Thanks for the fix.

Copy link
Contributor

@v4hn v4hn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixvd According to my current knowledge you could have (squash)merged the patch after your review.

I will leave this open for you to merge 😸

Thank you for the patch @tylerjw ! I might have been affected by this a few weeks ago as well...

@felixvd felixvd merged commit 023b11d into moveit:master Oct 2, 2020
@felixvd
Copy link
Contributor

felixvd commented Oct 2, 2020

You're right!

@tylerjw tylerjw mentioned this pull request Oct 23, 2020
57 tasks
tylerjw added a commit to tylerjw/moveit that referenced this pull request Oct 23, 2020
tylerjw added a commit to tylerjw/moveit that referenced this pull request Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug simple improvements This issue can likely be resolved in less than a day
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants