Crash with out-of bounds assert when changing kits

[treapster]

I'm on manjaro, geonkick 3.2.0-1 lv2 from manjaro repo. I tried to reproduce on the latest github release but ardour reported errors("Missing entry method in ...") when scanning, so i couldn't check the 3.3.0.

1. Open ardour.
2. Add midi track with geonkick lv2 in regular 32 channel mode..
3. Select any kit from presets, play some notes using virtual or real keyboard. Switch drum kits periodically.
4. After switching drum kits a few times, double clicking on a new kit from presets will cause ardour to hang and crash a second later with the following journalctl log.

янв 17 23:00:46 sasus plasmashell[35800]: /usr/include/c++/13.2.1/bits/stl_vector.h:1144: std::vector<_Tp, _Alloc>::const_reference std::vector<_Tp, _Alloc>
::operator[](size_type) const [with _Tp = int; _Alloc = std::allocator<int>; const_reference = const int&; size_type = long unsigned int]: Assertion '__n < 
this->size()' failed.
янв 17 23:00:46 sasus systemd[1]: Started Process Core Dump (PID 35955/UID 0).
янв 17 23:00:50 sasus systemd-coredump[35956]: Process 35800 (ArdourGUI) of user 1000 dumped core.

                                                  Stack trace of thread 35800:
                                                  #0  0x00007faeb75d583c n/a (libc.so.6 + 0x8e83c)
                                                  #1  0x00007faeb7585668 raise (libc.so.6 + 0x3e668)
                                                  #2  0x00007faeb756d4b8 abort (libc.so.6 + 0x264b8)
                                                  #3  0x00007faeb7a133b2 _ZSt21__glibcxx_assert_failPKciS0_S0_ (libstdc++.so.6 + 0xdd3b2)
                                                  #4  0x00007fae383d4068 n/a (geonkick_lv2.so + 0x9a068)
                                                  #5  0x00007fae3839c1f2 n/a (geonkick_lv2.so + 0x621f2)
                                                  #6  0x00007fae383c47f9 n/a (geonkick_lv2.so + 0x8a7f9)
                                                  #7  0x00007fae383d23ee n/a (geonkick_lv2.so + 0x983ee)
                                                  #8  0x00007fae383662c2 n/a (geonkick_lv2.so + 0x2c2c2)
                                                  #9  0x00007fae980f517f n/a (libsuil_x11_in_gtk2.so + 0x217f)
                                                  #10 0x00007faeb8e3a3ee n/a (libglib-2.0.so.0 + 0x5b3ee)
                                                  #11 0x00007faeb8e38f69 n/a (libglib-2.0.so.0 + 0x59f69)
                                                  #12 0x00007faeb8e97367 n/a (libglib-2.0.so.0 + 0xb8367)
                                                  #13 0x00007faeb8e39b97 g_main_loop_run (libglib-2.0.so.0 + 0x5ab97)
                                                  #14 0x00007faeb8a9d473 gtk_main (libgtk-x11-2.0.so.0 + 0x138473)
                                                  #15 0x00007faeb916ef59 _ZN9Gtkmm2ext2UI3runER8Receiver (libgtkmm2ext.so.0 + 0x5bf59)
                                                  #16 0x000055eb4efb66c7 main (ardour-8.2.0 + 0x4aa6c7)
                                                  #17 0x00007faeb756ecd0 n/a (libc.so.6 + 0x27cd0)
                                                  #18 0x00007faeb756ed8a __libc_start_main (libc.so.6 + 0x27d8a)
                                                  #19 0x000055eb4efbbfc5 n/a (ardour-8.2.0 + 0x4affc5)

... traces of other ardour threads here ...

[iurienistor]

Hi, thanks, if will check if I can reproduce. If this happens in 3.2, than also should in 3.3, but is good that not in 3.3 only because 3.3 has a lot of changes in the core DSP. This crash I think is related to GUI part only, I will check.

