qqiangwu / cppsafe

Cpp lifetime safety profile static analyzer
MIT License
45 stars 1 forks source link

rocksdb: member function pset build #25

Closed qqiangwu closed 5 months ago

qqiangwu commented 7 months ago
template <class T>
class channel {
 public:
  explicit channel() : eof_(false) {}

  channel(const channel&) = delete;
  void operator=(const channel&) = delete;

  void sendEof() {
    std::lock_guard<std::mutex> lk(lock_);
    eof_ = true;
    cv_.notify_all();
  }

  bool eof() {
    std::lock_guard<std::mutex> lk(lock_);
    return buffer_.empty() && eof_;
  }

  size_t size() const {
    std::lock_guard<std::mutex> lk(lock_);
    return buffer_.size();
  }

  // writes elem to the queue
  void write(T&& elem) {
    std::unique_lock<std::mutex> lk(lock_);
    buffer_.emplace(std::forward<T>(elem));
    cv_.notify_one();
  }

  /// Moves a dequeued element onto elem, blocking until an element
  /// is available.
  // returns false if EOF
  bool read(T& elem) {
    std::unique_lock<std::mutex> lk(lock_);
    cv_.wait(lk, [&] { return eof_ || !buffer_.empty(); });
    if (eof_ && buffer_.empty()) {
      return false;
    }
    elem = std::move(buffer_.front());
    buffer_.pop();
    cv_.notify_one();
    return true;
  }

 private:
  std::condition_variable cv_;
  mutable std::mutex lock_;
  std::queue<T> buffer_;
  bool eof_;
};

generates:

In file included from /Users/wuqq/dev/rocksdb-main/utilities/backup/backup_engine.cc:48:
/Users/wuqq/dev/rocksdb-main/util/channel.h:58:5: warning: use a moved-from object
   58 |     buffer_.pop();
      |     ^~~~~~~
