Rust: Panic in Firefox (rusqlite/lru_cache/linked_hash_map) built with recent Rust nightlies

[myfreeweb]

Firefox built with rustc 1.49.0-nightly (beb5ae474 2020-10-04) on FreeBSD/amd64 crashes very quickly with this:

* thread #1, name = 'BgIOThreadPool #2', stop reason = signal SIGSEGV
…
    frame #15: 0x000000080f38e698 libxul.so`core::panicking::panic::h68b0075d98bb8971 at panicking.rs:50:5
    frame #16: 0x000000080ea3f47c libxul.so`linked_hash_map::LinkedHashMap$LT$K$C$V$C$S$GT$::ensure_guard_node::hf152f3bcf37aa17d [inlined] core::mem::uninitialized::h942e45808eca740e at mod.rs:659:9
    frame #17: 0x000000080ea3f463 libxul.so`linked_hash_map::LinkedHashMap$LT$K$C$V$C$S$GT$::ensure_guard_node::hf152f3bcf37aa17d(self=<unavailable>) at lib.rs:174
    frame #18: 0x000000080ea3f59e libxul.so`linked_hash_map::LinkedHashMap$LT$K$C$V$C$S$GT$::insert::h1840c6a0a3863e66(self=0x000000083ab36750, k=Arc<str> @ 0x00007fffd6768f98, v=RawStatement @ 0x00007fffd6768ff0) at lib.rs:304:9
    frame #19: 0x000000080ea39585 libxul.so`lru_cache::LruCache$LT$K$C$V$C$S$GT$::insert::hb5c3cb3e465236d8(self=0x000000083ab36750, k=<unavailable>, v=<unavailable>) at lib.rs:119:23
    frame #20: 0x000000080ea417c2 libxul.so`rusqlite::cache::StatementCache::cache_stmt::h5b1d9082df40c120(self=<unavailable>, stmt=RawStatement @ 0x00007fffd6769140) at cache.rs:149:13
    frame #21: 0x000000080ea41403 libxul.so`_$LT$rusqlite..cache..CachedStatement$u20$as$u20$core..ops..drop..Drop$GT$::drop::h6046c68aae8bc005(self=<unavailable>) at cache.rs:87:13
    frame #22: 0x000000080d75025f libxul.so`core::ptr::drop_in_place::h4eb54bda8caf00cd((null)=0x00007fffd6769330) at mod.rs:175:1
    frame #23: 0x000000080d74fbff libxul.so`sql_support::conn_ext::ConnExt::execute_named_cached::h09f5e419dbd12fc4(self=<unavailable>, sql=<unavailable>, params=&[(&str, &ToSql)] @ 0x0000000a7d679590) at conn_ext.rs:78:5
    frame #24: 0x000000080d755d01 libxul.so`webext_storage::db::put_meta::h98717a8131394191(db=<unavailable>, key=(data_ptr = "", length = 14), value=<unavailable>) at db.rs:176:5
    frame #25: 0x000000080d7470d4 libxul.so`_$LT$webext_storage..sync..bridge..BridgedEngine$u20$as$u20$sync15_traits..bridged_engine..BridgedEngine$GT$::set_last_sync::h2e5c6e50ce8664c2(self=<unavailable>, last_sync_millis=0) at bridge.rs:50:9
    frame #26: 0x000000080d72c014 libxul.so`_$LT$webext_storage_bridge..store..LazyStore$u20$as$u20$sync15_traits..bridged_engine..BridgedEngine$GT$::set_last_sync::hf753dca2d35a20d4(self=<unavailable>, last_sync_millis=0) at store.rs:146:12
    frame #27: 0x000000080d729bef libxul.so`golden_gate::task::FerryTask$LT$N$GT$::inner_run::h45ad2dd4388f3741(self=<unavailable>) at task.rs:223:17
    frame #28: 0x000000080d72ae3b libxul.so`_$LT$golden_gate..task..FerryTask$LT$N$GT$$u20$as$u20$moz_task..Task$GT$::run::h543514311cc308d3(self=0x000000083a29b6a0) at task.rs:265:37
    frame #29: 0x000000080e9ea752 libxul.so`moz_task::TaskRunnable::run::hf300adf99dd3a70a(self=0x0000000833cde300) at lib.rs:219:17
    frame #30: 0x000000080e9eabb9 libxul.so`moz_task::TaskRunnable::Run::hbca50f12f2b5c3ee(self=<unavailable>) at method.rs:204:19
    frame #31: 0x000000080e9ea9bd libxul.so`moz_task::TaskRunnable::allocate::Run::hd242ffa8e19937f7(this=0x0000000833cde300) at lib.rs:163:10
    frame #32: 0x0000000808d8e397 libxul.so`mozilla::TaskQueue::Runner::Run(this=0x00000008394db180) at TaskQueue.cpp:158:20
    frame #33: 0x0000000808d9ae3e libxul.so`nsThreadPool::Run(this=<unavailable>) at nsThreadPool.cpp:299:14
