compile libpanda with clang (#2041)

* clang * report * safety.h * fix lines * cleaner * better * remove this * revert this * fix * tici build * revert * revert * fix
2024-09-25 20:14:12 -07:00 · 2024-09-25 20:14:12 -07:00 · fb67bda00e
parent 38cab7d501
commit fb67bda00e
12 changed files with 81 additions and 79 deletions
--- a/.gitignore
+++ b/.gitignore
@ -29,3 +29,6 @@ nosetests.xml
 *.gcno
 tests/safety/coverage-out
 tests/safety/coverage.info
+
+*.profraw
+*.profdata
--- a/4
+++ b/4
@ -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/
--- 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
--- a/4
+++ b/4
@ -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')
--- 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;
--- 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
--- a/board/safety/safety_hyundai.h
+++ b/board/safety/safety_hyundai.h
@ -49,15 +49,18 @@ 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) {
+  }
+  if (addr == 0x4F1) {
    cnt = (GET_BYTE(to_push, 3) >> 4) & 0xFU;
-  } else {
  } 
  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;
 }
--- 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;
--- a/board/safety/safety_volkswagen_mqb.h
+++ b/board/safety/safety_volkswagen_mqb.h
@ -45,16 +45,18 @@ 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];

--- 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;
--- 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)
--- 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