[PATCH] D33588: Fix two sources of UB in __next_hash_pow2 (from __hash_table)

[Vedant Kumar via Phabricator via cfe-commits]

vsk updated this revision to Diff 100461.
vsk edited the summary of this revision.
vsk added a comment.

Thanks @EricWF for pointing me to the right place to add a test. I've tried to follow the style used by the existing tests. PTAL.


https://reviews.llvm.org/D33588

Files:
  include/__hash_table
  test/libcxx/containers/unord/next_pow2.pass.cpp


Index: test/libcxx/containers/unord/next_pow2.pass.cpp
===================================================================
--- /dev/null
+++ test/libcxx/containers/unord/next_pow2.pass.cpp
@@ -0,0 +1,80 @@
+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// REQUIRES: long_tests
+
+// Not a portable test
+
+// <__hash_table>
+
+// size_t __next_hash_pow2(size_t n);
+
+// If n <= 1, return n. If n is a power of 2, return n.
+// Otherwise, return the next power of 2.
+
+#include <__hash_table>
+#include <unordered_map>
+#include <cassert>
+
+#include <iostream>
+
+bool
+is_power_of_two(unsigned long n)
+{
+    return __builtin_popcount(n) == 1;
+}
+
+void
+test_next_pow2()
+{
+    assert(!is_power_of_two(0));
+    assert(is_power_of_two(1));
+    assert(is_power_of_two(2));
+    assert(!is_power_of_two(3));
+
+    assert(std::__next_hash_pow2(0) == 0);
+    assert(std::__next_hash_pow2(1) == 1);
+
+    for (std::size_t n = 2; n < (sizeof(std::size_t) * 8 - 1); ++n)
+    {
+        std::size_t pow2 = 1ULL << n;
+        assert(std::__next_hash_pow2(pow2) == pow2);
+    }
+
+    for (std::size_t n : {3, 7, 9, 15, 127, 129})
+    {
+        std::size_t npow2 = std::__next_hash_pow2(n);
+        assert(is_power_of_two(npow2) && npow2 > n);
+    }
+}
+
+// Note: this is only really useful when run with -fsanitize=undefined.
+void
+fuzz_unordered_map_reserve(unsigned num_inserts,
+                           unsigned num_reserve1,
+                           unsigned num_reserve2)
+{
+    std::unordered_map<uint64_t, unsigned long> m;
+    m.reserve(num_reserve1);
+    for (unsigned I = 0; I < num_inserts; ++I) m[I] = 0;
+    m.reserve(num_reserve2);
+    assert(m.bucket_count() >= num_reserve2);
+}
+
+int main()
+{
+    test_next_pow2();
+
+    for (unsigned num_inserts = 0; num_inserts <= 64; ++num_inserts)
+        for (unsigned num_reserve1 = 1; num_reserve1 <= 64; ++num_reserve1)
+            for (unsigned num_reserve2 = 1; num_reserve2 <= 64; ++num_reserve2)
+                fuzz_unordered_map_reserve(num_inserts, num_reserve1, num_reserve2);
+
+    return 0;
+}
Index: include/__hash_table
===================================================================
--- include/__hash_table
+++ include/__hash_table
@@ -136,7 +136,7 @@
 size_t
 __next_hash_pow2(size_t __n)
 {
-    return size_t(1) << (std::numeric_limits<size_t>::digits - __clz(__n-1));
+    return (__n > 1) ? (size_t(1) << (std::numeric_limits<size_t>::digits - __clz(__n-1))) : __n;
 }
 
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33588.100461.patch
Type: text/x-patch
Size: 2807 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170526/932bc69e/attachment-0001.bin>