Skip to content

Commit

Permalink
Add simple static checker based on clang-query
Browse files Browse the repository at this point in the history
This add a simple static checker based on clang-query, which is a
tool that could be described as a "clever grep" for abstract syntax
trees (ASTs).

As an initial proof of usefulness, this commit adds these checks:
  - No uses of floating point types.
  - No use of certain reserved identifiers (e.g., "mem(...)", bitcoin-core#829).
  - No use of memcmp (bitcoin-core#823).

The checks are easily extensible.

The main purpose is to run the checker on CI, and this commit adds
the checker to the Travis CI script.

This currently requires clang-query version at least 10. (However,
it's not required not compile with clang version 10, or with clang
at all. Just the compiler flags must be compatible with clang.)
Clang-query simply uses the clang compiler as a backend for
generating the AST.

In order to determine the compile in which the code is supposed to be
compiled (e.g., compiler flags such as -D defines and include paths),
it reads a compilation database in JSON format. There are multiple ways
to generate this database. The easiest way to obtain such a database is
to use a tool that intercepts the make process and build the database.

On Travis CI, we currently use "bear" for this purpose. It's a natural
choice because there is an Ubuntu package for it. If you want to run
this locally, bear is a good choice but other tools such as compiledb
(Python) are available.
  • Loading branch information
real-or-random committed Oct 22, 2020
1 parent c6b6b8f commit 4831f2e
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 0 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ libtool
*~
*.log
*.trs
compile_commands.commands.json
compile_commands.json
src/libsecp256k1-config.h
src/libsecp256k1-config.h.in
src/ecmult_static_context.h
Expand Down
10 changes: 10 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ osx_image: xcode10.1
addons:
apt:
packages:
- clang-tools-10
- libgmp-dev
- valgrind
- libtool-bin
- bear
compiler:
- clang
- gcc
Expand Down Expand Up @@ -43,27 +45,32 @@ matrix:
addons:
apt:
packages:
- clang-tools-10
- gcc-multilib
- libgmp-dev:i386
- valgrind
- libtool-bin
- bear
- libc6-dbg:i386
- compiler: clang
env: HOST=i686-linux-gnu
os: linux
addons:
apt:
packages:
- clang-tools-10
- gcc-multilib
- valgrind
- libtool-bin
- bear
- libc6-dbg:i386
- compiler: gcc
env: HOST=i686-linux-gnu
os: linux
addons:
apt:
packages:
- clang-tools-10
- gcc-multilib
- valgrind
- libtool-bin
Expand All @@ -74,6 +81,7 @@ matrix:
addons:
apt:
packages:
- clang-tools-10
- gcc-multilib
- libgmp-dev:i386
- valgrind
Expand Down Expand Up @@ -104,5 +112,7 @@ after_script:
- cat ./exhaustive_tests.log
- cat ./valgrind_ctime_test.log
- cat ./bench.log
- cat ./clang-query.log
- cat ./compile_commands.json
- $CC --version
- valgrind --version
39 changes: 39 additions & 0 deletions clang-query.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#!/bin/sh

set -u

matcher=$(cat <<'EOF'
set print-matcher true
enable output detailed-ast
### Expressions of any floating point type (unless in a system header)
match expr(allOf(unless(isExpansionInSystemHeader()), hasType(realFloatingPointType())))
### Calls to memcmp (secp256k1_memcmp_var should be used instead)
match callExpr(callee(functionDecl(hasName("memcmp"))))
### Reserved identifiers (unless in a system header) with external linkage or at file scope.
# Any function is in file scope. Any variable with static storage (unless static local variable) is in file scope.
# Allowed exceptions: __builtin_expect
# We need the "::" due to LLVM bug 47879.
match namedDecl(allOf(unless(isExpansionInSystemHeader()), anyOf(hasExternalFormalLinkage(), functionDecl(), varDecl(allOf(hasStaticStorageDuration(), unless(isStaticLocal())))), allOf(matchesName("^::(_|((mem|is|to|str|wcs)[a-z]))"), unless(hasAnyName("__builtin_expect")))))
### Reserved type names (unless in a system header)
# Allowed exceptions: uint128_t, int128_t, __int128_t, __uint128_t (the latter two are "implicit", i.e., added by the compiler)
match typedefDecl(allOf(unless(isExpansionInSystemHeader()), matchesName("(^::u?int)|(_t$)"), unless(hasAnyName("int128_t", "uint128_t", "__int128_t", "__uint128_t"))))
quit
EOF
)

# Poor man's JSON parser.
# This is not great but it works with the output of all common tools and it does not need extra dependencies.
files=$(grep 'file' compile_commands.json | uniq | cut -d '"' -f 4)
echo "Running clang-query on files:"
echo "$files"

output=$(echo "$matcher" | ${CLANG_QUERY:-clang-query} "$@" $files)
status=$?
echo "$output"
echo
exit $status
16 changes: 16 additions & 0 deletions contrib/travis.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ fi

if [ -n "$BUILD" ]
then
if [ "$TRAVIS_OS_NAME" = "linux" ] && [ "$TRAVIS_COMPILER" = "clang" ]
then
# Use bear to generate compile_commands.json
# This needs to be the first make command because otherwise make does not invoke the compiler because the files are up to date.
# We need to update this to "bear -- make" when we move to bear 3.0.
bear make -j2
fi
make -j2 "$BUILD"
fi
if [ "$RUN_VALGRIND" = "yes" ]
Expand Down Expand Up @@ -66,3 +73,12 @@ if [ "$CTIMETEST" = "yes" ]
then
./libtool --mode=execute valgrind --error-exitcode=42 ./valgrind_ctime_test > valgrind_ctime_test.log 2>&1
fi

# This would also run on gcc builds but there's no need to run it for both compilers.
if [ "$TRAVIS_OS_NAME" = "linux" ] && [ "$TRAVIS_COMPILER" = "clang" ]
then
# Abuse `script` to run make clang-query believe it's running with a terminal (see https://superuser.com/a/1434381/404598).
# This lets us write color codes to clang-query.log.
# (Passing `--extra-arg=-fcolor-diagnostics` to clang-query works only if stdout but not stderr is redirected to a file...)
CLANG_QUERY=clang-query-10 script -efqc "./clang-query.sh 2>&1" clang-query.log > /dev/null
fi

2 comments on commit 4831f2e

@ysangkok
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@real-or-random what do you mean by "However, it's not required not compile with clang version 10" in the commit message?

@real-or-random
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@real-or-random what do you mean by "However, it's not required not compile with clang version 10" in the commit message?

Just a typo. I meant to write "However, it's not required to compile with clang version 10". Clang-query works even the compilation database was produced when compiling with gcc (as the flags are compatible).

Please sign in to comment.