https://bugs.kde.org/show_bug.cgi?id=472270
Bug ID: 472270
Summary: Crash when ignoring whitespace and diffing files with
trailing whitespace
Classification: Applications
Product: kdiff3
Version: 1.10.5
Platform: Other
OS: Linux
Status: REPORTED
Severity: normal
Priority: NOR
Component: application
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---
Created attachment 160299
--> https://bugs.kde.org/attachment.cgi?id=160299&action=edit
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 are receiving this mail because:
You are watching all bug changes.
