search

ngscopeclient / scopehal-apps

ngscopeclient and other client applications for libscopehal.
https://www.ngscopeclient.org/
BSD 3-Clause "New" or "Revised" License
535 stars 83 forks source link

ngscopeclient: segfault when closing waveform with FilterPropertiesDialog open #599

Closed hansemro closed 11 months ago

hansemro commented 11 months ago

Setup

Steps to replicate segfault:

  1. Open ngscopeclient. (glscopeclient is not affected)
  2. Navigate to Add->Import and import a waveform of any supported format (Agilent/Rigol BIN, CSV, VCD, etc.)
  3. With the Filter Properties Dialog window opened, close the waveform group with the imported waveform. This leads to segfault.

https://github.com/glscopeclient/scopehal-apps/assets/40348686/b1d4ca1d-bcc8-472c-8f21-8131a0750210

Backtrace:

Thread 1 "ngscopeclient" received signal SIGSEGV, Segmentation fault.
0x00007ffff44aeb42 in __cxxabiv1::__dynamic_cast (src_ptr=0x555556886c80, src_type=0x555555b7ea20 <typeinfo for OscilloscopeChannel>, dst_type=0x555555b7ea40 <typeinfo for Filter>, src2dst=0)
    at /usr/src/debug/gcc/gcc/libstdc++-v3/libsupc++/dyncast.cc:58
Downloading source file /usr/src/debug/gcc/gcc/libstdc++-v3/libsupc++/dyncast.cc
58      /usr/src/debug/gcc/gcc/libstdc++-v3/libsupc++/dyncast.cc: Directory not empty.                                                                                                            
(gdb) bt
#0  0x00007ffff44aeb42 in __cxxabiv1::__dynamic_cast(void const*, __cxxabiv1::__class_type_info const*, __cxxabiv1::__class_type_info const*, ptrdiff_t)
    (src_ptr=0x555556886c80, src_type=0x555555b7ea20 <typeinfo for OscilloscopeChannel>, dst_type=0x555555b7ea40 <typeinfo for Filter>, src2dst=0)
    at /usr/src/debug/gcc/gcc/libstdc++-v3/libsupc++/dyncast.cc:58
#1  0x000055555594b256 in ChannelPropertiesDialog::DoRender() (this=this@entry=0x5555568869b0) at /home/hansemro/Documents/scopehal-apps/src/ngscopeclient/ChannelPropertiesDialog.cpp:208
#2  0x000055555596795e in FilterPropertiesDialog::DoRender() (this=<optimized out>) at /home/hansemro/Documents/scopehal-apps/src/ngscopeclient/FilterPropertiesDialog.cpp:143
#3  0x000055555594f085 in Dialog::Render() (this=0x5555568869b0) at /home/hansemro/Documents/scopehal-apps/src/ngscopeclient/Dialog.cpp:80
#4  0x000055555599638a in MainWindow::RenderUI() (this=0x555555f872a0) at /home/hansemro/Documents/scopehal-apps/src/ngscopeclient/MainWindow.cpp:488
#5  0x0000555555a60b55 in VulkanWindow::Render() (this=0x555555f872a0) at /home/hansemro/Documents/scopehal-apps/src/ngscopeclient/VulkanWindow.cpp:436
#6  0x00005555555c247f in main(int, char**) (argc=<optimized out>, argv=<optimized out>) at /home/hansemro/Documents/scopehal-apps/src/ngscopeclient/main.cpp:127
azonenberg commented 11 months ago

This smells like a use-after-free caused by the channel properties dialog not holding a reference open to the channel.

Filters are destroyed when no viewport or another filter is using them, so if the dialog tries to use the filter after it'll crash.

azonenberg commented 11 months ago

And yes, that's exactly the bug.

=================================================================
==3659315==ERROR: AddressSanitizer: heap-use-after-free on address 0x6130002c06f8 at pc 0x7ffff75ea983 bp 0x7fffffffc200 sp 0x7fffffffb9b0
READ of size 10 at 0x6130002c06f8 thread T0
    #0 0x7ffff75ea982 in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:806
    #1 0x555556609595 in void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) /usr/include/c++/10/bits/basic_string.tcc:225
    #2 0x5555566881cf in InstrumentChannel::GetHwname[abi:cxx11]() /ceph/fast/home/azonenberg/code/scopehal-apps/lib/scopehal/../scopehal/InstrumentChannel.h:89
    #3 0x5555566ea740 in FilterPropertiesDialog::DoRender() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/FilterPropertiesDialog.cpp:141
    #4 0x55555668cf64 in Dialog::Render() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/Dialog.cpp:80
    #5 0x5555566e9259 in FilterPropertiesDialog::Render() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/FilterPropertiesDialog.cpp:67
    #6 0x55555674c37e in MainWindow::RenderUI() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/MainWindow.cpp:488
    #7 0x55555696d911 in VulkanWindow::Render() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/VulkanWindow.cpp:436
    #8 0x55555674a6ed in MainWindow::Render() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/MainWindow.cpp:355
    #9 0x5555569fb5a9 in main /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/main.cpp:127
    #10 0x7ffff2941d09 in __libc_start_main ../csu/libc-start.c:308
    #11 0x5555559478f9 in _start (/ceph/fast/home/azonenberg/code/scopehal-apps/asan-build/src/ngscopeclient/ngscopeclient+0x3f38f9)

