Skip to content

Account for possible 0-sized elements in vector iterators #7738

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

Closed
wants to merge 2 commits into from

Conversation

alexcrichton
Copy link
Member

Closes #7733

@thestinger
Copy link
Contributor

I don't think this is the right solution because it will have runtime overhead. It really just needs to be a pointer compare and a pointer addition, rather than 3 operations.

It's possible it could be solved by making a nonzero_offset pointer method, and using that instead.

There are also some other broken things (not just iterators) like slices. It seems like fixed-size arrays might have other problems too.

@alexcrichton
Copy link
Member Author

I believe that slices are correct. They use offset to figure out the start, but that doesn't really matter. Their length is calculated just fine with nonzero_size_of.

Fixed-size arrays also don't appear to be wrong. Their actual size may be questionable, but the iteration works just fine over them.

I'm not quite comfortable with a nonzero_offset method because it doesn't really make sense. The struct itself is 0-sized so there's not really an address to memory containing it. I think that this needs to know how big the vector actually is in memory. Slices don't really matter so long as their size field is correct, but the actual vector representations still have a concept of length somehow and I'm just not sure what that is...

If we used a nonzero_offset method, then you would be generating pointers past the bounds of the vector in theory. If the vector were truly 0-sized (and just some length on the side), then generating pointers beyond the base of the vector would be invalid, right? Even if you don't actually read the memory, it seems wrong to get pointers to other data.

Also, I don't think that anyone else should seriously be using a nonzero_offset method (I could be wrong here), so adding it to ptr seems like a bad idea.

@thestinger
Copy link
Contributor

@alexcrichton: they're definitely wrong, the case I reported in the bug is with a slice

@alexcrichton
Copy link
Member Author

I specifically added a test for the case you mentioned, and it passes. As in in this request they're fixed, but it wasn't anything wrong with the slice, it was just the iterator.

@thestinger
Copy link
Contributor

Ah, I see what you mean, I didn't realize repr used the iterator.

@alexcrichton
Copy link
Member Author

repr was actually fixed previously (6d4d2c9)

@thestinger
Copy link
Contributor

@alexcrichton: I'm not really understanding why xs.slice(0, 2).to_owned() hits this bug in iterators

@alexcrichton
Copy link
Member Author

Oh good point, are you sure you're using the most recent version of the compiler? Turns out the iterators are just another issue.

@thestinger
Copy link
Contributor

@alexcrichton: I'm at f92b75a right now, should that be working?

@alexcrichton
Copy link
Member Author

It should...

[~/code/rust2] $ ./x86_64-apple-darwin/stage2/bin/rustc bar.rs                                          
warning: no debug symbols in executable (-arch x86_64)
[~/code/rust2] $ cat bar.rs                                                                             
struct Foo;
fn main() {
    let xs = ~[Foo, Foo, Foo];
    assert_eq!(fmt!("%?", xs.slice(0, 2).to_owned()), ~"~[{}, {}]");
[~/code/rust2] $ ./bar
[~/code/rust2] $

@thestinger
Copy link
Contributor

strcat@thinktank i ~/projects/rust doc % rustc -v
rustc 0.8-pre (f92b75a 2013-07-11 13:28:38 -0700)
host: x86_64-unknown-linux-gnu
strcat@thinktank i ~/projects/rust doc % rustc foo.rs
strcat@thinktank i ~/projects/rust doc % ./foo 
~[]
strcat@thinktank i ~/projects/rust doc % cat foo.rs 
fn main() {
    struct Foo;
    let xs = ~[Foo, Foo, Foo];
    println(fmt!("%?", xs.slice(0, 2).to_owned())) // prints `~[]`
}

With your test:

rust: task failed at 'left: ~"~[]" does not equal right: ~"~[{}, {}]"', foo.rs:4
rust: domain main @0x1ce31b0 root task failed

@alexcrichton
Copy link
Member Author

Huh, that's weird. My version is 02adf6c which is this commit + some extras. This should definitely be working on linux because there's a test at the end of repr.rs which exercises pretty much this exact functionality. Also, repr doesn't use iter() at all.

I'm building the current master right now, there's definitely something odd going on...

@thestinger
Copy link
Contributor

For anyone else looking at this, we narrowed it down on IRC to also being caused by iterators. It's confusing having two implementations of to_owned - one with iterators, and one without... hopefully the duplication is reduced more soon.

@alexcrichton
Copy link
Member Author

If 0-sized element dereferencing doesn't read memory, then we're safe. We already figured out that pointers to zero-sized structs are just kinda pointers to nothing, so I think that this is OK.

@thestinger
Copy link
Contributor

Landing with #7736.

@thestinger thestinger closed this Jul 12, 2013
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

Successfully merging this pull request may close these issues.

vectors don't handle zero-size elements correctly
2 participants