Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when deleting control nodes from the Default Voice MSEG #7401

Closed
jhillest opened this issue Dec 16, 2023 · 15 comments · Fixed by #7459
Closed

Crash when deleting control nodes from the Default Voice MSEG #7401

jhillest opened this issue Dec 16, 2023 · 15 comments · Fixed by #7459
Labels
Bug Report Item submitted using the Bug Report template

Comments

@jhillest
Copy link

Surge XT Version
1.3.makepkg.20a8e16ab

  • Version:
  • Plugin Type: VST3, Standalone
  • Bitness: [64-bit]

Reproduction Steps:
Steps to reproduce the behaviour, starting from default Init Saw patch:

  1. Change LFO1 waveform to MSEG
  2. Open the MSEG editing window.
  3. Right-click anywhere in the graph window, choose "Create" and select "Default Voice MSEG" from the list.
  4. Double-click on one of the control nodes at either step 2 or 3 to remove it.
  5. Double-click on the last control node.

Expected Behavior:
Deletion of the last control node, without crash.

Computer Information (please complete the following!):

  • OS: Arch Linux 6.6.7-zen1-1-zen
  • Host: Ardour
  • Version: 8.2, codenamed "Tracks and Traces"

Additional Information:
VST3 plugin crashes Ardour when triggering this behaviour.
Standalone version also crashes.

@jhillest jhillest added the Bug Report Item submitted using the Bug Report template label Dec 16, 2023
@mkruselj mkruselj added this to the Surge XT 1.3.1 milestone Dec 16, 2023
@baconpaul
Copy link
Collaborator

hmmm i can't recreate that on standalone on macOS. double click on last point just takes me from n to n-1 segments in envelope mode. i'll be in linux sometime this week so I'll try then.

anyone else can repro on their system?

@mkruselj mkruselj added the Awaiting User Information Self-explanatory label Dec 22, 2023
@mkruselj mkruselj added the Cannot Reproduce Issue that cannot be reproduced on our side, or user doesn't have a consistent reproducible case label Jan 9, 2024
@mkruselj
Copy link
Collaborator

mkruselj commented Jan 9, 2024

Cannot reproduce on Windows, either.

@baconpaul
Copy link
Collaborator

Hmm I trie this on my linux box last night also with no crash.

Any chance of a stack trace or anything?

@mkruselj
Copy link
Collaborator

@jhillest Some feedback would be appreciated, thanks!

@jhillest
Copy link
Author

I've never done a backtrace before.
I used these instructions to make one with gdb - hope it helps.

backtrace.log

@jhillest
Copy link
Author

A friend reminded me about strace and gave me instructions on how to use it.
strace.txt

@baconpaul
Copy link
Collaborator

Thank you! I’m away this weekend but will look next week to see what I can glean from these.

crashes every time for you yeah?

@jhillest
Copy link
Author

Yes @baconpaul - crashes every time, without fail.

@baconpaul
Copy link
Collaborator

Oh I wonder if this is an “arch compiles prod software with asserts on” issue.

Arch can crash with (say) an inverted clamp where Ubuntu will not because arch default build keeps asserts on in prod.

Not anything to do but I literally just had this thought pop in my head so wanted to jot it down as note to self for next week

@baconpaul
Copy link
Collaborator

Yeah look at this from the crash log

#1  0x00007ffff76ac8a3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
No locals.
#2  0x00007ffff765c668 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
        ret = <optimized out>
#3  0x00007ffff76444b8 in __GI_abort () at abort.c:79
        save_stage = 1
        act = {__sigaction_handler = {sa_handler = 0x20, sa_sigaction = 0x20}, sa_mask = {__val = {19447805, 202, 19639080, 140737488338688, 1, 0, 0, 1065353216, 0, 0, 5840896, 74656208, 13, 0, 0, 1065353216}}, sa_flags = 13, sa_restorer = 0x0}
#4  0x00007ffff78dd3b2 in std::__glibcxx_assert_fail (file=file@entry=0x128bffd "/usr/include/c++/13.2.1/array", line=line@entry=202, function=function@entry=0x12bab28 "constexpr std::array<_Tp, _Nm>::value_type& std::array<_Tp, _Nm>::operator[](size_type) [with _Tp = float; long unsigned int _Nm = 128; reference = float&; size_type = long unsigned int]", condition=condition@entry=0x128bfea "__n < this->size()") at /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/debug.cc:61
No locals.
#5  0x0000000000a7d658 in std::array<float, 128ul>::operator[] (__n=<optimized out>, this=<optimized out>) at /usr/include/c++/13.2.1/array:202
        __PRETTY_FUNCTION__ = <optimized out>
#6  Surge::Overlays::MSEGCanvas::drawAxis (this=this@entry=0x47c5fd0, g=...) at /usr/src/debug/surge-xt/surge-xt/src/surge-xt/gui/overlays/MSEGEditor.cpp:807
        pxs = <optimized out>
        pxe = <optimized out>
        mcolor = {argb = {{internal = 1291845631, components = {b = 255 '\377', g = 255 '\377', r = 255 '\377', a = 76 'L'}}}}
        ls = <optimized out>
        le = <optimized out>
        r = <optimized out>

so lemme see if i can figure out what in drawAxis is oob on that array.

@baconpaul
Copy link
Collaborator

ahh and now i can trap it on macOS too. Got it. Will have fix soon.

baconpaul added a commit to baconpaul/surge that referenced this issue Jan 21, 2024
A case where you deleted a loop node marker which reset
the loop would result in a draw error since we didn't
consistently use the constraineed loop endpoints to
indedx the segments. This caused a crash in some situations.

Closes surge-synthesizer#7401
@baconpaul baconpaul removed Awaiting User Information Self-explanatory Cannot Reproduce Issue that cannot be reproduced on our side, or user doesn't have a consistent reproducible case labels Jan 21, 2024
baconpaul added a commit that referenced this issue Jan 21, 2024
A case where you deleted a loop node marker which reset
the loop would result in a draw error since we didn't
consistently use the constraineed loop endpoints to
indedx the segments. This caused a crash in some situations.

Closes #7401
@baconpaul
Copy link
Collaborator

baconpaul commented Jan 21, 2024

OK I believe this is fixed @jhillest

so

  1. Thank you very much. Good bug reports make good software
  2. If you use the binaries, a new nightly will be available in about an hour
  3. If you build from source, pull any commit at or newer than 4bb5486

Since I was finally able to repro it on mac, i closed this issue with the merge, but if you still get it please let us know and we can re-open

@baconpaul
Copy link
Collaborator

(and just so i remember, mac in RelWithDebInfo behaves like arch in this case)

@jhillest
Copy link
Author

jhillest commented Jan 22, 2024

@baconpaul
I built Surge from source. Ended up with version 1.3.main.14417985.
Tested standalone and copied Surge XP.clap to /usr/lib/clap/ and fired up Bitwig.
I did not bother with the VST3 version.

Your fix works!

Also:

  1. I'm equally grateful to have access to this amazing software. 🤩
  2. It's both encouraging and uplifting to read that you think my bug report is good. 🥹
  3. Thanks for the reminder @mkruselj. 👌😊
  4. Thanks to my friend who kindly informed me about strace - you know who you are. 😉

I'll be looking forward to submit more bug reports in the future! 🤓

Cordial regards
/Joachim

@baconpaul
Copy link
Collaborator

Wonderful!

Also lesson learned - if we get an arch bug report, try and repro it with macos/RelDebInfo :)

Have fun using surge! (and also short circuit this year)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Report Item submitted using the Bug Report template
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants