changeset 52507:e5534cc91a10

8211926: Catastrophic size_t underflow in BitMap::*_large methods Reviewed-by: kbarrett, stuefe
author shade
date Sat, 10 Nov 2018 20:47:28 +0100
parents 18bd95c0e463
children 6f5948597697
files src/hotspot/share/utilities/bitMap.cpp src/hotspot/share/utilities/bitMap.hpp src/hotspot/share/utilities/bitMap.inline.hpp test/hotspot/gtest/utilities/test_bitMap_large.cpp
diffstat 4 files changed, 143 insertions(+), 5 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/utilities/bitMap.cpp	Sat Nov 10 19:27:52 2018 +0100
+++ b/src/hotspot/share/utilities/bitMap.cpp	Sat Nov 10 20:47:28 2018 +0100
@@ -263,14 +263,24 @@
   }
 }
 
+bool BitMap::is_small_range_of_words(idx_t beg_full_word, idx_t end_full_word) {
+  // There is little point to call large version on small ranges.
+  // Need to check carefully, keeping potential idx_t underflow in mind.
+  // The threshold should be at least one word.
+  STATIC_ASSERT(small_range_words >= 1);
+  return (beg_full_word + small_range_words >= end_full_word);
+}
+
 void BitMap::set_large_range(idx_t beg, idx_t end) {
   verify_range(beg, end);
 
   idx_t beg_full_word = word_index_round_up(beg);
   idx_t end_full_word = word_index(end);
 
-  assert(end_full_word - beg_full_word >= 32,
-         "the range must include at least 32 bytes");
+  if (is_small_range_of_words(beg_full_word, end_full_word)) {
+    set_range(beg, end);
+    return;
+  }
 
   // The range includes at least one full word.
   set_range_within_word(beg, bit_index(beg_full_word));
@@ -284,7 +294,7 @@
   idx_t beg_full_word = word_index_round_up(beg);
   idx_t end_full_word = word_index(end);
 
-  if (end_full_word - beg_full_word < 32) {
+  if (is_small_range_of_words(beg_full_word, end_full_word)) {
     clear_range(beg, end);
     return;
   }
@@ -368,8 +378,10 @@
   idx_t beg_full_word = word_index_round_up(beg);
   idx_t end_full_word = word_index(end);
 
-  assert(end_full_word - beg_full_word >= 32,
-         "the range must include at least 32 bytes");
+  if (is_small_range_of_words(beg_full_word, end_full_word)) {
+    par_at_put_range(beg, end, value);
+    return;
+  }
 
   // The range includes at least one full word.
   par_put_range_within_word(beg, bit_index(beg_full_word), value);
--- a/src/hotspot/share/utilities/bitMap.hpp	Sat Nov 10 19:27:52 2018 +0100
+++ b/src/hotspot/share/utilities/bitMap.hpp	Sat Nov 10 20:47:28 2018 +0100
@@ -72,6 +72,10 @@
   static const bm_word_t find_ones_flip = 0;
   static const bm_word_t find_zeros_flip = ~(bm_word_t)0;
 
+  // Threshold for performing small range operation, even when large range
+  // operation was requested. Measured in words.
+  static const size_t small_range_words = 32;
+
  protected:
   // Return the position of bit within the word that contains it (e.g., if
   // bitmap words are 32 bits, return a number 0 <= n <= 31).
@@ -117,6 +121,8 @@
 
   static void clear_range_of_words(bm_word_t* map, idx_t beg, idx_t end);
 
+  static bool is_small_range_of_words(idx_t beg_full_word, idx_t end_full_word);
+
   // The index of the first full word in a range.
   idx_t word_index_round_up(idx_t bit) const;
 
--- a/src/hotspot/share/utilities/bitMap.inline.hpp	Sat Nov 10 19:27:52 2018 +0100
+++ b/src/hotspot/share/utilities/bitMap.inline.hpp	Sat Nov 10 20:47:28 2018 +0100
@@ -237,10 +237,12 @@
 }
 
 inline void BitMap::set_large_range_of_words(idx_t beg, idx_t end) {
+  assert(beg <= end, "underflow");
   memset(_map + beg, ~(unsigned char)0, (end - beg) * sizeof(bm_word_t));
 }
 
 inline void BitMap::clear_large_range_of_words(idx_t beg, idx_t end) {
+  assert(beg <= end, "underflow");
   memset(_map + beg, 0, (end - beg) * sizeof(bm_word_t));
 }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/gtest/utilities/test_bitMap_large.cpp	Sat Nov 10 20:47:28 2018 +0100
@@ -0,0 +1,118 @@
+/*
+ * Copyright (c) 2018, Red Hat Inc. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include "precompiled.hpp"
+#include "utilities/bitMap.inline.hpp"
+#include "unittest.hpp"
+
+// Bitmap size should be large enough to accept large operations.
+static const BitMap::idx_t BITMAP_SIZE = 8192;
+
+// The test would like to fuzz indexes in this window. Having the fuzz
+// window at bitmap word size makes sure the test would touch every combination
+// of indexes (un)aligned on word boundary.
+static const BitMap::idx_t FUZZ_WINDOW = sizeof(BitMap::bm_word_t) * 8;
+
+static void verify_set(CHeapBitMap& map, BitMap::idx_t l, BitMap::idx_t r) {
+  for (BitMap::idx_t c = l; c < r; c++) {
+    EXPECT_TRUE(map.at(c));
+  }
+}
+
+static void verify_unset(CHeapBitMap& map, BitMap::idx_t l, BitMap::idx_t r) {
+  for (BitMap::idx_t c = l; c < r; c++) {
+    EXPECT_FALSE(map.at(c));
+  }
+}
+
+TEST(BitMap, clear_large_range) {
+  CHeapBitMap map(BITMAP_SIZE);
+
+  map.set_range(0, BITMAP_SIZE);
+  verify_set(map, 0, BITMAP_SIZE);
+
+  for (size_t size_class = 0; size_class <= BITMAP_SIZE; size_class = MAX2<size_t>(1, size_class*2)) {
+    for (BitMap::idx_t l = 0; l < FUZZ_WINDOW; l++) {
+      for (BitMap::idx_t tr = l; tr < FUZZ_WINDOW; tr++) {
+        BitMap::idx_t r = MIN2(BITMAP_SIZE, size_class + tr); // avoid overflow
+
+        map.clear_large_range(l, r);
+        verify_unset(map, l, r);
+        verify_set(map, 0, l);
+        verify_set(map, r, BITMAP_SIZE);
+
+        // Restore cleared
+        map.set_range(l, r);
+        verify_set(map, l, r);
+      }
+    }
+  }
+}
+
+TEST(BitMap, set_large_range) {
+  CHeapBitMap map(BITMAP_SIZE);
+
+  map.clear();
+  verify_unset(map, 0, BITMAP_SIZE);
+
+  for (size_t size_class = 0; size_class <= BITMAP_SIZE; size_class = MAX2<size_t>(1, size_class*2)) {
+    for (BitMap::idx_t l = 0; l < FUZZ_WINDOW; l++) {
+      for (BitMap::idx_t tr = l; tr < FUZZ_WINDOW; tr++) {
+        BitMap::idx_t r = MIN2(BITMAP_SIZE, size_class + tr); // avoid overflow
+
+        map.set_large_range(l, r);
+        verify_set(map, l, r);
+        verify_unset(map, 0, l);
+        verify_unset(map, r, BITMAP_SIZE);
+
+        // Restore set
+        map.clear_range(l, r);
+        verify_unset(map, l, r);
+      }
+    }
+  }
+}
+
+TEST(BitMap, par_at_put_large_range) {
+  CHeapBitMap map(BITMAP_SIZE);
+
+  map.clear();
+  verify_unset(map, 0, BITMAP_SIZE);
+
+  for (size_t size_class = 0; size_class <= BITMAP_SIZE; size_class = MAX2<size_t>(1, size_class*2)) {
+    for (BitMap::idx_t l = 0; l < FUZZ_WINDOW; l++) {
+      for (BitMap::idx_t tr = l; tr < FUZZ_WINDOW; tr++) {
+        BitMap::idx_t r = MIN2(BITMAP_SIZE, size_class + tr); // avoid overflow
+
+        map.par_at_put_large_range(l, r, true);
+        verify_set(map, l, r);
+        verify_unset(map, 0, l);
+        verify_unset(map, r, BITMAP_SIZE);
+
+        // Restore set
+        map.clear_range(l, r);
+        verify_unset(map, l, r);
+      }
+    }
+  }
+}