[treapster]

Thanks for a quick reply! The weird part is that the only place where std::vector\<int> comes up in the codebase is percussionIdList in GeonkickApi, at least according to github search, and all accesses to it seem to be properly checked. But i hope you'll be able to reproduce and get a proper stacktrace pointing at the right spot.

[iurienistor]

@treapster Tested on Manjaro (latest release) and Geonkick (3.2.0) on Ardour (8.2.0), all from repository. Even using a MIDI sequence of notes in a loop and at the same time switching the kits I cannot reproduce the crash. Tested on Ubuntu, and I cannot reproduce. Also, if I download the GNU/Linux binaries from the release Geonkick 3.3.0 from https://geonkick.org and replace the binaries and all files at /usr/lib/lv2/geonkick.lv2 on Manjaro, Geonkick 3.3.0 is loaded ok in Arduor, and also I cannot reproduce the problem. If you can reproduce and can get more info about the stack frames, please, attach them here.

[treapster]

Thanks for your efforts. Regarding the latest release from github, it was my bad and ardour errors were because i was setting vst2 path in ardour instead of LV2_PATH env variable, and it tried to read lv2 plugin as vst2. After loading 3.3.0 release properly in ardour i could not reproduce the error, but it may be because the binary is built without assertions. If you can confirm that github releases are built with NDEBUG, then i think there is no point in trying to reproduce it in github release. I also tried reproducing with repo version again, and it seems like aside from changing presets you also need to click through instruments in the kit tab before and after changing preset to cause crash. It is not super determenistic bug but i'll try to build the plugin with assertions and debug symbols and get you a traceback.

[iurienistor]

"you can confirm that github releases are built with NDEBUG" -- yes, they are built with NDEBUG.

[treapster]

Although judging from the code in stdlib(/usr/include/c++/13.2.1/debug/assertions.h for manjaro version), _GLIBCXX_ASSERTIONS in question depends on _GLIBCXX_DEBUG being defined and does not care about NDEBUG, so apparently the version from manjaro repo was built with _GLIBCXX_DEBUG or at least _GLIBCXX_ASSERTIONS.

[treapster]

I built with -DCMAKE_CXX_FLAGS="-D_GLIBCXX_DEBUG" -DCMAKE_BUILD_TYPE=Debug but despite debug type i still don't get line numbers in traceback. But now we at least got method names!

