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

Non-contiguous bit ranges #24

Closed
danlehmann opened this issue May 8, 2023 · 11 comments
Closed

Non-contiguous bit ranges #24

danlehmann opened this issue May 8, 2023 · 11 comments

Comments

@danlehmann
Copy link
Owner

Occasionally, bit ranges aren't contiguous. An example would be the x64 REX prefix, which provides the upper-most bit in a u4 to extend the u3 of a later byte.

Not very common, but we could support it

@danlehmann
Copy link
Owner Author

What syntax to use here? Rust itself doesn't have non-contiguous ranges, so we have to make up a syntax.

We could list ranges like this:

#[bits(36..=37, 39..40, rw)]

Or introduce a range combiner:

#[bits(36..=37 + 39..40, rw)]

Or have a more array-like syntax:

#[bits([36..=37, 39..40], rw)]

@vortexofdoom
Copy link
Contributor

I think of the listed 3 options, I like the feel of the 1st the most, but the 2nd and 3rd have the benefit of semantically separating the bit ranges from the rw modifier.

I'd also probably parse them as actual ranges instead of manually, because it would make it much easier to interweave single bits with bit ranges, so rather than 36..=37, 39..40 being the only way to represent it, 36..38, 39 or any combination of valid expressions that resulted in the same bits being referenced could also work. I thought about refactoring to that when I implemented spanned errors, but ended up deciding against it at the time.

@danlehmann
Copy link
Owner Author

Another feature where this is quite useful is RISC-V immediate values: They are a pain to take apart otherwise

@danlehmann
Copy link
Owner Author

Looking at the various options again, I think I prefer option 3 now. I think that's the option that I have seen other projects pick the most often when confronted with similar issues

@danlehmann
Copy link
Owner Author

Option 2 should probably be disqualified. The following pattern might be confusing:

1..=9 + 15

According to this proposal it would mean "1 to 9 (inclusive), followed by 15". In regular Rust however, this would be parsed as 1..=(9+15)

@danlehmann
Copy link
Owner Author

Implemented via option 3: #52

@mtoohey31
Copy link

Thanks for implementing this! Perfect timing too, cause I just realized I needed this today :) It looks like I might be running into a bug though. I've defined the following bitfield:

#[bitfield(u64, default = 0)]
struct SegmentDescriptor {
    #[bits([0..=15, 16..=19], rw)]
    limit: u20,
    #[bits([16..=31, 32..=39, 56..=63], rw)]
    base: u32,
    #[bits(40..=43, rw)]
    r#type: u4,
    #[bit(44, rw)]
    system: bool,
}

When I try to compile though, I get the following errors:

error[E0308]: mismatched types
 --> src/global_descriptor_table.rs:7:1
  |
7 | #[bitfield(u64, default = 0)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u64`, found `u32`
  |
  = note: this error originates in the attribute macro `bitfield` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0308]: mismatched types
  --> src/global_descriptor_table.rs:7:1
   |
7  | #[bitfield(u64, default = 0)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u32`, found `u64`
...
12 |     base: u32,
   |           --- expected `u32` because of return type
   |
   = note: this error originates in the attribute macro `bitfield` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: no implementation for `u64 | u32`
 --> src/global_descriptor_table.rs:7:1
  |
7 | #[bitfield(u64, default = 0)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `u64 | u32`
  |
  = help: the trait `BitOr<u32>` is not implemented for `u64`
  = help: the following other types implement trait `BitOr<Rhs>`:
            <u64 as BitOr>
            <u64 as BitOr<NonZeroU64>>
            <u64 as BitOr<&u64>>
            <&'a u64 as BitOr<u64>>
            <&u64 as BitOr<&u64>>
  = note: this error originates in the attribute macro `bitfield` (in Nightly builds, run with -Z macro-backtrace for more info)

Here's a more minimal example:

#[bitfield(u64)]
struct Foo {
    #[bits([0..=15, 16..=31], rw)]
    bar: u32,
}

So far, it seems the issue occurs when using the non-contiguous syntax for a u32 field inside a u33 or larger bitfield.

Does this look like a bug to you, or am I maybe missing something in how I'm trying to use this?

@danlehmann
Copy link
Owner Author

Thanks for the report. I can reproduce this. Working on a fix.

Unrelated to the issue, but #[bits([0..=15, 16..=19], rw)] actually ends up being contiguous again. The first item is the one that ends up at the bottom of the u20. You probably want them swapped? [16..=19, 0..=15]

danlehmann added a commit that referenced this issue Feb 7, 2024
This was reported in issue #24. The fix itself is two characters - added
a ton of test coverage to go through all combinations of regular and
arbitrary ints.
@danlehmann
Copy link
Owner Author

@mtoohey31 The fix was pretty straightforward - missing braces in the generated code: #53

I'll merge this asap and publish a version.

danlehmann added a commit that referenced this issue Feb 7, 2024
…#53)

This was reported in issue #24. The fix itself is two characters - added
a ton of test coverage to go through all combinations of regular and
arbitrary ints.
@danlehmann
Copy link
Owner Author

Fix merged and published. Please give 1.3.1 a shot

@mtoohey31
Copy link

Thanks for the super quick fix @danlehmann! I can confirm that the issue is fixed for me!

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