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

[bluenote10]

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 '' 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: ::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::open::{{closure}} at /home/fabian/.cargo/registry/src/github.com-1ecc6299db9ec823/portaudio-0.7.0/src/stream.rs:1309 18: as core::ops::function::FnMut>::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.