From 803a517b48ddd01689cc9c6feb0f4b7b19c317e6 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Sat, 23 May 2020 06:48:40 -0700 Subject: [PATCH] Misc things for ASSERT_STATUS_CHECKED, also gcc 4.8.5 (#6871) Summary: * Print stack trace on status checked failure * Make folly_synchronization_distributed_mutex_test a parallel test * Disable ldb_test.py and rocksdb_dump_test.sh with ASSERT_STATUS_CHECKED (broken) * Fix shadow warning in random_access_file_reader.h reported by gcc 4.8.5 (ROCKSDB_NO_FBCODE), also https://github.com/facebook/rocksdb/issues/6866 * Work around compiler bug on max_align_t for gcc < 4.9 * Remove an apparently wrong comment in status.h * Use check_some in Travis config (for proper diagnostic output) * Fix ignored Status in loop in options_helper.cc Pull Request resolved: https://github.com/facebook/rocksdb/pull/6871 Test Plan: manual, CI Reviewed By: ajkr Differential Revision: D21706619 Pulled By: pdillinger fbshipit-source-id: daf6364173d6689904eb394461a69a11f5bee2cb --- .travis.yml | 2 +- Makefile | 11 +++++++---- file/random_access_file_reader.h | 4 ++-- include/rocksdb/status.h | 18 +++++++++++++++--- options/options_helper.cc | 21 +++++++++++---------- third-party/folly/folly/lang/Align.h | 9 +++++++++ 6 files changed, 45 insertions(+), 20 deletions(-) diff --git a/.travis.yml b/.travis.yml index c3e6e91ec9..4b5111f06c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -300,7 +300,7 @@ script: mkdir build && cd build && cmake -DJNI=1 .. -DCMAKE_BUILD_TYPE=Release $OPT && make -j4 rocksdb rocksdbjni ;; status_checked) - OPT=-DTRAVIS V=1 ASSERT_STATUS_CHECKED=1 make -j4 check + OPT=-DTRAVIS V=1 ASSERT_STATUS_CHECKED=1 make -j4 check_some ;; esac notifications: diff --git a/Makefile b/Makefile index 7d425335b2..3141c59b44 100644 --- a/Makefile +++ b/Makefile @@ -633,10 +633,6 @@ TESTS = \ timer_test \ db_with_timestamp_compaction_test \ -ifeq ($(USE_FOLLY_DISTRIBUTED_MUTEX),1) - TESTS += folly_synchronization_distributed_mutex_test -endif - PARALLEL_TEST = \ backupable_db_test \ db_bloom_filter_test \ @@ -660,6 +656,11 @@ PARALLEL_TEST = \ write_prepared_transaction_test \ write_unprepared_transaction_test \ +ifeq ($(USE_FOLLY_DISTRIBUTED_MUTEX),1) + TESTS += folly_synchronization_distributed_mutex_test + PARALLEL_TEST += folly_synchronization_distributed_mutex_test +endif + # options_settable_test doesn't pass with UBSAN as we use hack in the test ifdef COMPILE_WITH_UBSAN TESTS := $(shell echo $(TESTS) | sed 's/\boptions_settable_test\b//g') @@ -1034,9 +1035,11 @@ check: all ifneq ($(PLATFORM), OS_AIX) $(PYTHON) tools/check_all_python.py ifeq ($(filter -DROCKSDB_LITE,$(OPT)),) +ifndef ASSERT_STATUS_CHECKED # not yet working with these tests $(PYTHON) tools/ldb_test.py sh tools/rocksdb_dump_test.sh endif +endif endif $(MAKE) check-format $(MAKE) check-buck-targets diff --git a/file/random_access_file_reader.h b/file/random_access_file_reader.h index 5dba52c58b..4fee67c92c 100644 --- a/file/random_access_file_reader.h +++ b/file/random_access_file_reader.h @@ -62,13 +62,13 @@ class RandomAccessFileReader { public: explicit RandomAccessFileReader( std::unique_ptr&& raf, const std::string& _file_name, - Env* env = nullptr, Statistics* stats = nullptr, uint32_t hist_type = 0, + Env* _env = nullptr, Statistics* stats = nullptr, uint32_t hist_type = 0, HistogramImpl* file_read_hist = nullptr, RateLimiter* rate_limiter = nullptr, const std::vector>& listeners = {}) : file_(std::move(raf)), file_name_(std::move(_file_name)), - env_(env), + env_(_env), stats_(stats), hist_type_(hist_type), file_read_hist_(file_read_hist), diff --git a/include/rocksdb/status.h b/include/rocksdb/status.h index 8b4ef1dc2e..6006fb16bb 100644 --- a/include/rocksdb/status.h +++ b/include/rocksdb/status.h @@ -16,7 +16,17 @@ #pragma once +#ifdef ROCKSDB_ASSERT_STATUS_CHECKED +#include +#include +#endif + #include + +#ifdef ROCKSDB_ASSERT_STATUS_CHECKED +#include "port/stack_trace.h" +#endif + #include "rocksdb/slice.h" namespace ROCKSDB_NAMESPACE { @@ -27,7 +37,11 @@ class Status { Status() : code_(kOk), subcode_(kNone), sev_(kNoError), state_(nullptr) {} ~Status() { #ifdef ROCKSDB_ASSERT_STATUS_CHECKED - assert(checked_); + if (!checked_) { + fprintf(stderr, "Failed to check Status\n"); + port::PrintStack(); + abort(); + } #endif // ROCKSDB_ASSERT_STATUS_CHECKED delete[] state_; } @@ -511,8 +525,6 @@ inline Status::Status(const Status& s, Severity sev) state_ = (s.state_ == nullptr) ? nullptr : CopyState(s.state_); } inline Status& Status::operator=(const Status& s) { - // The following condition catches both aliasing (when this == &s), - // and the common case where both s and *this are ok. if (this != &s) { #ifdef ROCKSDB_ASSERT_STATUS_CHECKED s.checked_ = true; diff --git a/options/options_helper.cc b/options/options_helper.cc index 99e5cd5d3d..4f5d5c28fb 100644 --- a/options/options_helper.cc +++ b/options/options_helper.cc @@ -1122,17 +1122,18 @@ Status OptionTypeInfo::ParseStruct( // This option represents the entire struct std::unordered_map opt_map; status = StringToMap(opt_value, &opt_map); - if (status.ok()) { - for (const auto& map_iter : opt_map) { - const auto iter = struct_map->find(map_iter.first); - if (iter != struct_map->end()) { - status = iter->second.Parse(config_options, map_iter.first, - map_iter.second, - opt_addr + iter->second.offset_); - } else { - return Status::InvalidArgument("Unrecognized option: ", + for (const auto& map_iter : opt_map) { + if (!status.ok()) { + break; + } + const auto iter = struct_map->find(map_iter.first); + if (iter != struct_map->end()) { + status = + iter->second.Parse(config_options, map_iter.first, map_iter.second, + opt_addr + iter->second.offset_); + } else { + status = Status::InvalidArgument("Unrecognized option: ", struct_name + "." + map_iter.first); - } } } } else if (StartsWith(opt_name, struct_name + ".")) { diff --git a/third-party/folly/folly/lang/Align.h b/third-party/folly/folly/lang/Align.h index fd00b14ff3..2a404b91d1 100644 --- a/third-party/folly/folly/lang/Align.h +++ b/third-party/folly/folly/lang/Align.h @@ -22,6 +22,15 @@ #include #include +// Work around bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56019 +#ifdef __GNUC__ +#if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 9) +namespace std { +using ::max_align_t; +} +#endif +#endif + namespace folly { // has_extended_alignment