/Users/wuqq/dev/rocksdb-main/utilities/backup/backup_engine.cc:1267:39: note: in instantiation of member function 'rocksdb::channel<rocksdb::(anonymous namespace)::BackupEngineImpl::CopyOrCreateWorkItem>::read' requested here
 1267 |       while (files_to_copy_or_create_.read(work_item)) {
      |                                       ^
/Users/wuqq/dev/rocksdb-main/util/channel.h:57:12: note: moved here
   57 |     elem = std::move(buffer_.front());
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/util/channel.h:59:5: warning: use a moved-from object
   59 |     cv_.notify_one();
      |     ^~~
/Users/wuqq/dev/rocksdb-main/util/channel.h:57:12: note: moved here
   57 |     elem = std::move(buffer_.front());
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~
31 warnings generated.
qqiangwu commented 6 months ago
void ThreadStatusUpdater::EraseDatabaseInfo(const void* db_key) {
  // Acquiring same lock as GetThreadList() to guarantee
  // a consistent view of global column family table (cf_info_map).
  std::lock_guard<std::mutex> lck(thread_list_mutex_);
  auto db_pair = db_key_map_.find(db_key);
  if (UNLIKELY(db_pair == db_key_map_.end())) {
    // In some occasional cases such as DB::Open fails, we won't
    // register ColumnFamilyInfo for a db.
    return;
  }

  for (auto cf_key : db_pair->second) {
    auto cf_pair = cf_info_map_.find(cf_key);
    if (cf_pair != cf_info_map_.end()) {
      cf_info_map_.erase(cf_pair);
    }
  }
  db_key_map_.erase(db_key);
}

gives:

/Users/wuqq/dev/rocksdb-main/monitoring/thread_status_updater.cc:276:20: warning: passing a possibly dangling pointer as argument
  276 |   for (auto cf_key : db_pair->second) {
      |                    ^
/Users/wuqq/dev/rocksdb-main/monitoring/thread_status_updater.cc:279:26: note: modified here
  279 |       cf_info_map_.erase(cf_pair);
      |                          ^~~~~~~
1 warning generated.
qqiangwu commented 6 months ago
  VersionBuilder* builder = nullptr;

  if (prev_missing_blob_file_high != kInvalidBlobFileNumber) {
    auto builder_iter = builders_.find(cfd->GetID());
    assert(builder_iter != builders_.end());
    builder = builder_iter->second->version_builder();
    assert(builder != nullptr);
  }

  // At this point, we have not yet applied the new version edits read from the
  // MANIFEST. We check whether we have any missing table and blob files.
  const bool prev_has_missing_files =
      !missing_files.empty() ||
      (prev_missing_blob_file_high != kInvalidBlobFileNumber &&
       prev_missing_blob_file_high >= builder->GetMinOldestBlobFileNumber());

  for (const auto& file : edit.GetDeletedFiles()) {
    uint64_t file_num = file.second;
    auto fiter = missing_files.find(file_num);
    if (fiter != missing_files.end()) {
      missing_files.erase(fiter);
    }
  }

gives:

/Users/wuqq/dev/rocksdb-main/db/version_edit_handler.cc:891:9: warning: passing a possibly dangling pointer as argument
  891 |     s = builder->LoadTableHandlers(
      |         ^~~~~~~
/Users/wuqq/dev/rocksdb-main/db/version_edit_handler.cc:811:7: note: modified here
  811 |       missing_files.erase(fiter);
      |       ^~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/db/version_edit_handler.cc:903:9: warning: passing a possibly dangling pointer as argument
  903 |     s = builder->SaveTo(version->storage_info());
      |         ^~~~~~~
/Users/wuqq/dev/rocksdb-main/db/version_edit_handler.cc:811:7: note: modified here
  811 |       missing_files.erase(fiter);
      |       ^~~~~~~~~~~~~
2 warnings generated.

psets:

/Users/wuqq/dev/rocksdb-main/db/version_edit_handler.cc:808:3: warning: pset(builder) = ((null), **this)
  808 |   __lifetime_pset(builder);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/db/version_edit_handler.cc:809:3: warning: pset(missing_files) = (**this)
  809 |   __lifetime_pset(missing_files);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/db/version_edit_handler.cc:817:3: warning: pset(builder) = ((null), **this)
  817 |   __lifetime_pset(builder);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/db/version_edit_handler.cc:817:3: warning: pset(builder) = ((invalid), (null), **this)
  817 |   __lifetime_pset(builder);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~
qqiangwu commented 6 months ago
void CondVar::Wait() {
#ifndef NDEBUG
  mu_->locked_ = false;
#endif
  PthreadCall("wait", pthread_cond_wait(&cv_, &mu_->mu_));
#ifndef NDEBUG
  mu_->locked_ = true;
#endif
  __lifetime_pset(mu_);
}
/Users/wuqq/dev/rocksdb-main/port/port_posix.cc:120:3: warning: pset(cv_) = (*this)
  120 |   __lifetime_pset(cv_);
      |   ^~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/port/port_posix.cc:121:3: warning: pset(mu_) = (**this)
  121 |   __lifetime_pset(mu_);
      |   ^~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/port/port_posix.cc:122:3: warning: pset(mu_->mu_) = ((global))
  122 |   __lifetime_pset(mu_->mu_);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/port/port_posix.cc:125:3: warning: dereferencing a dangling pointer
  125 |   mu_->locked_ = true;
      |   ^~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/port/port_posix.cc:123:41: note: modified here
  123 |   PthreadCall("wait", pthread_cond_wait(&cv_, &mu_->mu_));
      |                                         ^~~~
/Users/wuqq/dev/rocksdb-main/port/port_posix.cc:127:3: warning: pset(mu_) = ((invalid))
  127 |   __lifetime_pset(mu_);
      |   ^~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/port/port_posix.cc:140:3: warning: dereferencing a dangling pointer
  140 |   mu_->locked_ = true;
      |   ^~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/port/port_posix.cc:138:36: note: modified here
  138 |   int err = pthread_cond_timedwait(&cv_, &mu_->mu_, &ts);
      |                                    ^~~~
6 warnings generated.
qqiangwu commented 6 months ago
      auto range_tombstone_iter = range_tombstone_iters_[level];
      if (range_tombstone_iter) {
        range_tombstone_iter->SeekInternalKey(
            current_search_key.GetInternalKey());
        // Invariants (rti) and (phi)
        if (range_tombstone_iter->Valid()) {
          // If range tombstone starts after `current_search_key`,
          // we should insert start key to heap as the range tombstone is not
          // active yet.
          InsertRangeTombstoneToMinHeap(
              level, comparator_->Compare(range_tombstone_iter->start_key(),
                                          pik) > 0 /* start_key */);
qqiangwu commented 6 months ago
    auto cfd = versions_->GetColumnFamilySet()->GetDefault();
    __lifetime_pset(cfd);
    if (table_type_ == TableTypeForTest::kMockTable) {
      ASSERT_EQ(compaction_job_stats_.num_output_files,
                expected_results.size());
      mock_table_factory_->AssertLatestFiles(expected_results);
    } else {
      assert(table_type_ == TableTypeForTest::kBlockBasedTable);
    }

    __lifetime_pset(mock_table_factory_);
    __lifetime_pset(cfd);
    auto output_files =
        cfd->current()->storage_info()->LevelFiles(output_level);

gives:

/Users/wuqq/dev/rocksdb-main/db/compaction/compaction_job_test.cc:428:5: warning: pset(cfd) = (**this)
  428 |     __lifetime_pset(cfd);
      |     ^~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/db/compaction/compaction_job_test.cc:437:5: warning: pset(mock_table_factory_) = (*this)
  437 |     __lifetime_pset(mock_table_factory_);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/db/compaction/compaction_job_test.cc:438:5: warning: pset(cfd) = ((invalid), **this)
  438 |     __lifetime_pset(cfd);
      |     ^~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/db/compaction/compaction_job_test.cc:440:9: warning: passing a possibly dangling pointer as argument
  440 |         cfd->current()->storage_info()->LevelFiles(output_level);
      |         ^~~
/Users/wuqq/dev/rocksdb-main/db/compaction/compaction_job_test.cc:432:7: note: modified here
  432 |       mock_table_factory_->AssertLatestFiles(expected_results);
      |       ^~~~~~~~~~~~~~~~~~~~~
qqiangwu commented 6 months ago
/Users/wuqq/dev/rocksdb-main/db/compaction/compaction_outputs.cc:521:1: warning: pset(lower_bound_guard) = (**this, *lower_bound_buf, *this)
  521 | __lifetime_pset(lower_bound_guard);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/db/compaction/compaction_outputs.cc:522:1: warning: pset(lower_bound) = ((null), lower_bound_guard)
  522 | __lifetime_pset(lower_bound);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/db/compaction/compaction_outputs.cc:592:46: warning: passing a dangling pointer as argument
  592 |         icmp.Compare(tombstone_end.Encode(), *lower_bound) <= 0) {
      |                                              ^~~~~~~~~~~~
/Users/wuqq/dev/rocksdb-main/db/compaction/compaction_outputs.cc:579:13: note: modified here
  579 |   auto it = range_del_agg_->NewIterator(lower_bound, upper_bound);
      |             ^~~~~~~~~~~~~~~~