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

Warn on useless comparisons (like GCC's -Wtype-limits) #3833

Closed
veddan opened this issue Oct 22, 2012 · 1 comment
Closed

Warn on useless comparisons (like GCC's -Wtype-limits) #3833

veddan opened this issue Oct 22, 2012 · 1 comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@veddan
Copy link
Contributor

veddan commented Oct 22, 2012

Consider this program:

fn main() {
        let x = ~[1, 2, 3, 4, 5];
        let mut i = x.len() - 1;
        while i >= 0 {
                if x[i] <= 3 {
                        io::println("At most three");
                }
                i -= 1;
        }
}

The behaviour one might expect here is for the program to print "At most three" three times and then exiting. The actual program output is:

At most three
At most three
At most three
rust: task failed at 'index out of bounds: the len is 5 but the index
is -1', uint.rs:6
rust: domain main @0xdd9c10 root task failed

The problem is that i is unsigned, so the comparison i >= 0 will always be true. With the addition of "-Wtype-limits" (GCC manual) this could easily be avoided.

veddan added a commit to veddan/rust that referenced this issue Oct 24, 2012
brson added a commit that referenced this issue Oct 24, 2012
Lint pass like GCC's -Wtype-limits (#3833)
@brson
Copy link
Contributor

brson commented Oct 24, 2012

Fixed.

@brson brson closed this as completed Oct 24, 2012
bors pushed a commit to rust-lang-ci/rust that referenced this issue May 15, 2021
RalfJung pushed a commit to RalfJung/rust that referenced this issue Aug 22, 2024
…r=RalfJung

Make Tree Borrows Provenance GC no longer produce stack overflows

Most functions operating on Tree Borrows' trees are carefully written to not cause stack overflows due to too much recursion. The one exception is [`Tree::keep_only_needed`](https://github.com/rust-lang/miri/blob/94f5588fafcc7d59fce60ca8f7af0208e6f618d4/src/borrow_tracker/tree_borrows/tree.rs#L724), which just uses regular recursion.
This function is part of the provenance GC, so it is called regularly for every allocation in the program.

Tests show that this is a problem in practice. For example, the test `fill::horizontal_line` in crate `tiny-skia` (version 0.11.4) is such a test.

This PR changes this, this test no now longer crashes. Instead, it succeeds (after a _long_ time).
RalfJung pushed a commit to RalfJung/rust that referenced this issue Aug 30, 2024
…r=RalfJung

Avoid extra copy by using `retain_mut` and moving the deletion into the closure

Fixes the FIXME introduced in rust-lang#3833. Thanks to `@dmitrii-ubskii` for the idea 🙂
RalfJung pushed a commit to RalfJung/rust that referenced this issue Aug 30, 2024
Follow-up on rust-lang#3833 and rust-lang#3835. In these PRs, the TB GC was fixed to no
longer cause a stack overflow. One test that motivated it was the test
`fill::horizontal_line` in `tiny_skia`. But not causing stack overflows
was not a large improvents, since it did not fix the fundamental issue:
The tree was too large. The test now ran, but it required gigabytes of
memory and hours of time, whereas it finishes within seconds in Stacked
Borrows.

The problem in that test was that it used [`slice::chunked`](https://doc.rust-lang.org/std/primitive.slice.html#method.chunks) to iterate
a slice in chunks. That iterator is written to reborrow at each call to
`next`, which creates a linear tree with a bunch of intermediary nodes,
which also fragments the `RangeMap` for that allocation.

The solution is to now compact the tree, so that these interior nodes
are removed. Care is taken to not remove nodes that are protected, or
that otherwise restrict their children.
RalfJung pushed a commit to RalfJung/rust that referenced this issue Aug 30, 2024
…e-gc, r=RalfJung

Make Tree Borrows Provenance GC compact the tree

Follow-up on rust-lang#3833 and rust-lang#3835. In these PRs, the TB GC was fixed to no longer cause a stack overflow. One test that motivated it was the test `fill::horizontal_line` in [`tiny-skia`](https://github.com/RazrFalcon/tiny-skia). But not causing stack overflows was not a large improvents, since it did not fix the fundamental issue: The tree was too large. The test now ran, but it required gigabytes of memory and hours of time (only for it to be OOM-killed 🤬), whereas it finishes within 24 seconds in Stacked Borrows. With this merged, it finishes in about 40 seconds under TB.

The problem in that test was that it used [`slice::chunked`](https://doc.rust-lang.org/std/primitive.slice.html#method.chunks) to iterate a slice in chunks. That iterator is written to reborrow at each call to `next`, which creates a linear tree with a bunch of intermediary nodes, which also fragments the `RangeMap` for that allocation.

The solution is to now compact the tree, so that these interior nodes are removed. Care is taken to not remove nodes that are protected, or that otherwise restrict their children.

I am currently only 99% sure that this is sound, and I do also think that this could compact even more. So `@Vanille-N` please also have a look at whether I got the compacting logic right.

For a more visual comparison, [here is a gist](https://gist.github.com/JoJoDeveloping/ae4a7f7c29335a4c233ef42d2f267b01) of what the tree looks like at one point during that test, with and without compacting.

This new GC requires a different iteration order during accesses (since the current one can make the error messages non-deterministic), so it is rebased on top of rust-lang#3843 and requires that PR to be merged first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

2 participants