Release/9.1 to master

.github/scripts/compute-region-sizes.py

@@ -0,0 +1,132 @@
+#!/usr/bin/env python3
+"""Compute per-linker-region byte usage for one .elf, from its companion
+.map file (Memory Configuration table) and `arm-none-eabi-objdump -h` output.
+
+Why: `arm-none-eabi-size -B` (Berkeley format, used for the flat flash/ram
+totals elsewhere in this pipeline) sums ALL writable sections into one
+"data"/"bss" pair regardless of which physical memory region they're linked
+into (e.g. F4/F7 parts split writable memory across RAM and CCM; H7 across
+RAM and DTCM). That single combined number is what CI historically reported
+as "RAM Delta", which misattributes growth that actually landed in a
+different, smaller, often more memory-pressured region. This script instead
+matches each section's load address against the target's own linker-defined
+memory regions (from its .map file), so the breakdown is exact and requires
+no per-MCU-family section-name table to maintain.
+
+Usage: compute-region-sizes.py <elf> <map-file> [size-tool]
+Prints a JSON object of {"<region>": <bytes>, ...} to stdout, one entry per
+writable region (attributes containing "w") that has nonzero used bytes.
+This is informational/non-gating, so any failure (missing/unparsable map,
+`size` tool error) prints "{}" and exits 0 rather than aborting the
+caller's build-artifact pipeline over a region breakdown it can live
+without - callers degrade gracefully to the flat flash/ram figures.
+"""
+import json
+import re
+import subprocess
+import sys
+
+
+def parse_memory_regions(map_path):
+    """Returns [(name, origin, end), ...] for writable, nonzero-length
+    regions, parsed from the map file's "Memory Configuration" table."""
+    try:
+        with open(map_path, encoding='utf-8', errors='replace') as f:
+            text = f.read()
+    except OSError:
+        return []
+
+    m = re.search(r'^Memory Configuration\s*\n\s*\n[^\n]*\n(.*?)\n\s*\nLinker script and memory map',
+                  text, re.MULTILINE | re.DOTALL)
+    if not m:
+        return []
+
+    regions = []
+    for line in m.group(1).splitlines():
+        parts = line.split()
+        if len(parts) < 3 or parts[0] == '*default*':
+            continue
+        name, origin_str, length_str = parts[0], parts[1], parts[2]
+        attrs = parts[3] if len(parts) > 3 else ''
+        try:
+            origin = int(origin_str, 16)
+            length = int(length_str, 16)
+        except ValueError:
+            continue
+        if length <= 0 or 'w' not in attrs:
+            continue
+        regions.append((name, origin, origin + length))
+    return regions
+
+
+def parse_section_sizes(elf_path, size_tool):
+    """Returns [(section_name, size, addr), ...] for allocated sections only
+    (the SHF_ALLOC ELF flag - i.e. sections that actually occupy memory at
+    runtime), via `objdump -h`'s two-line-per-section format. Filtering on
+    that flag, rather than on address, is what correctly tells apart
+    never-placed debug/symbol metadata (addr 0) from a real, zero-origin
+    section like F7/H7's ITCM-resident .tcm_code (FAST_CODE).
+
+    Derives the objdump binary from `size_tool` (same toolchain bin dir,
+    "-size" -> "-objdump") rather than taking a separate CLI argument, since
+    both are always installed side by side."""
+    objdump_tool = re.sub(r'-size$', '-objdump', size_tool)
+    out = subprocess.run([objdump_tool, '-h', elf_path], capture_output=True, text=True, check=True).stdout
+    lines = out.splitlines()
+    sections = []
+    for i, line in enumerate(lines):
+        m = re.match(r'\s*\d+\s+(\S+)\s+([0-9a-fA-F]+)\s+([0-9a-fA-F]+)\s+[0-9a-fA-F]+\s+[0-9a-fA-F]+', line)
+        if not m:
+            continue
+        name, size_str, addr_str = m.groups()
+        flags_line = lines[i + 1] if i + 1 < len(lines) else ''
+        if 'ALLOC' not in flags_line:
+            continue
+        size, addr = int(size_str, 16), int(addr_str, 16)
+        if size <= 0:
+            continue
+        sections.append((name, size, addr))
+    return sections
+
+
+def assign_sections(sections, regions):
+    """Sums (name, size, addr) `sections` into whichever of `regions`
+    [(name, start, end), ...] each falls within. Split out from `compute()`
+    so the matching logic can be unit tested without touching the
+    filesystem or spawning a subprocess."""
+    usage = {name: 0 for name, _, _ in regions}
+    for _name, size, addr in sections:
+        for name, start, end in regions:
+            if start <= addr < end:
+                usage[name] += size
+                break
+
+    return {name: bytes_ for name, bytes_ in usage.items() if bytes_ > 0}
+
+
+def compute(elf_path, map_path, size_tool):
+    regions = parse_memory_regions(map_path)
+    if not regions:
+        return {}
+
+    return assign_sections(parse_section_sizes(elf_path, size_tool), regions)
+
+
+def main():
+    if len(sys.argv) not in (3, 4):
+        print('usage: compute-region-sizes.py <elf> <map-file> [size-tool]', file=sys.stderr)
+        sys.exit(1)
+    elf_path, map_path = sys.argv[1], sys.argv[2]
+    size_tool = sys.argv[3] if len(sys.argv) == 4 else 'arm-none-eabi-size'
+
+    try:
+        result = compute(elf_path, map_path, size_tool)
+    except (OSError, subprocess.CalledProcessError) as e:
+        print(f'compute-region-sizes.py: {e} - falling back to no region breakdown', file=sys.stderr)
+        result = {}
+
+    print(json.dumps(result))
+
+
+if __name__ == '__main__':
+    main()