…

I think the previous nightly before that had the issue too, but it's a recent issue, whatever I was using before the last couple upgrades (probably something from September) was fine. Switching to stable fixed the issue.

(doing a debug build now)

[myfreeweb]

The message is:

Hit MOZ_CRASH(attempted to leave type linked_hash_map::Node<std::sync::Arc<str>, raw_statement::RawStatement> uninitialized, which is invalid) at /home/greg/.rustup/toolchains/nightly-x86_64-unknown-freebsd/lib/rustlib/src/rust/library/core/src/mem/mod.rs:659

(a bit silly that non-debug builds of Firefox only output that message into the crash dump magic thing and not onto stderr..)

(lldb) frame sel 10
frame #10: 0x00000008119e613c libxul.so`linked_hash_map::LinkedHashMap$LT$K$C$V$C$S$GT$::ensure_guard_node::ha74213912ea83131 [inlined] core::mem::uninitialized::hfd5548125f921871 at mod.rs:659:9
   656  pub unsafe fn uninitialized<T>() -> T {
   657      // SAFETY: the caller must guarantee that an unitialized value is valid for `T`.
   658      unsafe {
-> 659          intrinsics::assert_uninit_valid::<T>();
   660          MaybeUninit::uninit().assume_init()
   661      }
   662  }
(lldb) frame sel 11
frame #11: 0x00000008119e6123 libxul.so`linked_hash_map::LinkedHashMap$LT$K$C$V$C$S$GT$::ensure_guard_node::ha74213912ea83131(self=<unavailable>) at lib.rs:174
   171          if self.head.is_null() {
   172              // allocate the guard node if not present
   173              unsafe {
-> 174                  self.head = Box::into_raw(Box::new(mem::uninitialized()));
   175                  (*self.head).next = self.head;
   176                  (*self.head).prev = self.head;
   177              }

Maybe the library is at fault here and new rustc just added more checks?

[Mark-Simulacrum]

Removing bisection/mcve -- this is a standard "code was UB" situation, caused by increased checking on mem::uninitialized with types that have invalid bitpatterns.

[myfreeweb]

Okay, I guess I should close the issue here if that's intentional? Though the fact that runtime checks are added to new compiler versions, not just compile-time checks, is a bit surprising.

Seems like the related issue is https://github.com/contain-rs/linked-hash-map/issues/97

[RalfJung]

This is definitely caused by https://github.com/rust-lang/rust/pull/71274.

@myfreeweb can you find out the exact version of linked-hash-map that was compiled for this? v0.5.3 (released back in May) should contain a fix for this problem.

[RalfJung]

Note in particular that the code you quoted is outdated:

   171          if self.head.is_null() {
   172              // allocate the guard node if not present
   173              unsafe {
-> 174                  self.head = Box::into_raw(Box::new(mem::uninitialized()));
   175                  (*self.head).next = self.head;
   176                  (*self.head).prev = self.head;
   177              }

In the latest version, this looks different.

This code was changed more than a year ago. So it looks like the version of linked-hash-map that you use is ancient.

Seems like the related issue is contain-rs/linked-hash-map#97

I don't think so, actually -- I think the relevant issue/PR is https://github.com/contain-rs/linked-hash-map/pull/100.

[scottmcm]

Though the fact that runtime checks are added to new compiler versions, not just compile-time checks, is a bit surprising.

It would be nice to just make this kind of thing a compilation error, but that's against the stability guarantee -- even if the call is known to be bad, the code might be written so that the UB cases are never executed, so we can't break it.

(There's arguably no runtime _check_ here; it's just the codegen level noticing UB and taking advantage of its freedom to emit whatever it wants there. And -- helpfully for debugging emitting a panic -- rather than the unreachable it could.)

[RalfJung]

We do actually have a lint to detect such problems so you can make it a compiler error in your crate with #[deny(invalid_values)]. However, a lint, being a static check, cannot be nearly as precise as this dynamic check that leads to a panic -- so the lint has many false negatives.

We cannot halt compilation in codegen when we emit the panic as this might be dead code -- even in correct unsafe code, there can be unreachable branches that would cause UB when reached. Making this a compilation error is akin to making any use of the unreachable_unchecked intrinsic a compilation error.

[myfreeweb]

looks like the version of linked-hash-map that you use is ancient

Well, mozilla-central has vendored 0.5.1, not me :) bugzilled