[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] xen/bitops: Fix break with in a for_each_set_bit() loop



for_each_set_bit()'s use of a double for loop had an accidental bug with a
break in the inner loop leading to an infinite outer loop.

Adjust for_each_set_bit() to avoid this behaviour, and add extend
test_for_each_set_bit() with a test case for this.

Fixes: ed26376f20bf ("xen/bitops: Introduce for_each_set_bit()")
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien@xxxxxxx>

Both GCC and Clang seem happy with this, even at -O1:

  https://godbolt.org/z/o6ohjrzsY
---
 xen/common/bitops.c      | 16 ++++++++++++++++
 xen/include/xen/bitops.h |  2 +-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/xen/common/bitops.c b/xen/common/bitops.c
index 91ae961440af..0edd62d25c28 100644
--- a/xen/common/bitops.c
+++ b/xen/common/bitops.c
@@ -110,6 +110,22 @@ static void __init test_for_each_set_bit(void)
 
     if ( ull != ull_res )
         panic("for_each_set_bit(uint64) expected %#"PRIx64", got 
%#"PRIx64"\n", ull, ull_res);
+
+    /* Check that we break from the middle of the loop */
+    ui = HIDE(0x80001008U);
+    ui_res = 0;
+    for_each_set_bit ( i, ui )
+    {
+        static __initdata unsigned int count;
+
+        if ( count++ > 1 )
+            break;
+
+        ui_res |= 1U << i;
+    }
+
+    if ( ui_res != 0x1008 )
+        panic("for_each_set_bit(break) expected 0x1008, got %#x\n", ui_res);
 }
 
 static void __init test_multiple_bits_set(void)
diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
index 79615fb89d04..448b2d3e0937 100644
--- a/xen/include/xen/bitops.h
+++ b/xen/include/xen/bitops.h
@@ -299,7 +299,7 @@ static always_inline attr_const unsigned int fls64(uint64_t 
x)
  * A copy of @val is taken internally.
  */
 #define for_each_set_bit(iter, val)                     \
-    for ( typeof(val) __v = (val); __v; )               \
+    for ( typeof(val) __v = (val); __v; __v = 0 )       \
         for ( unsigned int (iter);                      \
               __v && ((iter) = ffs_g(__v) - 1, true);   \
               __v &= __v - 1 )

base-commit: e0058760a0c7935ad0690d8b9babb9050eceedf0
-- 
2.39.5




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.