Rust backend can panic because of 'attempt to multiply with overflow'

[bluenote10]

trafficstars

An easy way to reproduce this problem is to use

faust2portaudiorust ./examples/gameaudio/rain.dsp

Running cargo run crashes with:

Faust Rust code running with Portaudio: sample-rate = 44100 buffer-size = 64
getNumInputs: 0
getNumOutputs: 2
thread '<unnamed>' panicked at 'attempt to multiply with overflow', src/main.rs:170:35
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
fatal runtime error: failed to initiate panic, error 5
[1]    20977 abort (core dumped)  cargo run

Full traceback

thread '<unnamed>' panicked at 'attempt to multiply with overflow', src/main.rs:170:35
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:77
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:61
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1028
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1412
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:65
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:50
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:188
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:205
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:464
  11: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:373
  12: rust_begin_unwind
             at src/libstd/panicking.rs:302
  13: core::panicking::panic_fmt
             at src/libcore/panicking.rs:139
  14: core::panicking::panic
             at src/libcore/panicking.rs:70
  15: rain_portaudiorust::mydsp::compute
             at src/main.rs:170
  16: rain_portaudiorust::run::{{closure}}
             at src/main.rs:248
  17: portaudio::stream::Stream<portaudio::stream::NonBlocking,F>::open::{{closure}}
             at /home/fabian/.cargo/registry/src/github.com-1ecc6299db9ec823/portaudio-0.7.0/src/stream.rs:1309
  18: <alloc::boxed::Box<F> as core::ops::function::FnMut<A>>::call_mut
             at /rustc/73528e339aae0f17a15ffa49a8ac608f50c6cf14/src/liballoc/boxed.rs:949
  19: portaudio::stream::stream_callback_proc
             at /home/fabian/.cargo/registry/src/github.com-1ecc6299db9ec823/portaudio-0.7.0/src/stream.rs:1365
  20: NonAdaptingProcess
             at src/common/pa_process.c:871
  21: PaUtil_EndBufferProcessing
             at src/common/pa_process.c:1562
  22: PaOSS_AudioThreadProc
             at src/hostapi/oss/pa_unix_oss.c:1726
  23: start_thread
  24: __clone
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
fatal runtime error: failed to initiate panic, error 5

The problematic line of code is:

let mut iTemp0: i32 = (1103515245 * (self.iRec0[1] + 12345));

The overflows here are probably on purpose for noise generation. We'll have to look into how this is possible in Rust.

Edit: It looks like std::num::Wrapping could be used to allow for intentionally-wrapped arithmetic.

CC @bkfox maybe you're more experienced on this.

[sletz]

The problem is known and yes the overflow is needed. And I would be interested to know the correct solution.

[bluenote10]

As a quick test I have edited the generated code to:

let mut iTemp0: Wrapping<i32> = (Wrapping(1103515245) * Wrapping(self.iRec0[1] + 12345));
self.iRec0[0] = (Wrapping(1103515245i32) * (iTemp0 + Wrapping(12345))).0;
let mut iRec1: i32 = iTemp0.0;

and then there is no panic. Since the example is noise anyway, it is hard to say if it has the right semantics, but most likely it does.

So basically the generator would have to use Wrapping<i32> (or u32?) to allow for overflows.

[sletz]

Wrapping<i32> is the semantic we want, but having to use this kind of ugly syntax is not really satisfactory. Is there any way in Rust to keep the standard syntax but be sure the underlying type is Wrapping<i32> ?

[bluenote10]

I'm no expert on this, but given Rust focus on being explicit about safety, it is likely that this is the only option.

My hacky example above maybe made it look more ugly than it has to be. Basically it comes down to:

• replacing the literals,
• replacing the types,
• unwrapping with .0 where necessary.

At least the first two could be made nicer by using type aliases and a macro for the literals. For instance:

use std::num::Wrapping;

type I32 = Wrapping<i32>;

macro_rules! w {
    ($lit:expr) => {
        Wrapping($lit)
    };
}

fn main() {
    // syntax becomes relatively lightweight
    let a: I32 = w!(1) * w!(2);
}

If the code uses these types consistently there shouldn't be many places where unwrapping is needed.

[sletz]

Yes, using macro is probably the way to go, I'll have a look ASAP. As curiosity, in what use-case are you testing the Rust backend ?

[bluenote10]

No hurry, I should have mentioned that this is only a problem in debug builds. For now it is easy to work-around via cargo run --release where Rust disables overflow checks anyway.

As curiosity, in what use-case are you testing the Rust backend?

I've always been a fan of Faust, but I never found the time to learn it. I heard that it has Rust support now, and thought that is a nice combination, and a good opportunity to look into it again. For now I'm just experimenting to see how it works / what is possible.

(It was actually pretty straightforward to get something running. Now I'm at a point where I can run examples through the portaudiorust architecture, but I have like ~1 sec audio pauses every 0.2 sec or so -- will investigate that, most likely an issue with portaudio. Edit: Turned out to be an issue with the duplex mode of portaudio, see https://github.com/RustAudio/rust-portaudio/issues/180. Switching to a pure output stream works fine, and is sufficient since I don't need audio input.)

[bkfox]

I actually see that there are standard methods implemented for overflowing integer operations. This would do the job:

let mut iTemp0: i32 = 1103515245.wrapping_mul(self.iRec0[1].wrapping_add(12345));

[plule]

I've opened a PR implementing the wrapping_* operations.

As of now, a possible workaround for now is to disable entirely the overflow checks. It can be done in Cargo.toml:

[profile.dev]
overflow-checks = false

This is however global to the entire project. I did not manage to scope it to the module with #![allow(arithmetic_overflow)], so I think the way to go is to use the verbose wrapping_* methods.

[bluenote10]

It can be done in Cargo.toml:

I also think your PR #802 is the way to go. I simply never had the time to figure out where it has to be integrated. Thanks for taking care of that!

I think the way to go is to use the verbose wrapping_* methods.

Yes, the generated code isn't supposed to be the most human-readable piece of code anyway. If we really care, we could later shorten it slightly by introducing a macro, but I'm not sure if it is worth the effort / added complexity.

[obsoleszenz]

@sletz Isn't this fixed by https://github.com/grame-cncm/faust/commit/47cc3a7a303df43d716bf2c27925c389eb4c0c73 ?

[sletz]

AFAICS it should yes.