Unsound Debug impl for collections::linked_list::IterMut

[steffahn]

use std::{collections::LinkedList, fmt, sync::Barrier, thread};

struct BadDebugBehavior(Vec<&'static i32>, Barrier, Barrier);
impl fmt::Debug for BadDebugBehavior {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        let elem = &self.0[0];
        self.1.wait();
        self.2.wait();
        write!(f, "{}", elem)
    }
}
fn main() {
    let l = Box::leak(Box::new(LinkedList::new()));
    l.push_back(BadDebugBehavior(vec![&0], Barrier::new(2), Barrier::new(2)));
    let mut i = l.iter_mut();
    let r = i.next().unwrap();
    let t = thread::spawn(move || {
        r.1.wait();
        r.0.push(&0);
        r.2.wait();
    });
    dbg!(&i);

    t.join().unwrap();
}

   Compiling playground v0.0.1 (/playground)
    Finished dev [unoptimized + debuginfo] target(s) in 1.67s
     Running `target/debug/playground`
[src/main.rs:22] &i = IterMut(
    [
timeout: the monitored command dumped core
/playground/tools/entrypoint.sh: line 11:     9 Segmentation fault      timeout --signal=KILL ${timeout} "$@"

(playground)

@rustbot label T-libs-impl, A-collections

---

Caused by violation of safety invariant as documented in IterMut

/// A mutable iterator over the elements of a `LinkedList`.
///
/// This `struct` is created by [`LinkedList::iter_mut()`]. See its
/// documentation for more.
#[stable(feature = "rust1", since = "1.0.0")]
pub struct IterMut<'a, T: 'a> {
    // We do *not* exclusively own the entire list here, references to node's `element`
    // have been handed out by the iterator! So be careful when using this; the methods
    // called must be aware that there can be aliasing pointers to `element`.
    list: &'a mut LinkedList<T>,
    head: Option<NonNull<Node<T>>>,
    tail: Option<NonNull<Node<T>>>,
    len: usize,
}

yet Debug implementation does

#[stable(feature = "collection_debug", since = "1.17.0")]
impl<T: fmt::Debug> fmt::Debug for IterMut<'_, T> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.debug_tuple("IterMut").field(&self.list).field(&self.len).finish()
    }
}

https://doc.rust-lang.org/nightly/src/alloc/collections/linked_list.rs.html#79

I’m not sure where the list field is really used at all, so maybe it can be replaced by a marker marker: PhantomData<&'a mut Node<T>>, similar to linked_list::Iter’s implementation.

[hameerabbasi]

Assigning P-critical according to the WG-prioritization discussion on Zulip and removing I-prioritize.

@rustbot modify labels -I-prioritize +P-critical

[RalfJung]

Good catch! Iterator debug implementations seem to be woefully under-audited in terms of soundness problems (this is not the first such issue).

[steffahn]

@RalfJung maybe you can help me out with how to run miri while hacking std (a question that came up for the PR fixing this: #85814 (comment)). I have no trouble compiling or testing a modified std with x.py, or linking a stage1 toolchain for rustc usage (so that cargo check or cargo build are working) so that I can play around with it in a crate, but I don’t know how to run miri on such a local crate depending-on/using a modified std.

[RalfJung]

Miri uses xargo for sysroot compilation, so setting XARGO_RUST_SRC will tell Miri which standard library to use. (It must be set to the library folder in a rustc checkout.) You can then use a standard Miri, no rustup toolchain linking is required.

[RalfJung]

Oh, also, if you want to run the alloc/core test suites in Miri, see https://github.com/RalfJung/miri-test-libstd.