Skip to content

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

Closed
@valpackett

Description

@valpackett
Contributor

Firefox built with rustc 1.49.0-nightly (beb5ae4 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)

Activity

added
I-prioritizeIssue: Indicates that prioritization has been requested for this issue.
on Oct 5, 2020
added
E-needs-bisectionCall for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc
E-needs-mcveCall for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example
O-freebsdOperating system: FreeBSD
on Oct 5, 2020
valpackett

valpackett commented on Oct 5, 2020

@valpackett
ContributorAuthor

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?

removed
O-freebsdOperating system: FreeBSD
E-needs-bisectionCall for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc
E-needs-mcveCall for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example
on Oct 5, 2020
Mark-Simulacrum

Mark-Simulacrum commented on Oct 5, 2020

@Mark-Simulacrum
Member

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

valpackett

valpackett commented on Oct 5, 2020

@valpackett
ContributorAuthor

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 contain-rs/linked-hash-map#97

RalfJung

RalfJung commented on Oct 5, 2020

@RalfJung
Member

This is definitely caused by #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

RalfJung commented on Oct 5, 2020

@RalfJung
Member

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 contain-rs/linked-hash-map#100.

scottmcm

scottmcm commented on Oct 6, 2020

@scottmcm
Member

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

RalfJung commented on Oct 6, 2020

@RalfJung
Member

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.

valpackett

valpackett commented on Oct 7, 2020

@valpackett
ContributorAuthor

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

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

removed
I-prioritizeIssue: Indicates that prioritization has been requested for this issue.
on Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: This is a bug.regression-from-stable-to-nightlyPerformance or correctness regression from stable to nightly.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @valpackett@RalfJung@jonas-schievink@Mark-Simulacrum@scottmcm

        Issue actions

          Panic in Firefox (rusqlite/lru_cache/linked_hash_map) built with recent Rust nightlies · Issue #77585 · rust-lang/rust