andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

rocksdb: Fix scan-build 'Called C++ object pointer is null' and 'Dereference of null pointer' bugs

Browse Source 
Summary:
In the existing implementation of `ASSERT*`, test termination happens in `~Tester`, which is called when instance of `Tester` goes out of scope. This is the cause of many scan-build bugs.

This diff changes `ASSERT*` to terminate the test immediately. Also added one suppression in `util/signal_test.cc`

scan-build bugs
before: http://home.fburl.com/~sugak/latest/index.html
after: http://home.fburl.com/~sugak/latest2/index.html

Test Plan:
Modify some test to fail an assertion and make sure that `ASSERT*` terminated the test.

Run `make analyze` and make sure no 'Called C++ object pointer is null' and 'Dereference of null pointer' bugs reported.

Run tests and make sure no failing tests:
```lang=bash
% make check
% USE_CLANG=1 make check
```

Reviewers: meyering, lgalanis, sdong, rven, igor

Reviewed By: igor

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D33381
This commit is contained in:
Igor Sugak 2015-02-13 15:10:47 -08:00
parent b3fd162267
commit 4e4b857841
2 changed files with 79 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
util/signal_test.cc
View File

@ -9,8 +9,11 @@
namespace {
void f0() {
char *p = nullptr;
#ifndef __clang_analyzer__
// cppcheck-suppress nullPointer
*p = 10; /* SIGSEGV here!! */
#endif
}
void f1() {

113
util/testharness.h
View File

@ -46,27 +46,19 @@ extern std::string TmpDir(Env* env = Env::Default());
// runs may be able to vary the seed.
extern int RandomSeed();
class TesterHelper;
// An instance of Tester is allocated to hold temporary state during
// the execution of an assertion.
class Tester {
friend class TesterHelper;
private:
bool ok_;
const char* fname_;
int line_;
std::stringstream ss_;
public:
Tester(const char* f, int l)
: ok_(true), fname_(f), line_(l) {
}
~Tester() {
if (!ok_) {
fprintf(stderr, "%s:%d:%s\n", fname_, line_, ss_.str().c_str());
port::PrintStack(2);
exit(1);
}
}
Tester() : ok_(true) {}
Tester& Is(bool b, const char* msg) {
if (!b) {
@ -118,38 +110,85 @@ class Tester {
}
return *this;
}
operator bool() const { return ok_; }
};
#define ASSERT_TRUE(c) ::rocksdb::test::Tester(__FILE__, __LINE__).Is((c), #c)
#define ASSERT_OK(s) ::rocksdb::test::Tester(__FILE__, __LINE__).IsOk((s))
#define ASSERT_NOK(s) ::rocksdb::test::Tester(__FILE__, __LINE__).IsNotOk((s))
#define ASSERT_EQ(a,b) ::rocksdb::test::Tester(__FILE__, __LINE__).IsEq((a),(b))
#define ASSERT_NE(a,b) ::rocksdb::test::Tester(__FILE__, __LINE__).IsNe((a),(b))
#define ASSERT_GE(a,b) ::rocksdb::test::Tester(__FILE__, __LINE__).IsGe((a),(b))
#define ASSERT_GT(a,b) ::rocksdb::test::Tester(__FILE__, __LINE__).IsGt((a),(b))
#define ASSERT_LE(a,b) ::rocksdb::test::Tester(__FILE__, __LINE__).IsLe((a),(b))
#define ASSERT_LT(a,b) ::rocksdb::test::Tester(__FILE__, __LINE__).IsLt((a),(b))
class TesterHelper {
private:
const char* fname_;
int line_;
#define TCONCAT(a,b) TCONCAT1(a,b)
#define TCONCAT1(a,b) a##b
public:
TesterHelper(const char* f, int l) : fname_(f), line_(l) {}
#define TEST(base,name) \
class TCONCAT(_Test_,name) : public base { \
public: \
void _Run(); \
static void _RunIt() { \
TCONCAT(_Test_,name) t; \
t._Run(); \
} \
}; \
bool TCONCAT(_Test_ignored_,name) = \
::rocksdb::test::RegisterTest(#base, #name, &TCONCAT(_Test_,name)::_RunIt); \
void TCONCAT(_Test_,name)::_Run()
void operator=(const Tester& tester) {
fprintf(stderr, "%s:%d:%s\n", fname_, line_, tester.ss_.str().c_str());
port::PrintStack(2);
exit(1);
}
};
// This is trying to solve:
// * Evaluate expression
// * Abort the test if the evaluation is not successful with the evaluation
// details.
// * Support operator << with ASSERT* for extra messages provided by the user
// code of ASSERT*
//
// For the third, we need to make sure that an expression at the end of macro
// supports << operator. But since we can have multiple of << we cannot abort
// inside implementation of operator <<, as we may miss some extra message. That
// is why there is TesterHelper with operator = which has lower precedence then
// operator <<, and it will be called after all messages from use code are
// accounted by <<.
//
// operator bool is added to Tester to make possible its declaration inside if
// statement and do not pollute its outer scope with the name tester. But in C++
// we cannot do any other operations inside if statement besides declaration.
// Then in order to get inside if body there are two options: make operator
// Tester::bool return true if ok_ == false or put the body into else part.
#define TEST_EXPRESSION_(expression) \
if (::rocksdb::test::Tester& tester = (expression)) \
; \
else \
::rocksdb::test::TesterHelper(__FILE__, __LINE__) = tester
#define ASSERT_TRUE(c) TEST_EXPRESSION_(::rocksdb::test::Tester().Is((c), #c))
#define ASSERT_OK(s) TEST_EXPRESSION_(::rocksdb::test::Tester().IsOk((s)))
#define ASSERT_NOK(s) TEST_EXPRESSION_(::rocksdb::test::Tester().IsNotOk((s)))
#define ASSERT_EQ(a, b) \
TEST_EXPRESSION_(::rocksdb::test::Tester().IsEq((a), (b)))
#define ASSERT_NE(a, b) \
TEST_EXPRESSION_(::rocksdb::test::Tester().IsNe((a), (b)))
#define ASSERT_GE(a, b) \
TEST_EXPRESSION_(::rocksdb::test::Tester().IsGe((a), (b)))
#define ASSERT_GT(a, b) \
TEST_EXPRESSION_(::rocksdb::test::Tester().IsGt((a), (b)))
#define ASSERT_LE(a, b) \
TEST_EXPRESSION_(::rocksdb::test::Tester().IsLe((a), (b)))
#define ASSERT_LT(a, b) \
TEST_EXPRESSION_(::rocksdb::test::Tester().IsLt((a), (b)))
#define TCONCAT(a, b) TCONCAT1(a, b)
#define TCONCAT1(a, b) a##b
#define TEST(base, name) \
class TCONCAT(_Test_, name) : public base { \
public: \
void _Run(); \
static void _RunIt() { \
TCONCAT(_Test_, name) t; \
t._Run(); \
} \
}; \
bool TCONCAT(_Test_ignored_, name) = ::rocksdb::test::RegisterTest( \
#base, #name, &TCONCAT(_Test_, name)::_RunIt); \
void TCONCAT(_Test_, name)::_Run()
// Register the specified test. Typically not used directly, but
// invoked via the macro expansion of TEST.
extern bool RegisterTest(const char* base, const char* name, void (*func)());
} // namespace test
} // namespace rocksdb