Skip to content

flate change bit reader Bits to usize #24719

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 7, 2025
Merged

flate change bit reader Bits to usize #24719

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a4c008
flate change bit reader bits to usize
ianic Aug 6, 2025
00a977c
flate same result tests on both 32 and 64 bit platforms
ianic Aug 6, 2025
bccec76
flate: simplify peekBitsEnding
ianic Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 12 additions & 23 deletions lib/std/compress/flate/Decompress.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ state: State,

err: ?Error,

/// TODO: change this to usize
const Bits = u64;
const Bits = usize;

const BlockType = enum(u2) {
stored = 0,
Expand Down Expand Up @@ -550,29 +549,19 @@ fn peekBitsEnding(d: *Decompress, comptime U: type) !U {
var u: Bits = 0;
var remaining_needed_bits = @bitSizeOf(U) - remaining_bits;
var i: usize = 0;
while (remaining_needed_bits >= 8) {
const byte = try specialPeek(in, next_bits, i);
u |= @as(Bits, byte) << @intCast(i * 8);
remaining_needed_bits -= 8;
while (remaining_needed_bits > 0) {
const peeked = in.peek(i + 1) catch |err| switch (err) {
error.ReadFailed => return error.ReadFailed,
error.EndOfStream => break,
};
u |= @as(Bits, peeked[i]) << @intCast(i * 8);
remaining_needed_bits -|= 8;
i += 1;
}
if (remaining_needed_bits != 0) {
const byte = try specialPeek(in, next_bits, i);
u |= @as(Bits, byte) << @intCast((i * 8) + remaining_needed_bits);
}
if (remaining_bits == 0 and i == 0) return error.EndOfStream;
return @truncate((u << remaining_bits) | next_bits);
}

/// If there is any unconsumed data, handles EndOfStream by pretending there
/// are zeroes afterwards.
fn specialPeek(in: *Reader, next_bits: Bits, i: usize) Reader.Error!u8 {
const peeked = in.peek(i + 1) catch |err| switch (err) {
error.ReadFailed => return error.ReadFailed,
error.EndOfStream => if (next_bits == 0 and i == 0) return error.EndOfStream else return 0,
};
return peeked[i];
}

fn tossBits(d: *Decompress, n: u4) !void {
const remaining_bits = d.remaining_bits;
const next_bits = d.next_bits;
Expand Down Expand Up @@ -1032,7 +1021,7 @@ test "failing invalid-tree01" {
try testFailure(.raw, @embedFile("testdata/fuzz/invalid-tree01.input"), error.IncompleteHuffmanTree);
}
test "failing invalid-tree02" {
try testFailure(.raw, @embedFile("testdata/fuzz/invalid-tree02.input"), error.EndOfStream);
try testFailure(.raw, @embedFile("testdata/fuzz/invalid-tree02.input"), error.IncompleteHuffmanTree);
Copy link
Member

Choose a reason for hiding this comment

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

but it is end of stream, isn't it? why are we pretending there are zeroes after the stream?

Copy link

Choose a reason for hiding this comment

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

Pretending there are zeros after the stream happens to fix the two panics I found

#24695 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I pass this case through infgen:

$infgen invalid-tree02.input -dd -m
! infgen 3.5 output
!
last                    ! 1
dynamic                 ! 10
count 257 11 19         ! 1111 01010 00000
code 18 2               ! 010 000 000
code 0 2                ! 010
code 3 1                ! 001 000 000 000 000 000 000 000 000 000
zeros 128               ! 1110101 11 000 000 000 000 000
zeros 128               ! 1110101 11
lens 3                  ! 0
lens 0                  ! 01
lens 3                  ! 0
lens 0                  ! 01
lens 3                  ! 0
lens 0                  ! 01
lens 3                  ! 0
lens 3                  ! 0
lens 3                  ! 0
lens 3                  ! 0
lens 3                  ! 0
lens 3                  ! 0
! litlen 256 3
! dist 1 3
! dist 3 3
! dist 5 3
! dist 6 3
! dist 7 3
! dist 8 3
! dist 9 3
! dist 10 3
end                     ! 000
                        ! 00

It can be decoded but fails to generate tree.
But I still need to explain myself every bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm slowly recollecting my understanding...

peekBits is used in two places; peekBits(u7) and peekBits(u15). It means that algorithm will need at most that amount of bits. After finding symbol it will know exact number of bits.
If it asks for 7 bits when there are less then 7 bits available peekBits should return that available bits. If there are 0 available bits then EndOfStream should be raised.

ref

Copy link
Member

Choose a reason for hiding this comment

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

ok well I think this is better than status quo, but it is still probably wrong unit tests because if the stream ends abruptly, it should not be trying to construct a huffman tree, or doing a match.

}
test "failing invalid-tree03" {
try testFailure(.raw, @embedFile("testdata/fuzz/invalid-tree03.input"), error.IncompleteHuffmanTree);
Expand Down Expand Up @@ -1065,7 +1054,7 @@ test "failing puff10" {
try testFailure(.raw, @embedFile("testdata/fuzz/puff10.input"), error.InvalidCode);
}
test "failing puff11" {
try testFailure(.raw, @embedFile("testdata/fuzz/puff11.input"), error.EndOfStream);
try testFailure(.raw, @embedFile("testdata/fuzz/puff11.input"), error.InvalidMatch);
}
test "failing puff12" {
try testFailure(.raw, @embedFile("testdata/fuzz/puff12.input"), error.InvalidDynamicBlockHeader);
Expand Down Expand Up @@ -1137,7 +1126,7 @@ test "deflate-stream" {
}

test "empty-distance-alphabet01" {
try testFailure(.raw, @embedFile("testdata/fuzz/empty-distance-alphabet01.input"), error.EndOfStream);
try testDecompress(.raw, @embedFile("testdata/fuzz/empty-distance-alphabet01.input"), "");
}

test "empty-distance-alphabet02" {
Expand Down
Loading