Created attachment 160299 [details] asan log of kdiff3 git d16aacc31841983a973faf2c1ce7989a7c72aee9 SUMMARY When I diff files with whitespace at the end, while ignoring whitespace differences, the app crashes. STEPS TO REPRODUCE 1. Compile kdiff3 with asan enabled. Without it, the crashes only occur randomly. 2. `kdiff3 file1 file2 file3` (sorry I cannot share the files) OBSERVED RESULT Segfault in EXPECTED RESULT No segfault. This bug actually consists of multiple interlocking related sources of undefined behavior. The code was changed in https://invent.kde.org/sdk/kdiff3/-/commit/f4b160a95e2079206dbf3bbd9de8e9546a499907#39f490613200d238a14dba98bf938d0cd0bb5b8e, after your most recent 1.10.5 release. However it doesn't actually fix the crash. - Now you mark lines as different if they differ in length, even if they only differ in whitespace and the loop body would mark them as identical. I think this change should be reverted? IDK. - That commit adds `if((p1 != p1End && p2 != p2End) && *p1 != *p2)` to avoid dereferencing an out-of-bounds pointer. But your latest git commit still crashes with asan enabled, because you check `isspace(p1->unicode()) && p1 != p1End`. If p1 points out of bounds, you dereference it *before* checking if it's OOB. - I'm not sure if .end() is technically valid to dereference because it's a null terminator. QString adds null terminators on some but not all method calls. I still think it's a bug to dereference .end(). - Even if you checked `p1 != p1End` before incrementing p1, the end-of-loop condition executes `++p1, ++p2`, resulting in a "past-the-end" pointer which isn't caught by `p1 != p1End` but is still invalid to dereference. - This is invalid *even if a null terminator is present!* I've patched all invalid derefs and increments at https://invent.kde.org/nyanpasu/kdiff3/-/tree/fix-segfault. This version of the program passes asan without errors. Even then, when closing the program, I get an unrelated segfault even without asan. If I enable asan, I get a log where apparently you're calling ~ProgressDialog() after main() exits: ==338259==ERROR: AddressSanitizer: SEGV on unknown address 0x000000004082 (pc 0x7f207a9025a0 bp 0x619000006480 sp 0x7ffdf69c9540 T0) ==338259==The signal is caused by a READ memory access. #0 0x7f207a9025a0 in QXcbConnection::removeWindowEventListener(unsigned int) (/usr/lib/libQt5XcbQpa.so.5+0x385a0) (BuildId: 0d9bddd568990cb856de2d0fbc95719075f2bb11) #1 0x7f207a91ede6 in QXcbWindow::destroy() (/usr/lib/libQt5XcbQpa.so.5+0x54de6) (BuildId: 0d9bddd568990cb856de2d0fbc95719075f2bb11) #2 0x7f207a91eee5 in QXcbWindow::~QXcbWindow() (/usr/lib/libQt5XcbQpa.so.5+0x54ee5) (BuildId: 0d9bddd568990cb856de2d0fbc95719075f2bb11) #3 0x7f207b68e0f5 (/usr/lib/qt/plugins/xcbglintegrations/libqxcb-glx-integration.so+0x80f5) (BuildId: 56d55ed2b339b27593c0687479bdc5caa4ca7928) #4 0x7f2083155919 in QWindowPrivate::destroy() (/usr/lib/libQt5Gui.so.5+0x155919) (BuildId: acccfa0e6cf0112180781234f2b5087fd4cd0fc0) #5 0x7f208399698b in QWidgetPrivate::deleteTLSysExtra() (/usr/lib/libQt5Widgets.so.5+0x19698b) (BuildId: b61c3153959c656fb8e955fe52184933b0a19c5f) #6 0x7f20839b8a68 in QWidget::destroy(bool, bool) (/usr/lib/libQt5Widgets.so.5+0x1b8a68) (BuildId: b61c3153959c656fb8e955fe52184933b0a19c5f) #7 0x7f208399ba42 in QWidget::~QWidget() (/usr/lib/libQt5Widgets.so.5+0x19ba42) (BuildId: b61c3153959c656fb8e955fe52184933b0a19c5f) #8 0x55ecdbfdd225 in ProgressDialog::~ProgressDialog() (/usr/bin/kdiff3+0x132a225) (BuildId: 44e4831ca1d69b2d07a18fdaada303168c6cc77b) #9 0x55ecdc0e3f8e in void std::_Destroy<ProgressDialog>(ProgressDialog*) (/usr/bin/kdiff3+0x1430f8e) (BuildId: 44e4831ca1d69b2d07a18fdaada303168c6cc77b) #10 0x55ecdc0e0574 in std::_Sp_counted_ptr_inplace<ProgressDialog, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (/usr/bin/kdiff3+0x142d574) (BuildId: 44e4831ca1d69b2d07a18fdaada303168c6cc77b) #11 0x55ecdbfcdc3b in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/include/c++/13.1.1/bits/shared_ptr_base.h:346 #12 0x55ecdbfd66e1 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /usr/include/c++/13.1.1/bits/shared_ptr_base.h:1071 #13 0x55ecdc085f9d in std::__shared_ptr<ProgressDialog, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() /usr/include/c++/13.1.1/bits/shared_ptr_base.h:1524 #14 0x55ecdc08600b in std::shared_ptr<ProgressDialog>::~shared_ptr() /usr/include/c++/13.1.1/bits/shared_ptr.h:175 #15 0x7f2082452065 in __run_exit_handlers /usr/src/debug/glibc/glibc/stdlib/exit.c:111 #16 0x7f20824521af in __GI_exit /usr/src/debug/glibc/glibc/stdlib/exit.c:141 #17 0x7f2082439856 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:74 #18 0x7f2082439909 in __libc_start_main_impl ../csu/libc-start.c:360 #19 0x55ecdbf9a144 in _start (/usr/bin/kdiff3+0x12e7144) (BuildId: 44e4831ca1d69b2d07a18fdaada303168c6cc77b) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV (/usr/lib/libQt5XcbQpa.so.5+0x385a0) (BuildId: 0d9bddd568990cb856de2d0fbc95719075f2bb11) in QXcbConnection::removeWindowEventListener(unsigned int) ==338259==ABORTING With asan disabled, gdb reports a partially different call stack: (gdb) bt #0 0x00007ffff64f5b5f in QThreadStorageData::get() const () at /usr/lib/libQt5Core.so.5 #1 0x00007ffff6b84012 in QOpenGLContext::currentContext() () at /usr/lib/libQt5Gui.so.5 #2 0x00007ffff6b518d1 in QSurface::~QSurface() () at /usr/lib/libQt5Gui.so.5 #3 0x00007ffff6b4bb12 in QWindow::~QWindow() () at /usr/lib/libQt5Gui.so.5 #4 0x00007ffff7396a39 in QWidgetPrivate::deleteTLSysExtra() () at /usr/lib/libQt5Widgets.so.5 #5 0x00007ffff73b8a69 in QWidget::destroy(bool, bool) () at /usr/lib/libQt5Widgets.so.5 #6 0x00007ffff739ba43 in QWidget::~QWidget() () at /usr/lib/libQt5Widgets.so.5 #7 0x0000555555668a79 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (this=0x555555ccae00) at /usr/include/c++/13.1.1/bits/shared_ptr_base.h:346 #8 std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (this=0x555555ccae00) at /usr/include/c++/13.1.1/bits/shared_ptr_base.h:317 #9 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/13.1.1/bits/shared_ptr_base.h:1071 #10 std::__shared_ptr<ProgressDialog, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/13.1.1/bits/shared_ptr_base.h:1524 #11 std::shared_ptr<ProgressDialog>::~shared_ptr() (this=<optimized out>, __in_chrg=<optimized out>) at /usr/include/c++/13.1.1/bits/shared_ptr.h:175 #12 0x00007ffff5e52066 in __run_exit_handlers (status=1, listp=0x7ffff5ff1760 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at exit.c:111 #13 0x00007ffff5e521b0 in __GI_exit (status=<optimized out>) at exit.c:141 #14 0x00007ffff5e39857 in __libc_start_call_main (main=main@entry=0x5555555a7da0 <main(int, char**)>, argc=argc@entry=4, argv=argv@entry=0x7fffffffe1c8) at ../sysdeps/nptl/libc_start_call_main.h:74 #15 0x00007ffff5e3990a in __libc_start_main_impl (main=0x5555555a7da0 <main(int, char**)>, argc=4, argv=0x7fffffffe1c8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe1b8) at ../csu/libc-start.c:360 #16 0x00005555555aa515 in _start () (I also rebased the code on master at https://invent.kde.org/nyanpasu/kdiff3/-/tree/fix-segfault-master (not tested), where I also removed the length check. Is that the right thing to do? IDK.) SOFTWARE/OS VERSIONS Operating System: Arch Linux KDE Plasma Version: 5.27.6 KDE Frameworks Version: 5.107.0 Qt Version: 5.15.10 Kernel Version: 6.4.1-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
You can also create a MR for this the exit crash has been reported previously and is a different root cause. Going to have a look at your fix.
Git commit 9c2901040c340696d9e7b9fea81bdc0b32a0fcb3 by Michael Reeves, on behalf of Nyan Pasu. Committed on 08/08/2023 at 17:47. Pushed by mreeves into branch 'master'. Fix segfault if ignoring whitespace FIXED-IN:1.10.5 M +48 -10 src/diff.cpp https://invent.kde.org/sdk/kdiff3/-/commit/9c2901040c340696d9e7b9fea81bdc0b32a0fcb3
Git commit 341bb0322026053400def3aa7f46e1ac171d0fc5 by Michael Reeves, on behalf of Nyan Pasu. Committed on 08/08/2023 at 17:47. Pushed by mreeves into branch '1.10'. Fix segfault if ignoring whitespace FIXED-IN:1.10.5 M +48 -10 src/diff.cpp https://invent.kde.org/sdk/kdiff3/-/commit/341bb0322026053400def3aa7f46e1ac171d0fc5