0x6130002c06f8 is located 184 bytes inside of 368-byte region [0x6130002c0640,0x6130002c07b0)
freed by thread T0 here:
    #0 0x7ffff765d467 in operator delete(void*, unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:172
    #1 0x7ffff61fd702 in ACCoupleFilter::~ACCoupleFilter() (/ceph/fast/home/azonenberg/code/scopehal-apps/asan-build/lib/scopeprotocols/libscopeprotocols.so+0x972702)
    #2 0x7ffff468dc47 in Filter::Release() /ceph/fast/home/azonenberg/code/scopehal-apps/lib/scopehal/Filter.cpp:93
    #3 0x5555569b599a in DisplayedChannel::~DisplayedChannel() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/WaveformArea.h:138
    #4 0x5555569e3407 in void __gnu_cxx::new_allocator<DisplayedChannel>::destroy<DisplayedChannel>(DisplayedChannel*) /usr/include/c++/10/ext/new_allocator.h:156
    #5 0x5555569e339a in void std::allocator_traits<std::allocator<DisplayedChannel> >::destroy<DisplayedChannel>(std::allocator<DisplayedChannel>&, DisplayedChannel*) /usr/include/c++/10/bits/alloc_traits.h:531
    #6 0x5555569e2f44 in std::_Sp_counted_ptr_inplace<DisplayedChannel, std::allocator<DisplayedChannel>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() /usr/include/c++/10/bits/shared_ptr_base.h:560
    #7 0x5555565f82ab in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/include/c++/10/bits/shared_ptr_base.h:158
    #8 0x5555565f1b17 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /usr/include/c++/10/bits/shared_ptr_base.h:733
    #9 0x555556779a0d in std::__shared_ptr<DisplayedChannel, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() /usr/include/c++/10/bits/shared_ptr_base.h:1183
    #10 0x555556779a4f in std::shared_ptr<DisplayedChannel>::~shared_ptr() /usr/include/c++/10/bits/shared_ptr.h:121
    #11 0x5555569dd86e in void std::_Destroy<std::shared_ptr<DisplayedChannel> >(std::shared_ptr<DisplayedChannel>*) /usr/include/c++/10/bits/stl_construct.h:140
    #12 0x5555569da1a3 in void std::_Destroy_aux<false>::__destroy<std::shared_ptr<DisplayedChannel>*>(std::shared_ptr<DisplayedChannel>*, std::shared_ptr<DisplayedChannel>*) /usr/include/c++/10/bits/stl_construct.h:152
    #13 0x5555569cef65 in void std::_Destroy<std::shared_ptr<DisplayedChannel>*>(std::shared_ptr<DisplayedChannel>*, std::shared_ptr<DisplayedChannel>*) /usr/include/c++/10/bits/stl_construct.h:185
    #14 0x5555569c5584 in void std::_Destroy<std::shared_ptr<DisplayedChannel>*, std::shared_ptr<DisplayedChannel> >(std::shared_ptr<DisplayedChannel>*, std::shared_ptr<DisplayedChannel>*, std::allocator<std::shared_ptr<DisplayedChannel> >&) /usr/include/c++/10/bits/alloc_traits.h:738
    #15 0x5555569c57f0 in std::vector<std::shared_ptr<DisplayedChannel>, std::allocator<std::shared_ptr<DisplayedChannel> > >::_M_erase_at_end(std::shared_ptr<DisplayedChannel>*) /usr/include/c++/10/bits/stl_vector.h:1796
    #16 0x5555569bc33e in std::vector<std::shared_ptr<DisplayedChannel>, std::allocator<std::shared_ptr<DisplayedChannel> > >::clear() /usr/include/c++/10/bits/stl_vector.h:1499
    #17 0x55555698f184 in WaveformArea::~WaveformArea() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/WaveformArea.cpp:274
    #18 0x5555567dae21 in void __gnu_cxx::new_allocator<WaveformArea>::destroy<WaveformArea>(WaveformArea*) /usr/include/c++/10/ext/new_allocator.h:156
    #19 0x5555567da8ba in void std::allocator_traits<std::allocator<WaveformArea> >::destroy<WaveformArea>(std::allocator<WaveformArea>&, WaveformArea*) /usr/include/c++/10/bits/alloc_traits.h:531
    #20 0x5555567da0bc in std::_Sp_counted_ptr_inplace<WaveformArea, std::allocator<WaveformArea>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() /usr/include/c++/10/bits/shared_ptr_base.h:560
    #21 0x5555565f82ab in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/include/c++/10/bits/shared_ptr_base.h:158
    #22 0x5555565f1b17 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /usr/include/c++/10/bits/shared_ptr_base.h:733
    #23 0x55555677a98b in std::__shared_ptr<WaveformArea, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() /usr/include/c++/10/bits/shared_ptr_base.h:1183
    #24 0x55555677a9a7 in std::shared_ptr<WaveformArea>::~shared_ptr() /usr/include/c++/10/bits/shared_ptr.h:121
    #25 0x5555567c45f6 in void std::_Destroy<std::shared_ptr<WaveformArea> >(std::shared_ptr<WaveformArea>*) /usr/include/c++/10/bits/stl_construct.h:140
    #26 0x5555567b5b5f in void std::_Destroy_aux<false>::__destroy<std::shared_ptr<WaveformArea>*>(std::shared_ptr<WaveformArea>*, std::shared_ptr<WaveformArea>*) /usr/include/c++/10/bits/stl_construct.h:152
    #27 0x5555567a1e56 in void std::_Destroy<std::shared_ptr<WaveformArea>*>(std::shared_ptr<WaveformArea>*, std::shared_ptr<WaveformArea>*) /usr/include/c++/10/bits/stl_construct.h:185
    #28 0x55555678eb63 in void std::_Destroy<std::shared_ptr<WaveformArea>*, std::shared_ptr<WaveformArea> >(std::shared_ptr<WaveformArea>*, std::shared_ptr<WaveformArea>*, std::allocator<std::shared_ptr<WaveformArea> >&) /usr/include/c++/10/bits/alloc_traits.h:738
    #29 0x5555569f4896 in std::vector<std::shared_ptr<WaveformArea>, std::allocator<std::shared_ptr<WaveformArea> > >::_M_erase_at_end(std::shared_ptr<WaveformArea>*) /usr/include/c++/10/bits/stl_vector.h:1796

