rhaiscript / rhai

Rhai - An embedded scripting language for Rust.
https://crates.io/crates/rhai
Apache License 2.0
3.63k stars 174 forks source link

check_struct_sizes and test_string_index test fails on 32 bit architectures #851

Open alexanderkjall opened 4 months ago

alexanderkjall commented 4 months ago

Running tests on an i386 version of Debian yields:

running 1 test
test tests::check_struct_sizes ... FAILED

failures:

---- tests::check_struct_sizes stdout ----
thread 'tests::check_struct_sizes' panicked at 'assertion failed: `(left == right)`
  left: `12`,
 right: `16`', src/tests.rs:18:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    tests::check_struct_sizes

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

And:

failures:

---- test_string_index stdout ----
thread 'test_string_index' panicked at 'attempt to add with overflow', src/eval/chaining.rs:418:53
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    test_string_index
schungx commented 4 months ago

Thanks for catching these... I don't get to test on 32-bit machines a lot so it would be difficult for me to catch these things.

schungx commented 4 months ago

For check_struct_sizes would you mind commenting out the offending line and then try again? Try to catch as many errors as possible and I'll fix them all at once.

Also, can you test with the current drop in this repo? There are changes to some of the source files, making line numbers not useful in identifying the source of the error.

alexanderkjall commented 4 months ago

sure :)

---- tests::check_struct_sizes stdout ----
thread 'tests::check_struct_sizes' panicked at 'assertion failed: `(left == right)`
  left: `12`,
 right: `16`', src/tests.rs:19:5

 ---- tests::check_struct_sizes stdout ----
thread 'tests::check_struct_sizes' panicked at 'assertion failed: `(left == right)`
  left: `12`,
 right: `8`', src/tests.rs:24:5
schungx commented 3 months ago

Please see if the new release resolves these...

alexanderkjall commented 3 months ago

I bumped the versions and reran the tests in Debian (on x86-64) and got this result:

---- tests::check_struct_sizes stdout ----
thread 'tests::check_struct_sizes' panicked at 'assertion failed: `(left == right)`
  left: `56`,
 right: `48`', src/tests.rs:80:5
stack backtrace:
   0: rust_begin_unwind
             at /usr/src/rustc-1.70.0/library/std/src/panicking.rs:578:5
   1: core::panicking::panic_fmt
             at /usr/src/rustc-1.70.0/library/core/src/panicking.rs:67:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /usr/src/rustc-1.70.0/library/core/src/panicking.rs:228:5
   4: rhai::tests::check_struct_sizes
             at ./src/tests.rs:80:5
   5: rhai::tests::check_struct_sizes::{{closure}}
             at ./src/tests.rs:6:25
   6: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.70.0/library/core/src/ops/function.rs:250:5
   7: core::ops::function::FnOnce::call_once
             at /usr/src/rustc-1.70.0/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

I guess (and haven't verified yet) that this is because that the versions of dependencies in Debian isn't the latest ones published on crates.io.

What do these unit test failues represent? Is there a risk that this causes security problems?

Edit: Note that it's not trivially reproducible by running the test-suite from git

schungx commented 3 months ago

I won't be too concerned. I have seen this before. As the Rust compiler evolves, they don't guarantee any particular binary layout of struct object or their sizes. Sometimes a compiler update (beta or even stable) will generate a particular struct layout that is sized differently (most of the time smaller). Sometimes it changes then change back in the next beta release etc.

Back on 29/12/2013, I have this code:

assert_eq!(
        size_of::<LexError>(),
        if cfg!(feature = "unstable") { 48 } else { 56 }
    )

So it was checking for an unstable compiler version that shrunk LexError down to 48 bytes. The check we then removed later on when the change stayed.

So I believe you may be using an earlier Rust compiler version?