diff --git a/tools/ci/check_cpp_guards.sh b/tools/ci/check_cpp_guards.sh index cc9f5fad12..558990d56e 100755 --- a/tools/ci/check_cpp_guards.sh +++ b/tools/ci/check_cpp_guards.sh @@ -1,6 +1,15 @@ #!/bin/bash # -# This script finds all files which do not contain "#ifdef __cplusplus", except the ones listed in check_cpp_guards_exceptions.txt +# This script finds all *.h files in public include dirs and checks for +# * compilation issues +# * c++ header guards +# +# The script fails if another a new header fails and not listed in check_cpp_guards_exceptions.txt +# +# Script arguments: +# --verbose: enables printing details if compilation fails (verbose is off by default) +# --process-all-files: all files are checked and then excluded, by default the excluded files are skipped and not checked +# --include-maxdepth=X: maximum directory depth for finding include files in include dirs (by default not specified=infinity) set -euo pipefail @@ -11,15 +20,94 @@ fi ##################### helper functions ##################### +# log_error +# +# Logs details on filename processing when the include fails +function log_error() +{ + if [ ! -z "$VERBOSE" ]; then + file=$1 + echo " verbose..." + echo "$LINE" + grep -A5 "error:" + echo "$LINE" + echo -n "$file" + fi +} + +# preproc_one_header +# +# Function to preprocess a header with CC and CXX and compares result +# Returns +# 0 -> Never returns zero (for easier return value expansion) +# 1 -> Cannot compile, but failed with "#error" directive (header seems OK) +# 2 -> Cannot compile with another issue, logged if verbose (header FAILs) +# 3 -> Both preprocessors produce zero out (header file is OK) +# 4 -> Both preprocessors produce the same, non-zero output (header file FAILs) +# 5 -> Both preprocessors produce different, non-zero output with extern "C" (header seems OK) +# 6 -> Both preprocessors produce different, non-zero output without extern "C" (header seems OK) +function preproc_one_header() +{ + file=$1 + if ! cpp_out=`xtensa-esp32-elf-g++ -w -P -E --std=c++17 ${file} ${FLAGS_INCLUDE_DIRS} 2>&1`; then + if [[ "$cpp_out" =~ "#error" ]]; then + return 1 + fi; + # Logging the compilation issue issue + echo "$cpp_out" | log_error $file + return 2 + fi; + if ! c99_out=`xtensa-esp32-elf-gcc -w -P -E --std=c99 ${file} ${FLAGS_INCLUDE_DIRS} 2>/dev/null`; then + # Should not happen, fail with compilation issue + return 2 + fi; + if diff_out=`diff <(echo "$cpp_out" ) <(echo "$c99_out")`; then + # checks zero output: contains spaces only if lines < 5 (for performace) + lines=`echo "$cpp_out" | wc -l` + if [ $lines -lt 5 ] && [ -z "${cpp_out// }" ]; then + return 3 + fi + return 4 + elif echo $diff_out | grep -q 'extern "C"'; then + return 5 + fi + return 6 +} + # check_one_header # # Function used to implement the logic of checking a single header. # Returns 0 on success (as a normal bash function would). -# Ideally shouldn't print anything. +# Ideally shouldn't print anything (if verbose, preproc_one_header outputs compilation errors) function check_one_header { include_file=$1 - grep -q '#ifdef __cplusplus' ${include_file} + # Procedure: + # 1) Preprocess the include file with C preprocessor and with CPP preprocessor + # - Pass the test if the preprocessor outputs are the same and whitespaces only (#define only header) + # - Fail the test if the preprocessor outputs are the same (but with some code) + # - If outputs different, continue with 2) + # 2) Strip out all include directives to generate "temp.h" + # 3) Preprocess the temp.h the same way in (1) + # - Pass the test if the preprocessor outputs are the same and whitespaces only (#include only header) + # - Fail the test if the preprocessor outputs are the same (but with some code) + # - If outputs different, pass the test + # + preproc_one_header ${include_file} || ret=$? + case $ret in + 1*) return 0 ;; # Pass if #error + 2*) return 2 ;; # Fail if other compile issue + 3*) return 0 ;; # Pass if zero out + 4*) return 1 ;; # Fail if the same non-zero out + [5-6]*) # Process further if different and non-zero + sed '/#include/d; /#error/d' ${include_file} > temp.h + case `preproc_one_header temp.h || echo $?` in + [1-3]*) return 0 ;; # Pass if compile issues (have stripped includes) + 4*) return 1 ;; # Fail if the same non-zero out + 5*) return 0 ;; # Pass if different and non-zero and with extern "C" + 6*) return 1 ;; # Fail if different and non-zero, but without extern "C" + esac + esac } # get_include_dir_list @@ -39,6 +127,9 @@ function get_include_dirs_list # build the list of include directories, only considering the directories in "components/" include_dirs="" for token in ${build_command}; do + if [[ "${token}" =~ ^-I.* ]]; then + flags_include_dirs+=" $token" + fi if [[ ! "${token}" =~ ^-I.*/components/.* ]]; then continue fi @@ -47,78 +138,112 @@ function get_include_dirs_list done export INCLUDE_DIRS=${include_dirs} + flags_include_dirs+=" -I $IDF_PATH/examples/get-started/blink/build/config" + export FLAGS_INCLUDE_DIRS=${flags_include_dirs} } -# check_include_dirs +# check_include_dirs # # Check all header files in directories listed in include_dirs variable. -# Places the list of header files with some issues into file. +# Places the list of header files where suspects missing C++ guard to file. +# Places the list of header files where compilation issues found to file. # Calls check_one_header function to check a single header file. function check_include_dirs { local include_dirs="$1" - local failed_headers_list_file="$2" + local missing_guard_list_file="$2" + local compile_error_list_file="$3" + local include_dir_maxdepth=${INCLUDE_DIR_MAXDEPTH:-} + local grep_expr=$(mktemp) - rm -f ${failed_headers_list_file} - touch ${failed_headers_list_file} + rm -f ${missing_guard_list_file} ${compile_error_list_file} + touch ${missing_guard_list_file} ${compile_error_list_file} + + if [ -z "$PROCESS_ALL_INCLUDES" ]; then + # generate grep expr: 1) strip comments, 2) generate '\|\|...' + exclusions=`cat $EXCLUSIONS_LIST | sed '/^#/d'` + echo $exclusions | sed 's/ /\\|/g' > ${grep_expr} + fi; for include_dir in ${include_dirs}; do echo "Processing include directory: ${include_dir}" - include_files=$(find ${include_dir} -name '*.h') + include_files=$(find ${include_dir} ${include_dir_maxdepth} -name '*.h') for include_file in ${include_files}; do - printf " Processing ${include_file}" - if ! check_one_header ${include_file}; then - printf " FAIL!\n" - echo ${include_file##${IDF_PATH}/} >> ${failed_headers_list_file} - else - printf " OK\n" + if [ -z "$PROCESS_ALL_INCLUDES" ] && echo "$include_file" | grep -q -f ${grep_expr}; then + printf " Excluding ${include_file}\n" + continue fi + printf " Processing ${include_file}" + check_one_header ${include_file} && ret=0 || ret=$? + case "$ret" in + 0*) printf " OK\n" ;; + 1*) + echo ${include_file##${IDF_PATH}/} >> ${missing_guard_list_file} + printf " FAIL (missing c++ guard)\n" ;; + 2*) + echo ${include_file##${IDF_PATH}/} >> ${compile_error_list_file} + printf " FAIL (cannot compile)\n" ;; + esac done done + rm -f ${grep_expr} } -# filter_exclusions +# filter_exclusions # -# Takes the list of files in file, and removes all files which +# Takes the list of files from stdin, and removes all files which # are listed in file. # Saves the final (filtered) list in file. # The exclusions list file may contain comments and empty lines. function filter_exclusions { - local input_list_file=$1 - local exclusions_list_file=$2 - local filtered_list_file=$3 - local temp_list=$(mktemp) + local exclusions_list_file=$1 + local filtered_list_file=$2 + local grep_expr=$(mktemp) - cp ${input_list_file} ${filtered_list_file} - touch ${temp_list} + # generate grep expr: 1) strip comments, 2) generate '\|\|...' + exclusions=`cat $exclusions_list_file | sed '/^#/d'` + echo $exclusions | sed 's/ /\\|/g' > ${grep_expr} - while read -r line; do - if [[ "${line}" =~ ^\# || -z "${line}" ]]; then - continue - fi - echo "Excluding ${line}" - grep -vE "${line}" ${filtered_list_file} > ${temp_list} || true - mv ${temp_list} ${filtered_list_file} - done < ${exclusions_list_file} - - rm -f ${temp_list} + # pass the expression as file (input goes from stdin) + grep -vf ${grep_expr} > ${filtered_list_file} || true + rm -f ${grep_expr} } ##################### main logic starts here ##################### EXCLUSIONS_LIST=${IDF_PATH}/tools/ci/check_cpp_guards_exceptions.txt MISSING_GUARDS_LIST=missing.txt +CANNOT_COMPILE_LIST=cannot_compile.txt FILTERED_LIST=filtered.txt LINE=--------------------------------------------------------------- +VERBOSE="" +PROCESS_ALL_INCLUDES="" +# Process command line arguments +for i in $@; do + if [[ "$i" =~ "--verbose" ]]; then + echo "$0: Enabling verbose output" + VERBOSE="1" + fi + if [[ "$i" =~ "--process-all-files" ]]; then + echo "$0: Processing all files" + PROCESS_ALL_INCLUDES="1" + fi + if [[ "$i" =~ --include-maxdepth=* ]]; then + INCLUDE_DIR_MAXDEPTH="-maxdepth ${i##--include-maxdepth=}" + echo "$0: Searching for includes with: $INCLUDE_DIR_MAXDEPTH" + fi +done +echo $LINE get_include_dirs_list -check_include_dirs "${INCLUDE_DIRS}" ${MISSING_GUARDS_LIST} -filter_exclusions ${MISSING_GUARDS_LIST} ${EXCLUSIONS_LIST} ${FILTERED_LIST} +check_include_dirs "${INCLUDE_DIRS}" ${MISSING_GUARDS_LIST} ${CANNOT_COMPILE_LIST} +cat ${MISSING_GUARDS_LIST} ${CANNOT_COMPILE_LIST} | filter_exclusions ${EXCLUSIONS_LIST} ${FILTERED_LIST} +echo $LINE if [ -s ${FILTERED_LIST} ]; then - echo "The following files don't have C++ guards:" + echo "The following files failed:" echo "" cat ${FILTERED_LIST} exit 1 diff --git a/tools/ci/check_cpp_guards_exceptions.txt b/tools/ci/check_cpp_guards_exceptions.txt index 4985524812..8fe96a4420 100644 --- a/tools/ci/check_cpp_guards_exceptions.txt +++ b/tools/ci/check_cpp_guards_exceptions.txt @@ -1,3 +1,5 @@ +### General ignore list +# components/xtensa/include/xtensa/* components/xtensa/include/* components/xtensa/esp32/include/xtensa/config/* @@ -15,52 +17,35 @@ components/soc/include/soc/* components/esp_rom/include/esp32s2beta/rom/rsa_pss.h -components/esp_common/include/esp_types.h -components/esp_common/include/esp_assert.h -components/esp_common/include/esp_bit_defs.h -components/esp_common/include/esp_task.h components/esp_common/include/esp_private/* -components/esp32/include/esp32/* -components/esp32/include/rom/* -components/esp32/include/esp_clk.h -components/esp32/include/esp_intr.h -components/esp32/include/esp_himem.h -components/esp32/include/esp_spiram.h -components/driver/include/driver/touch_pad.h +components/esp32/include/esp32/brownout.h +components/esp32/include/esp32/spiram.h +components/esp32/include/esp32/cache_err_int.h components/driver/include/driver/sdmmc_defs.h components/driver/include/driver/sdmmc_types.h -components/driver/include/driver/rtc_cntl.h -components/driver/include/driver/sigmadelta.h -components/esp_event/include/esp_event_loop.h +components/lwip/include/apps/dhcpserver/dhcpserver_options.h +components/lwip/include/apps/dhcpserver/dhcpserver.h -components/lwip/include/apps/dhcpserver/* -components/lwip/include/apps/esp_sntp.h -components/lwip/lwip/src/include/compat/* -components/lwip/lwip/src/include/lwip/* -components/lwip/lwip/src/include/netif/* -components/lwip/port/esp32/include/* +components/lwip/lwip/src/include/lwip/priv/memp_std.h +components/lwip/port/esp32/include/arch/cc.h +components/lwip/port/esp32/include/debug/lwip_debug.h +components/lwip/port/esp32/include/arch/cc.h components/vfs/include/sys/dirent.h -components/esp_wifi/include/esp_private/* components/esp_wifi/include/esp_wifi_netif.h components/esp_wifi/include/esp_wifi_default.h components/esp_wifi/esp32/include/phy_init_data.h -components/tcpip_adapter/include/tcpip_adapter_compatible/tcpip_adapter_compat.h -components/tcpip_adapter/include/tcpip_adapter.h -components/tcpip_adapter/include/tcpip_adapter_types.h - components/spi_flash/include/spi_flash_chip_issi.h components/spi_flash/include/spi_flash_chip_gd.h components/spi_flash/include/memspi_host_driver.h components/spi_flash/include/spi_flash_chip_driver.h components/spi_flash/include/spi_flash_chip_generic.h -components/bootloader_support/include/esp_flash_data_types.h components/bootloader_support/include/esp_app_format.h components/wpa_supplicant/include/* @@ -71,17 +56,12 @@ components/mbedtls/mbedtls/include/mbedtls/* components/espcoredump/include/esp_core_dump.h -components/perfmon/include/perfmon.h - -components/asio/* components/coap/* components/nghttp/* components/cbor/* components/esp-tls/private_include/* -components/esp_http_server/include/http_server.h - components/protobuf-c/* components/mdns/include/mdns_console.h @@ -95,7 +75,6 @@ components/fatfs/vfs/vfs_fat_internal.h components/fatfs/src/ffconf.h components/freemodbus/common/include/esp_modbus_common.h -components/freemodbus/common/include/mbcontroller.h components/freemodbus/common/include/esp_modbus_master.h components/freemodbus/common/include/esp_modbus_slave.h @@ -105,14 +84,16 @@ components/json/cJSON/* components/libsodium/* -components/mqtt/esp-mqtt/include/mqtt_supported_features.h -components/mqtt/esp-mqtt/include/mqtt_config.h - -components/openssl/include/platform/ssl_opt.h - components/spiffs/include/spiffs_config.h components/unity/unity/src/unity_internals.h -components/unity/include/priv/setjmp.h +components/unity/include/unity_config.h +components/unity/include/unity_test_runner.h +### Here are the files that do not compile for some reason +# +components/esp_rom/include/esp32s2beta/rom/spi_flash.h +components/app_trace/include/esp_sysview_trace.h +components/esp_gdbstub/include/esp_gdbstub.h +components/openssl/include/platform/ssl_pm.h diff --git a/tools/ci/config/pre_check.yml b/tools/ci/config/pre_check.yml index f057904728..34bd440b89 100644 --- a/tools/ci/config/pre_check.yml +++ b/tools/ci/config/pre_check.yml @@ -165,5 +165,5 @@ check_header_files_cpp_guards: - $BOT_LABEL_BUILD - $BOT_LABEL_REGULAR_TEST script: - - tools/ci/check_cpp_guards.sh + - tools/ci/check_cpp_guards.sh --verbose --process-all-files