.github/scripts/compute-region-sizes.test.py

@@ -0,0 +1,107 @@
+#!/usr/bin/env python3
+"""Unit tests for compute-region-sizes.py (per-linker-region size breakdown).
+
+Run with: python3 .github/scripts/compute-region-sizes.test.py
+
+Pure-logic tests only - no filesystem/subprocess access, matching the
+convention in size-diff-comment.test.js. Exercises assign_sections()
+directly with synthetic (name, size, addr) sections rather than building
+real .elf/.map files.
+"""
+import importlib.util
+import pathlib
+import unittest
+
+_SPEC = importlib.util.spec_from_file_location(
+    'compute_region_sizes',
+    pathlib.Path(__file__).parent / 'compute-region-sizes.py',
+)
+crs = importlib.util.module_from_spec(_SPEC)
+_SPEC.loader.exec_module(crs)
+
+
+class AssignSectionsTest(unittest.TestCase):
+    def test_zero_address_allocated_section_is_counted(self):
+        # F7/H7's ITCM_RAM originates at 0x0 and holds real, allocated
+        # .tcm_code (FAST_CODE) - this must not be dropped as if it were
+        # unallocated metadata.
+        regions = [('ITCM_RAM', 0x00000000, 0x00004000)]
+        sections = [('.tcm_code', 22, 0x00000000)]
+        self.assertEqual(crs.assign_sections(sections, regions), {'ITCM_RAM': 22})
+
+    def test_non_allocated_sections_are_never_passed_in(self):
+        # parse_section_sizes() is responsible for excluding non-ALLOC
+        # sections (debug info, .comment, .ARM.attributes, symtab/strtab)
+        # before assign_sections() ever sees them - simulate that here by
+        # simply not including any such section in the input.
+        regions = [('ITCM_RAM', 0x00000000, 0x00004000)]
+        sections = [('.tcm_code', 22, 0x00000000)]
+        result = crs.assign_sections(sections, regions)
+        self.assertEqual(sum(result.values()), 22)
+
+    def test_normal_ram_region_matching_unaffected(self):
+        regions = [('RAM', 0x20000000, 0x20020000), ('CCM', 0x10000000, 0x10010000)]
+        sections = [
+            ('.data', 100, 0x20000000),
+            ('.bss', 200, 0x20000100),
+            ('.ccm_bss', 50, 0x10000000),
+        ]
+        self.assertEqual(crs.assign_sections(sections, regions), {'RAM': 300, 'CCM': 50})
+
+    def test_section_outside_any_region_is_dropped(self):
+        regions = [('RAM', 0x20000000, 0x20020000)]
+        sections = [('.some_other', 100, 0x90000000)]
+        self.assertEqual(crs.assign_sections(sections, regions), {})
+
+    def test_empty_regions_yields_empty_result(self):
+        self.assertEqual(crs.assign_sections([('.data', 10, 0x20000000)], []), {})
+
+
+class ParseSectionSizesObjdumpParsingTest(unittest.TestCase):
+    """Exercises the objdump -h text parsing in isolation by feeding it
+    through the same regex/flag logic parse_section_sizes() uses, without
+    invoking a real toolchain."""
+
+    def _parse(self, objdump_output):
+        import re
+        lines = objdump_output.splitlines()
+        sections = []
+        for i, line in enumerate(lines):
+            m = re.match(r'\s*\d+\s+(\S+)\s+([0-9a-fA-F]+)\s+([0-9a-fA-F]+)\s+[0-9a-fA-F]+\s+[0-9a-fA-F]+', line)
+            if not m:
+                continue
+            name, size_str, addr_str = m.groups()
+            flags_line = lines[i + 1] if i + 1 < len(lines) else ''
+            if 'ALLOC' not in flags_line:
+                continue
+            size, addr = int(size_str, 16), int(addr_str, 16)
+            if size <= 0:
+                continue
+            sections.append((name, size, addr))
+        return sections
+
+    def test_itcm_section_at_zero_address_is_allocated(self):
+        out = (
+            "t2.elf:     file format elf32-littlearm\n\n"
+            "Sections:\n"
+            "Idx Name          Size      VMA       LMA       File off  Algn\n"
+            "  0 .text         00000018  08000000  08000000  00010000  2**3\n"
+            "                  CONTENTS, ALLOC, LOAD, READONLY, CODE\n"
+            "  1 .tcm_code     00000016  00000000  08000018  00020000  2**1\n"
+            "                  CONTENTS, ALLOC, LOAD, READONLY, CODE\n"
+            "  2 .comment      00000033  00000000  00000000  00020016  2**0\n"
+            "                  CONTENTS, READONLY\n"
+            "  3 .ARM.attributes 0000002e  00000000  00000000  00020049  2**0\n"
+            "                  CONTENTS, READONLY\n"
+        )
+        sections = self._parse(out)
+        names = {name for name, _, _ in sections}
+        self.assertIn('.tcm_code', names)
+        self.assertNotIn('.comment', names)
+        self.assertNotIn('.ARM.attributes', names)
+        tcm = next(s for s in sections if s[0] == '.tcm_code')
+        self.assertEqual(tcm, ('.tcm_code', 0x16, 0x00000000))
+
+
+if __name__ == '__main__':
+    unittest.main()

