`bitvec` contains unsound global state
Closed this issue · 4 comments
XAMPPRocky commented
Similar to #228, I've noticed that I've been getting different results from bitvec depending on whether other code has run or not. For example if you clone librasn/rasn@5820db2 and run the following invocation it will pass, where as if you run the invocation below it, ax_uper will fail with a different encoding result. There's zero mutable static state in my crate and the only part that is getting corrupted is the encoding of the strings, which leads me to believe that bitvec is behaving incredibly odd.
cargo test --test personnel ax_uper
cargo test --test personnel
Trolldemorted commented
did you manage to reproduce this in a minimal repro?
XAMPPRocky commented
Unfortunately I don't have the time to create minimal reproductions.
Trolldemorted commented
I have checked out librasn/rasn@5820db2, fixed the Cargo.toml (hopefully with no side effects):
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -45,7 +45,7 @@ bitvec = { version = "1.0.1", default-features = false, features = ["alloc"] }
rasn-derive = { version = "0.6.0", path = "macros", optional = true }
chrono = { version = "0.4.19", default-features = false, features = ["alloc"] }
konst = { version = "0.2.13", default-features = false }
-nom-bitvec = { package = "bitvec-nom", version = "0.2.0", git = "https://github.com/XAMPPRocky/nom-bitvec.git" }
+nom-bitvec = { package = "bitvec-nom2", version = "0.2.0", git = "https://github.com/XAMPPRocky/nom-bitvec.git" }
arrayvec = { version = "0.7.2", default-features = false }
either = { version = "1.8.0", default-features = false }
once_cell = { version = "1.16.0", default-features = false, features = ["race", "alloc"] }and was able to reproduce the failing tests. Then I threw out tests until I was down to two candidates where one would fail if both were active: ax_uper and unconstrained_aper.
To avoid missunderstandings with your test macro, I also rolled out the content:
#[test]
fn test() {
unconstrained_aper();
ax_uper();
}
fn unconstrained_aper() {
println!("unconstrained_aper");
const EXPECTED: &[u8] = &[
0x82, 0x4A, 0xDF, 0xA3, 0x70, 0x0D, 0x00, 0x5A, 0x7B, 0x74, 0xF4, 0xD0,
0x02, 0x66, 0x11, 0x13, 0x4F, 0x2C, 0xB8, 0xFA, 0x6F, 0xE4, 0x10, 0xC5,
0xCB, 0x76, 0x2C, 0x1C, 0xB1, 0x6E, 0x09, 0x37, 0x0F, 0x2F, 0x20, 0x35,
0x01, 0x69, 0xED, 0xD3, 0xD3, 0x40, 0x10, 0x2D, 0x2C, 0x3B, 0x38, 0x68,
0x01, 0xA8, 0x0B, 0x4F, 0x6E, 0x9E, 0x9A, 0x02, 0x18, 0xB9, 0x6A, 0xDD,
0x8B, 0x16, 0x2C, 0x41, 0x69, 0xF5, 0xE7, 0x87, 0x70, 0x0C, 0x20, 0x59,
0x5B, 0xF7, 0x65, 0xE6, 0x10, 0xC5, 0xCB, 0x57, 0x2C, 0x1B, 0xB1, 0x6E,
];
let default: PersonnelRecord = PersonnelRecord::default();
assert_eq!(
EXPECTED,
rasn::uper::encode(&default).unwrap()
);
}
fn ax_uper() {
println!("ax_uper");
const EXPECTED: &[u8] = &[0x9e, 0x00, 0x06, 0x00, 0x04, 0x0a, 0x46, 0x90];
let default: Ax = Ax::default();
assert_eq!(
EXPECTED,
rasn::uper::encode(&default).unwrap()
);
}Now this is where things get interesting:
- if you execute as given above, we get a fail in
ax_uper([....], 74, 144expected[...], 36, 112received). - if you do both tests individually (i.e. comment out one of the two calls in
test()), they pass. - if we swap them, we get a panic:
thread 'test' panicked at src\types\strings\constrained.rs:39:34:
no entry found for key
stack backtrace:
0: std::panicking::begin_panic_handler
at /rustc/1a06ac5b5d7c9331e8de1aa1fd7e9d3533034b44/library\std\src\panicking.rs:645
1: core::panicking::panic_fmt
at /rustc/1a06ac5b5d7c9331e8de1aa1fd7e9d3533034b44/library\core\src\panicking.rs:72
2: core::panicking::panic_display
at /rustc/1a06ac5b5d7c9331e8de1aa1fd7e9d3533034b44/library\core\src\panicking.rs:178
3: core::panicking::panic_str
at /rustc/1a06ac5b5d7c9331e8de1aa1fd7e9d3533034b44/library\core\src\panicking.rs:152
4: core::option::expect_failed
at /rustc/1a06ac5b5d7c9331e8de1aa1fd7e9d3533034b44/library\core\src\option.rs:1985
5: enum2$<core::option::Option<ref$<u32> > >::expect
at /rustc/1a06ac5b5d7c9331e8de1aa1fd7e9d3533034b44\library\core\src\option.rs:894
6: alloc::collections::btree::map::impl$88::index<u32,u32,u32,alloc::alloc::Global>
at /rustc/1a06ac5b5d7c9331e8de1aa1fd7e9d3533034b44\library\alloc\src\collections\btree\map.rs:2328
7: rasn::types::strings::constrained::StaticPermittedAlphabet::to_index_string<rasn::types::strings::visible::VisibleString>
at .\src\types\strings\constrained.rs:39
8: rasn::per::enc::Encoder::encode_known_multipler_string<rasn::types::strings::visible::VisibleString>
at .\src\per\enc.rs:176
9: rasn::per::enc::impl$2::encode_visible_string
at .\src\per\enc.rs:644
10: rasn::types::strings::visible::impl$4::encode_with_tag_and_constraints<rasn::per::enc::Encoder>
at .\src\types\strings\visible.rs:60
11: rasn::enc::Encode::encode<rasn::types::strings::visible::VisibleString,rasn::per::enc::Encoder>
at .\src\enc.rs:18
12: personnel::impl$35::encode_with_tag_and_constraints::closure$0<rasn::per::enc::Encoder>
at .\tests\personnel.rs:64
13: rasn::per::enc::impl$2::encode_sequence<personnel::Name,personnel::impl$35::encode_with_tag_and_constraints::closure_env$0<rasn::per::enc::Encoder> >
at .\src\per\enc.rs:830
14: personnel::impl$35::encode_with_tag_and_constraints<rasn::per::enc::Encoder>
at .\tests\personnel.rs:64
15: rasn::enc::Encode::encode<personnel::Name,rasn::per::enc::Encoder>
at .\src\enc.rs:18
16: personnel::impl$21::encode_with_tag_and_constraints::closure$0<rasn::per::enc::Encoder>
at .\tests\personnel.rs:4
17: rasn::per::enc::impl$2::encode_set<personnel::PersonnelRecord,personnel::impl$21::encode_with_tag_and_constraints::closure_env$0<rasn::per::enc::Encoder> >
at .\src\per\enc.rs:845
18: personnel::impl$21::encode_with_tag_and_constraints<rasn::per::enc::Encoder>
at .\tests\personnel.rs:4
19: rasn::enc::Encode::encode<personnel::PersonnelRecord,rasn::per::enc::Encoder>
at .\src\enc.rs:18
20: rasn::per::encode<personnel::PersonnelRecord>
at .\src\per.rs:32
21: rasn::uper::encode<personnel::PersonnelRecord>
at .\src\uper.rs:14
22: personnel::unconstrained_aper
at .\tests\personnel.rs:403
23: personnel::test
at .\tests\personnel.rs:385
24: personnel::test::closure$0
at .\tests\personnel.rs:383
25: core::ops::function::FnOnce::call_once<personnel::test::closure_env$0,tuple$<> >
at /rustc/1a06ac5b5d7c9331e8de1aa1fd7e9d3533034b44\library\core\src\ops\function.rs:250
26: core::ops::function::FnOnce::call_once
at /rustc/1a06ac5b5d7c9331e8de1aa1fd7e9d3533034b44/library\core\src\ops\function.rs:250
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
test test ... FAILED
I had a brief look at your code and your index_map function looks broken:
fn index_map() -> &'static alloc::collections::BTreeMap<u32, u32> {
static MAP: OnceBox<BTreeMap<u32, u32>> = OnceBox::new();
MAP.get_or_init(|| {
Box::new(
Self::CHARACTER_SET
.into_iter()
.copied()
.enumerate()
.map(|(i, e)| (e, i as u32))
.collect(),
)
})
}While Self::CHARACTER_SET correctly resolves to the type's constant, static MAP: OnceBox<BTreeMap<u32, u32>> = OnceBox::new(); will be the same OnceBox for all implementing types. unconstrained_aper and ax_uper apparently use different types (NumericString vs VisibleString), so
- if you execute only one of the tests, everything works
- if you execute the one with the bigger charset first, you get wrong results
- if you execute the one with the smaller charset first, things go boom because you are accessing the btreemap out of bounds
XAMPPRocky commented
Thank you for investigating, we can close this and continue on the rasn repo.