From: Igor Canadi <icanadi@fb.com>
Date: Wed, 29 Apr 2015 17:52:31 +0000 (-0700)
Subject: Fix possible SIGSEGV in CompactRange (github issue #596)
X-Git-Tag: rocksdb-3.10.2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a11863cb76ccbe3edc5aedfad473688a1185f934;p=rocksdb.git

Fix possible SIGSEGV in CompactRange (github issue #596)

Summary: For very detailed explanation of what's happening read this: https://github.com/facebook/rocksdb/issues/596

Test Plan: make check + new unit test

Reviewers: yhchiang, anthony, rven

Reviewed By: rven

Subscribers: adamretter, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37779

Conflicts:
	db/db_test.cc
---

diff --git a/db/db_impl.cc b/db/db_impl.cc
index ca462b693..6de853576 100644
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@ -1243,7 +1243,8 @@ Status DBImpl::CompactRange(ColumnFamilyHandle* column_family,
   {
     InstrumentedMutexLock l(&mutex_);
     Version* base = cfd->current();
-    for (int level = 1; level < cfd->NumberLevels(); level++) {
+    for (int level = 1; level < base->storage_info()->num_non_empty_levels();
+         level++) {
       if (base->storage_info()->OverlapInLevel(level, begin, end)) {
         max_level_with_files = level;
       }
diff --git a/db/db_test.cc b/db/db_test.cc
index a27511cf5..3c013e756 100644
--- a/db/db_test.cc
+++ b/db/db_test.cc
@@ -11963,6 +11963,25 @@ TEST_F(DBTest, EmptyCompactedDB) {
   Close();
 }
 
+// Github issue #596
+TEST_F(DBTest, HugeNumberOfLevels) {
+  Options options = CurrentOptions();
+  options.write_buffer_size = 2 * 1024 * 1024;         // 2MB
+  options.max_bytes_for_level_base = 2 * 1024 * 1024;  // 2MB
+  options.num_levels = 12;
+  options.max_background_compactions = 10;
+  options.max_bytes_for_level_multiplier = 2;
+  options.level_compaction_dynamic_level_bytes = true;
+  DestroyAndReopen(options);
+
+  Random rnd(301);
+  for (int i = 0; i < 300000; ++i) {
+    ASSERT_OK(Put(Key(i), RandomString(&rnd, 1024)));
+  }
+
+  ASSERT_OK(db_->CompactRange(nullptr, nullptr));
+}
+
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {
diff --git a/db/file_indexer.cc b/db/file_indexer.cc
index c59036bd6..222cca9c0 100644
--- a/db/file_indexer.cc
+++ b/db/file_indexer.cc
@@ -20,6 +20,9 @@ FileIndexer::FileIndexer(const Comparator* ucmp)
 size_t FileIndexer::NumLevelIndex() const { return next_level_index_.size(); }
 
 size_t FileIndexer::LevelIndexSize(size_t level) const {
+  if (level >= next_level_index_.size()) {
+    return 0;
+  }
   return next_level_index_[level].num_index;
 }
 
diff --git a/db/file_indexer_test.cc b/db/file_indexer_test.cc
index 85e083546..98fea47fe 100644
--- a/db/file_indexer_test.cc
+++ b/db/file_indexer_test.cc
@@ -11,6 +11,7 @@
 #include "db/file_indexer.h"
 #include "db/dbformat.h"
 #include "db/version_edit.h"
+#include "port/stack_trace.h"
 #include "rocksdb/comparator.h"
 #include "util/testharness.h"
 #include "util/testutil.h"
@@ -343,6 +344,7 @@ TEST_F(FileIndexerTest, mixed) {
 }  // namespace rocksdb
 
 int main(int argc, char** argv) {
+  rocksdb::port::InstallStackTraceHandler();
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
 }
diff --git a/db/version_set.cc b/db/version_set.cc
index 17cddcb9f..fd6d88fae 100644
--- a/db/version_set.cc
+++ b/db/version_set.cc
@@ -1184,6 +1184,10 @@ bool Version::Unref() {
 bool VersionStorageInfo::OverlapInLevel(int level,
                                         const Slice* smallest_user_key,
                                         const Slice* largest_user_key) {
+  if (level >= num_non_empty_levels_) {
+    // empty level, no overlap
+    return false;
+  }
   return SomeFileOverlapsRange(*internal_comparator_, (level > 0),
                                level_files_brief_[level], smallest_user_key,
                                largest_user_key);
@@ -1227,6 +1231,11 @@ int VersionStorageInfo::PickLevelForMemTableOutput(
 void VersionStorageInfo::GetOverlappingInputs(
     int level, const InternalKey* begin, const InternalKey* end,
     std::vector<FileMetaData*>* inputs, int hint_index, int* file_index) {
+  if (level >= num_non_empty_levels_) {
+    // this level is empty, no overlapping inputs
+    return;
+  }
+
   inputs->clear();
   Slice user_begin, user_end;
   if (begin != nullptr) {
diff --git a/util/autovector.h b/util/autovector.h
index 9362536d3..c9befe965 100644
--- a/util/autovector.h
+++ b/util/autovector.h
@@ -190,16 +190,16 @@ class autovector {
 
   bool empty() const { return size() == 0; }
 
-  // will not check boundry
   const_reference operator[](size_type n) const {
+    assert(n < size());
     return n < kSize ? values_[n] : vect_[n - kSize];
   }
 
   reference operator[](size_type n) {
+    assert(n < size());
     return n < kSize ? values_[n] : vect_[n - kSize];
   }
 
-  // will check boundry
   const_reference at(size_type n) const {
     assert(n < size());
     return (*this)[n];