.github/scripts/extract-size-report.sh

@@ -6,7 +6,14 @@
 # Usage: extract-size-report.sh <build-dir> <output-json> [size-tool]
 #
 # flash = .text + .data (what's programmed into flash)
-# ram   = .data + .bss  (what's reserved in RAM at runtime)
+# ram   = .data + .bss  (what's reserved in RAM at runtime, summed across
+#         EVERY writable memory region the target has — e.g. RAM+CCM on
+#         F4/F7 parts, RAM+DTCM on H7. It's a total, not one region.)
+# regions = { "<name>": <bytes>, ... } — the same total broken out per
+#         linker memory region (RAM, CCM, DTCM, ...), computed from the
+#         build's own .map file via compute-region-sizes.py. Omitted for a
+#         target whose .map file is missing, so consumers must treat it as
+#         optional.
 #
 # Runs inside the (unprivileged) build job on the PR's own checkout, so a
 # PR could in principle modify this script to misreport its own numbers.
@@ -46,6 +53,8 @@ if [ "${#ELFS[@]}" -eq 0 ]; then
     exit 0
 fi
 
+SCRIPT_DIR=$(dirname "${BASH_SOURCE[0]}")
+
 JQ_ARGS=()
 for elf in "${ELFS[@]}"; do
     target=$(basename "$elf" .elf)
@@ -56,13 +65,29 @@ for elf in "${ELFS[@]}"; do
     flash=$((text + data))
     ram=$((data + bss))
 
-    JQ_ARGS+=(--argjson "entry_${#JQ_ARGS[@]}" "{\"target\":\"${target}\",\"flash\":${flash},\"ram\":${ram}}")
+    # cmake's stm32.cmake/at32.cmake link every target with -Wl,-Map,<elf>.map
+    # (alongside -Wl,--print-memory-usage), so the per-region breakdown this
+    # script computes here matches exactly what the linker itself reported at
+    # build time. A missing map (e.g. a toolchain change that stops emitting
+    # one) degrades to just the flat flash/ram totals above, not a hard error.
+    map="${elf}.map"
+    regions='{}'
+    if [ -f "$map" ]; then
+        regions=$(python3 "${SCRIPT_DIR}/compute-region-sizes.py" "$elf" "$map" "$SIZE_TOOL") || regions='{}'
+    fi
+
+    JQ_ARGS+=(--argjson "entry_${#JQ_ARGS[@]}" "{\"target\":\"${target}\",\"flash\":${flash},\"ram\":${ram},\"regions\":${regions}}")
 done
 
 # Build via jq rather than manual string concatenation, so the target name
 # (an .elf basename, not otherwise validated) is JSON-escaped properly
-# instead of relying on it never containing a special character.
-jq -n "${JQ_ARGS[@]}" 'reduce $ARGS.named[] as $e ({}; .[$e.target] = {flash: $e.flash, ram: $e.ram})' \
-    > "$OUTPUT_JSON"
+# instead of relying on it never containing a special character. Omit
+# "regions" entirely when empty rather than storing a misleading {} that
+# would read as "this target has no writable memory regions".
+jq -n "${JQ_ARGS[@]}" '
+    reduce $ARGS.named[] as $e ({};
+        .[$e.target] = {flash: $e.flash, ram: $e.ram}
+            + (if ($e.regions | length) > 0 then {regions: $e.regions} else {} end)
+    )' > "$OUTPUT_JSON"
 
 echo "Wrote size report for ${#ELFS[@]} target(s) to $OUTPUT_JSON"

.github/scripts/size-diff-comment.js

@@ -4,7 +4,16 @@
 // dependency so it can be unit tested directly and reused (via require)
 // from the actions/github-script step in ci-size-report.yml.
 //
-// Report shape: { "<target>": { "flash": <bytes>, "ram": <bytes> }, ... }
+// Report shape: { "<target>": { "flash": <bytes>, "ram": <bytes>,
+//   "regions": { "<name>": <bytes>, ... } }, ... }
+//
+// "regions" is optional and, when present, breaks "ram" down by linker
+// memory region (e.g. RAM/CCM on F4/F7 parts, RAM/DTCM on H7) — "ram" alone
+// is the sum of those regions and stays purely additive/informational once
+// a regional breakdown exists. A region delta is only rendered when BOTH
+// the PR and baseline entries carry "regions" for that target; if either
+// side predates the field (e.g. an old stored baseline), the row falls
+// back to the combined "ram" figure instead of a partial breakdown.
 
 'use strict';
 
