[FL-3080] fbt: PVS support (#2286)

* 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: あく <alleteam@gmail.com>

.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 \
+          ./fbt COMPACT=1 PVSNOBROWSER=1 firmware_pvs || WARNINGS=1
-              -a GA:1,2,3 \
-              -t fullhtml \
-              --indicate-warnings \
-              PVS-Studio.log \
-              -o reports/${DEFAULT_TARGET}-${SUFFIX} || 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'

.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

.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",

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.Replace(FW_RESOURCES=resources)
-    env["FW_RESOURCES"] = resources
     assetsenv.Alias("resources", resources)
 
 Return("assetslib")

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

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

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

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

lib/subghz/blocks/math.h

@@ -5,8 +5,16 @@
 #include <stddef.h>
 
 #define bit_read(value, bit) (((value) >> (bit)) & 0x01)
-#define bit_set(value, bit) ((value) |= (1UL << (bit)))
+#define bit_set(value, bit)           \
-#define bit_clear(value, bit) ((value) &= ~(1UL << (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)))
 

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;

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

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 = []
 

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
 """

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

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

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)