From 341610b8a18fbff13521c3441ae30f4a3bb865fe Mon Sep 17 00:00:00 2001 From: hedger Date: Tue, 17 Jan 2023 15:55:49 +0300 Subject: [PATCH] [FL-3080] fbt: PVS support (#2286) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fbt: added firmware_pvscheck & firmware_pvs targets for *nix * fbt: pvs support on Windows * fbt: additional fixes & docs for PVS support * fbt: fixes for updater env configuration * github: reworked pvs workflow * vscode: added PVS shortcut * pvs: added --ignore-ccache flag * fbt: pvs: opening web browser if there are warnings * fbt: pvs: added browser handler for mac * github: fixed report path for PVS * fbt: pvs: fixed report upload path * removed intentional PVS warning * fixed more PVS warnings * fixed secplus_v1 PVS warning * fbt: added PVSNOBROWSER flag * github: setting PVSNOBROWSER for pvs runs * fbt: less debug output Co-authored-by: あく --- .github/workflows/pvs_studio.yml | 27 +----- .pvsoptions | 2 +- .vscode/example/tasks.json | 6 ++ assets/SConscript | 6 +- documentation/fbt.md | 1 + firmware.scons | 21 +++++ furi/core/core_defines.h | 6 +- lib/lfrfid/protocols/protocol_fdx_b.c | 2 +- lib/subghz/blocks/math.h | 12 ++- lib/subghz/protocols/chamberlain_code.c | 4 +- lib/subghz/protocols/secplus_v1.c | 2 +- scripts/fbt_tools/fbt_extapps.py | 2 + scripts/fbt_tools/fbt_help.py | 2 + scripts/fbt_tools/pvsstudio.py | 106 ++++++++++++++++++++++++ site_scons/commandline.scons | 5 ++ site_scons/extapps.scons | 5 ++ 16 files changed, 174 insertions(+), 35 deletions(-) create mode 100644 scripts/fbt_tools/pvsstudio.py diff --git a/.github/workflows/pvs_studio.yml b/.github/workflows/pvs_studio.yml index 46ee8801..50f8f0aa 100644 --- a/.github/workflows/pvs_studio.yml +++ b/.github/workflows/pvs_studio.yml @@ -43,36 +43,15 @@ jobs: fi python3 scripts/get_env.py "--event_file=${{ github.event_path }}" "--type=$TYPE" - - name: 'Make reports directory' + - name: 'Supply PVS credentials' run: | - rm -rf reports/ - mkdir reports - - - name: 'Generate compile_comands.json' - run: | - ./fbt COMPACT=1 version_json proto_ver icons firmware_cdb dolphin_internal dolphin_blocking _fap_icons api_syms - - - name: 'Static code analysis' - run: | - source scripts/toolchain/fbtenv.sh pvs-studio-analyzer credentials ${{ secrets.PVS_STUDIO_CREDENTIALS }} - pvs-studio-analyzer analyze \ - @.pvsoptions \ - -C gccarm \ - -j$(grep -c processor /proc/cpuinfo) \ - -f build/f7-firmware-DC/compile_commands.json \ - -o PVS-Studio.log - name: 'Convert PVS-Studio output to html and detect warnings' id: pvs-warn run: | WARNINGS=0 - plog-converter \ - -a GA:1,2,3 \ - -t fullhtml \ - --indicate-warnings \ - PVS-Studio.log \ - -o reports/${DEFAULT_TARGET}-${SUFFIX} || WARNINGS=1 + ./fbt COMPACT=1 PVSNOBROWSER=1 firmware_pvs || WARNINGS=1 echo "warnings=${WARNINGS}" >> $GITHUB_OUTPUT - name: 'Upload artifacts to update server' @@ -84,7 +63,7 @@ jobs: chmod 600 ./deploy_key; rsync -avrzP --mkpath \ -e 'ssh -p ${{ secrets.RSYNC_DEPLOY_PORT }} -i ./deploy_key' \ - reports/ ${{ secrets.RSYNC_DEPLOY_USER }}@${{ secrets.RSYNC_DEPLOY_HOST }}:/home/data/firmware-pvs-studio-report/"${BRANCH_NAME}/"; + build/f7-firmware-DC/pvsreport/ ${{ secrets.RSYNC_DEPLOY_USER }}@${{ secrets.RSYNC_DEPLOY_HOST }}:/home/data/firmware-pvs-studio-report/"${BRANCH_NAME}/${{steps.names.outputs.default_target}}-${{steps.names.outputs.suffix}}/"; rm ./deploy_key; - name: 'Find Previous Comment' diff --git a/.pvsoptions b/.pvsoptions index 31bc4b80..ca1b2b57 100644 --- a/.pvsoptions +++ b/.pvsoptions @@ -1 +1 @@ ---rules-config .pvsconfig -e lib/fatfs -e lib/fnv1a-hash -e lib/FreeRTOS-Kernel -e lib/heatshrink -e lib/libusb_stm32 -e lib/littlefs -e lib/mbedtls -e lib/micro-ecc -e lib/microtar -e lib/mlib -e lib/qrcode -e lib/ST25RFAL002 -e lib/STM32CubeWB -e lib/u8g2 -e lib/nanopb -e */arm-none-eabi/* -e applications/plugins/dap_link/lib/free-dap +--ignore-ccache -C gccarm --rules-config .pvsconfig -e lib/fatfs -e lib/fnv1a-hash -e lib/FreeRTOS-Kernel -e lib/heatshrink -e lib/libusb_stm32 -e lib/littlefs -e lib/mbedtls -e lib/micro-ecc -e lib/microtar -e lib/mlib -e lib/qrcode -e lib/ST25RFAL002 -e lib/STM32CubeWB -e lib/u8g2 -e lib/nanopb -e */arm-none-eabi/* -e applications/plugins/dap_link/lib/free-dap diff --git a/.vscode/example/tasks.json b/.vscode/example/tasks.json index beedf886..28e67d45 100644 --- a/.vscode/example/tasks.json +++ b/.vscode/example/tasks.json @@ -105,6 +105,12 @@ "type": "shell", "command": "./fbt COMPACT=1 DEBUG=0 FORCE=1 flash_usb_full" }, + { + "label": "[Debug] Create PVS-Studio report", + "group": "build", + "type": "shell", + "command": "./fbt firmware_pvs" + }, { "label": "[Debug] Build FAPs", "group": "build", diff --git a/assets/SConscript b/assets/SConscript index 63141829..ef5d83c7 100644 --- a/assets/SConscript +++ b/assets/SConscript @@ -54,13 +54,14 @@ assetsenv.Alias("proto_ver", proto_ver) # Gather everything into a static lib assets_parts = (icons, proto, dolphin_blocking, dolphin_internal, proto_ver) +env.Replace(FW_ASSETS_HEADERS=assets_parts) assetslib = assetsenv.Library("${FW_LIB_NAME}", assets_parts) assetsenv.Install("${LIB_DIST_DIR}", assetslib) # Resources for SD card - +env.SetDefault(FW_RESOURCES=None) if assetsenv["IS_BASE_FIRMWARE"]: # External dolphin animations dolphin_external = assetsenv.DolphinExtBuilder( @@ -92,8 +93,7 @@ if assetsenv["IS_BASE_FIRMWARE"]: ) # Exporting resources node to external environment - env["FW_ASSETS_HEADERS"] = assets_parts - env["FW_RESOURCES"] = resources + env.Replace(FW_RESOURCES=resources) assetsenv.Alias("resources", resources) Return("assetslib") diff --git a/documentation/fbt.md b/documentation/fbt.md index 7b1aa8b4..5166d0ab 100644 --- a/documentation/fbt.md +++ b/documentation/fbt.md @@ -56,6 +56,7 @@ To run cleanup (think of `make clean`) for specified targets, add the `-c` optio - `get_stlink` - output serial numbers for attached STLink probes. Used for specifying an adapter with `OPENOCD_ADAPTER_SERIAL=...`. - `lint`, `format` - run clang-format on the C source code to check and reformat it according to the `.clang-format` specs. - `lint_py`, `format_py` - run [black](https://black.readthedocs.io/en/stable/index.html) on the Python source code, build system files & application manifests. +- `firmware_pvs` - generate a PVS Studio report for the firmware. Requires PVS Studio to be availabe on your system's `PATH`. - `cli` - start a Flipper CLI session over USB. ### Firmware targets diff --git a/firmware.scons b/firmware.scons index d674bf16..8a52650c 100644 --- a/firmware.scons +++ b/firmware.scons @@ -15,6 +15,7 @@ env = ENV.Clone( ("compilation_db", {"COMPILATIONDB_COMSTR": "\tCDB\t${TARGET}"}), "fwbin", "fbt_apps", + "pvsstudio", ], COMPILATIONDB_USE_ABSPATH=False, BUILD_DIR=fw_build_meta["build_dir"], @@ -69,6 +70,8 @@ env = ENV.Clone( ], }, }, + SDK_APISYMS=None, + _APP_ICONS=None, ) @@ -273,6 +276,24 @@ Precious(fwcdb) NoClean(fwcdb) Alias(fwenv["FIRMWARE_BUILD_CFG"] + "_cdb", fwcdb) +pvscheck = fwenv.PVSCheck("pvsreport.log", fwcdb) +Depends( + pvscheck, + [ + fwenv["FW_VERSION_JSON"], + fwenv["FW_ASSETS_HEADERS"], + fwenv["SDK_APISYMS"], + fwenv["_APP_ICONS"], + ], +) +Alias(fwenv["FIRMWARE_BUILD_CFG"] + "_pvscheck", pvscheck) +AlwaysBuild(pvscheck) +Precious(pvscheck) + +pvsreport = fwenv.PVSReport(None, pvscheck, REPORT_DIR=Dir("pvsreport")) +Alias(fwenv["FIRMWARE_BUILD_CFG"] + "_pvs", pvsreport) +AlwaysBuild(pvsreport) + # If current configuration was explicitly requested, generate compilation database # and link its directory as build/latest if should_gen_cdb_and_link_dir(fwenv, BUILD_TARGETS): diff --git a/furi/core/core_defines.h b/furi/core/core_defines.h index a0f50aff..03a364ab 100644 --- a/furi/core/core_defines.h +++ b/furi/core/core_defines.h @@ -93,7 +93,11 @@ extern "C" { #endif #ifndef FURI_BIT_CLEAR -#define FURI_BIT_CLEAR(x, n) ((x) &= ~(1UL << (n))) +#define FURI_BIT_CLEAR(x, n) \ + ({ \ + __typeof__(x) _x = (1); \ + (x) &= ~(_x << (n)); \ + }) #endif #define FURI_SW_MEMBARRIER() asm volatile("" : : : "memory") diff --git a/lib/lfrfid/protocols/protocol_fdx_b.c b/lib/lfrfid/protocols/protocol_fdx_b.c index 855356f2..dd54cffb 100644 --- a/lib/lfrfid/protocols/protocol_fdx_b.c +++ b/lib/lfrfid/protocols/protocol_fdx_b.c @@ -244,7 +244,7 @@ LevelDuration protocol_fdx_b_encoder_yield(ProtocolFDXB* protocol) { static uint64_t protocol_fdx_b_get_national_code(const uint8_t* data) { uint64_t national_code = bit_lib_get_bits_32(data, 0, 32); national_code = national_code << 32; - national_code |= bit_lib_get_bits_32(data, 32, 6) << (32 - 6); + national_code |= (uint64_t)bit_lib_get_bits_32(data, 32, 6) << (32 - 6); bit_lib_reverse_bits((uint8_t*)&national_code, 0, 64); return national_code; } diff --git a/lib/subghz/blocks/math.h b/lib/subghz/blocks/math.h index 87c209f7..dcea3da5 100644 --- a/lib/subghz/blocks/math.h +++ b/lib/subghz/blocks/math.h @@ -5,8 +5,16 @@ #include #define bit_read(value, bit) (((value) >> (bit)) & 0x01) -#define bit_set(value, bit) ((value) |= (1UL << (bit))) -#define bit_clear(value, bit) ((value) &= ~(1UL << (bit))) +#define bit_set(value, bit) \ + ({ \ + __typeof__(value) _one = (1); \ + (value) |= (_one << (bit)); \ + }) +#define bit_clear(value, bit) \ + ({ \ + __typeof__(value) _one = (1); \ + (value) &= ~(_one << (bit)); \ + }) #define bit_write(value, bit, bitvalue) (bitvalue ? bit_set(value, bit) : bit_clear(value, bit)) #define DURATION_DIFF(x, y) (((x) < (y)) ? ((y) - (x)) : ((x) - (y))) diff --git a/lib/subghz/protocols/chamberlain_code.c b/lib/subghz/protocols/chamberlain_code.c index 3650a986..32f4e952 100644 --- a/lib/subghz/protocols/chamberlain_code.c +++ b/lib/subghz/protocols/chamberlain_code.c @@ -280,9 +280,9 @@ static bool subghz_protocol_chamb_code_to_bit(uint64_t* data, uint8_t size) { uint64_t data_tmp = data[0]; uint64_t data_res = 0; for(uint8_t i = 0; i < size; i++) { - if((data_tmp & 0xF) == CHAMBERLAIN_CODE_BIT_0) { + if((data_tmp & 0xFll) == CHAMBERLAIN_CODE_BIT_0) { bit_write(data_res, i, 0); - } else if((data_tmp & 0xF) == CHAMBERLAIN_CODE_BIT_1) { + } else if((data_tmp & 0xFll) == CHAMBERLAIN_CODE_BIT_1) { bit_write(data_res, i, 1); } else { return false; diff --git a/lib/subghz/protocols/secplus_v1.c b/lib/subghz/protocols/secplus_v1.c index 75a44a26..a1161101 100644 --- a/lib/subghz/protocols/secplus_v1.c +++ b/lib/subghz/protocols/secplus_v1.c @@ -224,7 +224,7 @@ static bool subghz_protocol_secplus_v1_encode(SubGhzProtocolEncoderSecPlus_v1* i instance->generic.data &= 0xFFFFFFFF00000000; instance->generic.data |= rolling; - if(rolling > 0xFFFFFFFF) { + if(rolling == 0xFFFFFFFF) { rolling = 0xE6000000; } if(fixed > 0xCFD41B90) { diff --git a/scripts/fbt_tools/fbt_extapps.py b/scripts/fbt_tools/fbt_extapps.py index f0015cf2..1199f5f8 100644 --- a/scripts/fbt_tools/fbt_extapps.py +++ b/scripts/fbt_tools/fbt_extapps.py @@ -32,6 +32,7 @@ def BuildAppElf(env, app): ext_apps_work_dir = env.subst("$EXT_APPS_WORK_DIR") app_work_dir = os.path.join(ext_apps_work_dir, app.appid) + env.SetDefault(_APP_ICONS=[]) env.VariantDir(app_work_dir, app._appdir, duplicate=False) app_env = env.Clone(FAP_SRC_DIR=app._appdir, FAP_WORK_DIR=app_work_dir) @@ -63,6 +64,7 @@ def BuildAppElf(env, app): icon_bundle_name=f"{app.fap_icon_assets_symbol if app.fap_icon_assets_symbol else app.appid }_icons", ) app_env.Alias("_fap_icons", fap_icons) + env.Append(_APP_ICONS=[fap_icons]) private_libs = [] diff --git a/scripts/fbt_tools/fbt_help.py b/scripts/fbt_tools/fbt_help.py index 8cce9335..afdb3666 100644 --- a/scripts/fbt_tools/fbt_help.py +++ b/scripts/fbt_tools/fbt_help.py @@ -31,6 +31,8 @@ Other: run linters format, format_py: run code formatters + firmware_pvs: + generate a PVS-Studio report For more targets & info, see documentation/fbt.md """ diff --git a/scripts/fbt_tools/pvsstudio.py b/scripts/fbt_tools/pvsstudio.py new file mode 100644 index 00000000..02014836 --- /dev/null +++ b/scripts/fbt_tools/pvsstudio.py @@ -0,0 +1,106 @@ +from SCons.Builder import Builder +from SCons.Action import Action +from SCons.Script import Delete, Mkdir, GetBuildFailures +import multiprocessing +import webbrowser +import atexit +import sys +import subprocess + +__no_browser = False + + +def _set_browser_action(target, source, env): + if env["PVSNOBROWSER"]: + global __no_browser + __no_browser = True + + +def emit_pvsreport(target, source, env): + target_dir = env["REPORT_DIR"] + if env["PLATFORM"] == "win32": + # Report generator on Windows emits to a subfolder of given output folder + target_dir = target_dir.Dir("fullhtml") + return [target_dir.File("index.html")], source + + +def atexist_handler(): + global __no_browser + if __no_browser: + return + + for bf in GetBuildFailures(): + if bf.node.exists and bf.node.name.endswith(".html"): + # macOS + if sys.platform == "darwin": + subprocess.run(["open", bf.node.abspath]) + else: + webbrowser.open(bf.node.abspath) + break + + +def generate(env): + env.SetDefault( + PVSNCORES=multiprocessing.cpu_count(), + PVSOPTIONS=[ + "@.pvsoptions", + "-j${PVSNCORES}", + # "--incremental", # kinda broken on PVS side + ], + PVSCONVOPTIONS=[ + "-a", + "GA:1,2,3", + "-t", + "fullhtml", + "--indicate-warnings", + ], + ) + + if env["PLATFORM"] == "win32": + env.SetDefault( + PVSCHECKBIN="CompilerCommandsAnalyzer.exe", + PVSCONVBIN="PlogConverter.exe", + ) + else: + env.SetDefault( + PVSCHECKBIN="pvs-studio-analyzer", + PVSCONVBIN="plog-converter", + ) + + if not env["VERBOSE"]: + env.SetDefault( + PVSCHECKCOMSTR="\tPVS\t${TARGET}", + PVSCONVCOMSTR="\tPVSREP\t${TARGET}", + ) + + env.Append( + BUILDERS={ + "PVSCheck": Builder( + action=Action( + '${PVSCHECKBIN} analyze ${PVSOPTIONS} -f "${SOURCE}" -o "${TARGET}"', + "${PVSCHECKCOMSTR}", + ), + suffix=".log", + src_suffix=".json", + ), + "PVSReport": Builder( + action=Action( + [ + Delete("${TARGET.dir}"), + # PlogConverter.exe and plog-converter have different behavior + Mkdir("${TARGET.dir}") if env["PLATFORM"] == "win32" else None, + Action(_set_browser_action, None), + '${PVSCONVBIN} ${PVSCONVOPTIONS} "${SOURCE}" -o "${REPORT_DIR}"', + ], + "${PVSCONVCOMSTR}", + ), + emitter=emit_pvsreport, + src_suffix=".log", + ), + } + ) + atexit.register(atexist_handler) + + +def exists(env): + return True diff --git a/site_scons/commandline.scons b/site_scons/commandline.scons index fc2534ed..6d01eb8f 100644 --- a/site_scons/commandline.scons +++ b/site_scons/commandline.scons @@ -235,6 +235,11 @@ vars.AddVariables( ("applications_user", False), ], ), + BoolVariable( + "PVSNOBROWSER", + help="Don't open browser after generating error repots", + default=False, + ), ) Return("vars") diff --git a/site_scons/extapps.scons b/site_scons/extapps.scons index b8f21056..4eff65cb 100644 --- a/site_scons/extapps.scons +++ b/site_scons/extapps.scons @@ -143,6 +143,11 @@ sdk_apisyms = appenv.SDKSymGenerator( "${BUILD_DIR}/assets/compiled/symbols.h", appenv["SDK_DEFINITION"] ) Alias("api_syms", sdk_apisyms) +ENV.Replace( + SDK_APISYMS=sdk_apisyms, + _APP_ICONS=appenv["_APP_ICONS"], +) + if appenv["FORCE"]: appenv.AlwaysBuild(sdk_source, sdk_tree, sdk_apicheck, sdk_apisyms)