bitvec contains unsound global state

[XAMPPRocky]

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]

did you manage to reproduce this in a minimal repro?

[XAMPPRocky]

Unfortunately I don't have the time to create minimal reproductions.

[Trolldemorted]

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, 144 expected [...], 36, 112 received).
• 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]

Thank you for investigating, we can close this and continue on the rasn repo.