Why are some stackoverflow causes not caught by Rust?

I wrote this function not paying attention that it was recursively calling itself:

pub fn test_data_scenario() -> Cow<'static, str> {
   Cow::Owned(format!(
     r#"data () {{
         {}
       }}
     "#,
     test_data_scenario(),
   ))
 }

I'm returning test data like this as a Cow merely because it is a tad bit less typing in my unit tests since my parser expects &str (not String) as its argument.

Sure enough, I got an obscure runtime error:

thread 'parser::lex::lex_tests::lex_tests::data_tests' has overflowed its stack
fatal runtime error: stack overflow
error: test failed, to rerun pass '-p app --bin app'

Caused by:
process didn't exit successfully: /root/git/parserly/src-tauri/target/debug/deps/app-084e26cc241d286c 'parser::lex::lex_tests::lex_tests' --format=json -Z unstable-options --show-output (signal: 6, SIGABRT: process abort signal)

Process finished with exit code 101

It made me wonder: Couldn't the Rust compiler detect that I'm building an infinite &str on the stack and complain? Or am I just living proof that idiot-proof systems eventually just lead to there being a bigger idiot? :poop:

str is borrowed from a String, which is a newtype for Vec<u8> so it should be heap-allocated. I think the stack overflow is from the function calls.

To address the broader question, how would the compiler know how much stack memory will be available when the program is run? It could even be cross-compiled to run on a different target system. It might make for a short test but you probably aren’t providing an API with recursion and don’t need to test how it handles recursion (and this proves that you basically can’t test for that). It’s a runtime error that can be avoided by better coding practices: just don’t recurse, which I’m sure you know.

The compiler works with what it knows at compile time, not what it can assume about runtime. I would like to know if clippy will catch recursion, though…

2 Likes

Well, if I copy-paste your code to playground, I get a helpful warning:

warning: function cannot return without recursing
 --> src/lib.rs:2:1
  |
2 | pub fn test_data_scenario() -> Cow<'static, str> {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
...
8 |      test_data_scenario(),
  |      -------------------- recursive call site
  |
  = note: `#[warn(unconditional_recursion)]` on by default
  = help: a `loop` may express intention better if this is on purpose

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=091ff80c360d595f90887e8c65f9ddeb

9 Likes

Out of curiosity, how are you compiling your code?

Like @scottmcm pointed out, the compiler emits a warning whenever it detects unconditional infinite recursion, so maybe your development environment wasn't emitting warnings properly.

2 Likes

Thanks @Michael-F-Bryan and @scottmcm. I sifted through the hundreds of warnings in my build and yup, it's in there. Busted.

I guess I'm so used to Rust throwing errors for any and all memory safety issues so I just sorta got lazy with warnings (telling myself I'll come back for them later). I'll pay more attention to warnings but hopefully my feedback is helpful for others that are expecting the same.

Update: and as I suspected, there's a simple flag that can be set to fail a build if there's any warnings. Not a panacea I'm sure, but that would definitely have caught this issue in my case:

1 Like

In fact, it's usually recommended to fix the warnings ASAP and/or to explicitly #[allow(...)] them with the helpful comment on why it was done (e.g. if it is a known false-positive). Rust tries very hard to warn you about potential logical mistakes, and it'd be better to find them while fixing the warning then to stumble into them in debugging.

5 Likes

As was mentioned, rustc is very careful about even the warnings it emits. You should get it clean before calling a change "done".

Or, if some of them are stylistic things that you intentionally don't want, turn them off. For example, you can do things like #![allow(nonstandard_style)] if you've intentionally chosen to use a different naming scheme.

(Though I'd definitely encourage sticking with the standard scheme. There's a few edge cases that are more likely to be caught when using the casing-based conventions that Rust uses by default.)

3 Likes

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.