Fix UB in folly/experimental/Bits.h
authorTom Jackson <tjackson@fb.com>
Thu, 1 Jun 2017 15:33:55 +0000 (08:33 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Thu, 1 Jun 2017 15:38:08 +0000 (08:38 -0700)
Summary:
Test output before the fix is applied:
```
folly/experimental/Bits.h:247:59: runtime error: signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'
     #0 folly/experimental/Bits.h:247 folly::Bits<...>::set(int*, unsigned long, unsigned long, int)
     #1 folly/experimental/test/BitsTest.cpp:228 std::enable_if<...>::type (anonymous namespace)::testSet<...>(unsigned char*, unsigned long, unsigned long, int)
     #2 folly/experimental/test/BitsTest.cpp:263 Bits_Boundaries_Test::TestBody()
    #17 folly/experimental/test/BitsTest.cpp:381 main
```

Reviewed By: philippv

Differential Revision: D5160789

fbshipit-source-id: 43f1926d58f1a5c019d4f8794d10a7a80a5c4749

folly/experimental/Bits.h
folly/experimental/test/BitsTest.cpp

index 191cb2d..03a18c5 100644 (file)
@@ -242,8 +242,7 @@ inline void Bits<T, Traits>::set(T* p, size_t bitStart, size_t count,
     size_t countInThisBlock = bitsPerBlock - offset;
     size_t countInNextBlock = count - countInThisBlock;
 
-    UnderlyingType thisBlock =
-        UnderlyingType(value & ((one << countInThisBlock) - 1));
+    UnderlyingType thisBlock = UnderlyingType(value & ones(countInThisBlock));
     UnderlyingType nextBlock = UnderlyingType(value >> countInThisBlock);
     if (std::is_signed<UnderlyingType>::value) {
       nextBlock &= ones(countInNextBlock);
index aeff9d3..0a1ec16 100644 (file)
@@ -255,6 +255,17 @@ T testValue(int bits) {
 }
 } // anonymous namespace
 
+TEST(Bits, Boundaries) {
+  uint8_t buf[20];
+  for (size_t offset = 0; offset <= 64; ++offset) {
+    for (size_t size = 0; size <= 32; ++size) {
+      int32_t value = testValue<int32_t>(size);
+      testSet<true>(buf, offset, size, value);
+      EXPECT_EQ(value, (testGet<true, int32_t>(buf, offset, size)));
+    }
+  }
+}
+
 template <size_t N>
 void accSize(size_t& w) {
   for (size_t s = 0; s <= N; ++s) {