Skip to content

Fix sign extension in BitField.getValue for top-bit fields#1749

Merged
garydgregory merged 1 commit into
apache:masterfrom
alhudz:bitfield-getvalue-sign-extension
Jul 17, 2026
Merged

Fix sign extension in BitField.getValue for top-bit fields#1749
garydgregory merged 1 commit into
apache:masterfrom
alhudz:bitfield-getvalue-sign-extension

Conversation

@alhudz

@alhudz alhudz commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

BitField.getValue(int) and getValue(long) read the field with getRawValue and then shift it down with an arithmetic >>. When the field sits on the top bit of the holder (bit 31 for the int overload, bit 63 for the long overload added in 3.21.0) the masked value is negative, so >> sign-extends and the field reads back negative. new BitField(0x8000000000000000L).getValue(0x8000000000000000L) gives -1 for a one-bit field whose value is 1, and new BitField(0xFF000000).getValue(0xFF000000) gives -1 from the int overload while getValue(long) returns 255 for the same field.

Use the unsigned >>> in both getters so the selected bits come down as magnitude. The value is reconstructed at the shift, so fixing it there also covers getShortValue/getByteValue, which delegate through getValue, and it leaves the mask handling from #1711 untouched. Fields that do not reach the top bit are unchanged because >> and >>> agree on a non-negative value.

@garydgregory garydgregory changed the title fix sign extension in BitField.getValue for top-bit fields Fix sign extension in BitField.getValue for top-bit fields Jul 17, 2026
@garydgregory
garydgregory requested a review from Copilot July 17, 2026 14:05
@garydgregory

Copy link
Copy Markdown
Member

Good find @alhudz , will merge, do you see other issues in the code base wit bit shifts?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes incorrect sign extension when extracting bitfields that occupy the top bit(s) of an int (bit 31) or long (bit 63) in BitField.getValue(...), ensuring extracted values reflect the intended unsigned magnitude.

Changes:

  • Switch BitField.getValue(int) to use unsigned right shift (>>>) to avoid sign extension for top-bit fields.
  • Switch BitField.getValue(long) to use unsigned right shift (>>>) to avoid sign extension for top-bit fields.
  • Add targeted regression tests covering top-bit and top-range masks for both int and long holders.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/main/java/org/apache/commons/lang3/BitField.java Uses unsigned right shift in getValue overloads to prevent sign extension when the masked value is negative.
src/test/java/org/apache/commons/lang3/BitFieldTest.java Adds regression tests validating correct extraction for fields at bit 31 and bit 63 (and high-range masks).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +155 to +156
// The int and long overloads must agree for the same field and holder.
assertEquals(topByte.getValue(0xFF000000L), 255L);
@garydgregory
garydgregory merged commit 98dc249 into apache:master Jul 17, 2026
20 of 21 checks passed
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

Successfully merging this pull request may close these issues.

3 participants