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

Condition handlers can unsoundly alias mutable stack variables #6459

Closed
brson opened this issue May 13, 2013 · 7 comments
Closed

Condition handlers can unsoundly alias mutable stack variables #6459

brson opened this issue May 13, 2013 · 7 comments

Comments

@brson
Copy link
Contributor

brson commented May 13, 2013

In this example, keep_reading is mutable and aliased.

    fn read_to_end(&mut self) -> ~[u8] {
        let mut buf = vec::with_capacity(DEFAULT_BUF_SIZE);
        let mut keep_reading = true;
        do read_error::cond.trap(|e| {
            if e.kind == EndOfFile {
                keep_reading = false;
            }
        }).in {
            while keep_reading {
                self.push_bytes(&mut buf, DEFAULT_BUF_SIZE)
            }
        }
        return buf;
    }
@brson
Copy link
Contributor Author

brson commented May 13, 2013

Although similar, I think finally doesn't have this problem because the regions are propagated correctly - finally is just a method call on a stack closure. Conditions though are unsafely put into TLS and presumably the region is lost from the returned condition object.

@nikomatsakis
Copy link
Contributor

The real problem here is the problem described in this blog post. (#2202)

@nikomatsakis
Copy link
Contributor

In particular, if we implemented the plan I had in mind, the variable would be "claimed" by both of the closures, leading to errors. This is a bit unfortunate, though, since in fact the code is probably safe.

@nikomatsakis
Copy link
Contributor

Thinking about this a bit more, I imagine we could distinguish fairly easily between variables that are borrowed (e.g., &mut keep_reading or &keep_reading) and variables that are merely assigned to and written from. Doing so would allow a case like this to work.

@metajack
Copy link
Contributor

metajack commented Aug 8, 2013

visiting for triage. nominating for backwards compat.

@catamorphism
Copy link
Contributor

Inherits from metabug. De-nominating

@alexcrichton
Copy link
Member

Closed by 0ac6e5a (tests checked in as part of #12158)

flip1995 pushed a commit to flip1995/rust that referenced this issue Dec 28, 2023
…-matches!, r=llogiq

6459: Check for redundant `matches!` with `Ready`, `Pending`, `V4`, `V6`

Fixes rust-lang#6459.

```
changelog: [`redundant_pattern_matching`]: Add checks for `Poll::{Ready,Pending}` and `IpAddr::{V4,V6}` in `matches!`
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants