diff --git a/.circleci/config.yml b/.circleci/config.yml index f835ccdf..0d6bdc44 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -10,6 +10,7 @@ workflows: - tsan - msan # Static analysis + - clang-analyze - clang-tidy - infer - static-analysis @@ -21,21 +22,21 @@ jobs: - image: ubuntu steps: - - checkout - run: &apt_install apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends clang cmake + git libconfig-dev libgtest-dev libopus-dev libsodium-dev libvpx-dev - llvm-dev ninja-build pkg-config + - checkout - run: CC=clang .circleci/cmake-asan tsan: @@ -44,8 +45,8 @@ jobs: - image: ubuntu steps: - - checkout - run: *apt_install + - checkout - run: CC=clang .circleci/cmake-tsan msan: @@ -65,17 +66,16 @@ jobs: - image: toxchat/infer steps: - - run: - apt-get update && - DEBIAN_FRONTEND=noninteractive - apt-get install -y --no-install-recommends - git - libopus-dev - libsodium-dev - libvpx-dev - pkg-config + - run: *apt_install - checkout - - run: infer --no-progress-bar -- cc toxav/*.c toxcore/*.c $(pkg-config --cflags opus vpx) + - run: infer --no-progress-bar -- cc + auto_tests/lossless_packet_test.c + testing/misc_tools.c + toxav/*.c + toxcore/*.c + toxencryptsave/*.c + -lpthread + $(pkg-config --cflags --libs libsodium opus vpx) static-analysis: working_directory: ~/work @@ -83,46 +83,35 @@ jobs: - image: ubuntu steps: + - run: *apt_install + - run: apt-get install -y --no-install-recommends cppcheck g++ llvm-dev - checkout - - run: - apt-get update && - DEBIAN_FRONTEND=noninteractive - apt-get install -y --no-install-recommends - clang - cppcheck - g++ - libconfig-dev - libgtest-dev - libopus-dev - libsodium-dev - libvpx-dev - llvm - run: other/analysis/check_logger_levels - run: other/analysis/run-check-recursion - run: other/analysis/run-clang - - run: other/analysis/run-clang-analyze - run: other/analysis/run-cppcheck - run: other/analysis/run-gcc + clang-analyze: + working_directory: ~/work + docker: + - image: ubuntu + + steps: + - run: *apt_install + - checkout + - run: other/analysis/run-clang-analyze + clang-tidy: working_directory: ~/work docker: - image: ubuntu steps: + - run: *apt_install + - run: apt-get install -y --no-install-recommends clang-tidy-12 - checkout - - run: - apt-get update && - DEBIAN_FRONTEND=noninteractive - apt-get install -y --no-install-recommends - build-essential - clang-tidy-11 - cmake - libconfig-dev - libopus-dev - libsodium-dev - libvpx-dev - - run: cmake . -B_build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON + - run: cmake . -B_build -GNinja -DCMAKE_EXPORT_COMPILE_COMMANDS=ON - run: other/analysis/run-clang-tidy || other/analysis/run-clang-tidy || diff --git a/.cirrus.yml b/.cirrus.yml index 56eb4ec0..86e200ea 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -50,6 +50,7 @@ bazel-asan_task: //c-toxcore/... -//c-toxcore/auto_tests:tcp_relay_test # TODO(robinlinden): Why does this pass locally but not in Cirrus? +# TODO(iphydf): Remove "|| true" once this works properly. bazel-msan_task: container: image: toxchat/toktok-stack:0.0.31-msan diff --git a/.github/settings.yml b/.github/settings.yml index 8c5ddf9e..64e7d1d5 100644 --- a/.github/settings.yml +++ b/.github/settings.yml @@ -14,6 +14,7 @@ branches: contexts: - "bazel-asan" - "bazel-debug" + - "bazel-msan" - "bazel-release" - "bazel-tsan" - "build-bootstrapd-docker" @@ -26,6 +27,7 @@ branches: - "CodeFactor" - "coverage-linux" - "ci/circleci: asan" + - "ci/circleci: clang-analyze" - "ci/circleci: clang-tidy" - "ci/circleci: infer" - "ci/circleci: static-analysis" diff --git a/.restyled.yaml b/.restyled.yaml index c7d4a6a3..8de1656a 100644 --- a/.restyled.yaml +++ b/.restyled.yaml @@ -1,6 +1,8 @@ --- exclude: - "**/*.api.h" + # shfmt doesn't support this file + - "other/analysis/run-clang-tidy" restylers: - astyle: diff --git a/auto_tests/encryptsave_test.c b/auto_tests/encryptsave_test.c index 3387d544..54291ac7 100644 --- a/auto_tests/encryptsave_test.c +++ b/auto_tests/encryptsave_test.c @@ -3,11 +3,8 @@ #include #include -#ifdef VANILLA_NACL -#include "../toxencryptsave/crypto_pwhash_scryptsalsa208sha256/crypto_pwhash_scryptsalsa208sha256.h" -#else +#ifndef VANILLA_NACL #include -#endif #include "../testing/misc_tools.h" #include "../toxcore/ccompat.h" @@ -217,3 +214,9 @@ int main(void) return 0; } +#else // VANILLA_NACL +int main(void) +{ + return 0; +} +#endif diff --git a/other/analysis/gen-file.sh b/other/analysis/gen-file.sh index b61e90af..17ff8b95 100644 --- a/other/analysis/gen-file.sh +++ b/other/analysis/gen-file.sh @@ -58,6 +58,10 @@ FIND_QUERY="$FIND_QUERY -and -not -name trace.cc" FIND_QUERY="$FIND_QUERY -and -not -name version_test.c" FIND_QUERY="$FIND_QUERY -and -not -wholename './testing/fuzzing/*'" +if [ "$SKIP_GTEST" == 1 ]; then + FIND_QUERY="$FIND_QUERY -and -not -name '*_test.cc'" +fi + readarray -t FILES <<<"$(eval "$FIND_QUERY")" (for i in "${FILES[@]}"; do diff --git a/other/analysis/run-check-recursion b/other/analysis/run-check-recursion index 741bac61..82bc6a9f 100755 --- a/other/analysis/run-check-recursion +++ b/other/analysis/run-check-recursion @@ -1,5 +1,7 @@ #!/bin/sh +set -e + cat toxav/*.c toxcore/*.c toxencryptsave/*.c | clang "$(pkg-config --cflags libsodium opus vpx)" \ -Itoxav -Itoxcore -Itoxencryptsave -S -emit-llvm -xc - -o- | diff --git a/other/analysis/run-clang b/other/analysis/run-clang index f1430770..b009455e 100755 --- a/other/analysis/run-clang +++ b/other/analysis/run-clang @@ -2,27 +2,34 @@ . other/analysis/gen-file.sh -echo "Running Clang compiler" -clang++ -o /dev/null amalgamation.cc \ - "${CPPFLAGS[@]}" \ - "${LDFLAGS[@]}" \ - -std=c++11 \ - -Werror \ - -Weverything \ - -Wno-alloca \ - -Wno-c++98-compat-pedantic \ - -Wno-c99-extensions \ - -Wno-conversion \ - -Wno-covered-switch-default \ - -Wno-disabled-macro-expansion \ - -Wno-documentation-deprecated-sync \ - -Wno-global-constructors \ - -Wno-missing-braces \ - -Wno-missing-field-initializers \ - -Wno-old-style-cast \ - -Wno-padded \ - -Wno-sign-compare \ - -Wno-unreachable-code-return \ - -Wno-unused-parameter \ - -Wno-used-but-marked-unused \ - -Wno-source-uses-openmp +set -e + +run() { + echo "Running Clang compiler in variant '$*'" + clang++ -o /dev/null amalgamation.cc \ + "${CPPFLAGS[@]}" \ + "${LDFLAGS[@]}" \ + "$@" \ + -std=c++11 \ + -Werror \ + -Weverything \ + -Wno-alloca \ + -Wno-c++98-compat-pedantic \ + -Wno-c99-extensions \ + -Wno-conversion \ + -Wno-covered-switch-default \ + -Wno-disabled-macro-expansion \ + -Wno-documentation-deprecated-sync \ + -Wno-global-constructors \ + -Wno-missing-braces \ + -Wno-missing-field-initializers \ + -Wno-old-style-cast \ + -Wno-padded \ + -Wno-sign-compare \ + -Wno-unreachable-code-return \ + -Wno-unused-parameter \ + -Wno-used-but-marked-unused \ + -Wno-source-uses-openmp +} + +. other/analysis/variants.sh diff --git a/other/analysis/run-clang-analyze b/other/analysis/run-clang-analyze index d790de00..05e94e78 100755 --- a/other/analysis/run-clang-analyze +++ b/other/analysis/run-clang-analyze @@ -2,7 +2,14 @@ . other/analysis/gen-file.sh -echo "Running Clang static analyzer" -clang++ --analyze amalgamation.cc \ - "${CPPFLAGS[@]}" \ - -std=c++11 +set -e + +run() { + echo "Running Clang static analyzer in variant '$*'" + clang++ --analyze amalgamation.cc \ + "${CPPFLAGS[@]}" \ + "$@" \ + -std=c++11 +} + +. other/analysis/variants.sh diff --git a/other/analysis/run-clang-tidy b/other/analysis/run-clang-tidy index 29cd4ab8..a847320f 100755 --- a/other/analysis/run-clang-tidy +++ b/other/analysis/run-clang-tidy @@ -1,6 +1,9 @@ -#!/bin/sh +#!/bin/bash +# TODO(iphydf): We might want some of these. For the ones we don't want, add a +# comment explaining why not. CHECKS="*" +CHECKS="$CHECKS,-altera-unroll-loops" CHECKS="$CHECKS,-android-cloexec-accept" CHECKS="$CHECKS,-android-cloexec-fopen" CHECKS="$CHECKS,-bugprone-not-null-terminated-result" @@ -18,24 +21,31 @@ CHECKS="$CHECKS,-llvmlibc-restrict-system-libc-headers" CHECKS="$CHECKS,-misc-redundant-expression" CHECKS="$CHECKS,-misc-unused-parameters" CHECKS="$CHECKS,-readability-else-after-return" +CHECKS="$CHECKS,-readability-function-cognitive-complexity" CHECKS="$CHECKS,-readability-inconsistent-declaration-parameter-name" CHECKS="$CHECKS,-readability-magic-numbers" CHECKS="$CHECKS,-readability-redundant-control-flow" +# TODO(iphydf): Maybe fix these? +CHECKS="$CHECKS,-altera-id-dependent-backward-branch" +CHECKS="$CHECKS,-altera-struct-pack-align" +CHECKS="$CHECKS,-bugprone-branch-clone" +CHECKS="$CHECKS,-bugprone-easily-swappable-parameters" +CHECKS="$CHECKS,-bugprone-implicit-widening-of-multiplication-result" +CHECKS="$CHECKS,-bugprone-integer-division" +CHECKS="$CHECKS,-bugprone-narrowing-conversions" +CHECKS="$CHECKS,-clang-analyzer-core.NonNullParamChecker" +CHECKS="$CHECKS,-clang-analyzer-core.NullDereference" +CHECKS="$CHECKS,-clang-analyzer-optin.portability.UnixAPI" +CHECKS="$CHECKS,-clang-analyzer-unix.Malloc" +CHECKS="$CHECKS,-clang-analyzer-valist.Uninitialized" +CHECKS="$CHECKS,-concurrency-mt-unsafe" +CHECKS="$CHECKS,-cppcoreguidelines-avoid-non-const-global-variables" +CHECKS="$CHECKS,-cppcoreguidelines-narrowing-conversions" +CHECKS="$CHECKS,-google-readability-casting" +CHECKS="$CHECKS,-misc-no-recursion" + ERRORS="*" -# TODO(iphydf): Maybe fix these? Otherwise don't show them, if they are useless. -ERRORS="$ERRORS,-bugprone-branch-clone" -ERRORS="$ERRORS,-bugprone-integer-division" -ERRORS="$ERRORS,-bugprone-narrowing-conversions" -ERRORS="$ERRORS,-clang-analyzer-core.NonNullParamChecker" -ERRORS="$ERRORS,-clang-analyzer-core.NullDereference" -ERRORS="$ERRORS,-clang-analyzer-optin.portability.UnixAPI" -ERRORS="$ERRORS,-clang-analyzer-unix.Malloc" -ERRORS="$ERRORS,-clang-analyzer-valist.Uninitialized" -ERRORS="$ERRORS,-cppcoreguidelines-avoid-non-const-global-variables" -ERRORS="$ERRORS,-cppcoreguidelines-narrowing-conversions" -ERRORS="$ERRORS,-google-readability-casting" -ERRORS="$ERRORS,-misc-no-recursion" # TODO(iphydf): Fix these. ERRORS="$ERRORS,-bugprone-macro-parentheses" @@ -45,19 +55,29 @@ ERRORS="$ERRORS,-cert-err34-c" ERRORS="$ERRORS,-cert-str34-c" ERRORS="$ERRORS,-clang-analyzer-security.insecureAPI.strcpy" ERRORS="$ERRORS,-hicpp-uppercase-literal-suffix" +ERRORS="$ERRORS,-readability-suspicious-call-argument" ERRORS="$ERRORS,-readability-uppercase-literal-suffix" set -eux -clang-tidy-11 \ - -p=_build \ - --extra-arg=-DMIN_LOGGER_LEVEL=LOGGER_LEVEL_TRACE \ - --checks="$CHECKS" \ - --warnings-as-errors="$ERRORS" \ - --use-color \ - other/bootstrap_daemon/src/*.c \ - other/*.c \ - toxav/*.c \ - toxcore/*.c \ - toxencryptsave/*.c \ - "$@" +run() { + echo "Running clang-tidy in variant '$*'" + EXTRA_ARGS=("$@") + for i in "${!EXTRA_ARGS[@]}"; do + EXTRA_ARGS[$i]="--extra-arg=${EXTRA_ARGS[$i]}" + done + clang-tidy-12 \ + -p=_build \ + --extra-arg=-DMIN_LOGGER_LEVEL=LOGGER_LEVEL_TRACE \ + "${EXTRA_ARGS[@]}" \ + --checks="$CHECKS" \ + --warnings-as-errors="$ERRORS" \ + --use-color \ + other/bootstrap_daemon/src/*.c \ + other/*.c \ + toxav/*.c \ + toxcore/*.c \ + toxencryptsave/*.c +} + +. other/analysis/variants.sh diff --git a/other/analysis/run-cppcheck b/other/analysis/run-cppcheck index 17d0ee25..871f08eb 100755 --- a/other/analysis/run-cppcheck +++ b/other/analysis/run-cppcheck @@ -1,5 +1,14 @@ #!/bin/bash +SKIP_GTEST=1 + . other/analysis/gen-file.sh -cppcheck amalgamation.cc "${CPPFLAGS[@]}" +set -e + +run() { + echo "Running cppcheck in variant '$*'" + cppcheck amalgamation.cc "${CPPFLAGS[@]}" "$@" +} + +. other/analysis/variants.sh diff --git a/other/analysis/run-gcc b/other/analysis/run-gcc index 5fe0fe68..05a89038 100755 --- a/other/analysis/run-gcc +++ b/other/analysis/run-gcc @@ -2,64 +2,71 @@ . other/analysis/gen-file.sh -echo "Running GCC" -# TODO(iphydf): Get rid of all VLAs, then enable -fstack-protector -Wstack-protector -g++ -O3 -o /dev/null amalgamation.cc \ - "${CPPFLAGS[@]}" \ - "${LDFLAGS[@]}" \ - -std=c++11 \ - -fdiagnostics-color=always \ - -Wall \ - -Wextra \ - -Werror \ - -Wno-aggressive-loop-optimizations \ - -Wno-error=null-dereference \ - -Wno-error=type-limits \ - -Wno-float-conversion \ - -Wno-format-signedness \ - -Wno-missing-field-initializers \ - -Wno-padded \ - -Wno-sign-compare \ - -Wno-sign-conversion \ - -Wno-switch-default \ - -Wno-unused-parameter \ - -Wstrict-aliasing=0 \ - -Wstrict-overflow=1 \ - \ - -Wmissing-declarations \ - -Wbool-compare \ - -Wcast-align \ - -Wcast-qual \ - -Wchar-subscripts \ - -Wdouble-promotion \ - -Wduplicated-cond \ - -Wempty-body \ - -Wenum-compare \ - -Wfloat-equal \ - -Wformat=2 \ - -Wframe-address \ - -Wframe-larger-than=133168 \ - -Wignored-qualifiers \ - -Wignored-attributes \ - -Winit-self \ - -Winline \ - -Wlarger-than=133120 \ - -Wmaybe-uninitialized \ - -Wmemset-transposed-args \ - -Wmisleading-indentation \ - -Wnonnull \ - -Wnonnull-compare \ - -Wnull-dereference \ - -Wodr \ - -Wredundant-decls \ - -Wreturn-type \ - -Wshadow \ - -Wsuggest-attribute=format \ - -Wundef \ - -Wunsafe-loop-optimizations \ - -Wunused-label \ - -Wunused-local-typedefs \ - -Wunused-value \ - -Wunused-but-set-parameter \ - -Wunused-but-set-variable \ - -fopenmp +set -e + +run() { + echo "Running GCC in variant '$*'" + # TODO(iphydf): Get rid of all VLAs, then enable -fstack-protector -Wstack-protector + g++ -O3 -o /dev/null amalgamation.cc \ + "${CPPFLAGS[@]}" \ + "${LDFLAGS[@]}" \ + "$@" \ + -std=c++11 \ + -fdiagnostics-color=always \ + -Wall \ + -Wextra \ + -Werror \ + -Wno-error=null-dereference \ + -Wno-error=type-limits \ + -Wno-aggressive-loop-optimizations \ + -Wno-float-conversion \ + -Wno-format-signedness \ + -Wno-missing-field-initializers \ + -Wno-padded \ + -Wno-sign-compare \ + -Wno-sign-conversion \ + -Wno-switch-default \ + -Wno-unused-parameter \ + -Wstrict-aliasing=0 \ + -Wstrict-overflow=1 \ + \ + -Wmissing-declarations \ + -Wbool-compare \ + -Wcast-align \ + -Wcast-qual \ + -Wchar-subscripts \ + -Wdouble-promotion \ + -Wduplicated-cond \ + -Wempty-body \ + -Wenum-compare \ + -Wfloat-equal \ + -Wformat=2 \ + -Wframe-address \ + -Wframe-larger-than=133168 \ + -Wignored-qualifiers \ + -Wignored-attributes \ + -Winit-self \ + -Winline \ + -Wlarger-than=133120 \ + -Wmaybe-uninitialized \ + -Wmemset-transposed-args \ + -Wmisleading-indentation \ + -Wnonnull \ + -Wnonnull-compare \ + -Wnull-dereference \ + -Wodr \ + -Wredundant-decls \ + -Wreturn-type \ + -Wshadow \ + -Wsuggest-attribute=format \ + -Wundef \ + -Wunsafe-loop-optimizations \ + -Wunused-label \ + -Wunused-local-typedefs \ + -Wunused-value \ + -Wunused-but-set-parameter \ + -Wunused-but-set-variable \ + -fopenmp +} + +. other/analysis/variants.sh diff --git a/other/analysis/run-infer b/other/analysis/run-infer index 0d9e33c6..43d66b04 100755 --- a/other/analysis/run-infer +++ b/other/analysis/run-infer @@ -3,6 +3,8 @@ # Infer ignores everything that's not in the "current file". SKIP_LINES=1 +set -e + . other/analysis/gen-file.sh infer --no-progress-bar -- clang++ -fsyntax-only amalgamation.cc "${CPPFLAGS[@]}" diff --git a/other/analysis/variants.sh b/other/analysis/variants.sh new file mode 100644 index 00000000..99570452 --- /dev/null +++ b/other/analysis/variants.sh @@ -0,0 +1,4 @@ +#!/bin/bash + +run +run -DVANILLA_NACL -I/usr/include/sodium diff --git a/toxcore/crypto_core.c b/toxcore/crypto_core.c index b044ec5c..3f029ef7 100644 --- a/toxcore/crypto_core.c +++ b/toxcore/crypto_core.c @@ -26,6 +26,9 @@ #include #include #include +#endif + +#ifndef crypto_box_MACBYTES #define crypto_box_MACBYTES (crypto_box_ZEROBYTES - crypto_box_BOXZEROBYTES) #endif