From d5fc6e055928d406a0e99077edb859af65233521 Mon Sep 17 00:00:00 2001 From: Ian Scott Date: Thu, 16 Apr 2026 23:53:34 -0600 Subject: [PATCH] Check for panic-causing printfs when compiling without PGDEBUG --- .github/workflows/build.yml | 31 +++++++++++---- sw/CMakeLists.txt | 28 +++++++++++++- sw/scripts/check_printf_leak.py | 68 +++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+), 8 deletions(-) create mode 100755 sw/scripts/check_printf_leak.py diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index eed5a03..ca774b8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -136,12 +136,28 @@ jobs: run: | mkdir -p $STAGING_DIR cp pgusinit/README.md $STAGING_DIR/README.md - - name: Get Tags - id: tag - uses: ildug/get-tag-action@v1 - - name: Generate release body + - name: Classify release + id: relinfo + shell: bash run: | - awk '/^# v${{ steps.tag.outputs.version }}/{a=1;next}/^# /{a=0}a' sw/CHANGELOG.md > release-body.md + VERSION="${GITHUB_REF_NAME#v}" + echo "version=$VERSION" >> "$GITHUB_OUTPUT" + if [[ "$VERSION" == *-* ]]; then + echo "prerelease=true" >> "$GITHUB_OUTPUT" + echo "base_version=${VERSION%%-*}" >> "$GITHUB_OUTPUT" + else + echo "prerelease=false" >> "$GITHUB_OUTPUT" + echo "base_version=$VERSION" >> "$GITHUB_OUTPUT" + fi + - name: Generate release body + shell: bash + run: | + VERSION="${{ steps.relinfo.outputs.version }}" + BASE="${{ steps.relinfo.outputs.base_version }}" + awk -v ver="$VERSION" '$0 ~ "^# v" ver "$" {a=1;next} /^# /{a=0} a' sw/CHANGELOG.md > release-body.md + if [ ! -s release-body.md ] && [ "$VERSION" != "$BASE" ]; then + awk -v ver="$BASE" '$0 ~ "^# v" ver "$" {a=1;next} /^# /{a=0} a' sw/CHANGELOG.md > release-body.md + fi - name: Download firmware artifact uses: actions/download-artifact@v4 with: @@ -156,11 +172,12 @@ jobs: uses: montudor/action-zip@v1 - name: Zip release run: | - zip -9 -r ../picogus-v${{ steps.tag.outputs.version }}.zip . + zip -9 -r ../picogus-v${{ steps.relinfo.outputs.version }}.zip . working-directory: ${{env.STAGING_DIR}} - name: Release uses: softprops/action-gh-release@v1 with: body_path: release-body.md - files: picogus-v${{ steps.tag.outputs.version }}.zip + files: picogus-v${{ steps.relinfo.outputs.version }}.zip draft: true + prerelease: ${{ steps.relinfo.outputs.prerelease }} diff --git a/sw/CMakeLists.txt b/sw/CMakeLists.txt index 28245f6..8beeb53 100644 --- a/sw/CMakeLists.txt +++ b/sw/CMakeLists.txt @@ -56,6 +56,7 @@ endfunction() option(PGDEBUG "Enable debug printf framework" OFF) +find_package(Python3 COMPONENTS Interpreter QUIET) # Global definitions across all targets add_compile_definitions( @@ -246,6 +247,18 @@ function(config_target TARGET_NAME MULTIFW) endif() pico_set_printf_implementation(${TARGET_NAME} none) endif() + if(NOT PGDEBUG AND Python3_FOUND) + # With printf stripped, __wrap_snprintf is the panic stub, so any + # stray printf call dead-loops the firmware. Fail the build if we + # find one. + add_custom_command(TARGET ${TARGET_NAME} POST_BUILD + COMMAND ${Python3_EXECUTABLE} + ${CMAKE_SOURCE_DIR}/scripts/check_printf_leak.py + ${CMAKE_OBJDUMP} ${CMAKE_ADDR2LINE} + $ + VERBATIM + ) + endif() pico_enable_stdio_semihosting(${TARGET_NAME} 0) endfunction() @@ -518,6 +531,7 @@ function(build_psg TARGET_NAME MULTIFW) ) target_sources(${TARGET_NAME} PRIVATE audio/volctrl.cpp + audio/audio_i2s_minimal.c square/square.cpp ) target_sources(${TARGET_NAME} PRIVATE psgplay.cpp) @@ -534,9 +548,12 @@ function(build_usb TARGET_NAME MULTIFW) pico_set_program_name(${TARGET_NAME} "picogus-usb") target_compile_definitions(${TARGET_NAME} PRIVATE USB_ONLY=1 + USE_CD_AUDIO_FIFO=1 ) - target_sources(${TARGET_NAME} PRIVATE + target_sources(${TARGET_NAME} PRIVATE audio/volctrl.cpp + audio/audio_fifo.c + audio/audio_i2s_minimal.c usbplay.cpp ) endfunction() @@ -665,6 +682,15 @@ if(PROJECT_TYPE STREQUAL "MULTIFW") if(NOT PGDEBUG) pico_set_printf_implementation(bootloader none) endif() + if(NOT PGDEBUG AND Python3_FOUND) + add_custom_command(TARGET bootloader POST_BUILD + COMMAND ${Python3_EXECUTABLE} + ${CMAKE_SOURCE_DIR}/scripts/check_printf_leak.py + ${CMAKE_OBJDUMP} ${CMAKE_ADDR2LINE} + $ + VERBATIM + ) + endif() add_dependencies(bootloader generate_flash_firmware_h) if (NOT UF2CREATE_FOUND) diff --git a/sw/scripts/check_printf_leak.py b/sw/scripts/check_printf_leak.py new file mode 100755 index 0000000..45bb95b --- /dev/null +++ b/sw/scripts/check_printf_leak.py @@ -0,0 +1,68 @@ +#!/usr/bin/env python3 + +# Used to fail the build if any non-panic code calls the raw printf-family +# wrappers that route to printf_none_assert. + +import re +import subprocess +import sys + + +def main(argv: list[str]) -> int: + if len(argv) != 4: + print( + "usage: check_printf_leak.py ", + file=sys.stderr, + ) + return 2 + + objdump, addr2line, elf = argv[1:4] + + dis = subprocess.run( + [objdump, "-d", elf], check=True, capture_output=True, text=True + ).stdout + + offenders = [] + call_re = re.compile( + r"^\s*([0-9a-f]+):\s.*\bbl\s+[0-9a-f]+\s+<(__wrap_v?s?n?printf)>" + ) + for line in dis.splitlines(): + m = call_re.match(line) + if not m: + continue + call_site, wrap = m.group(1), m.group(2) + res = subprocess.run( + [addr2line, "-fe", elf, call_site], + check=True, + capture_output=True, + text=True, + ) + lines = res.stdout.strip().splitlines() + if not lines: + continue + caller = lines[0].strip() + # Skip pico_stdio wrappers calling each other internally + # (__wrap_printf -> __wrap_vprintf etc) + if caller.startswith("__wrap_"): + continue + location = lines[1].strip() if len(lines) > 1 else "" + offenders.append((call_site, wrap, caller, location)) + + if not offenders: + return 0 + + print( + f"error: {elf} contains printf call(s) that will panic:", + file=sys.stderr, + ) + for addr, wrap, caller, loc in offenders: + print(f" {loc}: {caller}() -> {wrap} [@0x{addr}]", file=sys.stderr) + print( + "Replace with DBG_PRINTF from sw/include/pg_debug.h (no-ops when PGDEBUG is off).", + file=sys.stderr, + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main(sys.argv))