commit 30244a8d81b82151ff7462c1554ba9401a70c935
author shaoboyan <shaoboyan@microsoft.com> Mon Jun 02 19:25:45 2025 -0700
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Jun 02 19:25:45 2025 -0700
tree 174583ec0ae04e098249a6d8d4c9941999f2df73
parent 1f561315a2504989f74d78dd07bb92f0face20e4

Fix advance() bug in BitSetRangeIterator

This CL fixes a bug in the `advance()` method of the
`BitSetRangeIterator` class. The bug caused the iterator
failed to handle fake bit set range introduced by the algorithm.

Tests don't catch this bug because they all happens to have offset
larger than bit set range size. Adding a new one to track this.

Bug:366291600
Change-Id: If21559d0c2997dcd30c2b07d50fa61a7f1bdd149
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/244095
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Shaobo Yan <shaoboyan@microsoft.com>
Reviewed-by: Geoff Lang <geofflang@chromium.org>

src/dawn/common/BitSetRangeIterator.h

diff --git a/src/dawn/common/BitSetRangeIterator.h b/src/dawn/common/BitSetRangeIterator.h
index 4a83a7d..85773c2 100644
--- a/src/dawn/common/BitSetRangeIterator.h
+++ b/src/dawn/common/BitSetRangeIterator.h
@@ -152,8 +152,10 @@
 
     mSize = rangeCount;
     mBits = ~mBits;
-    mBits <<= mOffset;
-    mBits >>= mOffset;
+
+    // Clear the bits for the current range.
+    mBits <<= rangeCount;
+    mBits >>= rangeCount;
 }
 
 // Helper to avoid needing to specify the template parameter size

src/dawn/tests/unittests/BitSetRangeIteratorTests.cpp

diff --git a/src/dawn/tests/unittests/BitSetRangeIteratorTests.cpp b/src/dawn/tests/unittests/BitSetRangeIteratorTests.cpp
index b64cf68..1798b47 100644
--- a/src/dawn/tests/unittests/BitSetRangeIteratorTests.cpp
+++ b/src/dawn/tests/unittests/BitSetRangeIteratorTests.cpp
@@ -38,15 +38,16 @@
 class BitSetRangeIteratorTest : public testing::Test {
   protected:
     template <size_t N>
-    void RunSingleBitRangesTests() {
+    void RunSingleBitSetRangeTests(uint32_t offset, uint32_t size) {
         std::bitset<N> stateBits;
-        uint32_t bitSize = stateBits.size();
 
         std::vector<std::pair<uint32_t, size_t>> expectedRanges;
-        expectedRanges.push_back({bitSize / 4u * 3u, 1});
+        expectedRanges.push_back({offset, size});
 
         for (const auto& range : expectedRanges) {
-            stateBits.set(range.first);
+            for (uint32_t i = 0; i < range.second; ++i) {
+                stateBits.set(range.first + i);
+            }
         }
 
         std::vector<std::pair<uint32_t, size_t>> foundRanges;
@@ -62,6 +63,11 @@
     }
 
     template <size_t N>
+    void RunSingleBitTests() {
+        RunSingleBitSetRangeTests<N>(N / 4u * 3u, 1);
+    }
+
+    template <size_t N>
     void RunConsecutiveBitRangesTests() {
         std::bitset<N> stateBits;
         uint32_t bitSize = stateBits.size();
@@ -139,20 +145,20 @@
 };
 
 // Test basic range iteration with single bits (each range has size 1)
-TEST_F(BitSetRangeIteratorTest, SingleBitRanges) {
+TEST_F(BitSetRangeIteratorTest, SingleBit) {
     // Smaller than 1 word
     {
-        RunSingleBitRangesTests<kBitsPerWord - 1>();
+        RunSingleBitTests<kBitsPerWord - 1>();
     }
 
     // Equal to 1 word
     {
-        RunSingleBitRangesTests<kBitsPerWord>();
+        RunSingleBitTests<kBitsPerWord>();
     }
 
     // Larger than 1 word
     {
-        RunSingleBitRangesTests<kBitsPerWord * 2 - 1>();
+        RunSingleBitTests<kBitsPerWord * 2 - 1>();
     }
 }
 
@@ -217,5 +223,23 @@
     EXPECT_EQ(4u, foundRanges[0].second);
 }
 
+// Test ranges that start from first bit.
+TEST_F(BitSetRangeIteratorTest, SingleBitSetRange) {
+    // Smaller than 1 word
+    {
+        RunSingleBitSetRangeTests<kBitsPerWord - 1>(0, kBitsPerWord - 1);
+    }
+
+    // Equal to 1 word
+    {
+        RunSingleBitSetRangeTests<kBitsPerWord>(0, kBitsPerWord - 1);
+    }
+
+    // Larger than 1 word
+    {
+        RunSingleBitSetRangeTests<kBitsPerWord * 2>(kBitsPerWord / 2, kBitsPerWord);
+    }
+}
+
 }  // anonymous namespace
 }  // namespace dawn