From: Tudor Bosman <tudorb@fb.com>
Date: Thu, 13 Dec 2012 20:16:32 +0000 (-0800)
Subject: Fix bug in Bits<T>::get / set
X-Git-Tag: v0.22.0~1109
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=06eaa4f3ae4e385b8e85ae76db9ffaa8103edad0;p=folly.git

Fix bug in Bits<T>::get / set

Summary:
Everything worked except for getting properly aligned full blocks
because 1ULL << 64 is invalid (the shift amount must be strictly less than
the value size in bits)

Test Plan: test added

Reviewed By: philipp@fb.com

FB internal diff: D657800
---

diff --git a/folly/experimental/Bits.h b/folly/experimental/Bits.h
index 214bd66a..fd97ddd7 100644
--- a/folly/experimental/Bits.h
+++ b/folly/experimental/Bits.h
@@ -156,7 +156,12 @@ struct Bits {
   // (bitStart < sizeof(T) * 8, bitStart + count <= sizeof(T) * 8)
   static UnderlyingType innerGet(const T* p, size_t bitStart, size_t count);
 
+  static constexpr UnderlyingType zero = UnderlyingType(0);
   static constexpr UnderlyingType one = UnderlyingType(1);
+
+  static constexpr UnderlyingType ones(size_t count) {
+    return count < bitsPerBlock ? (one << count) - 1 : ~zero;
+  }
 };
 
 template <class T, class Traits>
@@ -180,8 +185,6 @@ template <class T, class Traits>
 inline void Bits<T, Traits>::set(T* p, size_t bitStart, size_t count,
                                  UnderlyingType value) {
   assert(count <= sizeof(UnderlyingType) * 8);
-  assert(count == sizeof(UnderlyingType) ||
-         (value & ~((one << count) - 1)) == 0);
   size_t idx = blockIndex(bitStart);
   size_t offset = bitOffset(bitStart);
   if (offset + count <= bitsPerBlock) {
@@ -217,7 +220,7 @@ inline void Bits<T, Traits>::innerSet(T* p, size_t offset, size_t count,
                                       UnderlyingType value) {
   // Mask out bits and set new value
   UnderlyingType v = Traits::loadRMW(*p);
-  v &= ~(((one << count) - 1) << offset);
+  v &= ~(ones(count) << offset);
   v |= (value << offset);
   Traits::store(*p, v);
 }
@@ -225,7 +228,7 @@ inline void Bits<T, Traits>::innerSet(T* p, size_t offset, size_t count,
 template <class T, class Traits>
 inline auto Bits<T, Traits>::innerGet(const T* p, size_t offset, size_t count)
   -> UnderlyingType {
-  return (Traits::load(*p) >> offset) & ((one << count) - 1);
+  return (Traits::load(*p) >> offset) & ones(count);
 }
 
 template <class T, class Traits>
diff --git a/folly/experimental/test/BitsTest.cpp b/folly/experimental/test/BitsTest.cpp
index 83dc8d06..d0151031 100644
--- a/folly/experimental/test/BitsTest.cpp
+++ b/folly/experimental/test/BitsTest.cpp
@@ -148,6 +148,7 @@ void runMultiBitTest64() {
   auto load = detail::BitsTraits<T>::load;
   T buf[] = {0x123456789abcdef0, 0x13579bdf2468ace0};
 
+  EXPECT_EQ(0x123456789abcdef0, load(Bits<T>::get(buf, 0, 64)));
   EXPECT_EQ(0xf0, load(Bits<T>::get(buf, 0, 8)));
   EXPECT_EQ(0x89abcdef, load(Bits<T>::get(buf, 4, 32)));
   EXPECT_EQ(0x189abcdef, load(Bits<T>::get(buf, 4, 33)));
@@ -156,6 +157,9 @@ void runMultiBitTest64() {
   EXPECT_EQ(0xd5555555, load(Bits<T>::get(buf, 4, 32)));
   EXPECT_EQ(0x1d5555555, load(Bits<T>::get(buf, 4, 33)));
   EXPECT_EQ(0xd55555550, load(Bits<T>::get(buf, 0, 36)));
+
+  Bits<T>::set(buf, 0, 64, 0x23456789abcdef01);
+  EXPECT_EQ(0x23456789abcdef01, load(Bits<T>::get(buf, 0, 64)));
 }
 
 TEST(Bits, MultiBit64) {