Outdated version of linked-hash-map crate in mozilla-central has UB that triggers assertions when built by nightly Rust
Categories
(Toolkit :: Storage, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox83 | --- | fixed |
People
(Reporter: val, Assigned: markh)
References
()
Details
Attachments
(1 file)
When built with nightly Rust, Firefox panics, seems like when doing Sync stuff (golden_gate/webext_storage/rusqlite/lru_cache/linked_hash_map):
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
See https://github.com/rust-lang/rust/issues/77585 — the undefined behavior that's now detected by nightly Rust has been fixed in the newest release of linked-hash-map, so Firefox should update.
Also in the next rusqlite version this dependency will change to a different, better maintained crate https://github.com/rusqlite/rusqlite/pull/811
Specifically, the linked-hash-map issues were fixed with https://github.com/contain-rs/linked-hash-map/commit/da9e3fb458d1a0f383b033ea41bdb466e3d96805 and https://github.com/contain-rs/linked-hash-map/pull/100, both of which are contained in v0.5.3 which was released back in May.
Assignee | ||
Comment 2•4 years ago
|
||
I'm going to put up a patch to vendor a newer application-services, which includes rusqlite 0.24.1 - which as mentioned above, no longer depends on linked-hash-map. However, that's still going to leave linked-hash-map 0.5.1 in the tree as it's used by serde_yaml
and yaml-rust
, via geckodriver
which appears to be test-only. So this crash should go away, but it isn't being banished as much as would be ideal.
What about updating the vendored linked-hash.map to 0.5.3? That's a semver-compatible update.
Assignee | ||
Comment 4•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 5•4 years ago
|
||
(In reply to Ralf Jung from comment #3)
What about updating the vendored linked-hash.map to 0.5.3? That's a semver-compatible update.
As above, that would involve an update to the geckodriver
package which I'm not the correct person to do.
Pushed by mhammond@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/41d48f6233e2 vendor a new app-services, which includes ruslite 0.24.1. r=eoger
Comment 7•4 years ago
|
||
Hello Mark !
Based on Comment 2, can this issue be set to NEW? Thank you !
Assignee | ||
Updated•4 years ago
|
Comment 8•4 years ago
|
||
bugherder |
That does not seem to fix the bug though, if old linked-hash-map is still in the crate tree?
Assignee | ||
Comment 10•4 years ago
|
||
(In reply to Ralf Jung from comment #9)
That does not seem to fix the bug though, if old linked-hash-map is still in the crate tree?
That's going to depend on how that crate is now used, right? Comment 0 says "Firefox panics, seems like when doing Sync stuff" and I believe that will no longer be true. Best I can tell, "geckodriver" is used for testing/automation.
Comment 11•4 years ago
|
||
That's going to depend on how that crate is now used, right?
Well, yes, I guess. Using it with any type that has a "niche" (i.e., that has some forbidden bit-patterns) will cause a panic. To me it seems easier to update the crate than to figure out if there is any such user (never mind the fact that this update pulls in a UB fix, i.e., a fix for a rather severe bug).
Assignee | ||
Comment 12•4 years ago
|
||
(In reply to Ralf Jung from comment #11)
To me it seems easier to update the crate
Great! Please file a bug for updating geckodriver and I'll help find a reviewer for your patch.
Comment 13•4 years ago
|
||
I'm afraid I have no familiarity with the workflow here, I just assumed that it would be about as easy as cargo update
for someone that knows how all this vendoring stuff works. If not, I am a bit worried about Firefox using outdated Rust crates -- but in the end that's obviously up to y'all. I think my time is more effectively spent on the Rust compiler and language itself.
Description
•