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

Bitfield macro has problems with overflow in release mode #21

Open
jounathaen opened this issue Mar 25, 2020 · 2 comments
Open

Bitfield macro has problems with overflow in release mode #21

jounathaen opened this issue Mar 25, 2020 · 2 comments

Comments

@jounathaen
Copy link

Hi,

I cannot really say whether this is a bug, as I do not have a windows machine and only executed that very module, but I discovered that the bitfield128! macro has issues in --release mode.

To reproduce, run cargo test --release.
The error I get is

error: attempt to shift left with overflow
   --> src/xxxxxxxxx
    |
65  | /                     self.$field.High64 <<
66  | |                         (128 - $r.end) >> (64 - $r.end + $r.start)
    | |______________________________________^
...
183 | /         bitfield128!(TEST_STRUCT_U128 AsUINT128: u64[
184 | |             a set_a[0..32],
185 | |             b set_b[32..64],
186 | |             c set_c[64..96],
187 | |             d set_d[96..128],
188 | |         ]);
    | |___________- in this macro invocation
    |
    = note: `#[deny(exceeding_bitshifts)]` on by default

Might be only due to the fact that I execute only the macro and the test on my Linux machine, but I observe kind of similar behaviour in one of our Windows pipelines as well, since around Feb 22.

@petrutlucian94
Copy link
Collaborator

Thanks for reporting this issue. I think previous rustc versions treated this as a warning.

Such overflows would occur in case of incorrectly defined bitfields, using values outside the [0, 128] range or having r.end - r.start > 64. We should probably add an assertion checking those values and have rustc ignore this warning since it won't be able to tell that we're already avoiding overflows with that assertion.

@epakskape
Copy link
Contributor

I created a new proposed PR for this that does not rely on allow(const_err): #22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants