# Libs Specializations and Unsoundness for 1.53

- "iter::Fuse is unsound with how specialization currently behaves around HRTB fn pointers" [rust#85863](https://github.com/rust-lang/rust/issues/85863)
- mentioned [last week](https://github.com/rust-lang/rust/issues/85863#issuecomment-856958496)
- opened by @**Frank Steffahn**
- reporter suggests possible fixes, on Zulip [some more thoughts](https://rust-lang.zulipchat.com/#narrow/stream/227806-t-compiler.2Fwg-prioritization/topic/Issues.20.2385863.20and.20.2385873)
- @**Josh Triplett** comments this issue will be discussed [in a follow-up T-lang meeting](https://github.com/rust-lang/rust/issues/85863#issuecomment-856958496)
- "TrustedRandomAccess optimization for Zip containing vec::IntoIter is unsound with how specialization currently behaves around HRTB fn pointers" [rust#85873](https://github.com/rust-lang/rust/issues/85873)
- same as above: issue will be discussed [in a follow-up T-lang meeting](https://github.com/rust-lang/rust/issues/85863#issuecomment-856958496)

@**Frank Steffahn** [elaborated](https://zulip-archive.rust-lang.org/238009tcompilermeetings/52925weekly2021061054818.html#242225674):

> This is indeed the case. #85975 is fixing #85969 which is a (not necessarily easy to hit) a behavioral regression introduced in beta 1.53, but no unsoundness. #85874 is about a soundness issue (#85873) that’s already present in stable, but not as old as the issue involving Fuse. That one (#85863) is, IIRC, as old as FusedIterator itself.
>
>
> The only connection between #85874 and #85975 is that they both involve TrustedRandomAccess

However, applying a simplistic sound/unsound predicate may be non-trivial. @**the8472** has made statements [like](https://github.com/rust-lang/rust/issues/85863#issuecomment-852276407):

> That sounds to me like some safety loopholes around specializations on FusedIterator and Copy were not unexpected since they tend to include unsafe code.

A table summarizing state proposed by Felix (aka his notes to self)

Issue | Unsound | When Injected | When/if fixed
-------|-----------|---------------|---------------
[#82303] | !unsound | ? ancient, <= 1.21 | ? not fixed
[#85969] | !unsound | 1.53 | 1.54, PR [#85975]
[#85873] | unsound | array, vec_deque in 1.53, PR [#81607]<br><br>vec believed in 1.48, PR [#70793] | no fix merged; proposed in PR [#85874]
[#85863] | unsound | ancient; since Fuse intro'd? | no fix merged nor PR available

[#70793]: https://github.com/rust-lang/rust/pull/70793
[#81607]: https://github.com/rust-lang/rust/issues/81607
[#82303]: https://github.com/rust-lang/rust/issues/82303
[#85863]: https://github.com/rust-lang/rust/issues/85863
[#85874]: https://github.com/rust-lang/rust/pull/85874
[#85969]: https://github.com/rust-lang/rust/issues/85969
[#85873]: https://github.com/rust-lang/rust/issues/85873
[#85975]: https://github.com/rust-lang/rust/pull/85975

## Meeting Discussion Notes

### Definitions

* **unsound:** purely safe code can cause UB because of this bug
* **incorrect:** the code does not perform to specification; this could lead to UB if combined with unsafe code, or simply incorrect results with safe code

### #82303

* Affects whether `next` method is called on base iterators
* when implementing `TrustedRandomAccess`, they are able to "skip ahead" where they were not before
* thus side-effects from `map` calls may not be visible
* This is arguably **incorrect** but not **unsound**
* certainly a behavioral change
* Q: Is there specified behavior in the docs?
* [Zip has some docs](https://github.com/rust-lang/rust/issues/82303#issuecomment-852187787)
* But in the end, does this matter?
* Seems like docs do not address this question with 100% specificity

### Consensus

* Backport
* #85975
* #85874, the parts pertaining to `vec_deque` and `array`
* Do not backport
* the parts of #85874 that pertain to `Vec`
* hypothetical future PRs that pertain to `Fuse` for #85863
* Why?
* Backport immediate regressions that were introduced in 1.53
* 1.48 was released in Nov 2020 and that is "long enough ago" that folks may be relying on the perf optimization
* Q: How much perf are we talking about? -- we don't have that data
* Assumption: this is hard to weaponize or run into accidentally
* Assumption: backporting with a week until release is high risk itself, so we should be minimal

#### How likely are folks to hit UB in practice?

Conclusion: not very likely

Some comments:
* I see a lot of the `zip` +/- `skip_while` and people using e.g. `map` instead of `inspect` for side effects, and Vec is **the** "canonical" collection to transform iterators into or from.
* AFAICT you *need* to use HRTB function pointers or trait objects to run into *observable* UB and use subtype coercion on a `Fuse<some iterator conatining that HRTB type>` or `Zip<some iterator conatining that HRTB type>`.
* the bugs here are caused by invoking next some portion of the way under one impl and then "changing horses" to the other mid-stream. I think this is pretty unlikely to hit people in practice.

#### Risks of excessive backport close to release

* WesleyWiser: Related to that, I'm a bit wary in general of making quick decisions the week prior to a release. The last thing we want is to introduce some kind of accidental regression in the backport to resolve an issue which has been on stable for a while and then having to ship a .1 release to fix it.
* For issues introduced in 1.53, I think it definitely makes sense to prevent them from hitting stable but for issues already on stable, I think the bar is a lot higher and that increases the closer we get to release.