``` янв 18 13:52:39 sasus plasmashell[70309]: /usr/include/c++/13.2.1/debug/vector:450: янв 18 13:52:39 sasus plasmashell[70309]: In function: янв 18 13:52:39 sasus plasmashell[70309]: std::debug::vector<_Tp, _Allocator>::const_reference std:: янв 18 13:52:39 sasus plasmashell[70309]: debug::vector<_Tp, _Allocator>::operator[](size_type) const [with _Tp = янв 18 13:52:39 sasus plasmashell[70309]: int; _Allocator = std::allocator; const_reference = const int&; янв 18 13:52:39 sasus plasmashell[70309]: size_type = long unsigned int] янв 18 13:52:39 sasus plasmashell[70309]: Error: attempt to subscript container with out-of-bounds index -1, but янв 18 13:52:39 sasus plasmashell[70309]: container only holds 5 elements. янв 18 13:52:39 sasus plasmashell[70309]: Objects involved in the operation: янв 18 13:52:39 sasus plasmashell[70309]: sequence "this" @ 0x7ffc009e62e0 { янв 18 13:52:39 sasus plasmashell[70309]: type = std::debug::vector >; янв 18 13:52:39 sasus plasmashell[70309]: } янв 18 13:52:39 sasus systemd[1]: Started Process Core Dump (PID 70477/UID 0). янв 18 13:52:42 sasus systemd-coredump[70478]: Process 70309 (ArdourGUI) of user 1000 dumped core. Stack trace of thread 70309: #0 0x00007ff90827d83c n/a (libc.so.6 + 0x8e83c) #1 0x00007ff90822d668 raise (libc.so.6 + 0x3e668) #2 0x00007ff9082154b8 abort (libc.so.6 + 0x264b8) #3 0x00007ff90867df52 _ZNK11__gnu_debug16_Error_formatter8_M_errorEv (libstdc++.so.6 + 0x9ff52) #4 0x00007ff8881a9203 _ZNKSt7__debug6vectorIiSaIiEEixEm (geonkick_lv2.so + 0xbd203) #5 0x00007ff8881a950a _ZNK8KitModel17percussionLevelerEi (geonkick_lv2.so + 0xbd50a) #6 0x00007ff888204032 _ZN17KitPercussionView13updateLevelerEv (geonkick_lv2.so + 0x118032) #7 0x00007ff8881bb1eb _ZN9KitWidget16onUpdateLevelersEv (geonkick_lv2.so + 0xcf1eb) #8 0x00007ff888253a19 _ZNKSt8functionIFvvEEclEv (geonkick_lv2.so + 0x167a19) #9 0x00007ff888253869 _ZN7RkTimer11callTimeoutEv (geonkick_lv2.so + 0x167869) #10 0x00007ff888256f01 _ZN12RkEventQueue16RkEventQueueImpl13processTimersEv (geonkick_lv2.so + 0x16af01) #11 0x00007ff88825359d _ZN12RkEventQueue13processTimersEv (geonkick_lv2.so + 0x16759d) #12 0x00007ff8882639fb _ZN6RkMain10RkMainImpl4execEb (geonkick_lv2.so + 0x1779fb) #13 0x00007ff88812d9db gkick_idle (geonkick_lv2.so + 0x419db) #14 0x00007ff8e8d9d17f n/a (libsuil_x11_in_gtk2.so + 0x217f) #15 0x00007ff909ae23ee n/a (libglib-2.0.so.0 + 0x5b3ee) #16 0x00007ff909ae0f69 n/a (libglib-2.0.so.0 + 0x59f69) #17 0x00007ff909b3f367 n/a (libglib-2.0.so.0 + 0xb8367) #18 0x00007ff909ae1b97 g_main_loop_run (libglib-2.0.so.0 + 0x5ab97) #19 0x00007ff909745473 gtk_main (libgtk-x11-2.0.so.0 + 0x138473) #20 0x00007ff909e16f59 _ZN9Gtkmm2ext2UI3runER8Receiver (libgtkmm2ext.so.0 + 0x5bf59) #21 0x000055c77dfd66c7 main (ardour-8.2.0 + 0x4aa6c7) #22 0x00007ff908216cd0 n/a (libc.so.6 + 0x27cd0) #23 0x00007ff908216d8a __libc_start_main (libc.so.6 + 0x27d8a) #24 0x000055c77dfdbfc5 n/a (ardour-8.2.0 + 0x4affc5) ```

[treapster]

Looks like the patch should be

diff --git a/src/kit_model.cpp b/src/kit_model.cpp
index d08387b..d3634f8 100644
--- a/src/kit_model.cpp
+++ b/src/kit_model.cpp
@@ -325,7 +325,7 @@ size_t KitModel::maxPercussionNumber() const
 int KitModel::percussionId(int index) const
 {
         const auto &ids = geonkickApi->ordredPercussionIds();
-        if (index < -1 || index > static_cast<decltype(index)>(ids.size() - 1))
+        if (index < 0 || index > static_cast<decltype(index)>(ids.size() - 1))
                 return -1;
         return ids[index];
 }

percussionIdList was indeed the culprit!

[iurienistor]

@treapster Hi, I'll thanks for the findings. I'll create a commit. :)

[iurienistor]

Here is the commit: https://github.com/Geonkick-Synthesizer/geonkick/commit/36a5049b16bbf7ce2ac57e408f25dc867f940452