From fb67bda00e18ef4529695c1b5367a8ba884916b4 Mon Sep 17 00:00:00 2001 From: Maxime Desroches Date: Wed, 25 Sep 2024 20:14:12 -0700 Subject: [PATCH 1/4] compile libpanda with clang (#2041) * clang * report * safety.h * fix lines * cleaner * better * remove this * revert this * fix * tici build * revert * revert * fix --- .gitignore | 3 +++ Dockerfile | 4 +++- README.md | 2 +- SConstruct | 4 ---- board/safety/safety_ford.h | 28 +++++++++++------------- board/safety/safety_gm.h | 5 ++--- board/safety/safety_hyundai.h | 25 +++++++++++++--------- board/safety/safety_hyundai_common.h | 5 ++--- board/safety/safety_volkswagen_mqb.h | 16 ++++++++------ board/safety/safety_volkswagen_pq.h | 4 ++-- tests/libpanda/SConscript | 32 +++++++++++++--------------- tests/safety/test.sh | 28 ++++++++++++------------ 12 files changed, 79 insertions(+), 77 deletions(-) diff --git a/.gitignore b/.gitignore index 640f25f8e0..8f7cb049b9 100644 --- a/.gitignore +++ b/.gitignore @@ -29,3 +29,6 @@ nosetests.xml *.gcno tests/safety/coverage-out tests/safety/coverage.info + +*.profraw +*.profdata diff --git a/Dockerfile b/Dockerfile index 5d3c1ffc1d..a6f94826da 100644 --- a/Dockerfile +++ b/Dockerfile @@ -18,7 +18,9 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ apt clean && \ cd /usr/lib/gcc/arm-none-eabi/* && \ rm -rf arm/ && \ - rm -rf thumb/nofp thumb/v6* thumb/v8* thumb/v7+fp thumb/v7-r+fp.sp + rm -rf thumb/nofp thumb/v6* thumb/v8* thumb/v7+fp thumb/v7-r+fp.sp && \ + apt-get update && apt-get install -y clang-17 && \ + ln -s $(which clang-17) /usr/bin/clang ENV CPPCHECK_DIR=/tmp/cppcheck COPY tests/misra/install.sh /tmp/ diff --git a/README.md b/README.md index 618a5b70d6..854e6a2522 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ In addition, we run the [ruff linter](https://github.com/astral-sh/ruff) and [my Setup dependencies: ```bash # Ubuntu -sudo apt-get install dfu-util gcc-arm-none-eabi python3-pip libffi-dev git +sudo apt-get install dfu-util gcc-arm-none-eabi python3-pip libffi-dev git clang-17 # macOS brew install --cask gcc-arm-embedded diff --git a/SConstruct b/SConstruct index aa13f59229..8a7609afe1 100644 --- a/SConstruct +++ b/SConstruct @@ -8,10 +8,6 @@ AddOption('--ubsan', action='store_true', help='turn on UBSan') -AddOption('--coverage', - action='store_true', - help='build with test coverage options') - AddOption('--compile_db', action='store_true', help='build clang compilation database') diff --git a/board/safety/safety_ford.h b/board/safety/safety_ford.h index 3ab8827b6e..50ff6f7139 100644 --- a/board/safety/safety_ford.h +++ b/board/safety/safety_ford.h @@ -28,10 +28,10 @@ static uint8_t ford_get_counter(const CANPacket_t *to_push) { if (addr == FORD_BrakeSysFeatures) { // Signal: VehVActlBrk_No_Cnt cnt = (GET_BYTE(to_push, 2) >> 2) & 0xFU; - } else if (addr == FORD_Yaw_Data_FD1) { + } + if (addr == FORD_Yaw_Data_FD1) { // Signal: VehRollYaw_No_Cnt cnt = GET_BYTE(to_push, 5); - } else { } return cnt; } @@ -43,10 +43,10 @@ static uint32_t ford_get_checksum(const CANPacket_t *to_push) { if (addr == FORD_BrakeSysFeatures) { // Signal: VehVActlBrk_No_Cs chksum = GET_BYTE(to_push, 3); - } else if (addr == FORD_Yaw_Data_FD1) { + } + if (addr == FORD_Yaw_Data_FD1) { // Signal: VehRollYawW_No_Cs chksum = GET_BYTE(to_push, 4); - } else { } return chksum; } @@ -60,14 +60,14 @@ static uint32_t ford_compute_checksum(const CANPacket_t *to_push) { chksum += GET_BYTE(to_push, 2) >> 6; // VehVActlBrk_D_Qf chksum += (GET_BYTE(to_push, 2) >> 2) & 0xFU; // VehVActlBrk_No_Cnt chksum = 0xFFU - chksum; - } else if (addr == FORD_Yaw_Data_FD1) { + } + if (addr == FORD_Yaw_Data_FD1) { chksum += GET_BYTE(to_push, 0) + GET_BYTE(to_push, 1); // VehRol_W_Actl chksum += GET_BYTE(to_push, 2) + GET_BYTE(to_push, 3); // VehYaw_W_Actl chksum += GET_BYTE(to_push, 5); // VehRollYaw_No_Cnt chksum += GET_BYTE(to_push, 6) >> 6; // VehRolWActl_D_Qf chksum += (GET_BYTE(to_push, 6) >> 4) & 0x3U; // VehYawWActl_D_Qf chksum = 0xFFU - chksum; - } else { } return chksum; @@ -79,11 +79,12 @@ static bool ford_get_quality_flag_valid(const CANPacket_t *to_push) { bool valid = false; if (addr == FORD_BrakeSysFeatures) { valid = (GET_BYTE(to_push, 2) >> 6) == 0x3U; // VehVActlBrk_D_Qf - } else if (addr == FORD_EngVehicleSpThrottle2) { + } + if (addr == FORD_EngVehicleSpThrottle2) { valid = ((GET_BYTE(to_push, 4) >> 5) & 0x3U) == 0x3U; // VehVActlEng_D_Qf - } else if (addr == FORD_Yaw_Data_FD1) { + } + if (addr == FORD_Yaw_Data_FD1) { valid = ((GET_BYTE(to_push, 6) >> 4) & 0x3U) == 0x3U; // VehYawWActl_D_Qf - } else { } return valid; } @@ -316,8 +317,7 @@ static int ford_fwd_hook(int bus_num, int addr) { // Forward all traffic from bus 0 onward bus_fwd = FORD_CAM_BUS; break; - } - case FORD_CAM_BUS: { + } case FORD_CAM_BUS: { if (ford_lkas_msg_check(addr)) { // Block stock LKAS and UI messages bus_fwd = -1; @@ -329,12 +329,10 @@ static int ford_fwd_hook(int bus_num, int addr) { bus_fwd = FORD_MAIN_BUS; } break; - } - default: { + } default: { // No other buses should be in use; fallback to do-not-forward bus_fwd = -1; - break; - } + break;} } return bus_fwd; diff --git a/board/safety/safety_gm.h b/board/safety/safety_gm.h index f0902a921a..10cb21f32c 100644 --- a/board/safety/safety_gm.h +++ b/board/safety/safety_gm.h @@ -227,12 +227,11 @@ static safety_config gm_init(uint16_t param) { {0x1E1, 2, 7}, {0x184, 2, 8}}; // camera bus gm_hw = GET_FLAG(param, GM_PARAM_HW_CAM) ? GM_CAM : GM_ASCM; - if (gm_hw == GM_ASCM) { gm_long_limits = &GM_ASCM_LONG_LIMITS; - } else if (gm_hw == GM_CAM) { + } + if (gm_hw == GM_CAM) { gm_long_limits = &GM_CAM_LONG_LIMITS; - } else { } #ifdef ALLOW_DEBUG diff --git a/board/safety/safety_hyundai.h b/board/safety/safety_hyundai.h index 3e9d23216c..cc2fd8c6b0 100644 --- a/board/safety/safety_hyundai.h +++ b/board/safety/safety_hyundai.h @@ -49,16 +49,19 @@ static uint8_t hyundai_get_counter(const CANPacket_t *to_push) { uint8_t cnt = 0; if (addr == 0x260) { cnt = (GET_BYTE(to_push, 7) >> 4) & 0x3U; - } else if (addr == 0x386) { + } + if (addr == 0x386) { cnt = ((GET_BYTE(to_push, 3) >> 6) << 2) | (GET_BYTE(to_push, 1) >> 6); - } else if (addr == 0x394) { + } + if (addr == 0x394) { cnt = (GET_BYTE(to_push, 1) >> 5) & 0x7U; - } else if (addr == 0x421) { + } + if (addr == 0x421) { cnt = GET_BYTE(to_push, 7) & 0xFU; - } else if (addr == 0x4F1) { - cnt = (GET_BYTE(to_push, 3) >> 4) & 0xFU; - } else { } + if (addr == 0x4F1) { + cnt = (GET_BYTE(to_push, 3) >> 4) & 0xFU; + } return cnt; } @@ -68,13 +71,15 @@ static uint32_t hyundai_get_checksum(const CANPacket_t *to_push) { uint8_t chksum = 0; if (addr == 0x260) { chksum = GET_BYTE(to_push, 7) & 0xFU; - } else if (addr == 0x386) { + } + if (addr == 0x386) { chksum = ((GET_BYTE(to_push, 7) >> 6) << 2) | (GET_BYTE(to_push, 5) >> 6); - } else if (addr == 0x394) { + } + if (addr == 0x394) { chksum = GET_BYTE(to_push, 6) & 0xFU; - } else if (addr == 0x421) { + } + if (addr == 0x421) { chksum = GET_BYTE(to_push, 7) >> 4; - } else { } return chksum; } diff --git a/board/safety/safety_hyundai_common.h b/board/safety/safety_hyundai_common.h index d83b396401..ee2ee32f2f 100644 --- a/board/safety/safety_hyundai_common.h +++ b/board/safety/safety_hyundai_common.h @@ -118,10 +118,9 @@ uint32_t hyundai_common_canfd_compute_checksum(const CANPacket_t *to_push) { if (len == 24) { crc ^= 0x819dU; - } else if (len == 32) { + } + if (len == 32) { crc ^= 0x9f5bU; - } else { - } return crc; diff --git a/board/safety/safety_volkswagen_mqb.h b/board/safety/safety_volkswagen_mqb.h index 40f7cbccc9..1484cd5b07 100644 --- a/board/safety/safety_volkswagen_mqb.h +++ b/board/safety/safety_volkswagen_mqb.h @@ -45,17 +45,19 @@ static uint32_t volkswagen_mqb_compute_crc(const CANPacket_t *to_push) { uint8_t counter = volkswagen_mqb_get_counter(to_push); if (addr == MSG_LH_EPS_03) { crc ^= (uint8_t[]){0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5}[counter]; - } else if (addr == MSG_ESP_05) { + } + if (addr == MSG_ESP_05) { crc ^= (uint8_t[]){0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07}[counter]; - } else if (addr == MSG_TSK_06) { + } + if (addr == MSG_TSK_06) { crc ^= (uint8_t[]){0xC4,0xE2,0x4F,0xE4,0xF8,0x2F,0x56,0x81,0x9F,0xE5,0x83,0x44,0x05,0x3F,0x97,0xDF}[counter]; - } else if (addr == MSG_MOTOR_20) { + } + if (addr == MSG_MOTOR_20) { crc ^= (uint8_t[]){0xE9,0x65,0xAE,0x6B,0x7B,0x35,0xE5,0x5F,0x4E,0xC7,0x86,0xA2,0xBB,0xDD,0xEB,0xB4}[counter]; - } else if (addr == MSG_GRA_ACC_01) { + } + if (addr == MSG_GRA_ACC_01) { crc ^= (uint8_t[]){0x6A,0x38,0xB4,0x27,0x22,0xEF,0xE1,0xBB,0xF8,0x80,0x84,0x49,0xC7,0x9E,0x1E,0x2B}[counter]; - } else { - // Undefined CAN message, CRC check expected to fail - } + } crc = volkswagen_crc8_lut_8h2f[crc]; return (uint8_t)(crc ^ 0xFFU); diff --git a/board/safety/safety_volkswagen_pq.h b/board/safety/safety_volkswagen_pq.h index 75979a5149..8fc811eba1 100644 --- a/board/safety/safety_volkswagen_pq.h +++ b/board/safety/safety_volkswagen_pq.h @@ -26,9 +26,9 @@ static uint8_t volkswagen_pq_get_counter(const CANPacket_t *to_push) { if (addr == MSG_LENKHILFE_3) { counter = (uint8_t)(GET_BYTE(to_push, 1) & 0xF0U) >> 4; - } else if (addr == MSG_GRA_NEU) { + } + if (addr == MSG_GRA_NEU) { counter = (uint8_t)(GET_BYTE(to_push, 2) & 0xF0U) >> 4; - } else { } return counter; diff --git a/tests/libpanda/SConscript b/tests/libpanda/SConscript index cc08907308..66dd563a75 100644 --- a/tests/libpanda/SConscript +++ b/tests/libpanda/SConscript @@ -1,14 +1,13 @@ +import os import platform -CC = 'gcc' -system = platform.system() -if system == 'Darwin': - # gcc installed by homebrew has version suffix (e.g. gcc-12) in order to be - # distinguishable from system one - which acts as a symlink to clang - CC += '-13' +COVERAGE_FLAGS = [ + '-fprofile-instr-generate', + '-fcoverage-mapping', +] env = Environment( - CC=CC, + CC='clang', CFLAGS=[ '-nostdlib', '-fno-builtin', @@ -18,9 +17,17 @@ env = Environment( ], CPPPATH=[".", "../../board/"], ) -if system == "Darwin": +if platform.system() == "Darwin": env.PrependENVPath('PATH', '/opt/homebrew/bin') +TICI = os.path.isfile('/TICI') +AGNOS = TICI +if AGNOS: + env['CC'] = 'gcc' +else: + env['CFLAGS'] += COVERAGE_FLAGS + env['LINKFLAGS'] += COVERAGE_FLAGS + if GetOption('ubsan'): flags = [ "-fsanitize=undefined", @@ -31,12 +38,3 @@ if GetOption('ubsan'): panda = env.SharedObject("panda.os", "panda.c") libpanda = env.SharedLibrary("libpanda.so", [panda]) - -if GetOption('coverage'): - env.Append( - CFLAGS=["-fprofile-arcs", "-ftest-coverage", "-fprofile-abs-path",], - LIBS=["gcov"], - ) - # GCC note file is generated by compiler, ensure we build it, and allow scons to clean it up - AlwaysBuild(panda) - env.SideEffect("panda.gcno", panda) diff --git a/tests/safety/test.sh b/tests/safety/test.sh index 13703b26a3..173c5b5c69 100755 --- a/tests/safety/test.sh +++ b/tests/safety/test.sh @@ -4,31 +4,31 @@ set -e DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null && pwd)" cd $DIR -# reset coverage data and generate gcc note file -rm -f ../libpanda/*.gcda -scons -j$(nproc) -D --coverage +rm -f safety_*.profraw safety.profdata +scons -j$(nproc) -D # run safety tests and generate coverage data HW_TYPES=( 6 9 ) for hw_type in "${HW_TYPES[@]}"; do echo "Testing HW_TYPE: $hw_type" - HW_TYPE=$hw_type pytest test_*.py + LLVM_PROFILE_FILE="safety_%m.profraw" HW_TYPE=$hw_type pytest test_*.py done -# generate and open report +# generate coverage report +llvm-profdata-17 merge -sparse safety_*.profraw -o safety.profdata + +# open html report if [ "$1" == "--report" ]; then - geninfo ../libpanda/ -o coverage.info - genhtml coverage.info -o coverage-out - sensible-browser coverage-out/index.html + llvm-cov-17 show -format=html -show-branches=count -instr-profile=safety.profdata ../libpanda/libpanda.so -sources ../../board/safety/safety_*.h ../../board/safety.h -o coverage_report + sensible-browser coverage_report/index.html fi -# test coverage -GCOV_OUTPUT=$(gcov -n ../libpanda/panda.c) -INCOMPLETE_COVERAGE=$(echo "$GCOV_OUTPUT" | paste -s -d' \n' | grep -E "File.*(safety\/safety_.*)|(safety)\.h" | grep -v "100.00%" || true) -if [ -n "$INCOMPLETE_COVERAGE" ]; then - echo "FAILED: Some files have less than 100% coverage:" +# test line coverage +INCOMPLETE_COVERAGE=$(llvm-cov-17 report -show-region-summary=false -show-branch-summary=false -instr-profile=safety.profdata ../libpanda/libpanda.so -sources ../../board/safety/safety_*.h ../../board/safety.h | awk '$7 != "100.00%"' | head -n -1) +if [ ! $(echo "$INCOMPLETE_COVERAGE" | wc -l) -eq 2 ]; then + echo "FAILED: Some files have less than 100% line coverage:" echo "$INCOMPLETE_COVERAGE" exit 1 else - echo "SUCCESS: All checked files have 100% coverage!" + echo "SUCCESS: All checked files have 100% line coverage!" fi From 78b49ab155f6f6cb819df16f0327c5768ed6bb66 Mon Sep 17 00:00:00 2001 From: Maxime Desroches Date: Wed, 25 Sep 2024 21:56:09 -0700 Subject: [PATCH 2/4] safety mutation tests (#2040) * mutation * clone * origin * get diff * better example * better * fix docker * work on push * make test fail * real change * test all modes * ignore * better * reco * fix * no libpanda on device * curl * nl * nl again * delete * clean * clean * this * add this back * cleanup --- .github/workflows/test.yaml | 14 ++++++++++++++ .gitignore | 1 + Dockerfile | 4 ++++ SConstruct | 4 ++++ tests/libpanda/SConscript | 9 +++++++++ tests/safety/install_mull.sh | 11 +++++++++++ tests/safety/mutation.sh | 20 ++++++++++++++++++++ 7 files changed, 63 insertions(+) create mode 100755 tests/safety/install_mull.sh create mode 100755 tests/safety/mutation.sh diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 5b62bf18fd..12d956eecf 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -103,6 +103,20 @@ jobs: timeout-minutes: 5 run: ${{ env.RUN }} "cd tests/misra && pytest -n8 test_mutation.py" + mutation: + name: Mutation tests + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v2 + with: + fetch-depth: 0 # need master to get diff + - name: Build Docker image + run: eval "$BUILD" + - name: Mutation tests + timeout-minutes: 5 + run: ${{ env.RUN }} "GIT_REF=${{ github.event_name == 'push' && github.ref == 'refs/heads/master' && github.event.before || 'origin/master' }} cd tests/safety && ./mutation.sh" + static_analysis: name: static analysis runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 8f7cb049b9..89eed6e4c5 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,4 @@ tests/safety/coverage.info *.profraw *.profdata +mull.yml diff --git a/Dockerfile b/Dockerfile index a6f94826da..670283dd47 100644 --- a/Dockerfile +++ b/Dockerfile @@ -22,6 +22,10 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ apt-get update && apt-get install -y clang-17 && \ ln -s $(which clang-17) /usr/bin/clang +RUN apt-get update && apt-get install -y curl && \ + curl -1sLf 'https://dl.cloudsmith.io/public/mull-project/mull-stable/setup.deb.sh' | bash && \ + apt-get update && apt-get install -y mull-17 + ENV CPPCHECK_DIR=/tmp/cppcheck COPY tests/misra/install.sh /tmp/ RUN /tmp/install.sh && rm -rf $CPPCHECK_DIR/.git/ diff --git a/SConstruct b/SConstruct index 8a7609afe1..6a97f4c170 100644 --- a/SConstruct +++ b/SConstruct @@ -12,6 +12,10 @@ AddOption('--compile_db', action='store_true', help='build clang compilation database') +AddOption('--mutation', + action='store_true', + help='generate mutation-ready code') + env = Environment( COMPILATIONDB_USE_ABSPATH=True, tools=["default", "compilation_db"], diff --git a/tests/libpanda/SConscript b/tests/libpanda/SConscript index 66dd563a75..fc33d85d35 100644 --- a/tests/libpanda/SConscript +++ b/tests/libpanda/SConscript @@ -28,6 +28,15 @@ else: env['CFLAGS'] += COVERAGE_FLAGS env['LINKFLAGS'] += COVERAGE_FLAGS +if GetOption('mutation'): + flags = [ + '-fpass-plugin=/usr/lib/mull-ir-frontend-17', + '-g', + '-grecord-command-line', + ] + env['CFLAGS'] += flags + env['LINKFLAGS'] += flags + if GetOption('ubsan'): flags = [ "-fsanitize=undefined", diff --git a/tests/safety/install_mull.sh b/tests/safety/install_mull.sh new file mode 100755 index 0000000000..75b1042ec3 --- /dev/null +++ b/tests/safety/install_mull.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env bash +set -e + +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null && pwd)" +cd $DIR + +if ! command -v "mull-runner-17" > /dev/null 2>&1; then + sudo apt-get update && sudo apt-get install -y curl clang-17 + curl -1sLf 'https://dl.cloudsmith.io/public/mull-project/mull-stable/setup.deb.sh' | sudo -E bash + sudo apt-get update && sudo apt-get install -y mull-17 +fi diff --git a/tests/safety/mutation.sh b/tests/safety/mutation.sh new file mode 100755 index 0000000000..23bba67c74 --- /dev/null +++ b/tests/safety/mutation.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +set -e + +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null && pwd)" +cd $DIR + +$DIR/install_mull.sh + +scons --mutation -j$(nproc) -D + +GIT_REF="${GIT_REF:-origin/master}" +echo -e "timeout: 10000\ngitDiffRef: $GIT_REF\ngitProjectRoot: ../../" > mull.yml + +SAFETY_MODELS=$(find * | grep "^test_.*\.py") +for safety_model in ${SAFETY_MODELS[@]}; do + echo "" + echo "" + echo -e "Testing mutations on : $safety_model" + mull-runner-17 --ld-search-path /lib/x86_64-linux-gnu/ ../libpanda/libpanda.so -test-program=./$safety_model +done From b6644f7a35c0982d5a724a9f11f77bce6db809ad Mon Sep 17 00:00:00 2001 From: Maxime Desroches Date: Thu, 26 Sep 2024 12:35:45 -0700 Subject: [PATCH 3/4] revert libpanda with clang (#2044) * revert * adapt this --- SConstruct | 4 ++++ board/safety/safety_ford.h | 28 +++++++++++----------- board/safety/safety_gm.h | 5 ++-- board/safety/safety_hyundai.h | 25 ++++++++------------ board/safety/safety_hyundai_common.h | 5 ++-- board/safety/safety_volkswagen_mqb.h | 16 ++++++------- board/safety/safety_volkswagen_pq.h | 4 ++-- tests/libpanda/SConscript | 35 ++++++++++++++++------------ tests/safety/test.sh | 28 +++++++++++----------- 9 files changed, 78 insertions(+), 72 deletions(-) diff --git a/SConstruct b/SConstruct index 6a97f4c170..11ff21738e 100644 --- a/SConstruct +++ b/SConstruct @@ -8,6 +8,10 @@ AddOption('--ubsan', action='store_true', help='turn on UBSan') +AddOption('--coverage', + action='store_true', + help='build with test coverage options') + AddOption('--compile_db', action='store_true', help='build clang compilation database') diff --git a/board/safety/safety_ford.h b/board/safety/safety_ford.h index 50ff6f7139..3ab8827b6e 100644 --- a/board/safety/safety_ford.h +++ b/board/safety/safety_ford.h @@ -28,10 +28,10 @@ static uint8_t ford_get_counter(const CANPacket_t *to_push) { if (addr == FORD_BrakeSysFeatures) { // Signal: VehVActlBrk_No_Cnt cnt = (GET_BYTE(to_push, 2) >> 2) & 0xFU; - } - if (addr == FORD_Yaw_Data_FD1) { + } else if (addr == FORD_Yaw_Data_FD1) { // Signal: VehRollYaw_No_Cnt cnt = GET_BYTE(to_push, 5); + } else { } return cnt; } @@ -43,10 +43,10 @@ static uint32_t ford_get_checksum(const CANPacket_t *to_push) { if (addr == FORD_BrakeSysFeatures) { // Signal: VehVActlBrk_No_Cs chksum = GET_BYTE(to_push, 3); - } - if (addr == FORD_Yaw_Data_FD1) { + } else if (addr == FORD_Yaw_Data_FD1) { // Signal: VehRollYawW_No_Cs chksum = GET_BYTE(to_push, 4); + } else { } return chksum; } @@ -60,14 +60,14 @@ static uint32_t ford_compute_checksum(const CANPacket_t *to_push) { chksum += GET_BYTE(to_push, 2) >> 6; // VehVActlBrk_D_Qf chksum += (GET_BYTE(to_push, 2) >> 2) & 0xFU; // VehVActlBrk_No_Cnt chksum = 0xFFU - chksum; - } - if (addr == FORD_Yaw_Data_FD1) { + } else if (addr == FORD_Yaw_Data_FD1) { chksum += GET_BYTE(to_push, 0) + GET_BYTE(to_push, 1); // VehRol_W_Actl chksum += GET_BYTE(to_push, 2) + GET_BYTE(to_push, 3); // VehYaw_W_Actl chksum += GET_BYTE(to_push, 5); // VehRollYaw_No_Cnt chksum += GET_BYTE(to_push, 6) >> 6; // VehRolWActl_D_Qf chksum += (GET_BYTE(to_push, 6) >> 4) & 0x3U; // VehYawWActl_D_Qf chksum = 0xFFU - chksum; + } else { } return chksum; @@ -79,12 +79,11 @@ static bool ford_get_quality_flag_valid(const CANPacket_t *to_push) { bool valid = false; if (addr == FORD_BrakeSysFeatures) { valid = (GET_BYTE(to_push, 2) >> 6) == 0x3U; // VehVActlBrk_D_Qf - } - if (addr == FORD_EngVehicleSpThrottle2) { + } else if (addr == FORD_EngVehicleSpThrottle2) { valid = ((GET_BYTE(to_push, 4) >> 5) & 0x3U) == 0x3U; // VehVActlEng_D_Qf - } - if (addr == FORD_Yaw_Data_FD1) { + } else if (addr == FORD_Yaw_Data_FD1) { valid = ((GET_BYTE(to_push, 6) >> 4) & 0x3U) == 0x3U; // VehYawWActl_D_Qf + } else { } return valid; } @@ -317,7 +316,8 @@ static int ford_fwd_hook(int bus_num, int addr) { // Forward all traffic from bus 0 onward bus_fwd = FORD_CAM_BUS; break; - } case FORD_CAM_BUS: { + } + case FORD_CAM_BUS: { if (ford_lkas_msg_check(addr)) { // Block stock LKAS and UI messages bus_fwd = -1; @@ -329,10 +329,12 @@ static int ford_fwd_hook(int bus_num, int addr) { bus_fwd = FORD_MAIN_BUS; } break; - } default: { + } + default: { // No other buses should be in use; fallback to do-not-forward bus_fwd = -1; - break;} + break; + } } return bus_fwd; diff --git a/board/safety/safety_gm.h b/board/safety/safety_gm.h index 10cb21f32c..f0902a921a 100644 --- a/board/safety/safety_gm.h +++ b/board/safety/safety_gm.h @@ -227,11 +227,12 @@ static safety_config gm_init(uint16_t param) { {0x1E1, 2, 7}, {0x184, 2, 8}}; // camera bus gm_hw = GET_FLAG(param, GM_PARAM_HW_CAM) ? GM_CAM : GM_ASCM; + if (gm_hw == GM_ASCM) { gm_long_limits = &GM_ASCM_LONG_LIMITS; - } - if (gm_hw == GM_CAM) { + } else if (gm_hw == GM_CAM) { gm_long_limits = &GM_CAM_LONG_LIMITS; + } else { } #ifdef ALLOW_DEBUG diff --git a/board/safety/safety_hyundai.h b/board/safety/safety_hyundai.h index cc2fd8c6b0..3e9d23216c 100644 --- a/board/safety/safety_hyundai.h +++ b/board/safety/safety_hyundai.h @@ -49,19 +49,16 @@ static uint8_t hyundai_get_counter(const CANPacket_t *to_push) { uint8_t cnt = 0; if (addr == 0x260) { cnt = (GET_BYTE(to_push, 7) >> 4) & 0x3U; - } - if (addr == 0x386) { + } else if (addr == 0x386) { cnt = ((GET_BYTE(to_push, 3) >> 6) << 2) | (GET_BYTE(to_push, 1) >> 6); - } - if (addr == 0x394) { + } else if (addr == 0x394) { cnt = (GET_BYTE(to_push, 1) >> 5) & 0x7U; - } - if (addr == 0x421) { + } else if (addr == 0x421) { cnt = GET_BYTE(to_push, 7) & 0xFU; - } - if (addr == 0x4F1) { + } else if (addr == 0x4F1) { cnt = (GET_BYTE(to_push, 3) >> 4) & 0xFU; - } + } else { + } return cnt; } @@ -71,15 +68,13 @@ static uint32_t hyundai_get_checksum(const CANPacket_t *to_push) { uint8_t chksum = 0; if (addr == 0x260) { chksum = GET_BYTE(to_push, 7) & 0xFU; - } - if (addr == 0x386) { + } else if (addr == 0x386) { chksum = ((GET_BYTE(to_push, 7) >> 6) << 2) | (GET_BYTE(to_push, 5) >> 6); - } - if (addr == 0x394) { + } else if (addr == 0x394) { chksum = GET_BYTE(to_push, 6) & 0xFU; - } - if (addr == 0x421) { + } else if (addr == 0x421) { chksum = GET_BYTE(to_push, 7) >> 4; + } else { } return chksum; } diff --git a/board/safety/safety_hyundai_common.h b/board/safety/safety_hyundai_common.h index ee2ee32f2f..d83b396401 100644 --- a/board/safety/safety_hyundai_common.h +++ b/board/safety/safety_hyundai_common.h @@ -118,9 +118,10 @@ uint32_t hyundai_common_canfd_compute_checksum(const CANPacket_t *to_push) { if (len == 24) { crc ^= 0x819dU; - } - if (len == 32) { + } else if (len == 32) { crc ^= 0x9f5bU; + } else { + } return crc; diff --git a/board/safety/safety_volkswagen_mqb.h b/board/safety/safety_volkswagen_mqb.h index 1484cd5b07..40f7cbccc9 100644 --- a/board/safety/safety_volkswagen_mqb.h +++ b/board/safety/safety_volkswagen_mqb.h @@ -45,19 +45,17 @@ static uint32_t volkswagen_mqb_compute_crc(const CANPacket_t *to_push) { uint8_t counter = volkswagen_mqb_get_counter(to_push); if (addr == MSG_LH_EPS_03) { crc ^= (uint8_t[]){0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5,0xF5}[counter]; - } - if (addr == MSG_ESP_05) { + } else if (addr == MSG_ESP_05) { crc ^= (uint8_t[]){0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07,0x07}[counter]; - } - if (addr == MSG_TSK_06) { + } else if (addr == MSG_TSK_06) { crc ^= (uint8_t[]){0xC4,0xE2,0x4F,0xE4,0xF8,0x2F,0x56,0x81,0x9F,0xE5,0x83,0x44,0x05,0x3F,0x97,0xDF}[counter]; - } - if (addr == MSG_MOTOR_20) { + } else if (addr == MSG_MOTOR_20) { crc ^= (uint8_t[]){0xE9,0x65,0xAE,0x6B,0x7B,0x35,0xE5,0x5F,0x4E,0xC7,0x86,0xA2,0xBB,0xDD,0xEB,0xB4}[counter]; - } - if (addr == MSG_GRA_ACC_01) { + } else if (addr == MSG_GRA_ACC_01) { crc ^= (uint8_t[]){0x6A,0x38,0xB4,0x27,0x22,0xEF,0xE1,0xBB,0xF8,0x80,0x84,0x49,0xC7,0x9E,0x1E,0x2B}[counter]; - } + } else { + // Undefined CAN message, CRC check expected to fail + } crc = volkswagen_crc8_lut_8h2f[crc]; return (uint8_t)(crc ^ 0xFFU); diff --git a/board/safety/safety_volkswagen_pq.h b/board/safety/safety_volkswagen_pq.h index 8fc811eba1..75979a5149 100644 --- a/board/safety/safety_volkswagen_pq.h +++ b/board/safety/safety_volkswagen_pq.h @@ -26,9 +26,9 @@ static uint8_t volkswagen_pq_get_counter(const CANPacket_t *to_push) { if (addr == MSG_LENKHILFE_3) { counter = (uint8_t)(GET_BYTE(to_push, 1) & 0xF0U) >> 4; - } - if (addr == MSG_GRA_NEU) { + } else if (addr == MSG_GRA_NEU) { counter = (uint8_t)(GET_BYTE(to_push, 2) & 0xF0U) >> 4; + } else { } return counter; diff --git a/tests/libpanda/SConscript b/tests/libpanda/SConscript index fc33d85d35..2ad405f5e2 100644 --- a/tests/libpanda/SConscript +++ b/tests/libpanda/SConscript @@ -1,13 +1,14 @@ -import os import platform -COVERAGE_FLAGS = [ - '-fprofile-instr-generate', - '-fcoverage-mapping', -] +CC = 'gcc' +system = platform.system() +if system == 'Darwin': + # gcc installed by homebrew has version suffix (e.g. gcc-12) in order to be + # distinguishable from system one - which acts as a symlink to clang + CC += '-13' env = Environment( - CC='clang', + CC=CC, CFLAGS=[ '-nostdlib', '-fno-builtin', @@ -17,19 +18,14 @@ env = Environment( ], CPPPATH=[".", "../../board/"], ) -if platform.system() == "Darwin": +if system == "Darwin": env.PrependENVPath('PATH', '/opt/homebrew/bin') -TICI = os.path.isfile('/TICI') -AGNOS = TICI -if AGNOS: - env['CC'] = 'gcc' -else: - env['CFLAGS'] += COVERAGE_FLAGS - env['LINKFLAGS'] += COVERAGE_FLAGS - if GetOption('mutation'): + env['CC'] = 'clang-17' flags = [ + '-fprofile-instr-generate', + '-fcoverage-mapping', '-fpass-plugin=/usr/lib/mull-ir-frontend-17', '-g', '-grecord-command-line', @@ -47,3 +43,12 @@ if GetOption('ubsan'): panda = env.SharedObject("panda.os", "panda.c") libpanda = env.SharedLibrary("libpanda.so", [panda]) + +if GetOption('coverage'): + env.Append( + CFLAGS=["-fprofile-arcs", "-ftest-coverage", "-fprofile-abs-path",], + LIBS=["gcov"], + ) + # GCC note file is generated by compiler, ensure we build it, and allow scons to clean it up + AlwaysBuild(panda) + env.SideEffect("panda.gcno", panda) diff --git a/tests/safety/test.sh b/tests/safety/test.sh index 173c5b5c69..13703b26a3 100755 --- a/tests/safety/test.sh +++ b/tests/safety/test.sh @@ -4,31 +4,31 @@ set -e DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null && pwd)" cd $DIR -rm -f safety_*.profraw safety.profdata -scons -j$(nproc) -D +# reset coverage data and generate gcc note file +rm -f ../libpanda/*.gcda +scons -j$(nproc) -D --coverage # run safety tests and generate coverage data HW_TYPES=( 6 9 ) for hw_type in "${HW_TYPES[@]}"; do echo "Testing HW_TYPE: $hw_type" - LLVM_PROFILE_FILE="safety_%m.profraw" HW_TYPE=$hw_type pytest test_*.py + HW_TYPE=$hw_type pytest test_*.py done -# generate coverage report -llvm-profdata-17 merge -sparse safety_*.profraw -o safety.profdata - -# open html report +# generate and open report if [ "$1" == "--report" ]; then - llvm-cov-17 show -format=html -show-branches=count -instr-profile=safety.profdata ../libpanda/libpanda.so -sources ../../board/safety/safety_*.h ../../board/safety.h -o coverage_report - sensible-browser coverage_report/index.html + geninfo ../libpanda/ -o coverage.info + genhtml coverage.info -o coverage-out + sensible-browser coverage-out/index.html fi -# test line coverage -INCOMPLETE_COVERAGE=$(llvm-cov-17 report -show-region-summary=false -show-branch-summary=false -instr-profile=safety.profdata ../libpanda/libpanda.so -sources ../../board/safety/safety_*.h ../../board/safety.h | awk '$7 != "100.00%"' | head -n -1) -if [ ! $(echo "$INCOMPLETE_COVERAGE" | wc -l) -eq 2 ]; then - echo "FAILED: Some files have less than 100% line coverage:" +# test coverage +GCOV_OUTPUT=$(gcov -n ../libpanda/panda.c) +INCOMPLETE_COVERAGE=$(echo "$GCOV_OUTPUT" | paste -s -d' \n' | grep -E "File.*(safety\/safety_.*)|(safety)\.h" | grep -v "100.00%" || true) +if [ -n "$INCOMPLETE_COVERAGE" ]; then + echo "FAILED: Some files have less than 100% coverage:" echo "$INCOMPLETE_COVERAGE" exit 1 else - echo "SUCCESS: All checked files have 100% line coverage!" + echo "SUCCESS: All checked files have 100% coverage!" fi From 08c95bf47ba6d0c5d11a7616e42e10b8dbde19eb Mon Sep 17 00:00:00 2001 From: Maxime Desroches Date: Thu, 26 Sep 2024 19:37:59 -0700 Subject: [PATCH 4/4] mutation tests: activate all mutation ops (#2047) * all * clean * cleaner * WORKING * test * cleanup --- tests/safety/mutation.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/safety/mutation.sh b/tests/safety/mutation.sh index 23bba67c74..e8c242cab3 100755 --- a/tests/safety/mutation.sh +++ b/tests/safety/mutation.sh @@ -6,10 +6,11 @@ cd $DIR $DIR/install_mull.sh -scons --mutation -j$(nproc) -D - GIT_REF="${GIT_REF:-origin/master}" -echo -e "timeout: 10000\ngitDiffRef: $GIT_REF\ngitProjectRoot: ../../" > mull.yml +GIT_ROOT=$(git rev-parse --show-toplevel) +echo -e "mutators:\n - cxx_all" > $GIT_ROOT/mull.yml +scons --mutation -j$(nproc) -D +echo -e "timeout: 10000\ngitDiffRef: $GIT_REF\ngitProjectRoot: $GIT_ROOT" >> $GIT_ROOT/mull.yml SAFETY_MODELS=$(find * | grep "^test_.*\.py") for safety_model in ${SAFETY_MODELS[@]}; do