fix(ci): report flash and static RAM usage separately

.github/workflows/flash_analysis.yml

@@ -28,9 +28,9 @@ jobs:
         target: [px4_fmu-v5x, px4_fmu-v6x]
     outputs:
       px4_fmu-v5x-bloaty-output: ${{ steps.gen-output.outputs.px4_fmu-v5x-bloaty-output }}
-      px4_fmu-v5x-bloaty-summary-map: ${{ steps.gen-output.outputs.px4_fmu-v5x-bloaty-summary-map }}
+      px4_fmu-v5x-memory-summary: ${{ steps.gen-output.outputs.px4_fmu-v5x-memory-summary }}
       px4_fmu-v6x-bloaty-output: ${{ steps.gen-output.outputs.px4_fmu-v6x-bloaty-output }}
-      px4_fmu-v6x-bloaty-summary-map: ${{ steps.gen-output.outputs.px4_fmu-v6x-bloaty-summary-map }}
+      px4_fmu-v6x-memory-summary: ${{ steps.gen-output.outputs.px4_fmu-v6x-memory-summary }}
     steps:
       - uses: runs-on/action@v2
       - uses: actions/checkout@v6
@@ -40,6 +40,12 @@ jobs:
       - name: Configure Git Safe Directory
         run: git config --system --add safe.directory '*'
 
+      - name: Test and preserve memory reporter
+        # The baseline checkout may not contain this version of the reporter.
+        run: |
+          python3 -m unittest discover -s Tools/ci -p test_firmware_size.py
+          cp Tools/ci/firmware_size.py "$RUNNER_TEMP/firmware_size.py"
+
       - name: Cache - Restore ccache (current)
         id: cache_current
         uses: actions/cache/restore@v5
@@ -85,10 +91,10 @@ jobs:
           make submodulesclean
           ccache -C
 
-      - name: If it's a PR checkout the base branch
+      - name: If it's a PR checkout the base of the merge commit
         if: ${{ github.event.pull_request }}
-        # As checkout creates a merge commit (merging the base branch into the PR branch), the base branch is the base for a diff of the PR changes.
-        run: git checkout ${{ github.event.pull_request.base.ref }}
+        # The base branch tip may already contain this PR (merged while the job was queued) or unrelated commits.
+        run: git checkout ${{ github.sha }}^1
 
       - name: If it's a push checkout the previous commit
         if: github.event_name == 'push'
@@ -130,29 +136,35 @@ jobs:
         id: bloaty-step
         with:
           bloaty-file-args: ./with-change.elf -- ./before-change.elf
-          bloaty-additional-args: -d sections,symbols -s vm -n 20
+          bloaty-additional-args: -d sections,symbols -s vm -n 20 --domain=vm
           output-to-summary: true
 
       - name: Generate output
         id: gen-output
+        env:
+          BLOATY_OUTPUT: ${{ steps.bloaty-step.outputs.bloaty-output }}
         run: |
+          summary=$(python3 "$RUNNER_TEMP/firmware_size.py" \
+            --before before-change.elf --after with-change.elf)
           EOF=$(dd if=/dev/urandom bs=15 count=1 status=none | base64)
-          echo "${{ matrix.target }}-bloaty-output<<$EOF" >> $GITHUB_OUTPUT
-          echo "${{ steps.bloaty-step.outputs.bloaty-output-encoded }}" >> $GITHUB_OUTPUT
-          echo "$EOF" >> $GITHUB_OUTPUT
-          echo "${{ matrix.target }}-bloaty-summary-map<<$EOF" >> $GITHUB_OUTPUT
-          echo '${{ steps.bloaty-step.outputs.bloaty-summary-map }}' >> $GITHUB_OUTPUT
-          echo "$EOF" >> $GITHUB_OUTPUT
+          {
+            echo "${{ matrix.target }}-memory-summary=$summary"
+            echo "${{ matrix.target }}-bloaty-output<<$EOF"
+            printf '%s\n' "$BLOATY_OUTPUT"
+            echo "$EOF"
+          } >> "$GITHUB_OUTPUT"
 
   post_pr_comment:
     name: Publish Results
     runs-on: [runs-on,runner=1cpu-linux-x64,image=ubuntu24-full-x64,"run-id=${{ github.run_id }}"]
     needs: [analyze_flash]
     env:
-      V5X-SUMMARY-MAP-ABS: ${{ fromJSON(fromJSON(needs.analyze_flash.outputs.px4_fmu-v5x-bloaty-summary-map).vm-absolute) }}
-      V5X-SUMMARY-MAP-PERC: ${{ fromJSON(fromJSON(needs.analyze_flash.outputs.px4_fmu-v5x-bloaty-summary-map).vm-percentage) }}
-      V6X-SUMMARY-MAP-ABS: ${{ fromJSON(fromJSON(needs.analyze_flash.outputs.px4_fmu-v6x-bloaty-summary-map).vm-absolute) }}
-      V6X-SUMMARY-MAP-PERC: ${{ fromJSON(fromJSON(needs.analyze_flash.outputs.px4_fmu-v6x-bloaty-summary-map).vm-percentage) }}
+      V5X_FLASH: ${{ fromJSON(needs.analyze_flash.outputs.px4_fmu-v5x-memory-summary).flash }}
+      V5X_RAM: ${{ fromJSON(needs.analyze_flash.outputs.px4_fmu-v5x-memory-summary).ram }}
+      V5X_CHANGED: ${{ fromJSON(needs.analyze_flash.outputs.px4_fmu-v5x-memory-summary).changed }}
+      V6X_FLASH: ${{ fromJSON(needs.analyze_flash.outputs.px4_fmu-v6x-memory-summary).flash }}
+      V6X_RAM: ${{ fromJSON(needs.analyze_flash.outputs.px4_fmu-v6x-memory-summary).ram }}
+      V6X_CHANGED: ${{ fromJSON(needs.analyze_flash.outputs.px4_fmu-v6x-memory-summary).changed }}
     if: github.event.pull_request
     steps:
       - name: Find Comment
@@ -166,15 +178,15 @@ jobs:
       - name: Set Build Time
         id: bt
         run: |
-          echo "timestamp=$(date +'%Y-%m-%dT%H:%M:%S')" >> $GITHUB_OUTPUT
+          echo "timestamp=$(date +'%Y-%m-%dT%H:%M:%S')" >> "$GITHUB_OUTPUT"
 
       - name: Write pr-comment artifact
-        # Skip only when the diff is exactly 0 bytes on all targets and no comment exists yet.
+        # Update existing comments even when both deltas return to zero.
         # This can't be moved to the job-level conditions, as GH actions don't allow a job-level if condition to access the env.
         if: |
           steps.fc.outputs.comment-id != '' ||
-          env.V5X-SUMMARY-MAP-ABS != 0 ||
-          env.V6X-SUMMARY-MAP-ABS != 0
+          env.V5X_CHANGED == 'true' ||
+          env.V6X_CHANGED == 'true'
         run: |
           mkdir -p pr-comment
           cat > pr-comment/manifest.json <<EOF
@@ -186,17 +198,23 @@ jobs:
           EOF
           cat > pr-comment/body.md <<'PR_COMMENT_BODY_EOF'
           <!-- pr-comment-poster:flash-analysis -->
-          ## 🔎 FLASH Analysis
+          ## 🔎 Flash and RAM Analysis
+
+          | Target | Flash change | Static RAM change |
+          | --- | ---: | ---: |
+          | px4_fmu-v5x | ${{ env.V5X_FLASH }} | ${{ env.V5X_RAM }} |
+          | px4_fmu-v6x | ${{ env.V6X_FLASH }} | ${{ env.V6X_RAM }} |
+
           <details>
-          <summary>px4_fmu-v5x [Total VM Diff: ${{ env.V5X-SUMMARY-MAP-ABS }} byte (${{ env.V5X-SUMMARY-MAP-PERC}} %)]</summary>
+          <summary>px4_fmu-v5x — Bloaty section/symbol breakdown</summary>
 
           ```
           ${{ needs.analyze_flash.outputs.px4_fmu-v5x-bloaty-output }}
           ```
           </details>
 
           <details>
