Merge remote-tracking branch 'tor-github/pr/1462'

This commit is contained in:
teor 2019-10-25 16:14:37 +10:00
commit f40eb2ead6
No known key found for this signature in database
GPG Key ID: 10FEAA0E7075672A
8 changed files with 298 additions and 120 deletions

View File

@ -117,6 +117,8 @@ addons:
- libseccomp-dev - libseccomp-dev
## zstd doesn't exist in Ubuntu Trusty ## zstd doesn't exist in Ubuntu Trusty
#- libzstd #- libzstd
## Optional build dependencies
- coccinelle
- shellcheck - shellcheck
## Conditional build dependencies ## Conditional build dependencies
## Always installed, so we don't need sudo ## Always installed, so we don't need sudo
@ -146,6 +148,7 @@ addons:
- pkg-config - pkg-config
## Optional build dependencies ## Optional build dependencies
- ccache - ccache
- coccinelle
- shellcheck - shellcheck
## Conditional build dependencies ## Conditional build dependencies
## Always installed, because manual brew installs are hard to get right ## Always installed, because manual brew installs are hard to get right
@ -202,6 +205,9 @@ install:
- if [[ "$CHUTNEY" != "" ]]; then pushd "$CHUTNEY_PATH"; git log -1 ; popd ; fi - if [[ "$CHUTNEY" != "" ]]; then pushd "$CHUTNEY_PATH"; git log -1 ; popd ; fi
## If we're running stem, show the stem version and commit ## If we're running stem, show the stem version and commit
- if [[ "$TEST_STEM" != "" ]]; then pushd stem; python -c "from stem import stem; print(stem.__version__);"; git log -1; popd; fi - if [[ "$TEST_STEM" != "" ]]; then pushd stem; python -c "from stem import stem; print(stem.__version__);"; git log -1; popd; fi
## Get the coccinelle version
## Installs are unreliable on macOS, so we just rely on brew list --versions
- if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then spatch --version; fi
## We don't want Tor tests to depend on default configuration file at ## We don't want Tor tests to depend on default configuration file at
## ~/.torrc. So we put some random bytes in there, to make sure we get build ## ~/.torrc. So we put some random bytes in there, to make sure we get build
## failures in case Tor is reading it during CI jobs. ## failures in case Tor is reading it during CI jobs.

View File

@ -158,37 +158,43 @@ include doc/include.am
include contrib/include.am include contrib/include.am
EXTRA_DIST+= \ EXTRA_DIST+= \
ChangeLog \ ChangeLog \
CONTRIBUTING \ CONTRIBUTING \
CODE_OF_CONDUCT \ CODE_OF_CONDUCT \
INSTALL \ INSTALL \
LICENSE \ LICENSE \
Makefile.nmake \ Makefile.nmake \
README \ README \
ReleaseNotes \ ReleaseNotes \
scripts/maint/checkIncludes.py \ scripts/maint/checkIncludes.py \
scripts/maint/checkSpace.pl \ scripts/maint/checkSpace.pl \
scripts/maint/checkShellScripts.sh \ scripts/maint/checkShellScripts.sh \
scripts/maint/practracker/README \ scripts/maint/practracker/README \
scripts/maint/practracker/exceptions.txt \ scripts/maint/practracker/exceptions.txt \
scripts/maint/practracker/includes.py \ scripts/maint/practracker/includes.py \
scripts/maint/practracker/metrics.py \ scripts/maint/practracker/metrics.py \
scripts/maint/practracker/practracker.py \ scripts/maint/practracker/practracker.py \
scripts/maint/practracker/practracker_tests.py \ scripts/maint/practracker/practracker_tests.py \
scripts/maint/practracker/problem.py \ scripts/maint/practracker/problem.py \
scripts/maint/practracker/testdata/.may_include \ scripts/maint/practracker/testdata/.may_include \
scripts/maint/practracker/testdata/a.c \ scripts/maint/practracker/testdata/a.c \
scripts/maint/practracker/testdata/b.c \ scripts/maint/practracker/testdata/b.c \
scripts/maint/practracker/testdata/ex0-expected.txt \ scripts/maint/practracker/testdata/ex0-expected.txt \
scripts/maint/practracker/testdata/ex0.txt \ scripts/maint/practracker/testdata/ex0.txt \
scripts/maint/practracker/testdata/ex1-expected.txt \ scripts/maint/practracker/testdata/ex1-expected.txt \
scripts/maint/practracker/testdata/ex1.txt \ scripts/maint/practracker/testdata/ex1.txt \
scripts/maint/practracker/testdata/ex1-overbroad-expected.txt \ scripts/maint/practracker/testdata/ex1-overbroad-expected.txt \
scripts/maint/practracker/testdata/ex.txt \ scripts/maint/practracker/testdata/ex.txt \
scripts/maint/practracker/testdata/header.h \ scripts/maint/practracker/testdata/header.h \
scripts/maint/practracker/testdata/not_c_file \ scripts/maint/practracker/testdata/not_c_file \
scripts/maint/practracker/test_practracker.sh \ scripts/maint/practracker/test_practracker.sh \
scripts/maint/practracker/util.py scripts/maint/practracker/util.py \
scripts/coccinelle/apply.sh \
scripts/coccinelle/check_cocci_parse.sh \
scripts/coccinelle/exceptions.txt \
scripts/coccinelle/test-operator-cleanup \
scripts/coccinelle/tor-coccinelle.h \
scripts/coccinelle/try_parse.sh
## This tells etags how to find mockable function definitions. ## This tells etags how to find mockable function definitions.
AM_ETAGSFLAGS=--regex='{c}/MOCK_IMPL([^,]+,\W*\([a-zA-Z0-9_]+\)\W*,/\1/s' AM_ETAGSFLAGS=--regex='{c}/MOCK_IMPL([^,]+,\W*\([a-zA-Z0-9_]+\)\W*,/\1/s'
@ -246,7 +252,7 @@ test: all
shellcheck: shellcheck:
$(top_srcdir)/scripts/maint/checkShellScripts.sh $(top_srcdir)/scripts/maint/checkShellScripts.sh
check-local: check-spaces check-changes check-includes check-best-practices shellcheck check-local: check-spaces check-changes check-includes check-best-practices shellcheck check-cocci
need-chutney-path: need-chutney-path:
@if test ! -d "$$CHUTNEY_PATH"; then \ @if test ! -d "$$CHUTNEY_PATH"; then \
@ -379,6 +385,9 @@ if USEPYTHON
@$(PYTHON) $(top_srcdir)/scripts/maint/practracker/practracker.py $(top_srcdir) $(TOR_PRACTRACKER_OPTIONS) @$(PYTHON) $(top_srcdir)/scripts/maint/practracker/practracker.py $(top_srcdir) $(TOR_PRACTRACKER_OPTIONS)
endif endif
check-cocci:
VERBOSE=1 $(top_srcdir)/scripts/coccinelle/check_cocci_parse.sh $(OWNED_TOR_C_FILES)
practracker-regen: practracker-regen:
$(PYTHON) $(top_srcdir)/scripts/maint/practracker/practracker.py --regen $(top_srcdir) $(PYTHON) $(top_srcdir)/scripts/maint/practracker/practracker.py --regen $(top_srcdir)

13
changes/ticket31919 Normal file
View File

@ -0,0 +1,13 @@
o Minor features (developer tools):
- Add a check_cocci_parse.sh script that checks that new code
is parseable by Coccinelle. Add an exceptions file for unparseable
files. Closes ticket 31919.
- Call the check_cocci_parse.sh script from a 'check-cocci' Makefile
target. Closes ticket 31919.
o Minor features (git scripts):
- Call the check_cocci_parse.sh script from the git commit and push hooks.
Closes ticket 31919.
- Skip unmodified source files when doing some existing git hook checks.
Related to ticket 31919.
o Minor features (continuous integration):
- Call the check_cocci_parse.sh script from Travis CI. Closes ticket 31919.

View File

@ -0,0 +1,52 @@
#!/bin/sh
# If we have coccinelle installed, run try_parse.sh on every filename passed
# as an argument. If no filenames are supplied, scan a standard Tor 0.3.5 or
# later directory layout.
#
# Uses the default coccinelle exceptions file, or $TOR_COCCI_EXCEPTIONS_FILE,
# if it is set.
#
# Use TOR_COCCI_EXCEPTIONS_FILE=/dev/null check_cocci_parse.sh to disable
# the default exception file.
#
# If spatch is not installed, remind the user to install it, but exit with
# a success error status.
scripts_cocci="$(dirname "$0")"
top="$scripts_cocci/../.."
try_parse="$scripts_cocci/try_parse.sh"
exitcode=0
export TOR_COCCI_EXCEPTIONS_FILE="${TOR_COCCI_EXCEPTIONS_FILE:-$scripts_cocci/exceptions.txt}"
if ! command -v spatch; then
echo "Install coccinelle's spatch to check cocci C parsing!"
exit "$exitcode"
fi
if test $# -ge 1 ; then
"$try_parse" "$@"
exitcode=$?
else
# This is the layout in 0.3.5
"$try_parse" \
src/lib/*/*.[ch] \
src/core/*/*.[ch] \
src/feature/*/*.[ch] \
src/app/*/*.[ch] \
src/test/*.[ch] \
src/test/*/*.[ch] \
src/tools/*.[ch]
exitcode=$?
fi
if test "$exitcode" != 0 ; then
echo "Please fix these cocci parsing errors in the above files"
echo "Set VERBOSE=1 for more details"
echo "Try running test-operator-cleanup or 'make autostyle-operators'"
echo "As a last resort, you can modify scripts/coccinelle/exceptions.txt"
fi
exit "$exitcode"

View File

@ -0,0 +1,24 @@
# A list of exception patterns for check_cocci_parse.sh
# Passed to 'grep -f'
src/lib/cc/compat_compiler.h
src/lib/container/handles.h
src/lib/container/map.c
src/lib/container/map.h
src/lib/container/order.c
src/lib/crypt_ops/crypto_rand.c
src/lib/fs/files.h
src/lib/log/util_bug.c
src/lib/pubsub/pubsub_macros.h
src/lib/smartlist_core/smartlist_foreach.h
src/lib/testsupport/testsupport.h
src/lib/tls/tortls.h
src/lib/tls/tortls_openssl.c
src/lib/tls/x509.h
src/lib/version/version.c
src/core/mainloop/connection.c
src/core/or/reasons.c
src/feature/dirclient/dirclient.c
src/feature/nodelist/networkstatus.c
src/test/test_address.c
src/test/test_hs_cache.c
src/test/test_hs_descriptor.c

View File

@ -2,6 +2,11 @@
# Echo the name of every argument of this script that is not "perfect" # Echo the name of every argument of this script that is not "perfect"
# according to coccinelle's --parse-c. # according to coccinelle's --parse-c.
#
# If $TOR_COCCI_EXCEPTIONS_FILE is non-empty, skip any files that match the
# patterns in the exception file, according to "grep -f"
#
# If VERBOSE is non-empty, log spatch errors and skipped files.
top="$(dirname "$0")/../.." top="$(dirname "$0")/../.."
@ -9,16 +14,28 @@ exitcode=0
for fn in "$@"; do for fn in "$@"; do
if spatch -macro_file_builtins "$top"/scripts/coccinelle/tor-coccinelle.h \ if test "${TOR_COCCI_EXCEPTIONS_FILE}" ; then
-I "$top" -I "$top"/src -I "$top"/ext --defined COCCI \ skip_fn=$(echo "$fn" | grep -f "${TOR_COCCI_EXCEPTIONS_FILE}")
if test "${skip_fn}" ; then
if test "${VERBOSE}" != ""; then
echo "Skipping '${skip_fn}'"
fi
continue
fi
fi
if spatch --macro-file-builtins \
"$top"/scripts/coccinelle/tor-coccinelle.h \
--defined COCCI \
--parse-c "$fn" \ --parse-c "$fn" \
2>/dev/null | grep "perfect = 1" > /dev/null; then 2>/dev/null | grep "perfect = 1" > /dev/null; then
: # it's perfect : # it's perfect
else else
echo "$fn" echo "$fn"
if test "${VERBOSE}" != ""; then if test "${VERBOSE}" != ""; then
spatch -macro_file_builtins "$top"/scripts/coccinelle/tor-coccinelle.h \ spatch --macro-file-builtins \
-I "$top" -I "$top"/src -I "$top"/ext --defined COCCI \ "$top"/scripts/coccinelle/tor-coccinelle.h \
--defined COCCI \
--parse-c "$fn" --parse-c "$fn"
fi fi
exitcode=1 exitcode=1

View File

@ -13,30 +13,61 @@ cd "$workdir" || exit 1
set -e set -e
if [ $# -eq 0 ]; then
# When called in pre-commit, check the files modified in this commit
CHECK_FILTER="git diff --cached --name-only --diff-filter=ACMR"
# Use the appropriate owned tor source list to filter the changed files
if [ -d src/lib ]; then
# This is the layout in 0.3.5
CHECK_FILES="$($CHECK_FILTER \
src/lib/*/*.[ch] \
src/core/*/*.[ch] \
src/feature/*/*.[ch] \
src/app/*/*.[ch] \
src/test/*.[ch] \
src/test/*/*.[ch] \
src/tools/*.[ch] \
)"
elif [ -d src/common ]; then
# This was the layout before 0.3.5
CHECK_FILES="$($CHECK_FILTER \
src/common/*/*.[ch] \
src/or/*/*.[ch] \
src/test/*.[ch] \
src/test/*/*.[ch] \
src/tools/*.[ch]
)"
fi
else
# When called in pre-push, concatenate the argument array
# Fails on special characters in file names
CHECK_FILES="$*"
fi
## General File Checks
if [ -n "$(ls ./changes/)" ]; then if [ -n "$(ls ./changes/)" ]; then
python scripts/maint/lintChanges.py ./changes/* python scripts/maint/lintChanges.py ./changes/*
fi fi
if [ -d src/lib ]; then if [ -e scripts/maint/checkShellScripts.sh ]; then
# This is the layout in 0.3.5 scripts/maint/checkShellScripts.sh
perl scripts/maint/checkSpace.pl -C \
src/lib/*/*.[ch] \
src/core/*/*.[ch] \
src/feature/*/*.[ch] \
src/app/*/*.[ch] \
src/test/*.[ch] \
src/test/*/*.[ch] \
src/tools/*.[ch]
elif [ -d src/common ]; then
# This was the layout before 0.3.5
perl scripts/maint/checkSpace.pl -C \
src/common/*/*.[ch] \
src/or/*/*.[ch] \
src/test/*.[ch] \
src/test/*/*.[ch] \
src/tools/*.[ch]
fi fi
if [ ! "$CHECK_FILES" ]; then
echo "No modified tor-owned source files, skipping further checks"
exit 0
fi
## Owned Source File Checks
printf "Modified tor-owned source files:\n%s\n" "$CHECK_FILES"
# We want word splitting here, because file names are space separated
# shellcheck disable=SC2086
perl scripts/maint/checkSpace.pl -C \
$CHECK_FILES
if test -e scripts/maint/practracker/includes.py; then if test -e scripts/maint/practracker/includes.py; then
python scripts/maint/practracker/includes.py python scripts/maint/practracker/includes.py
fi fi
@ -54,6 +85,13 @@ if [ -e "${PT_DIR}/practracker.py" ]; then
fi fi
fi fi
if [ -e scripts/maint/checkShellScripts.sh ]; then if [ -e scripts/coccinelle/check_cocci_parse.sh ]; then
scripts/maint/checkShellScripts.sh
# Run a verbose cocci parse check on the changed files
# (spatch is slow, so we don't want to check all the files.)
#
# We want word splitting here, because file names are space separated
# shellcheck disable=SC2086
VERBOSE=1 scripts/coccinelle/check_cocci_parse.sh \
$CHECK_FILES
fi fi

View File

@ -16,91 +16,110 @@
# The following sample script was used as starting point: # The following sample script was used as starting point:
# https://github.com/git/git/blob/master/templates/hooks--pre-push.sample # https://github.com/git/git/blob/master/templates/hooks--pre-push.sample
# Are you adding a new check to the git hooks?
# - Common checks belong in the pre-commit hook
# - Push-only checks belong in the pre-push hook
echo "Running pre-push hook" echo "Running pre-push hook"
z40=0000000000000000000000000000000000000000 z40=0000000000000000000000000000000000000000
upstream_name=${TOR_UPSTREAM_REMOTE_NAME:-"upstream"} upstream_name=${TOR_UPSTREAM_REMOTE_NAME:-"upstream"}
# Are you adding a new check to the git hooks?
# - Common checks belong in the pre-commit hook
# - Push-only checks belong in the pre-push hook
#
# Call the pre-commit hook for the common checks, if it is executable.
workdir=$(git rev-parse --show-toplevel) workdir=$(git rev-parse --show-toplevel)
if [ -x "$workdir/.git/hooks/pre-commit" ]; then
if ! "$workdir"/.git/hooks/pre-commit; then
exit 1
fi
fi
remote="$1" remote="$1"
remote_name=$(git remote --verbose | grep "$2" | awk '{print $1}' | head -n 1) remote_name=$(git remote --verbose | grep "$2" | awk '{print $1}' | head -n 1)
if [[ "$remote_name" != "$upstream_name" ]]; then
echo "Not pushing to upstream - refraining from further checks"
exit 0
fi
ref_is_upstream_branch() { ref_is_upstream_branch() {
if [ "$1" == "refs/heads/master" ] || if [ "$1" == "refs/heads/master" ] ||
[[ "$1" == refs/heads/release-* ]] || [[ "$1" == refs/heads/release-* ]] ||
[[ "$1" == refs/heads/maint-* ]] [[ "$1" == refs/heads/maint-* ]]; then
then return 1
return 1 fi
fi
} }
# shellcheck disable=SC2034 # shellcheck disable=SC2034
while read -r local_ref local_sha remote_ref remote_sha while read -r local_ref local_sha remote_ref remote_sha
do do
if [ "$local_sha" = $z40 ] if [ "$local_sha" = $z40 ]; then
then # Handle delete
# Handle delete :
: else
else if [ "$remote_sha" = $z40 ]; then
if [ "$remote_sha" = $z40 ] # New branch, examine commits not in master
then range="master...$local_sha"
# New branch, examine all commits else
range="$local_sha" # Update to existing branch, examine new commits
else range="$remote_sha..$local_sha"
# Update to existing branch, examine new commits fi
range="$remote_sha..$local_sha"
fi
if (ref_is_upstream_branch "$local_ref" == 0 || # Call the pre-commit hook for the common checks, if it is executable
ref_is_upstream_branch "$remote_ref" == 0) && # Only check the files newly modified in this branch
[ "$local_ref" != "$remote_ref" ] CHECK_FILTER="git diff --name-only --diff-filter=ACMR $range"
then # Use the appropriate owned tor source list to filter the changed files
if [ "$remote" == "origin" ] if [ -d src/lib ]; then
then # This is the layout in 0.3.5
echo >&2 "Not pushing: $local_ref to $remote_ref" CHECK_FILES="$($CHECK_FILTER \
echo >&2 "If you really want to push this, use --no-verify." src/lib/*/*.[ch] \
exit 1 src/core/*/*.[ch] \
else src/feature/*/*.[ch] \
continue src/app/*/*.[ch] \
fi src/test/*.[ch] \
fi src/test/*/*.[ch] \
src/tools/*.[ch] \
)"
elif [ -d src/common ]; then
# This was the layout before 0.3.5
CHECK_FILES="$($CHECK_FILTER \
src/common/*/*.[ch] \
src/or/*/*.[ch] \
src/test/*.[ch] \
src/test/*/*.[ch] \
src/tools/*.[ch]
)"
fi
# Check for fixup! commit # We want word splitting here, because file names are space separated
commit=$(git rev-list -n 1 --grep '^fixup!' "$range") # shellcheck disable=SC2086
if [ -n "$commit" ] if ! "$workdir/"scripts/git/pre-commit.git-hook $CHECK_FILES; then
then exit 1
echo >&2 "Found fixup! commit in $local_ref, not pushing" fi
echo >&2 "If you really want to push this, use --no-verify."
exit 1
fi
# Check for squash! commit if [[ "$remote_name" != "$upstream_name" ]]; then
commit=$(git rev-list -n 1 --grep '^squash!' "$range") echo "Not pushing to upstream - refraining from further checks"
if [ -n "$commit" ] continue
then fi
echo >&2 "Found squash! commit in $local_ref, not pushing"
echo >&2 "If you really want to push this, use --no-verify." if (ref_is_upstream_branch "$local_ref" == 0 ||
exit 1 ref_is_upstream_branch "$remote_ref" == 0) &&
fi [ "$local_ref" != "$remote_ref" ]; then
fi if [ "$remote" == "origin" ]; then
echo >&2 "Not pushing: $local_ref to $remote_ref"
echo >&2 "If you really want to push this, use --no-verify."
exit 1
else
continue
fi
fi
# Check for fixup! commit
commit=$(git rev-list -n 1 --grep '^fixup!' "$range")
if [ -n "$commit" ]; then
echo >&2 "Found fixup! commit in $local_ref, not pushing"
echo >&2 "If you really want to push this, use --no-verify."
exit 1
fi
# Check for squash! commit
commit=$(git rev-list -n 1 --grep '^squash!' "$range")
if [ -n "$commit" ]; then
echo >&2 "Found squash! commit in $local_ref, not pushing"
echo >&2 "If you really want to push this, use --no-verify."
exit 1
fi
fi
done done
exit 0 exit 0