Skip to content

Commit

Permalink
libsepol, libsemanage: add a macro to silence static analyzer warning…
Browse files Browse the repository at this point in the history
…s in tests

Several static analyzers (clang's one, Facebook Infer, etc.) warn about
NULL pointer dereferences after a call to CU_ASSERT_PTR_NOT_NULL_FATAL()
in the test code written using CUnit framework. This is because this
CUnit macro is too complex for them to understand that the pointer
cannot be NULL: it is translated to a call to CU_assertImplementation()
with an argument as TRUE in order to mean that the call is fatal if the
asserted condition failed (cf.
http://cunit.sourceforge.net/doxdocs/group__Framework.html).

A possible solution could consist in replacing the
CU_ASSERT_..._FATAL() calls by assert() ones, as most static analyzers
know about assert(). Nevertheless this seems to go against CUnit's API.

An alternative solution consists in overriding CU_ASSERT_..._FATAL()
macros in order to expand to assert() after a call to the matching
CU_ASSERT_...() non-fatal macro. This appears to work fine and to remove
many false-positive warnings from various static analyzers.

As this substitution should only occur when using static analyzer, put
it under #ifdef __CHECKER__, which is the macro used by sparse when
analyzing the Linux kernel.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
  • Loading branch information
fishilico authored and stephensmalley committed Sep 30, 2019
1 parent eca4ee4 commit 120681c
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 1 deletion.
2 changes: 2 additions & 0 deletions libsemanage/tests/test_utilities.c
Expand Up @@ -34,6 +34,8 @@
#include <string.h>
#include <unistd.h>

#include "utilities.h"

void test_semanage_is_prefix(void);
void test_semanage_split_on_space(void);
void test_semanage_split(void);
Expand Down
26 changes: 26 additions & 0 deletions libsemanage/tests/utilities.h
Expand Up @@ -41,6 +41,32 @@
CU_ASSERT_STRING_EQUAL(__str, __str2); \
} while (0)


/* Override CU_*_FATAL() in order to help static analyzers by really asserting that an assertion holds */
#ifdef __CHECKER__

#undef CU_ASSERT_FATAL
#define CU_ASSERT_FATAL(value) do { \
int _value = (value); \
CU_ASSERT(_value); \
assert(_value); \
} while (0)

#undef CU_FAIL_FATAL
#define CU_FAIL_FATAL(msg) do { \
CU_FAIL(msg); \
assert(0); \
} while (0)

#undef CU_ASSERT_PTR_NOT_NULL_FATAL
#define CU_ASSERT_PTR_NOT_NULL_FATAL(value) do { \
const void *_value = (value); \
CU_ASSERT_PTR_NOT_NULL(_value); \
assert(_value != NULL); \
} while (0)

#endif /* __CHECKER__ */

#define I_NULL -1
#define I_FIRST 0
#define I_SECOND 1
Expand Down
29 changes: 29 additions & 0 deletions libsepol/tests/helpers.h
Expand Up @@ -24,9 +24,38 @@

#include <sepol/policydb/policydb.h>
#include <sepol/policydb/conditional.h>
#include <CUnit/Basic.h>

/* helper functions */

/* Override CU_*_FATAL() in order to help static analyzers by really asserting that an assertion holds */
#ifdef __CHECKER__

#include <assert.h>

#undef CU_ASSERT_FATAL
#define CU_ASSERT_FATAL(value) do { \
int _value = (value); \
CU_ASSERT(_value); \
assert(_value); \
} while (0)

#undef CU_FAIL_FATAL
#define CU_FAIL_FATAL(msg) do { \
CU_FAIL(msg); \
assert(0); \
} while (0)

#undef CU_ASSERT_PTR_NOT_NULL_FATAL
#define CU_ASSERT_PTR_NOT_NULL_FATAL(value) do { \
const void *_value = (value); \
CU_ASSERT_PTR_NOT_NULL(_value); \
assert(_value != NULL); \
} while (0)

#endif /* __CHECKER__ */


/* Load a source policy into p. policydb_init will called within this function.
*
* Example: test_load_policy(p, POLICY_BASE, 1, "foo", "base.conf") will load the
Expand Down
2 changes: 2 additions & 0 deletions libsepol/tests/test-common.c
Expand Up @@ -26,6 +26,8 @@

#include <CUnit/Basic.h>

#include "helpers.h"

void test_sym_presence(policydb_t * p, const char *id, int sym_type, unsigned int scope_type, unsigned int *decls, unsigned int len)
{
scope_datum_t *scope;
Expand Down
2 changes: 2 additions & 0 deletions libsepol/tests/test-deps.c
Expand Up @@ -66,6 +66,8 @@
#include <sepol/debug.h>
#include <sepol/handle.h>

#include "helpers.h"

#define BASE_MODREQ_TYPE_GLOBAL 0
#define BASE_MODREQ_ATTR_GLOBAL 1
#define BASE_MODREQ_OBJ_GLOBAL 2
Expand Down
1 change: 1 addition & 0 deletions libsepol/tests/test-expander-attr-map.c
Expand Up @@ -21,6 +21,7 @@

#include "test-expander-attr-map.h"
#include "test-common.h"
#include "helpers.h"

#include <sepol/policydb/policydb.h>
#include <CUnit/Basic.h>
Expand Down
1 change: 1 addition & 0 deletions libsepol/tests/test-expander-roles.c
Expand Up @@ -22,6 +22,7 @@

#include "test-expander-roles.h"
#include "test-common.h"
#include "helpers.h"

#include <sepol/policydb/policydb.h>
#include <CUnit/Basic.h>
Expand Down
1 change: 1 addition & 0 deletions libsepol/tests/test-expander-users.c
Expand Up @@ -21,6 +21,7 @@
*/

#include "test-expander-users.h"
#include "helpers.h"

#include <sepol/policydb/policydb.h>
#include <CUnit/Basic.h>
Expand Down
6 changes: 5 additions & 1 deletion scripts/run-scan-build
Expand Up @@ -22,7 +22,11 @@ export RUBYLIB="$DESTDIR/$(${RUBY:-ruby} -e 'puts RbConfig::CONFIG["vendorlibdir

# Build and analyze
make -C .. CC=clang clean distclean -j"$(nproc)"
scan-build -analyze-headers -o "$OUTPUTDIR" make -C .. CC=clang DESTDIR="$DESTDIR" install install-pywrap install-rubywrap all test
scan-build -analyze-headers -o "$OUTPUTDIR" make -C .. \
CC=clang \
DESTDIR="$DESTDIR" \
CFLAGS="-O2 -Wall -D__CHECKER__ -I$DESTDIR/usr/include" \
install install-pywrap install-rubywrap all test

# Reduce the verbosity in order to keep the message from scan-build saying
# "scan-build: Run 'scan-view /.../output-scan-build/2018-...' to examine bug reports.
Expand Down

0 comments on commit 120681c

Please sign in to comment.