Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Attempt to subtract with overflow when output exceeds terminal height #582

Closed
smoelius opened this issue Sep 2, 2023 · 3 comments · Fixed by #586
Closed

Attempt to subtract with overflow when output exceeds terminal height #582

smoelius opened this issue Sep 2, 2023 · 3 comments · Fixed by #586

Comments

@smoelius
Copy link
Contributor

smoelius commented Sep 2, 2023

I am observing attempt to subtract with overflow on this line:

*last_line_count = real_len - self.orphan_lines_count + shift;

self.lines.len() >= self.orphan_lines_count holds prior to entering this loop:

for (idx, line) in self.lines.iter().enumerate() {

And real_len >= idx holds at the start of each iteration of that loop.

Those two facts causes me to squint at this if block:

if real_len + diff > term_height {
break;
}

In particular, commenting out that if block causes my panics to go away.

I don't completely understand the purpose of that if block, but maybe it can go away?

(indicatif is a great tool, BTW.)

@smoelius
Copy link
Contributor Author

smoelius commented Sep 7, 2023

Here are steps to reproduce the bug.

  1. Create a debug build of Necessist, e.g.:
git clone https://github.com/trailofbits/necessist
cd necessist
cargo build
  1. Create a project with a test like this:
#[test]
fn many_lines() {
    let mut s = String::new();
    for _ in 0..100 {
        s.push('\n');
    }
    None::<()>.expect(&s);
}
  1. Within the project containing the above test, run Necessist:
path-to-clone/target/debug/necessist

You should see something like this:

thread 'main' panicked at 'attempt to subtract with overflow', .../.cargo/registry/src/index.crates.io-6f17d22bba15001f/indicatif-0.17.6/src/draw_target.rs:534:28

@smoelius smoelius changed the title Attempt to subtract with overflow Attempt to subtract with overflow when output exceeds terminal height Sep 7, 2023
@djc
Copy link
Member

djc commented Sep 7, 2023

@smoelius thanks for the detailed issue and reproduction case. Unfortunately I don't have much time to dig into this in detail at the moment. The code you're referencing here was introduced in #563, maybe read up on that to figure out if/why the branch was necessary? Probably the easiest fix is to just slap a saturating_sub() on it, and I'm happy to merge that PR, but if you want to look into it a bit more to figure out the proper invariants (and add some comments to document them for future readers) that would of course be much appreciated!

@smoelius
Copy link
Contributor Author

smoelius commented Sep 7, 2023

Thank you very much for providing the background. I will try to look into this more and propose a fix.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants