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

Stack Overflow with Specific Optimization Levels When Running Test Cases #702

Closed
cicilzx opened this issue Mar 18, 2024 · 6 comments · Fixed by #703
Closed

Stack Overflow with Specific Optimization Levels When Running Test Cases #702

cicilzx opened this issue Mar 18, 2024 · 6 comments · Fixed by #703
Labels
A-parse Area: Parsing TOML C-bug Category: Things not working as expected

Comments

@cicilzx
Copy link

cicilzx commented Mar 18, 2024

Hi, I've noticed that when running test cases for this crate, I get a stack overflow when using certain specific optimization level (-C opt-level=0), not sure if it's a problem with the crate or with rustc.

I am using cargo version: cargo 1.77.0-nightly (7bb7b5395 2024-01-20), and below is an example of triggering the issue.

By using the following command:

RUSTFLAGS="-C opt-level=2" cargo test

The tests will run successfully like this:

......
test crates/toml_edit/src/ser/mod.rs - ser::to_string (line 103) ... ok
test crates/toml_edit/src/ser/value.rs - ser::value::ValueSerializer (line 15) ... ok
test crates/toml_edit/src/de/value.rs - de::value::ValueDeserializer (line 13) ... ok
test crates/toml_edit/src/item.rs - item::value (line 362) ... ok

test result: ok. 32 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.61s

However, if I change the optimization level to 0, for example, with the command:

cargo clean
RUSTFLAGS="-C opt-level=0" cargo test

It will produce the following stackoverflow:

test parse::stray_cr ... ok
test stackoverflow::inline_dotted_key_recursion_limit ... ok
test stackoverflow::dotted_key_recursion_limit ... ok
test stackoverflow::table_key_recursion_limit ... ok

thread 'stackoverflow::array_recursion_limit' has overflowed its stack
fatal runtime error: stack overflow
error: test failed, to rerun pass `-p toml_edit --test testsuite`

Caused by:
  process didn't exit successfully: `/home/cici/toml/target/debug/deps/testsuite-9882ff360a6a395e` (signal: 6, SIGABRT: process abort signal)
@epage
Copy link
Member

epage commented Mar 18, 2024

I wonder if this is a duplicate of #653 (which I closed due to lack of reproduction case)

I'm also able to reproduce this with

$ CARGO_PROFILE_DEV_OPT_LEVEL=0 cargo test

The odd thing is that the dev profile defaults to opt-level = 0, so not sure why changing this flag makes a difference.

@epage
Copy link
Member

epage commented Mar 18, 2024

If I drop the recursion limit from 128 to 64, it passes.

@epage epage added C-bug Category: Things not working as expected A-parse Area: Parsing TOML labels Mar 18, 2024
@cicilzx
Copy link
Author

cicilzx commented Mar 18, 2024

I wonder if this is a duplicate of #653 (which I closed due to lack of reproduction case)

I'm also able to reproduce this with

$ CARGO_PROFILE_DEV_OPT_LEVEL=0 cargo test

The odd thing is that the dev profile defaults to opt-level = 0, so not sure why changing this flag makes a difference.

I think the reproduction case is the test case tests/testsuite/stackoverflow.rs/array_recursion_limit(), I could reproduce it with this command:

RUSTFLAGS="-C opt-level=0" cargo test --package toml_edit --test testsuite --features parse --features display -- stackoverflow::array_recursion_limit --exact --nocapture

But this case will pass if opt-level is set as 1/2/3/s/z. Is it possible that this is a bug related with rustc optimization?

@epage
Copy link
Member

epage commented Mar 18, 2024

Likely the problem is that the optimizer is reducing the stack size, making the recursion cheaper and allowing deeper recursion. The biggest red flag is about the dev profile's defaults.

@cicilzx
Copy link
Author

cicilzx commented Mar 18, 2024

I wonder if this is a duplicate of #653 (which I closed due to lack of reproduction case)

I'm also able to reproduce this with

$ CARGO_PROFILE_DEV_OPT_LEVEL=0 cargo test

The odd thing is that the dev profile defaults to opt-level = 0, so not sure why changing this flag makes a difference.

In your Cargo.toml (https://github.com/toml-rs/toml/blob/main/Cargo.toml), the opt-level is set as 1, so it's not using the default dev profile.

epage added a commit to epage/toml_edit that referenced this issue Mar 18, 2024
epage added a commit to epage/toml_edit that referenced this issue Mar 18, 2024
@epage
Copy link
Member

epage commented Mar 18, 2024

Wow, hadn't realize that was in there as it isn't part of my standard template.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parse Area: Parsing TOML C-bug Category: Things not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants