From 4e34e7e0d6b497a2016e54ebe42fb1700cee0a30 Mon Sep 17 00:00:00 2001 From: liu weikai Date: Sat, 27 Jun 2026 00:30:30 +0800 Subject: [PATCH] chore(esp): guard meshtastic stack hygiene --- .github/workflows/ci.yml | 3 + AGENTS.md | 8 + .../chat/infra/meshtastic/mt_adapter.h | 1 + .../src/chat/infra/meshtastic/mt_adapter.cpp | 5 +- scripts/check_esp_stack_hygiene.py | 266 ++++++++++++++++++ 5 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 scripts/check_esp_stack_hygiene.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7fbf1d5..d816ec82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,9 @@ jobs: - name: Verify shared/platform UI boundaries run: python3 scripts/check_platform_ui_boundaries.py + - name: Verify ESP stack hygiene + run: python3 scripts/check_esp_stack_hygiene.py + - name: Verify repository root has no build artifacts run: python3 scripts/check_root_build_artifacts.py diff --git a/AGENTS.md b/AGENTS.md index 125138fd..26bc34fd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,6 +57,14 @@ This project is indexed by GitNexus as **trail-mate** (43886 symbols, 82034 rela - Keep serial monitoring short and bounded unless the user explicitly asks for a longer capture. - Prefer CI/release-time artifacts for routine build outputs. Do not rebuild locally on every small change. +## ESP Stack Hygiene + +- Treat large protocol, config, frame, protobuf/nanopb, and byte-buffer objects as forbidden automatic locals on ESP task stacks. +- In ESP BLE/Meshtastic hot paths, do not create local `MeshtasticBleFrame`, `meshtastic_*` frame/config objects, or large byte arrays. Use member scratch storage, fixed-depth ring slots, static storage with clear ownership, or caller-provided output storage. +- Do not introduce `std::deque` in ESP BLE/Meshtastic bridge headers. Prefer fixed-depth ring buffers with explicit full/drop policy. +- Run `python3 scripts/check_esp_stack_hygiene.py` before committing changes that touch ESP BLE, Meshtastic bridge, app config save/load, or Android BLE connection flows. On Windows, if `python`/`python3` resolves to the Microsoft Store alias, use the real Python executable path reported by `where.exe python`. +- If the stack guard fails, fix the ownership/storage model. Do not bypass it with a local exception unless the user explicitly accepts the stack budget and the reason is documented next to the exception. + ## PowerShell Command Discipline - Keep PowerShell commands short and single-purpose. Avoid dense one-liners with complex quoting, nested shells, or long chained pipelines. diff --git a/platform/esp/arduino_common/include/platform/esp/arduino_common/chat/infra/meshtastic/mt_adapter.h b/platform/esp/arduino_common/include/platform/esp/arduino_common/chat/infra/meshtastic/mt_adapter.h index 3007d559..c1b4924a 100644 --- a/platform/esp/arduino_common/include/platform/esp/arduino_common/chat/infra/meshtastic/mt_adapter.h +++ b/platform/esp/arduino_common/include/platform/esp/arduino_common/chat/infra/meshtastic/mt_adapter.h @@ -204,6 +204,7 @@ class MtAdapter : public chat::IMeshAdapter std::array payload{}; meshtastic_MeshPacket packet = meshtastic_MeshPacket_init_zero; meshtastic_MqttClientProxyMessage proxy = meshtastic_MqttClientProxyMessage_init_zero; + meshtastic_ServiceEnvelope envelope = meshtastic_ServiceEnvelope_init_zero; }; struct TxScratchBuffers diff --git a/platform/esp/arduino_common/src/chat/infra/meshtastic/mt_adapter.cpp b/platform/esp/arduino_common/src/chat/infra/meshtastic/mt_adapter.cpp index 92a2afd0..a35b55e2 100644 --- a/platform/esp/arduino_common/src/chat/infra/meshtastic/mt_adapter.cpp +++ b/platform/esp/arduino_common/src/chat/infra/meshtastic/mt_adapter.cpp @@ -145,7 +145,7 @@ int8_t coreRadioSnr(float snr) void mt_diag_log(const char* fmt, ...) { - char buf[192] = {}; + char buf[160] = {}; va_list args; va_start(args, fmt); vsnprintf(buf, sizeof(buf), fmt, args); @@ -1209,8 +1209,9 @@ bool MtAdapter::queueMqttProxyPublish(const meshtastic_MeshPacket& packet, auto& scratch = mqtt_scratch_; std::memset(&scratch.proxy, 0, sizeof(scratch.proxy)); + std::memset(&scratch.envelope, 0, sizeof(scratch.envelope)); std::string node_id = mqttNodeIdString(); - meshtastic_ServiceEnvelope env = meshtastic_ServiceEnvelope_init_zero; + auto& env = scratch.envelope; env.packet = const_cast(&packet); env.channel_id = const_cast(channel_id); env.gateway_id = const_cast(node_id.c_str()); diff --git a/scripts/check_esp_stack_hygiene.py b/scripts/check_esp_stack_hygiene.py new file mode 100644 index 00000000..1fc022a2 --- /dev/null +++ b/scripts/check_esp_stack_hygiene.py @@ -0,0 +1,266 @@ +#!/usr/bin/env python3 +"""Reject high-risk ESP stack allocations in protocol hot paths.""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path +import os +import re +import subprocess +import sys + + +REPO_ROOT = Path(__file__).resolve().parent.parent + +SOURCE_SUFFIXES = {".c", ".cc", ".cpp", ".cxx", ".h", ".hpp", ".hh"} +IMPLEMENTATION_SUFFIXES = {".c", ".cc", ".cpp", ".cxx"} + +HOT_PATH_PREFIXES = ( + "platform/esp/arduino_common/src/chat/infra/meshtastic/", + "modules/core_phone/src/meshtastic/", +) + +HOT_PATH_FILES = { + "platform/esp/arduino_common/src/ble/meshtastic_ble.cpp", +} + +HOT_HEADER_PREFIXES = ( + "platform/esp/arduino_common/include/platform/esp/arduino_common/chat/infra/meshtastic/", + "modules/core_phone/include/phone/meshtastic/", +) + +HOT_HEADER_FILES = { + "platform/esp/arduino_common/include/ble/meshtastic_ble.h", +} + +APP_CONFIG_PATHS = { + "platform/esp/arduino_common/src/app_config_store.cpp", + "platform/esp/arduino_common/src/app_context.cpp", +} + +HIGH_RISK_PROTOCOL_TYPES = ( + "MeshtasticBleFrame", + "phone::meshtastic::MeshtasticBleFrame", + "meshtastic_FromRadio", + "meshtastic_ToRadio", + "meshtastic_MeshPacket", + "meshtastic_Data", + "meshtastic_AdminMessage", + "meshtastic_MqttClientProxyMessage", + "meshtastic_ServiceEnvelope", +) + +HIGH_RISK_CONFIG_TYPES = ( + "AppConfig", + "app::AppConfig", + "MeshConfig", + "chat::MeshConfig", +) + +RAW_ARRAY_STACK_LIMIT = 192 + +LARGE_ARRAY_SIZE_TOKENS = ( + "MeshtasticBleFrame::kMaxPayload", + "meshtastic_FromRadio_size", + "meshtastic_ToRadio_size", + "kMaxEncoded", + "MAX_PACKET_SIZE", + "TM_C6_MAX_PAYLOAD", +) + + +@dataclass(frozen=True) +class Violation: + path: Path + line_number: int + rule: str + line: str + detail: str + + +def normalize(path: Path) -> str: + return path.relative_to(REPO_ROOT).as_posix() + + +def is_under(relative: str, prefixes: tuple[str, ...]) -> bool: + return any(relative.startswith(prefix) for prefix in prefixes) + + +def git_tracked_files() -> list[Path]: + try: + result = subprocess.run( + ["git", "ls-files"], + cwd=REPO_ROOT, + check=True, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + except (OSError, subprocess.CalledProcessError): + return [] + return [REPO_ROOT / line for line in result.stdout.splitlines() if line] + + +def walk_source_files() -> list[Path]: + tracked = git_tracked_files() + if tracked: + return [path for path in tracked if path.suffix.lower() in SOURCE_SUFFIXES] + + source_files: list[Path] = [] + excluded_dirs = {".git", ".pio", ".tmp", "build", "dist", "third_party"} + for root, dir_names, file_names in os.walk(REPO_ROOT): + dir_names[:] = [name for name in dir_names if name not in excluded_dirs] + root_path = Path(root) + for file_name in file_names: + path = root_path / file_name + if path.suffix.lower() in SOURCE_SUFFIXES: + source_files.append(path) + return source_files + + +def type_decl_pattern(type_name: str) -> re.Pattern[str]: + escaped = re.escape(type_name) + return re.compile( + rf"^\s*(?!static\b)(?:(?:const|volatile)\s+)*{escaped}\s+" + rf"([A-Za-z_][A-Za-z0-9_]*)\s*(?:[={{;(])" + ) + + +PROTOCOL_TYPE_PATTERNS = tuple( + (type_name, type_decl_pattern(type_name)) for type_name in HIGH_RISK_PROTOCOL_TYPES +) +CONFIG_TYPE_PATTERNS = tuple( + (type_name, type_decl_pattern(type_name)) for type_name in HIGH_RISK_CONFIG_TYPES +) + +RAW_ARRAY_PATTERN = re.compile( + r"^\s*(?!static\b)(?:(?:const|volatile)\s+)*" + r"(?:uint8_t|std::uint8_t|char|unsigned\s+char|std::byte)\s+" + r"([A-Za-z_][A-Za-z0-9_]*)\s*\[\s*([A-Za-z0-9_:]+)\s*\]" +) + +STD_ARRAY_PATTERN = re.compile( + r"^\s*(?!static\b)(?:(?:const|volatile)\s+)*" + r"std::array\s*<\s*(?:uint8_t|std::uint8_t|char|unsigned\s+char|std::byte)\s*,\s*" + r"([A-Za-z0-9_:]+)\s*>\s+([A-Za-z_][A-Za-z0-9_]*)" +) + +STD_DEQUE_PATTERN = re.compile(r"\bstd::deque\s*<") + + +def is_large_array_size(size_token: str) -> bool: + if size_token.isdigit(): + return int(size_token) >= RAW_ARRAY_STACK_LIMIT + return size_token in LARGE_ARRAY_SIZE_TOKENS + + +def collect_violations() -> list[Violation]: + violations: list[Violation] = [] + + for path in walk_source_files(): + relative = normalize(path) + suffix = path.suffix.lower() + in_hot_impl = suffix in IMPLEMENTATION_SUFFIXES and ( + relative in HOT_PATH_FILES or is_under(relative, HOT_PATH_PREFIXES) + ) + in_hot_header = relative in HOT_HEADER_FILES or is_under(relative, HOT_HEADER_PREFIXES) + in_app_config = relative in APP_CONFIG_PATHS + + if not in_hot_impl and not in_hot_header and not in_app_config: + continue + + try: + lines = path.read_text(encoding="utf-8", errors="ignore").splitlines() + except OSError as exc: + violations.append(Violation(path, 0, "read-error", "", str(exc))) + continue + + for line_number, line in enumerate(lines, start=1): + stripped = line.strip() + if not stripped or stripped.startswith("//"): + continue + + if in_hot_header and STD_DEQUE_PATTERN.search(line): + violations.append( + Violation( + path, + line_number, + "esp-hot-path-std-deque", + stripped, + "Use a fixed-depth ring buffer with an explicit drop policy.", + ) + ) + + if in_hot_impl: + for type_name, pattern in PROTOCOL_TYPE_PATTERNS: + if pattern.search(line): + violations.append( + Violation( + path, + line_number, + "esp-hot-path-large-local", + stripped, + f"Move local {type_name} storage to member scratch/static storage or fill an existing slot.", + ) + ) + + raw_array = RAW_ARRAY_PATTERN.search(line) + if raw_array and is_large_array_size(raw_array.group(2)): + violations.append( + Violation( + path, + line_number, + "esp-hot-path-large-array", + stripped, + "Move large raw byte arrays off the task stack.", + ) + ) + + std_array = STD_ARRAY_PATTERN.search(line) + if std_array and is_large_array_size(std_array.group(1)): + violations.append( + Violation( + path, + line_number, + "esp-hot-path-large-std-array", + stripped, + "Move large std::array byte buffers off the task stack.", + ) + ) + + if in_app_config: + for type_name, pattern in CONFIG_TYPE_PATTERNS: + if pattern.search(line): + violations.append( + Violation( + path, + line_number, + "esp-app-config-large-local", + stripped, + f"Do not place {type_name} on app/config task stacks; use existing context storage or scratch.", + ) + ) + + return violations + + +def main() -> int: + violations = collect_violations() + if not violations: + print("ESP stack hygiene check passed.") + return 0 + + print("ESP stack hygiene check failed:") + for violation in violations: + relative = normalize(violation.path) + location = f"{relative}:{violation.line_number}" if violation.line_number else relative + print(f"- [{violation.rule}] {location}") + if violation.line: + print(f" {violation.line}") + print(f" {violation.detail}") + return 1 + + +if __name__ == "__main__": + sys.exit(main())