From 73adbb319cacea04ac52e2f51be72260dd4c0f87 Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Sat, 15 May 2021 00:12:50 -0600 Subject: [PATCH] AK: Don't read past the end in BitmapView::count_in_range() The current code is factored such that reads to the entirety of the last byte should be dropped. This was relying on the fact that last would be one past the end in that case. Instead of actually reading that byte when it's completely out of bounds of the bitmask, just skip reads that would be invalid. Add more tests to make sure that the behavior is correct for byte aligned reads of byte aligned bitmaps. --- AK/BitmapView.h | 11 +++++++---- Tests/AK/TestBitmap.cpp | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/AK/BitmapView.h b/AK/BitmapView.h index 4e012030a7..a2ae0342a7 100644 --- a/AK/BitmapView.h +++ b/AK/BitmapView.h @@ -52,7 +52,7 @@ public: return 0; static const u8 bitmask_first_byte[8] = { 0xFF, 0xFE, 0xFC, 0xF8, 0xF0, 0xE0, 0xC0, 0x80 }; - static const u8 bitmask_last_byte[8] = { 0x0, 0x1, 0x3, 0x7, 0xF, 0x1F, 0x3F, 0x7F }; + static const u8 bitmask_last_byte[8] = { 0x00, 0x1, 0x3, 0x7, 0xF, 0x1F, 0x3F, 0x7F }; size_t count; const u8* first = &m_data[start / 8]; @@ -64,9 +64,12 @@ public: count = __builtin_popcount(byte); } else { count = __builtin_popcount(byte); - byte = *last; - byte &= bitmask_last_byte[(start + len) % 8]; - count += __builtin_popcount(byte); + // Don't access *last if it's out of bounds + if (last < &m_data[size_in_bytes()]) { + byte = *last; + byte &= bitmask_last_byte[(start + len) % 8]; + count += __builtin_popcount(byte); + } if (++first < last) { const u32* ptr32 = (const u32*)(((FlatPtr)first + sizeof(u32) - 1) & ~(sizeof(u32) - 1)); if ((const u8*)ptr32 > last) diff --git a/Tests/AK/TestBitmap.cpp b/Tests/AK/TestBitmap.cpp index b4326dcd54..464e81c0c8 100644 --- a/Tests/AK/TestBitmap.cpp +++ b/Tests/AK/TestBitmap.cpp @@ -248,3 +248,31 @@ TEST_CASE(count_in_range) test_with_value(true); test_with_value(false); } + +TEST_CASE(byte_aligned_access) +{ + { + Bitmap bitmap(16, true); + EXPECT_EQ(bitmap.count_in_range(0, 16, true), 16u); + EXPECT_EQ(bitmap.count_in_range(8, 8, true), 8u); + EXPECT_EQ(bitmap.count_in_range(0, 8, true), 8u); + EXPECT_EQ(bitmap.count_in_range(4, 8, true), 8u); + } + { + Bitmap bitmap(16, false); + bitmap.set_range(4, 8, true); + EXPECT_EQ(bitmap.count_in_range(0, 16, true), 8u); + EXPECT_EQ(bitmap.count_in_range(8, 8, true), 4u); + EXPECT_EQ(bitmap.count_in_range(0, 8, true), 4u); + EXPECT_EQ(bitmap.count_in_range(4, 8, true), 8u); + } + { + Bitmap bitmap(8, false); + bitmap.set(2, true); + bitmap.set(4, true); + EXPECT_EQ(bitmap.count_in_range(0, 2, true), 0u); + EXPECT_EQ(bitmap.count_in_range(0, 4, true), 1u); + EXPECT_EQ(bitmap.count_in_range(0, 8, true), 2u); + EXPECT_EQ(bitmap.count_in_range(4, 4, true), 1u); + } +}