From 35a25a3fb9b5a61a389e853f8be37932487edd03 Mon Sep 17 00:00:00 2001
From: Peter Dillinger <peterd@fb.com>
Date: Fri, 22 May 2020 11:15:44 -0700
Subject: [PATCH] Fix/expand ASSERT_STATUS_CHECKED build, add to Travis (#6870)

Summary:
Fixed some option handling code that recently broke the
ASSERT_STATUS_CHECKED build for options_test.

Added all other existing tests that pass under ASSERT_STATUS_CHECKED to
the whitelist.

Added a Travis configuration to run all whitelisted tests with
ASSERT_STATUS_CHECKED. (Someday we might enable this check by default in
debug builds.)
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6870

Test Plan: ASSERT_STATUS_CHECKED=1 make check, Travis

Reviewed By: ajkr

Differential Revision: D21704374

Pulled By: pdillinger

fbshipit-source-id: 15daef98136a19d7a6843fa0c9ec08738c2ac693
---
 .travis.yml               | 15 ++++++++++
 Makefile                  | 59 ++++++++++++++++++++++++++++++++++-----
 options/options_helper.cc |  9 +++---
 3 files changed, 71 insertions(+), 12 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 75a770827..c3e6e91ec 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -53,6 +53,7 @@ env:
   - JOB_NAME=cmake-gcc9 # 3-5 minutes
   - JOB_NAME=cmake-gcc9-c++20 # 3-5 minutes
   - JOB_NAME=cmake-mingw # 3 minutes
+  - JOB_NAME=status_checked
 
 matrix:
   exclude:
@@ -197,6 +198,17 @@ matrix:
     os: linux
     arch: ppc64le
     env: JOB_NAME=cmake-gcc9-c++20
+  - if: type = pull_request
+    os : osx
+    env: JOB_NAME=status_checked
+  - if: type = pull_request
+    os : linux
+    arch: arm64
+    env: JOB_NAME=status_checked
+  - if: type = pull_request
+    os: linux
+    arch: ppc64le
+    env: JOB_NAME=status_checked
 
 install:
   - if [ "${TRAVIS_OS_NAME}" == osx ]; then
@@ -287,6 +299,9 @@ 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
+      ;;
     esac
 notifications:
     email:
diff --git a/Makefile b/Makefile
index a192365a1..7d425335b 100644
--- a/Makefile
+++ b/Makefile
@@ -667,12 +667,54 @@ endif
 ifdef ASSERT_STATUS_CHECKED
 	# This is a new check for which we will add support incrementally. The
 	# whitelist can be removed once support is fully added.
-        TESTS_WHITELIST = \
-		options_test \
+	TESTS_WHITELIST = \
+		arena_test \
+		autovector_test \
+		blob_file_addition_test \
+		blob_file_garbage_test \
+		bloom_test \
+		cassandra_format_test \
+		cassandra_row_merge_test \
+		cassandra_serialize_test \
+		cleanable_test \
+		coding_test \
+		crc32c_test \
+		dbformat_test \
+		defer_test \
+		dynamic_bloom_test \
+		event_logger_test \
+		file_indexer_test \
+		folly_synchronization_distributed_mutex_test \
+		hash_table_test \
+		hash_test \
+		heap_test \
+		histogram_test \
+		inlineskiplist_test \
+		io_posix_test \
+		iostats_context_test \
+		memkind_kmem_allocator_test \
+		merger_test \
+		mock_env_test \
+		object_registry_test \
 		options_settable_test \
-		io_posix_test
-        TESTS := $(filter $(TESTS_WHITELIST),$(TESTS))
-        PARALLEL_TEST := $(filter $(TESTS_WHITELIST),$(PARALLEL_TEST))
+		options_test \
+		random_test \
+		range_del_aggregator_test \
+		range_tombstone_fragmenter_test \
+		repeatable_thread_test \
+		skiplist_test \
+		slice_test \
+		statistics_test \
+		thread_local_test \
+		timer_queue_test \
+		timer_test \
+		util_merge_operators_test \
+		version_edit_test \
+		work_queue_test \
+		write_controller_test \
+
+	TESTS := $(filter $(TESTS_WHITELIST),$(TESTS))
+	PARALLEL_TEST := $(filter $(TESTS_WHITELIST),$(PARALLEL_TEST))
 endif
 SUBSET := $(TESTS)
 ifdef ROCKSDBTESTS_START
@@ -972,6 +1014,9 @@ CLEAN_FILES += t LOG $(TMPD)
 watch-log:
 	$(WATCH) --interval=0 'sort -k7,7nr -k4,4gr LOG|$(quoted_perl_command)'
 
+dump-log:
+	bash -c '$(quoted_perl_command)' < LOG
+
 # If J != 1 and GNU parallel is installed, run the tests in parallel,
 # via the check_0 rule above.  Otherwise, run them sequentially.
 check: all
@@ -1283,9 +1328,9 @@ db_repl_stress: tools/db_repl_stress.o $(LIBOBJECTS) $(TESTUTIL)
 
 arena_test: memory/arena_test.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)
-	
+
 memkind_kmem_allocator_test: memory/memkind_kmem_allocator_test.o $(LIBOBJECTS) $(TESTHARNESS)
-	$(AM_LINK)	
+	$(AM_LINK)
 
 autovector_test: util/autovector_test.o $(LIBOBJECTS) $(TESTHARNESS)
 	$(AM_LINK)
diff --git a/options/options_helper.cc b/options/options_helper.cc
index 2f8ce4c55..99e5cd5d3 100644
--- a/options/options_helper.cc
+++ b/options/options_helper.cc
@@ -1221,13 +1221,13 @@ Status OptionTypeInfo::SerializeStruct(
     std::string elem_name;
     const auto opt_info = Find(opt_name, *struct_map, &elem_name);
     if (opt_info == nullptr) {
-      return Status::InvalidArgument("Unrecognized option: ", opt_name);
+      status = Status::InvalidArgument("Unrecognized option: ", opt_name);
     } else if (opt_info->ShouldSerialize()) {
-      return opt_info->Serialize(config_options, opt_name + "." + elem_name,
-                                 opt_addr + opt_info->offset_, value);
+      status = opt_info->Serialize(config_options, opt_name + "." + elem_name,
+                                   opt_addr + opt_info->offset_, value);
     }
   }
-  return Status::OK();
+  return status;
 }
 
 template <typename T>
@@ -1329,7 +1329,6 @@ bool OptionTypeInfo::StructsAreEqual(
     const std::string& opt_name, const char* this_addr, const char* that_addr,
     std::string* mismatch) {
   assert(struct_map);
-  Status status;
   bool matches = true;
   std::string result;
   if (EndsWith(opt_name, struct_name)) {