-          <summary>px4_fmu-v6x [Total VM Diff: ${{ env.V6X-SUMMARY-MAP-ABS }} byte (${{ env.V6X-SUMMARY-MAP-PERC }} %)]</summary>
+          <summary>px4_fmu-v6x — Bloaty section/symbol breakdown</summary>
 
           ```
           ${{ needs.analyze_flash.outputs.px4_fmu-v6x-bloaty-output }}
@@ -209,8 +227,8 @@ jobs:
       - name: Upload pr-comment artifact
         if: |
           steps.fc.outputs.comment-id != '' ||
-          env.V5X-SUMMARY-MAP-ABS != 0 ||
-          env.V6X-SUMMARY-MAP-ABS != 0
+          env.V5X_CHANGED == 'true' ||
+          env.V6X_CHANGED == 'true'
         uses: actions/upload-artifact@v7
         with:
           name: pr-comment

Tools/ci/firmware_size.py

@@ -0,0 +1,85 @@
+#!/usr/bin/env python3
+"""Compare flash image and static RAM usage in bare-metal firmware ELFs."""
+
+import argparse
+import json
+import os
+from pathlib import Path
+import subprocess
+
+
+def memory_usage(elf: Path) -> dict[str, int]:
+    # Sections rather than program headers: ld may map the ELF header into the
+    # first LOAD segment, below the flash origin, depending on its page size.
+    headers = subprocess.check_output(
+        ["arm-none-eabi-objdump", "--section-headers", "--wide", str(elf)],
+        text=True,
+        env={**os.environ, "LC_ALL": "C"},
+    )
+    image_start = None
+    image_end = 0
+    ram = 0
+
+    for line in headers.splitlines():
+        fields = line.split(maxsplit=7)
+        if len(fields) < 8 or not fields[0].isdigit():
+            continue
+
+        size, vma, lma = (int(value, 16) for value in fields[2:5])
+        flags = {flag.strip() for flag in fields[7].split(",")}
+        if "ALLOC" not in flags or not size:
+            continue
+
+        in_image = {"LOAD", "CONTENTS"} <= flags
+        if in_image:
+            image_start = lma if image_start is None else min(image_start, lma)
+            image_end = max(image_end, lma + size)
+
+        # Only code and constants execute in place. Initialized data and RAM
+        # functions are copied out of the image; NOLOAD sections reserve RAM.
+        if not (in_image and vma == lma):
+            ram += size
+
+    if image_start is None:
+        raise ValueError(f"{elf}: no flash load image found")
+
+    # Include alignment gaps in the programmed image, as objcopy -O binary does.
+    return {"flash": image_end - image_start, "ram": ram}
+
+
+def indicator(delta: int) -> str:
+    if delta > 1000:
+        return "🔴 "
+    if delta > 100:
+        return "🟡 "
+    if delta < -100:
+        return "🟢 "
+    return ""
+
+
+def format_change(before: int, after: int) -> str:
+    delta = after - before
+    percentage = f"{delta / before:+.2%}" if before else "n/a"
+    return f"{indicator(delta)}{delta:+,} B ({percentage})"
+
+
+def summarize(before: dict[str, int], after: dict[str, int]) -> dict:
+    return {
+        "flash": format_change(before["flash"], after["flash"]),
+        "ram": format_change(before["ram"], after["ram"]),
+        "changed": before != after,
+    }
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--before", type=Path, required=True)
+    parser.add_argument("--after", type=Path, required=True)
+    args = parser.parse_args()
+    before = memory_usage(args.before)
+    after = memory_usage(args.after)
+    print(json.dumps(summarize(before, after)))
+
+
+if __name__ == "__main__":
+    main()

Tools/ci/test_firmware_size.py

@@ -0,0 +1,133 @@
+"""Exercise memory accounting against ARM ELFs produced by GNU binutils."""
+
+import json
+from pathlib import Path
+import subprocess
+import sys
+import tempfile
+import unittest
+
+from firmware_size import format_change, memory_usage, summarize
+
+
+class FirmwareSizeTest(unittest.TestCase):
+    def setUp(self):
+        self.directory = tempfile.TemporaryDirectory()
+        self.addCleanup(self.directory.cleanup)
+        self.root = Path(self.directory.name)
+        self.linker = self.root / "firmware.ld"
+        self.linker.write_text("""
+MEMORY
+{
+    FLASH (rx) : ORIGIN = 0x08008000, LENGTH = 2016K
+    ITCM (rwx) : ORIGIN = 0, LENGTH = 16K
+    RAM (rw) : ORIGIN = 0x20020000, LENGTH = 368K
+    SRAM4 (rw) : ORIGIN = 0x38000000, LENGTH = 64K
+}
+SECTIONS
+{
+    .text : { *(.text) } > FLASH
+    .ramfunc : { *(.ramfunc) } > ITCM AT > FLASH
+    .data : { *(.data) } > RAM AT > FLASH
+    .bss (NOLOAD) : { *(.bss) } > RAM
+    .noinit (NOLOAD) : { *(.noinit) } > SRAM4
+    .debug_info 0 : { *(.debug_info) }
+}
+""")
+
+    def build(self, name="firmware", *, text=32, data=16, bss=32,
+              ramfunc=16, reserved=16, debug=16, linker_flags=()):
+        source = self.root / f"{name}.s"
+        obj = source.with_suffix(".o")
+        elf = source.with_suffix(".elf")
+        source.write_text(f"""
+.section .text,"ax"
+.space {text}, 1
+.section .ramfunc,"ax"
+.space {ramfunc}, 2
+.section .data,"aw"
+.space {data}, 3
+.section .bss,"aw",%nobits
+.space {bss}
+.section .noinit,"aw",%nobits
+.space {reserved}
+.section .debug_info,"",%progbits
+.space {debug}, 4
+""")
+        subprocess.run(["arm-none-eabi-as", str(source), "-o", str(obj)], check=True)
+        subprocess.run([
+            "arm-none-eabi-ld", *linker_flags, "-T", str(self.linker), str(obj), "-o", str(elf)
+        ], check=True)
+        return elf
+
+    def usage(self, **kwargs):
+        elf = self.build(**kwargs)
+        usage = memory_usage(elf)
+        binary = elf.with_suffix(".bin")
+        subprocess.run([
+            "arm-none-eabi-objcopy", "-O", "binary", str(elf), str(binary)
+        ], check=True)
+        self.assertEqual(usage["flash"], binary.stat().st_size)
+        return usage
+
+    def test_bss_and_reserved_buffers_only_use_ram(self):
+        before = self.usage()
+        self.assertEqual(before, {"flash": 64, "ram": 80})
+        for change in ({"bss": 96}, {"reserved": 80}):
+            with self.subTest(change=change):
+                after = self.usage(**change)
+                self.assertEqual(after, {"flash": 64, "ram": 144})
+
+    def test_initialized_data_and_ram_code_use_both(self):
+        for change in ({"data": 32}, {"ramfunc": 32}):
+            with self.subTest(change=change):
+                self.assertEqual(self.usage(**change), {"flash": 80, "ram": 96})
+
+    def test_debug_information_uses_neither(self):
+        self.assertEqual(self.usage(), self.usage(debug=4096))
+
+    def test_flash_includes_alignment_between_load_segments(self):
+        self.linker.write_text(self.linker.read_text().replace(
+            ".ramfunc :", ".ramfunc : ALIGN(32)"
+        ))
+        self.assertEqual(
+            self.usage(text=33), {"flash": 96, "ram": 80}
+        )
+
+    def test_elf_header_in_first_load_segment(self):
+        # A page size larger than the flash origin's alignment makes ld map the
+        # ELF header into the first LOAD segment, below the flash origin.
+        self.assertEqual(
+            self.usage(linker_flags=("-z", "max-page-size=0x10000")),
+            {"flash": 64, "ram": 80},
+        )
+
+    def test_opposing_changes_do_not_cancel_comment(self):
+        result = summarize({"flash": 1024, "ram": 1024}, {"flash": 1088, "ram": 960})
+        self.assertTrue(result["changed"])
+        self.assertEqual(result["flash"], "+64 B (+6.25%)")
+        self.assertEqual(result["ram"], "-64 B (-6.25%)")
+        self.assertFalse(summarize({"flash": 1, "ram": 0}, {"flash": 1, "ram": 0})["changed"])
+        self.assertEqual(format_change(0, 4488), "🔴 +4,488 B (n/a)")
+
+    def test_change_indicator(self):
+        for delta, expected in ((1001, "🔴 "), (1000, "🟡 "), (101, "🟡 "), (100, ""),
+                                (-100, ""), (-101, "🟢 "), (-5000, "🟢 ")):
+            with self.subTest(delta=delta):
+                self.assertTrue(format_change(10000, 10000 + delta).startswith(
+                    f"{expected}{delta:+,} B"))
+
+    def test_cli_from_outside_checkout(self):
+        before = self.build("before")
+        after = self.build("after", bss=96)
+        output = subprocess.check_output([
+            sys.executable, str(Path(__file__).with_name("firmware_size.py").resolve()),
+            "--before", str(before), "--after", str(after),
+        ], cwd=self.root, text=True)
+        self.assertEqual(json.loads(output), {
+            "flash": "+0 B (+0.00%)", "ram": "+64 B (+80.00%)", "changed": True,
+        })
+
+
+if __name__ == "__main__":
+    unittest.main()