Fix/expand ASSERT_STATUS_CHECKED build, add to Travis

[pdillinger]

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.)

Test Plan: ASSERT_STATUS_CHECKED=1 make check, Travis

[ajkr]

Thanks for finding all these tests that already pass and adding to travis to prevent regression! I thought we were only at 2/167 passing but we're actually at 44/167, that's great.

[facebook-github-bot]

@pdillinger has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@pdillinger merged this pull request in 35a25a3.

[pdillinger]

The new Travis status_checked build failed https://travis-ci.org/github/facebook/rocksdb/jobs/690119387, but @ajkr landed this with accept2ship.

I think the problem is that it's up to the compiler whether

Status s;
... // initial value of s not read
if (x) {
  s = something;
} else {
  s = something else;
}
check(s);

constructs and destructs a Status that goes unchecked. I think debug builds need to disallow any reading from default-initialized statuses, thus (in whitelisted code) forcing explicit initialization to OK() if a status might be read.

[pdillinger]

I think the problem is that it's up to the compiler whether

Not so sure about that. We'll see in #6871, where we now have stack traces on failures 😄

[ajkr]

The new Travis status_checked build failed https://travis-ci.org/github/facebook/rocksdb/jobs/690119387, but @ajkr landed this with accept2ship.

Sorry, didn't know what checks were in place with that button. Seems not the ones we want.

I think we need to add a check for status for each iteration of this loop body:

rocksdb/options/options_helper.cc

Lines 1126 to 1135 in 35a25a3

	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: ",
	struct_name + "." + map_iter.first);
	}

. I haven't figured out why it's compiler dependent...

[ajkr]

@pdillinger - Ha, I think the order of the unordered_map changed between compiler versions. When I run in older versions, the status is non-ok on the second iteration. In newer versions the status is non-ok on the first iteration (and we check status just above the loop).