Created attachment 156787 [details] Terminal log including app logs and stack traces, from triggering use-after-free errors in Valgrind SUMMARY KDiff3 sometimes crashes when opening the diff view between two files, reloading and cancelling, entering merge view of two files, etc. STEPS TO REPRODUCE 1. Build KDiff3 with debug symbols. (The Arch packages lack debug symbols *and* lack a kdiff3-debug package for some reason.) 2. Run `valgrind path/to/kdiff3-dbg file1 file2`. (I can't share the files I was diffing locally, but they were rather long. I initially encountered the crash outside of Valgrind, but it generates better stack traces. Alternatively you could build with asan too.) 3. Enable word wrap (affects DiffTextWindowData::draw() near the crash site, you *may* be able to reproduce the crash without it but I haven't tested). 4. Try playing with the diff view, refreshing with F5, clicking the merge toolbar button, etc. The bug doesn't trigger reliably. OBSERVED RESULT ==437501== Invalid read of size 1 ==437501== at 0x1BB767: isEqualAB (diff.h:287) ==437501== by 0x1BB767: Diff3Line::getLineInfo(e_SrcSelector, bool, LineRef&, std::shared_ptr<DiffList>&, std::shared_ptr<DiffList>&, QFlags<ChangeFlag>&, QFlags<ChangeFlag>&) const (diff.cpp:1486) ==437501== by 0x1B246D: DiffTextWindowData::draw(RLPainter&, QRect const&, int, LineRef const&) (difftextwindow.cpp:1259) ==437501== by 0x1B2A39: DiffTextWindow::paintEvent(QPaintEvent*) (difftextwindow.cpp:1182) ==437501== by 0x5090513: QWidget::event(QEvent*) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x5059B5B: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x5E93F47: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib/libQt5Core.so.5.15.8) ==437501== by 0x50843EA: QWidgetPrivate::sendPaintEvent(QRegion const&) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x50857B5: QWidgetPrivate::drawWidget(QPaintDevice*, QRegion const&, QPoint const&, QFlags<QWidgetPrivate::DrawWidgetFlag>, QPainter*, QWidgetRepaintManager*) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x506588F: ??? (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x50901E3: QWidget::event(QEvent*) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x4B0E41D: KXmlGuiWindow::event(QEvent*) (in /usr/lib/libKF5XmlGui.so.5.103.0) ==437501== by 0x5059B5B: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== Address 0x29b0a89e is 30 bytes inside a block of size 96 free'd ==437501== at 0x4844B5F: operator delete(void*, unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==437501== by 0x199969: deallocate (new_allocator.h:158) ==437501== by 0x199969: deallocate (alloc_traits.h:496) ==437501== by 0x199969: _M_put_node (stl_list.h:522) ==437501== by 0x199969: _M_clear (list.tcc:81) ==437501== by 0x199969: clear (stl_list.h:1594) ==437501== by 0x199969: KDiff3App::mainInit(TotalDiffStatus*, QFlags<InitFlag>) (pdiff.cpp:118) ==437501== by 0x5EC4A70: ??? (in /usr/lib/libQt5Core.so.5.15.8) ==437501== by 0x504CEC6: QAction::triggered(bool) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x50528C6: QAction::activate(QAction::ActionEvent) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x505298D: QAction::event(QEvent*) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x5059B5B: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x5E93F47: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib/libQt5Core.so.5.15.8) ==437501== by 0x5710FAC: QShortcutMap::dispatchEvent(QKeyEvent*) (in /usr/lib/libQt5Gui.so.5.15.8) ==437501== by 0x5707816: QShortcutMap::tryShortcut(QKeyEvent*) (in /usr/lib/libQt5Gui.so.5.15.8) ==437501== by 0x56C7037: QWindowSystemInterface::handleShortcutEvent(QWindow*, unsigned long, int, QFlags<Qt::KeyboardModifier>, unsigned int, unsigned int, unsigned int, QString const&, bool, unsigned short) (in /usr/lib/libQt5Gui.so.5.15.8) ==437501== by 0x56DC971: QGuiApplicationPrivate::processKeyEvent(QWindowSystemInterfacePrivate::KeyEvent*) (in /usr/lib/libQt5Gui.so.5.15.8) ==437501== Block was alloc'd at ==437501== at 0x4842003: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==437501== by 0x1BDC92: allocate (new_allocator.h:137) ==437501== by 0x1BDC92: allocate (alloc_traits.h:464) ==437501== by 0x1BDC92: _M_get_node (stl_list.h:518) ==437501== by 0x1BDC92: _M_create_node<const Diff3Line&> (stl_list.h:710) ==437501== by 0x1BDC92: _M_insert<const Diff3Line&> (stl_list.h:2005) ==437501== by 0x1BDC92: push_back (stl_list.h:1306) ==437501== by 0x1BDC92: Diff3LineList::calcDiff3LineListUsingAB(DiffList const*) (diff.cpp:123) ==437501== by 0x19B25D: KDiff3App::mainInit(TotalDiffStatus*, QFlags<InitFlag>) (pdiff.cpp:181) ==437501== by 0x5EC4A70: ??? (in /usr/lib/libQt5Core.so.5.15.8) ==437501== by 0x504CEC6: QAction::triggered(bool) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x50528C6: QAction::activate(QAction::ActionEvent) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x505298D: QAction::event(QEvent*) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x5059B5B: QApplicationPrivate::notify_helper(QObject*, QEvent*) (in /usr/lib/libQt5Widgets.so.5.15.8) ==437501== by 0x5E93F47: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /usr/lib/libQt5Core.so.5.15.8) ==437501== by 0x5710FAC: QShortcutMap::dispatchEvent(QKeyEvent*) (in /usr/lib/libQt5Gui.so.5.15.8) ==437501== by 0x5707816: QShortcutMap::tryShortcut(QKeyEvent*) (in /usr/lib/libQt5Gui.so.5.15.8) ==437501== by 0x56C7037: QWindowSystemInterface::handleShortcutEvent(QWindow*, unsigned long, int, QFlags<Qt::KeyboardModifier>, unsigned int, unsigned int, unsigned int, QString const&, bool, unsigned short) (in /usr/lib/libQt5Gui.so.5.15.8) And other messages (see attachment). Unfortunately these stack traces are incomplete and don't extend down to main(), so I don't know if any of these call stacks originate from reentrant event loops (eg. in KDiff3App::mainInit() after freeing). For line numbers, reference https://invent.kde.org/sdk/kdiff3/-/tree/1.10.0. Looks like DiffTextWindowData::m_diff3WrapLineVector is holding onto stale Diff3WrapLine::pD3L (Diff3Line*), pointing into KDiff3App::m_diff3LineList even after it gets cleared. DiffTextWindow::recalcWordWrapHelper() assigns Diff3WrapLine& d3wl = d->m_diff3WrapLineVector[wrapLineIdx], and d3wl.pD3L = (*d->getDiff3LineVector())[i] (DiffTextWindowData::mDiff3LineVector). `const Diff3LineVector* mDiff3LineVector` is initialized in `DiffTextWindow::init` (called in KDiff3App::mainInit and passing in &mDiff3LineVector), and IDK if mutated later on. I *think* mDiff3LineVector is initialized by m_diff3LineList.calcDiff3LineVector(mDiff3LineVector), finally completing the chain from free (KDiff3App::m_diff3LineList) to use (m_diff3WrapLineVector). I think `KDiff3App::mainInit` runs and returns while the progress dialog remains visible. ---- Running under gdb, I found that when a use-after-free error appears, it's from a reentrant event loop called from within KDiff3App::mainInit (pp.setInformation(i18n("Loading A")); -> setInformationSig -> signals2::signal), after clearing the vectors but before calling DiffTextWindow::init() with the new contents. The call stack is long, should I post it as an attachment? EXPECTED RESULT No crash. SOFTWARE/OS VERSIONS Operating System: Arch Linux KDE Plasma Version: 5.27.1 KDE Frameworks Version: 5.103.0 Qt Version: 5.15.8 Kernel Version: 6.1.12-zen1-1-zen (64-bit) Graphics Platform: X11 Processors: 12 × AMD Ryzen 5 5600X 6-Core Processor Memory: 15.5 GiB of RAM Graphics Processor: AMD Radeon RX 570 Series Manufacturer: Gigabyte Technology Co., Ltd. Product Name: B550M DS3H ADDITIONAL INFORMATION Unsure if related to Bug 466374.
Created attachment 156788 [details] Backtrace of the use-after-free triggered by a reentrant event loop in KDiff3App::mainInit.
Thanks. The valigrind information is actually more value in this type of situation as shows memory management issues that may otherwise be hard to find. In this a shared_ptr was being converted to raw form which left a dangling pointer in DiffTextWindowData. Knowing where the crash happens often isn't enough for a case like this. Needed to the allocation and deallocation points as well. This has been corrected locally but something about my setup/usage doesn't trigger the issue.
Git commit 7b7e556970bbc85a920021f61d33afc816eea30e by Michael Reeves. Committed on 27/02/2023 at 23:27. Pushed by mreeves into branch '1.10'. Don't convert std::shared_ptr<Diff3Line> to a bare pointer This prevents use after free reported in https://bugs.kde.org/show_bug.cgi?id=466524 In addition reset all three DiffTextWindows and MergeResultWindow on reload. This fixes a second bad memmory access visable in the valgrind log. FIXED-IN:10.0.1 M +1 -1 src/diff.cpp M +2 -2 src/diff.h M +7 -7 src/difftextwindow.cpp M +2 -0 src/mergeresultwindow.cpp M +1 -5 src/mergeresultwindow.h M +6 -2 src/pdiff.cpp https://invent.kde.org/sdk/kdiff3/commit/7b7e556970bbc85a920021f61d33afc816eea30e
Git commit 119648a3102f5f21074cc3c181045e209eefcfe3 by Michael Reeves. Committed on 27/02/2023 at 23:09. Pushed by mreeves into branch 'master'. Don't convert std::shared_ptr<Diff3Line> to a bare pointer This prevents use after free reported in https://bugs.kde.org/show_bug.cgi?id=466524 In addition reset all three DiffTextWindows and MergeResultWindow on reload. This fixes a second bad memmory access visable in the valgrind log. FIXED-IN:10.0.1 M +1 -1 src/diff.cpp M +2 -2 src/diff.h M +7 -7 src/difftextwindow.cpp M +2 -0 src/mergeresultwindow.cpp M +0 -5 src/mergeresultwindow.h M +6 -2 src/pdiff.cpp https://invent.kde.org/sdk/kdiff3/commit/119648a3102f5f21074cc3c181045e209eefcfe3
I've found a fairly reliable way to trigger the crash (without the patch): - Run kdiff3 without valgrind. - Resize the kdiff3 window quickly in circles (a 1000 Hz mouse may help, though I could reproduce the crash with a Dell office mouse as well). - While resizing in circles, hold F5 to refresh the diff view, then let go of the mouse to allow the F5 events to reach the window. - Repeat until KDiff3 crashes (usually takes me around 3 times). Can you reproduce that way on your end? As a separate bug, resizing the window causes the texture to become scrambled, with width changes causing the old contents to be drawn "slanted", likely by interpreting the same linear 2D texture with a different width (as well as the window contents being incorrect and being filled with copies of the colored status margins).
Valgrind does not run clean without this patch. Didn't crash but could have.
*** Bug 457472 has been marked as a duplicate of this bug. ***
*** Bug 466374 has been marked as a duplicate of this bug. ***