Fix zero-sized reference to deallocated memory #91797
Conversation
rust-highfive
added
the
S-waiting-on-review
|
Running |
|
Sorry, I don't have enough familiar with this code to review :) r? rust-lang/libs |
camelid
added
the
T-libs
|
@the8472 I'm going to r+ this as I think it should address the immediate problem, but I suspect a more thorough patch which fixes the Splice Drop(?) impl to not create such a Drain would be better. In general I think we should be able to expect that the iterator is pointing to a valid range inside Drain -- the fact that's not the case to me suggests that Splice is not properly resetting Drain after modifying the backing vector (e.g., be re-initing the slice here to be freshly created, not from deallocated memory). With this patch it's probably fine, but seems easy to get wrong in the future, so likely good to fix at the root in some sense. |
|
@bors r+ p=1 -- fixing unsoundness |
|
📌 Commit 9063b64 has been approved by |
bors
added
S-waiting-on-bors
Well, at least it's documented now that |
| let drop_ptr = iter.as_slice().as_ptr(); | ||
|
|
||
| // forget iter so there's no aliasing reference | ||
| drop(iter); |
There was a problem hiding this comment.
Does iter have an actual impl Drop that used to be run here?
There was a problem hiding this comment.
No, it wasn't actually necessary. I thought slice::Iter internally holds a slice reference that would be invalidated by mutating the vec through other pointers. But it actually contains pointers.
There was a problem hiding this comment.
Also, at least under Stacked Borrows, droping a reference doesn't help with anything -- it only marks the reference as active the moment it gets dropped.
…askrgr Rollup of 8 pull requests Successful merges: - rust-lang#90081 (Make `intrinsics::write_bytes` const) - rust-lang#91643 (asm: Allow using r9 (ARM) and x18 (AArch64) if they are not reserved by the current target) - rust-lang#91737 (Make certain panicky stdlib functions behave better under panic_immediate_abort) - rust-lang#91750 (rustdoc: Add regression test for Iterator as notable trait on &T) - rust-lang#91764 (Do not ICE when suggesting elided lifetimes on non-existent spans.) - rust-lang#91780 (Remove hir::Node::hir_id.) - rust-lang#91797 (Fix zero-sized reference to deallocated memory) - rust-lang#91806 (Make `Unique`s methods `const`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
fixes #91772
r? @camelid