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

NaN formula values crash Surge XT when a note is played #5566

Closed
xard-dev opened this issue Dec 6, 2021 · 7 comments · Fixed by #5576
Closed

NaN formula values crash Surge XT when a note is played #5566

xard-dev opened this issue Dec 6, 2021 · 7 comments · Fixed by #5576
Labels
Bug Report Item submitted using the Bug Report template Modulation Modulation related issues
Milestone

Comments

@xard-dev
Copy link

xard-dev commented Dec 6, 2021

Bug Description:
NaN formula values crash Surge XT when a note is played.

Surge Version
Surge XT 0.99.main.007a427

  • Version:
  • Plugin Type: LV2
  • Bitness: 64-bit

Reproduction Steps:
Steps to reproduce the behavior:

  1. Open first example preset "01 A Simple Formula".
  2. Click on "FORM 1.1".
  3. Click on the "Pen icon" to open Formula editor.
  4. Change row "pclamp = limit_range(modstate["intphase"], 0, 10) / 10" tail to "... / 0".
  5. Play a note.

Expected Behavior:
No sound is played and additionally an error message is displayed in some method.

Screenshots:
image

Computer Information (please complete the following information):

  • OS: Ubuntu 20.04 LTS
  • Host: Carla / Standalone
  • Version: 2.1.0
@xard-dev xard-dev added the Bug Report Item submitted using the Bug Report template label Dec 6, 2021
@baconpaul
Copy link
Collaborator

Great reports this morning! Thanks so much!!

@mkruselj mkruselj added the Modulation Modulation related issues label Dec 6, 2021
@mkruselj mkruselj changed the title XT - NaN formula values crash Surge XT when a note is played NaN formula values crash Surge XT when a note is played Dec 6, 2021
@mkruselj mkruselj added this to the Surge XT 1.0 milestone Dec 6, 2021
@mkruselj
Copy link
Collaborator

mkruselj commented Dec 6, 2021

Just FYI no need to put XT in the title since we're not going back to fixing 1.9 so everything that is reported already basically pertains to XT. Thanks!

@xard-dev
Copy link
Author

xard-dev commented Dec 6, 2021

Alright, I'll drop the prefix then.

@baconpaul
Copy link
Collaborator

on my mac this doesn't crash but does of course produce garbage
i wonder if you have fpe on in your host
but i can put a std::is_finite() check when i pull the values from lua back into c++ world

@baconpaul
Copy link
Collaborator

but first let me fire up linux and see if it crashes there.

@xard-dev
Copy link
Author

xard-dev commented Dec 6, 2021

Confirmed that the crash happens on standalone as well.

@baconpaul
Copy link
Collaborator

Right so there's a few things happening here

First of all the juce path renderer on linux throws exceptions with nans. Fine.

But more importantly, our limit_range assumes a finite input so with a nan all our checks blow up. The particular stack in this case happens in SurgeVoice where we use the pitch to interpolate across a 2^x interpolation array in the filter blah blah and it would be fine if we were accidentally out of bounds as long as we are finite

I don't want to make limit_range check std::isfinite. We use that everywhere. So the fix I propose - since we have made sure all the rest of the modulators never make nan and we can't guarantee that with lua - is to just check isfinite on the outbound of the formula modulator, set a flag, and voila.

Will do that now.

baconpaul added a commit to baconpaul/surge that referenced this issue Dec 6, 2021
NaN and Inf from formula would rip through the engine and
blow through various checks and so on. So inside formula
do an explicit std::isfinite and set a flag if they return
these.

Closes surge-synthesizer#5566
baconpaul added a commit to baconpaul/surge that referenced this issue Dec 6, 2021
NaN and Inf from formula would rip through the engine and
blow through various checks and so on. So inside formula
do an explicit std::isfinite and set a flag if they return
these.

Closes surge-synthesizer#5566
baconpaul added a commit to baconpaul/surge that referenced this issue Dec 6, 2021
NaN and Inf from formula would rip through the engine and
blow through various checks and so on. So inside formula
do an explicit std::isfinite and set a flag if they return
these. Also add a unit test of the behavior.

Closes surge-synthesizer#5566
baconpaul added a commit that referenced this issue Dec 6, 2021
NaN and Inf from formula would rip through the engine and
blow through various checks and so on. So inside formula
do an explicit std::isfinite and set a flag if they return
these. Also add a unit test of the behavior.

Closes #5566
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 Modulation Modulation related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants