Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crossbeam-epoch internal::local_size unit test fails on 32-bit architectures #597

Closed
decathorpe opened this issue Nov 12, 2020 · 1 comment

Comments

@decathorpe
Copy link

failures:
---- internal::local_size stdout ----
thread 'internal::local_size' panicked at 'assertion failed: `(left == right)`
  left: `2040`,
 right: `1020`', src/internal.rs:379:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
    internal::local_size

The same error occurs on both i686 and armv7hl, on fedora rawhide, with stable Rust 1.47.0. The test is fine on all 64-bit architectures that are available to me (x86_64, aarch64, ppc64le, and s390x).

Looking at the source code for the failing test
https://github.com/crossbeam-rs/crossbeam/blob/master/crossbeam-epoch/src/internal.rs#L375

it seems the intent is to check if Local is equal or smaller than 2040 bytes (at least that's what the comment says), but the code in the test actually checks for equality: assert_eq!(2040, core::mem::size_of::<Local>());, not a "less than or equal" test.

Should this be changed to assert!(core::mem::size_of::<Local>() <= 2040); or something like that? I think this value is half the expected value because pointers are half as big on 32-bit architectures ... Or would it make sense to actually compare using a usize value (the architecture dependent pointer size)?

PS: Since this looks like an issue in the test and not a code issue, I'll ignore the failure for now, so fixing this is not urgent :)

@taiki-e
Copy link
Member

taiki-e commented Nov 12, 2020

Ideally, we need to test i686 linux-gnu on CI by using cross. (We are not currently testing platforms other than x86_64 linux&windows.)
However, some of the channel tests (deadline/timeout-related) are pretty fragile. (After all, that's why #578 and #591 can't be merged yet. That's also why macOS hasn't been tested in CI.)

bors bot added a commit that referenced this issue Nov 22, 2020
599: Relaxed size constrait of Local (closes #597) r=taiki-e a=jeehoonkang

@decathorpe Thank you for reporting this.  It's also kinda hotfix.  As @taiki-e said, we really need CI for i686 and other 32-bit architectures... (#598).

r? @taiki-e 

Co-authored-by: Jeehoon Kang <jeehoon.kang@kaist.ac.kr>
bors bot added a commit that referenced this issue Nov 22, 2020
599: Relaxed size constrait of Local (closes #597) r=taiki-e a=jeehoonkang

@decathorpe Thank you for reporting this.  It's also kinda hotfix.  As @taiki-e said, we really need CI for i686 and other 32-bit architectures... (#598).

r? @taiki-e 

Co-authored-by: Jeehoon Kang <jeehoon.kang@kaist.ac.kr>
@bors bors bot closed this as completed in 9512d9c Nov 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants