mirror of
https://github.com/esphome/esphome.git
synced 2026-08-23 06:36:23 +00:00
Refuse inconsistent memory-layout configs, validate the cached linker script
Unknown or conflicting VTABLES_IN_* defines raise instead of warning and picking arbitrarily (a typo previously won the sorted pick and died in the SDK header's #error). Custom MMU sizes without the CUSTOM knob raise too: PlatformIO warns but its defaults win the compile line, while here the user's tokens would win and compile against a layout the linker script does not implement. The cached linker script is re-checked for its SECTIONS block so a truncated file regenerates, the testing-mode flash-ld read gets the same OSError guard as the gcc spawn, the ParseFlags docstring states its actual coverage, and the ld test mocks are quiet so the clean path asserts no warnings.
This commit is contained in:
@@ -215,10 +215,15 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig:
|
||||
# build flags, whose iteration order varies between processes.
|
||||
vtables_knobs = sorted(name for name in defines if name.startswith("VTABLES_IN_"))
|
||||
known_vtables = {"VTABLES_IN_FLASH", "VTABLES_IN_DRAM", "VTABLES_IN_IRAM"}
|
||||
# A typo would otherwise win the sorted pick and end in the SDK header's
|
||||
# #error, and a conflicting pair would resolve arbitrarily; both are
|
||||
# config errors, not build-time surprises
|
||||
if unknown := [k for k in vtables_knobs if k not in known_vtables]:
|
||||
_LOGGER.warning("Unknown VTABLES_IN_* define(s): %s", ", ".join(unknown))
|
||||
raise EsphomeError(f"Unknown VTABLES_IN_* define(s): {', '.join(unknown)}")
|
||||
if len(vtables_knobs) > 1:
|
||||
_LOGGER.warning("Multiple VTABLES_IN_* defines; using %s", vtables_knobs[0])
|
||||
raise EsphomeError(
|
||||
f"Conflicting VTABLES_IN_* defines: {', '.join(vtables_knobs)}"
|
||||
)
|
||||
vtables = vtables_knobs[0] if vtables_knobs else "VTABLES_IN_FLASH"
|
||||
|
||||
mmu = next((variant for knob, variant in _MMU_VARIANTS if knob in defines), None)
|
||||
@@ -237,13 +242,14 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig:
|
||||
)
|
||||
else:
|
||||
if "MMU_IRAM_SIZE" in defines or "MMU_ICACHE_SIZE" in defines:
|
||||
# Same diagnostic the PlatformIO builder prints: without the
|
||||
# knob the linker script keeps the default layout while the
|
||||
# compile line carries the custom sizes
|
||||
_LOGGER.warning(
|
||||
"Detected custom MMU flags; use "
|
||||
"-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM to disable the "
|
||||
"default configuration"
|
||||
# PlatformIO only warns here and appends its defaults last so
|
||||
# they win the compile line; in this generator the user's
|
||||
# tokens would come last instead, compiling against a memory
|
||||
# layout the linker script does not implement. Refuse rather
|
||||
# than reproduce the upstream footgun with worse odds.
|
||||
raise EsphomeError(
|
||||
"Custom MMU_IRAM_SIZE/MMU_ICACHE_SIZE build flags require "
|
||||
"-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM"
|
||||
)
|
||||
mmu = list(_MMU_DEFAULT)
|
||||
|
||||
@@ -306,7 +312,10 @@ def _project_flags(
|
||||
Every entry is shell-lexed the way PlatformIO's ``ParseFlags`` does, so a
|
||||
linker flag anywhere in an entry reaches the link line and
|
||||
``build_unflags`` matches individual tokens (``-Os`` inside ``-Os -g3``).
|
||||
Lexed tokens are re-quoted at emission via ``_shell_token``.
|
||||
Only the flag forms ESPHome emits are classified (``-Wl,``/``-L``/``-l``
|
||||
and compile flags); rarities like plain-form ``-T``/``-u``/``-Xlinker``
|
||||
route to the compile line, unlike full ParseFlags. Lexed tokens are
|
||||
re-quoted at emission via ``_shell_token``.
|
||||
"""
|
||||
compile_flags: list[str] = []
|
||||
link_flags: list[str] = []
|
||||
@@ -372,11 +381,20 @@ def generate_ld_scripts(
|
||||
# any behavioral edit in build_surgery self-invalidates the cache
|
||||
+ f" {build_surgery.surgery_fingerprint()}"
|
||||
)
|
||||
if not (
|
||||
output.is_file()
|
||||
and stamp.is_file()
|
||||
and stamp.read_text(encoding="utf-8") == stamp_content
|
||||
):
|
||||
|
||||
def _cached_ld_is_valid() -> bool:
|
||||
if not (
|
||||
output.is_file()
|
||||
and stamp.is_file()
|
||||
and stamp.read_text(encoding="utf-8") == stamp_content
|
||||
):
|
||||
return False
|
||||
# A truncated or externally edited script must force regeneration,
|
||||
# not be reused on existence alone (the SECTIONS check below only
|
||||
# guards the generation path)
|
||||
return "SECTIONS" in output.read_text(encoding="utf-8")
|
||||
|
||||
if not _cached_ld_is_valid():
|
||||
try:
|
||||
result = subprocess.run(
|
||||
cmd, capture_output=True, text=True, check=False, close_fds=False
|
||||
@@ -409,10 +427,17 @@ def generate_ld_scripts(
|
||||
# A patched copy of the flash ld in the build dir; resolved through
|
||||
# the same -L path as the SDK original it shadows.
|
||||
flash_ld = framework / "tools" / "sdk" / "ld" / flash_ld_name
|
||||
try:
|
||||
flash_ld_text = flash_ld.read_text(encoding="utf-8")
|
||||
except OSError as err:
|
||||
# Same half-extracted-cache hazard as the gcc spawn above
|
||||
raise EsphomeError(
|
||||
f"Could not read {flash_ld}: {err}; run 'esphome clean-all' and retry"
|
||||
) from err
|
||||
write_file_if_changed(
|
||||
ld_dir / f"testing_{flash_ld_name}",
|
||||
build_surgery.apply_testing_memory_patches(
|
||||
flash_ld.read_text(encoding="utf-8"),
|
||||
flash_ld_text,
|
||||
("dram0_0_seg", "irom0_0_seg"),
|
||||
),
|
||||
)
|
||||
|
||||
@@ -153,7 +153,7 @@ def test_defines_match_platformio_builder() -> None:
|
||||
]
|
||||
|
||||
|
||||
def _make_framework(tmp_path: Path) -> dict[str, Path]:
|
||||
def _make_framework(tmp_path: Path) -> InstalledPaths:
|
||||
framework = tmp_path / "framework"
|
||||
core = framework / "cores" / "esp8266"
|
||||
core.mkdir(parents=True)
|
||||
@@ -270,9 +270,14 @@ def test_generate_ld_scripts(tmp_path: Path) -> None:
|
||||
|
||||
paths = _make_framework(tmp_path)
|
||||
_set_flags("-DFP_IN_IROM")
|
||||
result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT)
|
||||
with patch.object(arduino8266.subprocess, "run", return_value=result) as mock_run:
|
||||
result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="")
|
||||
with (
|
||||
patch.object(arduino8266.subprocess, "run", return_value=result) as mock_run,
|
||||
patch.object(arduino8266._LOGGER, "warning") as mock_warn,
|
||||
):
|
||||
ld_dir = _run_generate_ld_scripts(paths)
|
||||
# A clean preprocessor run must be quiet
|
||||
mock_warn.assert_not_called()
|
||||
content = (ld_dir / "local.eagle.app.v6.common.ld").read_text()
|
||||
assert RATETABLE_RULE in content
|
||||
cmd = mock_run.call_args[0][0]
|
||||
@@ -297,6 +302,20 @@ def test_generate_ld_scripts(tmp_path: Path) -> None:
|
||||
mock_run.assert_called_once()
|
||||
|
||||
|
||||
def test_generate_ld_scripts_corrupt_cache_regenerates(tmp_path: Path) -> None:
|
||||
"""A truncated cached linker script regenerates even with a fresh stamp."""
|
||||
paths = _make_framework(tmp_path)
|
||||
result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="")
|
||||
with patch.object(arduino8266.subprocess, "run", return_value=result):
|
||||
ld_dir = _run_generate_ld_scripts(paths)
|
||||
output = ld_dir / "local.eagle.app.v6.common.ld"
|
||||
output.write_text("truncated garbage")
|
||||
with patch.object(arduino8266.subprocess, "run", return_value=result) as mock_run:
|
||||
_run_generate_ld_scripts(paths)
|
||||
mock_run.assert_called_once()
|
||||
assert RATETABLE_RULE in output.read_text()
|
||||
|
||||
|
||||
def test_generate_ld_scripts_failure(tmp_path: Path) -> None:
|
||||
|
||||
paths = _make_framework(tmp_path)
|
||||
@@ -320,7 +339,7 @@ def test_generate_ld_scripts_testing_mode(tmp_path: Path) -> None:
|
||||
"}\n"
|
||||
)
|
||||
CORE.testing_mode = True
|
||||
result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT)
|
||||
result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="")
|
||||
with patch.object(arduino8266.subprocess, "run", return_value=result):
|
||||
ld_dir = _run_generate_ld_scripts(paths)
|
||||
patched = (ld_dir / "testing_eagle.flash.4m.ld").read_text()
|
||||
@@ -415,15 +434,14 @@ def test_flag_defines_joins_spaced_define() -> None:
|
||||
assert "" not in defines
|
||||
|
||||
|
||||
def test_build_config_custom_mmu_without_knob_warns(
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""Custom MMU sizes without the CUSTOM knob keep the default layout and
|
||||
warn, as the PlatformIO builder does."""
|
||||
def test_build_config_custom_mmu_without_knob_raises() -> None:
|
||||
"""Custom MMU sizes without the CUSTOM knob would compile against a
|
||||
layout the linker script does not implement; refuse instead of warning
|
||||
(PlatformIO warns, but its defaults win the compile line; ours would
|
||||
not)."""
|
||||
_set_flags("-DMMU_IRAM_SIZE=0xC000")
|
||||
config = _resolve_build_config(_flag_defines(set()))
|
||||
assert config.mmu_defines == ["MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x8000"]
|
||||
assert "Detected custom MMU flags" in caplog.text
|
||||
with pytest.raises(EsphomeError, match="PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM"):
|
||||
_resolve_build_config(_flag_defines(set()))
|
||||
|
||||
|
||||
def test_flag_defines_lexes_quoted_single_tokens() -> None:
|
||||
@@ -534,15 +552,18 @@ def test_flag_defines_respects_unflags() -> None:
|
||||
assert config.vtables == "VTABLES_IN_FLASH"
|
||||
|
||||
|
||||
def test_vtables_unknown_and_conflicting_warn(
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
_set_flags("-DVTABLES_IN_BANANA", "-DVTABLES_IN_DRAM")
|
||||
config = _resolve_build_config(_flag_defines(set()))
|
||||
assert "Unknown VTABLES_IN_*" in caplog.text
|
||||
assert "Multiple VTABLES_IN_*" in caplog.text
|
||||
# Deterministic pick, as before
|
||||
assert config.vtables == "VTABLES_IN_BANANA"
|
||||
def test_vtables_unknown_raises() -> None:
|
||||
"""A typo'd knob would win the sorted pick and die in the SDK header's
|
||||
#error; fail by name at generation instead."""
|
||||
_set_flags("-DVTABLES_IN_BANANA")
|
||||
with pytest.raises(EsphomeError, match="Unknown VTABLES_IN_.*BANANA"):
|
||||
_resolve_build_config(_flag_defines(set()))
|
||||
|
||||
|
||||
def test_vtables_conflicting_raises() -> None:
|
||||
_set_flags("-DVTABLES_IN_DRAM", "-DVTABLES_IN_IRAM")
|
||||
with pytest.raises(EsphomeError, match="Conflicting VTABLES_IN_"):
|
||||
_resolve_build_config(_flag_defines(set()))
|
||||
|
||||
|
||||
def test_project_flags_empty_lib_flags_warn(
|
||||
|
||||
Reference in New Issue
Block a user