previously allocated by thread T0 here:
    #0 0x7ffff765c647 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x7ffff6756746 in ACCoupleFilter::CreateInstance(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /ceph/fast/home/azonenberg/code/scopehal-apps/lib/scopeprotocols/ACCoupleFilter.h:49
    #2 0x7ffff468dfed in Filter::CreateFilter(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /ceph/fast/home/azonenberg/code/scopehal-apps/lib/scopehal/Filter.cpp:114
    #3 0x55555675aa8a in MainWindow::CreateFilter(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, WaveformArea*, StreamDescriptor, bool, bool) /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/MainWindow.cpp:1177
    #4 0x5555569b0c0a in WaveformArea::FilterSubmenu(std::shared_ptr<DisplayedChannel>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, Filter::Category) /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/WaveformArea.cpp:2795
    #5 0x5555569af44a in WaveformArea::FilterMenu(std::shared_ptr<DisplayedChannel>) /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/WaveformArea.cpp:2734
    #6 0x5555569ae719 in WaveformArea::ChannelButton(std::shared_ptr<DisplayedChannel>, unsigned long) /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/WaveformArea.cpp:2719
    #7 0x5555569915f1 in WaveformArea::Render(int, int, ImVec2) /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/WaveformArea.cpp:523
    #8 0x5555569e6a8b in WaveformGroup::Render() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/WaveformGroup.cpp:221
    #9 0x55555674bec4 in MainWindow::RenderUI() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/MainWindow.cpp:473
    #10 0x55555696d911 in VulkanWindow::Render() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/VulkanWindow.cpp:436
    #11 0x55555674a6ed in MainWindow::Render() /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/MainWindow.cpp:355
    #12 0x5555569fb5a9 in main /ceph/fast/home/azonenberg/code/scopehal-apps/src/ngscopeclient/main.cpp:127
    #13 0x7ffff2941d09 in __libc_start_main ../csu/libc-start.c:308

SUMMARY: AddressSanitizer: heap-use-after-free ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:806 in __interceptor_memcpy
hansemro commented 11 months ago

Fix is working, but the filter properties dialog window is still open after closing the waveform group. Is this expected?

azonenberg commented 11 months ago

Yes, that was intended behavior for this particular fix.

The idea is, if you have the dialog open you probably still care about that channel and might want to add it to another plot or something. The filter will be deleted if you close the dialog without doing so.