Miri reports undefined behaviour triggered by test suite

[ssokolow]

For any crate that doesn't use #[forbid(unsafe_code)], the very first thing I do before considering it for use it to git clone --depth=1 it and run cargo +nightly miri test on it... this crate didn't pass.

% git clone --depth=1 https://github.com/shepmaster/sxd-document.git
Cloning into 'sxd-document'...
remote: Enumerating objects: 24, done.
remote: Counting objects: 100% (24/24), done.
remote: Compressing objects: 100% (20/20), done.
remote: Total 24 (delta 1), reused 6 (delta 0), pack-reused 0
Unpacking objects: 100% (24/24), done.
Checking connectivity... done.
% cd sxd-document
% cargo +nightly miri test
    Updating crates.io index
  Downloaded peresil v0.3.0
  Downloaded typed-arena v1.7.0
  Downloaded 2 crates (23.5 KB) in 0.63s
   Compiling typed-arena v1.7.0
   Compiling peresil v0.3.0
   Compiling sxd-document v0.3.2 (/home/ssokolow/src/sxd-document)
    Finished test [unoptimized + debuginfo] target(s) in 20.41s
     Running target/x86_64-unknown-linux-gnu/debug/deps/sxd_document-2ca61747ef9df327

running 194 tests
test dom::test::attributes_belong_to_a_document ... ok
test dom::test::attributes_can_be_iterated ... ok
test dom::test::attributes_can_be_removed ... ok
test dom::test::attributes_can_be_removed_from_parent ... ok
test dom::test::attributes_can_be_reset ... ok
test dom::test::attributes_know_their_element ... ok
test dom::test::can_return_a_populated_package ... error: Undefined Behavior: trying to reborrow for SharedReadWrite at alloc260646, but parent tag <695071> does not have an appropriate item in the borrow stack
    --> src/raw.rs:521:9
     |
521  |         parent_r.children.push(child);
     |         ^^^^^^^^^^^^^^^^^ trying to reborrow for SharedReadWrite at alloc260646, but parent tag <695071> does not have an appropriate item in the borrow stack
     |
     = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
     = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
             
     = note: inside `raw::Connections::append_root_child::<raw::ChildOfRoot>` at src/raw.rs:521:9
note: inside `dom::Root::append_child::<dom::Element>` at src/dom.rs:174:9
    --> src/dom.rs:174:9
     |
174  |         self.document.connections.append_root_child(child.as_raw());
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `dom::test::can_return_a_populated_package::populate` at src/dom.rs:1608:17
    --> src/dom.rs:1608:17
     |
1608 |                 doc.root().append_child(element);
     |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `dom::test::can_return_a_populated_package` at src/dom.rs:1614:23
    --> src/dom.rs:1614:23
     |
1614 |         let package = populate();
     |                       ^^^^^^^^^^
note: inside closure at src/dom.rs:1601:5
    --> src/dom.rs:1601:5
     |
1601 | /     fn can_return_a_populated_package() {
1602 | |         fn populate() -> Package {
1603 | |             let package = Package::new();
1604 | |             {
...    |
1617 | |         assert_qname_eq!(element.name(), "hello");
1618 | |     }
     | |_____^
     = note: inside `<[closure@src/dom.rs:1601:5: 1618:6] as std::ops::FnOnce<()>>::call_once - shim` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
     = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
     = note: inside `test::__rust_begin_short_backtrace::<fn()>` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:516:5
     = note: inside closure at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:507:30
     = note: inside `<[closure@test::run_test::{closure#2}] as std::ops::FnOnce<()>>::call_once - shim(vtable)` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
     = note: inside `<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send> as std::ops::FnOnce<()>>::call_once` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1328:9
     = note: inside `<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>> as std::ops::FnOnce<()>>::call_once` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:322:9
     = note: inside `std::panicking::r#try::do_call::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>, ()>` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:379:40
     = note: inside `std::panicking::r#try::<(), std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>>` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:343:19
     = note: inside `std::panic::catch_unwind::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>, ()>` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:396:14
     = note: inside `test::run_test_in_process` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:538:18
     = note: inside closure at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:449:39
     = note: inside `test::run_test::run_test_inner` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:474:13
     = note: inside `test::run_test` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:504:28
     = note: inside `test::run_tests::<[closure@test::run_tests_console::{closure#2}]>` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:283:13
     = note: inside `test::run_tests_console` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/console.rs:289:5
     = note: inside `test::test_main` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:121:15
     = note: inside `test::test_main_static` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:140:5
     = note: inside `main`
     = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
     = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
     = note: inside closure at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:66:18
     = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
     = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:379:40
     = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:343:19
     = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:396:14
     = note: inside `std::rt::lang_start_internal` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:51:25
     = note: inside `std::rt::lang_start::<()>` at /home/ssokolow/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:65:5
     = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

error: test failed, to rerun pass '--lib'

Miri isn't exhaustive, but I consider it to be the bare minimum that the tests for a crate which uses unsafe or has it in its transitive dependencies must pass and, once you've fixed this, I'd strongly recommend adding it to your CI runs, similar to how I'd want a C or C++ codebase to run their tests under LLVM's various sanitizers (i.e. ASan, UBSan, etc.)