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
Increment self.index before calling `Iterator::self.a.__iterator_ge…
#81741
Increment self.index before calling `Iterator::self.a.__iterator_ge…
#81741
Conversation
…t_unchecked` in `Zip` `TrustedRandomAccess` specialization Otherwise if `Iterator::self.a.__iterator_get_unchecked` panics the index would not have been incremented yet and another call to `Iterator::next` would read from the same index again, which is not allowed according to the API contract of `TrustedRandomAccess` for `!Clone`. Fixes rust-lang#81740
|
r? @KodrAus (rust-highfive has picked a reviewer for you, use r? to override) |
|
@bors r+ |
|
|
…pecialization-panic-safety, r=KodrAus Increment `self.index` before calling `Iterator::self.a.__iterator_ge… …`t_unchecked` in `Zip` `TrustedRandomAccess` specialization Otherwise if `Iterator::self.a.__iterator_get_unchecked` panics the index would not have been incremented yet and another call to `Iterator::next` would read from the same index again, which is not allowed according to the API contract of `TrustedRandomAccess` for `!Clone`. Fixes rust-lang#81740
…as-schievink Rollup of 16 pull requests Successful merges: - rust-lang#79983 (fix indefinite article in cell.rs) - rust-lang#81479 (Allow casting mut array ref to mut ptr) - rust-lang#81741 (Increment `self.index` before calling `Iterator::self.a.__iterator_ge…) - rust-lang#81850 (use RWlock when accessing os::env) - rust-lang#81947 (Relax ItemCtxt::to_ty lifetime) - rust-lang#81954 (RELEASES.md 1.50: Group platform support notes together) - rust-lang#81955 (bootstrap: Locate llvm-dwp based on llvm-config bindir) - rust-lang#81959 (Fix assosiated typo) - rust-lang#81964 (Fix documentation not showing on localStorage error) - rust-lang#81968 (bootstrap: fix wrong docs installation path) - rust-lang#81975 (Seal the CommandExt, OsStrExt and OsStringExt traits) - rust-lang#81990 (Make suggestion of changing mutability of arguments broader) - rust-lang#81993 (Update Clippy) - rust-lang#81994 (Improve long explanation for E0542 and E0546) - rust-lang#81995 (Fix suggestion to introduce explicit lifetime) - rust-lang#81997 (dist: include src/build_helper as part of the crate graph for rustc-dev) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…pecialization-panic-safety, r=KodrAus Increment `self.index` before calling `Iterator::self.a.__iterator_ge… …`t_unchecked` in `Zip` `TrustedRandomAccess` specialization Otherwise if `Iterator::self.a.__iterator_get_unchecked` panics the index would not have been incremented yet and another call to `Iterator::next` would read from the same index again, which is not allowed according to the API contract of `TrustedRandomAccess` for `!Clone`. Fixes rust-lang#81740
Rollup of 12 pull requests Successful merges: - rust-lang#79983 (fix indefinite article in cell.rs) - rust-lang#81506 (HWAddressSanitizer support) - rust-lang#81741 (Increment `self.index` before calling `Iterator::self.a.__iterator_ge…) - rust-lang#81850 (use RWlock when accessing os::env) - rust-lang#81911 (GAT/const_generics: Allow with_opt_const_param to return GAT param def_id) - rust-lang#81947 (Relax ItemCtxt::to_ty lifetime) - rust-lang#81954 (RELEASES.md 1.50: Group platform support notes together) - rust-lang#81955 (bootstrap: Locate llvm-dwp based on llvm-config bindir) - rust-lang#81959 (Fix assosiated typo) - rust-lang#81964 (Fix documentation not showing on localStorage error) - rust-lang#81968 (bootstrap: fix wrong docs installation path) - rust-lang#81975 (Seal the CommandExt, OsStrExt and OsStringExt traits) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…pecialization-panic-safety, r=KodrAus Increment `self.index` before calling `Iterator::self.a.__iterator_ge… …`t_unchecked` in `Zip` `TrustedRandomAccess` specialization Otherwise if `Iterator::self.a.__iterator_get_unchecked` panics the index would not have been incremented yet and another call to `Iterator::next` would read from the same index again, which is not allowed according to the API contract of `TrustedRandomAccess` for `!Clone`. Fixes rust-lang#81740
Rollup of 11 pull requests Successful merges: - rust-lang#79775 (Fix injected errors when running doctests on a crate named after a keyword) - rust-lang#81479 (Allow casting mut array ref to mut ptr) - rust-lang#81506 (HWAddressSanitizer support) - rust-lang#81666 (Don't release Miri if its tests only failed on Windows) - rust-lang#81734 (Ensures `make` tests run under /bin/dash (if available), like CI, and fixes a Makefile) - rust-lang#81741 (Increment `self.index` before calling `Iterator::self.a.__iterator_ge…) - rust-lang#81850 (use RWlock when accessing os::env) - rust-lang#81911 (GAT/const_generics: Allow with_opt_const_param to return GAT param def_id) - rust-lang#82022 (Push a `char` instead of a `str` with len one into a String) - rust-lang#82023 (Remove unnecessary lint allow attrs on example) - rust-lang#82030 (Use `Iterator::all` instead of open-coding it) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 10 pull requests Successful merges: - rust-lang#79775 (Fix injected errors when running doctests on a crate named after a keyword) - rust-lang#81012 (Stabilize the partition_point feature) - rust-lang#81479 (Allow casting mut array ref to mut ptr) - rust-lang#81506 (HWAddressSanitizer support) - rust-lang#81741 (Increment `self.index` before calling `Iterator::self.a.__iterator_ge…) - rust-lang#81850 (use RWlock when accessing os::env) - rust-lang#81911 (GAT/const_generics: Allow with_opt_const_param to return GAT param def_id) - rust-lang#82022 (Push a `char` instead of a `str` with len one into a String) - rust-lang#82023 (Remove unnecessary lint allow attrs on example) - rust-lang#82030 (Use `Iterator::all` instead of open-coding it) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 10 pull requests Successful merges: - rust-lang#79775 (Fix injected errors when running doctests on a crate named after a keyword) - rust-lang#81012 (Stabilize the partition_point feature) - rust-lang#81479 (Allow casting mut array ref to mut ptr) - rust-lang#81506 (HWAddressSanitizer support) - rust-lang#81741 (Increment `self.index` before calling `Iterator::self.a.__iterator_ge…) - rust-lang#81850 (use RWlock when accessing os::env) - rust-lang#81911 (GAT/const_generics: Allow with_opt_const_param to return GAT param def_id) - rust-lang#82022 (Push a `char` instead of a `str` with len one into a String) - rust-lang#82023 (Remove unnecessary lint allow attrs on example) - rust-lang#82030 (Use `Iterator::all` instead of open-coding it) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
|
Is it worth (or even possible) to add a testcase for this? I run the libcore test site in Miri every day, so it makes sense to have a test that "used to trigger UB" and thereby ensure that it does not do that any more. Currently the test suite passes -- testing an iterator properly is hard, and I guess the test suite simply does not hit the code paths that cause UB. |
|
@RalfJung That would make sense but should probably wait until the other recent issues with the I'll try to not forget and do that once those are fixed. |
|
I was specifically thinking of #82289 |
…
t_uncheckedinZipTrustedRandomAccessspecializationOtherwise if
Iterator::self.a.__iterator_get_uncheckedpanics theindex would not have been incremented yet and another call to
Iterator::nextwould read from the same index again, which is notallowed according to the API contract of
TrustedRandomAccessfor!Clone.Fixes #81740