@@ -15,9 +24,9 @@
 // manager as a possible coverage gap, not decided unilaterally here.
 const REPRESENTATIVE_TARGETS = ['MATEKF405', 'MATEKF722', 'MATEKF765', 'MATEKH743'];
 
-// Below this magnitude a delta is noise (rounding/toolchain jitter), not a
-// real change worth calling out.
-const NOISE_THRESHOLD_BYTES = 256;
+// Below these magnitudes a delta is noise, not worth flagging.
+const FLASH_NOISE_THRESHOLD_BYTES = 4096;
+const RAM_NOISE_THRESHOLD_BYTES = 1024;
 
 function formatDelta(deltaBytes, baseBytes) {
     const sign = deltaBytes > 0 ? '+' : deltaBytes < 0 ? '' : '±';
@@ -45,6 +54,25 @@ function diffSizeReports(prReport, baselineReport) {
 
         const flashDelta = pr.flash - base.flash;
         const ramDelta = pr.ram - base.ram;
+
+        // Only trust a per-region breakdown when both sides have one - a
+        // region missing from just one side (schema drift, or a region a
+        // target gained/lost) would otherwise render a misleading partial
+        // delta for that region.
+        let regionDeltas;
+        if (pr.regions && base.regions) {
+            const names = Array.from(new Set([...Object.keys(pr.regions), ...Object.keys(base.regions)])).sort();
+            regionDeltas = names.map((name) => {
+                const prBytes = pr.regions[name] || 0;
+                const baseBytes = base.regions[name] || 0;
+                return { name, delta: prBytes - baseBytes, baseBytes };
+            });
+        }
+
+        const notable = regionDeltas
+            ? Math.abs(flashDelta) >= FLASH_NOISE_THRESHOLD_BYTES || regionDeltas.some((r) => Math.abs(r.delta) >= RAM_NOISE_THRESHOLD_BYTES)
+            : Math.abs(flashDelta) >= FLASH_NOISE_THRESHOLD_BYTES || Math.abs(ramDelta) >= RAM_NOISE_THRESHOLD_BYTES;
+
         return {
             target,
             status: 'compared',
@@ -54,7 +82,8 @@ function diffSizeReports(prReport, baselineReport) {
             baseRam: base.ram,
             flashDelta,
             ramDelta,
-            notable: Math.abs(flashDelta) >= NOISE_THRESHOLD_BYTES || Math.abs(ramDelta) >= NOISE_THRESHOLD_BYTES,
+            regionDeltas,
+            notable,
         };
     });
 }
@@ -95,7 +124,9 @@ function renderComment({ prReport, baselineReport, shortSha, baselineCommit, bas
         for (const row of rows) {
             if (row.status === 'compared') {
                 const flashCell = formatDelta(row.flashDelta, row.baseFlash);
-                const ramCell = formatDelta(row.ramDelta, row.baseRam);
+                const ramCell = row.regionDeltas
+                    ? row.regionDeltas.map((r) => `${r.name}: ${formatDelta(r.delta, r.baseBytes)}`).join('<br>')
+                    : formatDelta(row.ramDelta, row.baseRam);
                 const notableMark = row.notable ? ' ⚠️' : '';
                 lines.push(`| ${row.target}${notableMark} | ${flashCell} | ${ramCell} |`);
             } else if (row.status === 'no-baseline') {
@@ -121,4 +152,11 @@ function renderComment({ prReport, baselineReport, shortSha, baselineCommit, bas
     return lines.join('\n').trimEnd() + '\n';
 }
 
-module.exports = { REPRESENTATIVE_TARGETS, NOISE_THRESHOLD_BYTES, diffSizeReports, renderComment, formatDelta };
+module.exports = {
+    REPRESENTATIVE_TARGETS,
+    FLASH_NOISE_THRESHOLD_BYTES,
+    RAM_NOISE_THRESHOLD_BYTES,
+    diffSizeReports,
+    renderComment,
+    formatDelta,
+};