From 3653f34f31993789ba2b616d0d93dec3edb40ed7 Mon Sep 17 00:00:00 2001 From: Yell-walkalone <12112088@qq.com> Date: Mon, 14 Sep 2026 17:52:01 +0800 Subject: [PATCH 1/3] fix(cli): stop direct-config param overrides crashing divergence checks Direct config/PDK parameters (maps_to=None) are applied through their explicit config target and have no legacy backend projection. _backend_leaf_keys now returns an empty tuple for them instead of calling .items() on None, and a diverging direct-config override reports its canonical parameter name rather than an invented backend leaf key. Covers both --set runs and project-level ecc.toml [params] runs, with registry-wide regressions over every schema mapping shape. --- chipcompiler/cli/project/effective_config.py | 9 +- test/cli/commands/test_effective_config.py | 107 +++++++++++++++++++ test/cli/params/test_registry.py | 38 +++++++ 3 files changed, 153 insertions(+), 1 deletion(-) diff --git a/chipcompiler/cli/project/effective_config.py b/chipcompiler/cli/project/effective_config.py index 45162449..8e19234e 100644 --- a/chipcompiler/cli/project/effective_config.py +++ b/chipcompiler/cli/project/effective_config.py @@ -410,6 +410,10 @@ def layer_divergences(cfg, assembled: dict, entry) -> list[str]: def _backend_leaf_keys(schema) -> tuple[str, ...]: """The flattened backend key names a schema's maps_to target produces.""" maps_to = schema.maps_to + # Direct config/PDK parameters are applied through their explicit target + # and intentionally have no legacy backend projection. + if maps_to is None: + return () if isinstance(maps_to, str): return (maps_to,) return tuple(".".join((subtree, leaf)) for subtree, leaf in maps_to.items()) @@ -439,7 +443,10 @@ def _diverging_lower_keys(overrides: dict, resolve_lower) -> tuple[list[str], se continue coerced, type_err = _validate_schema_type(lower_value, schema) if type_err or coerced != override_value: - diverging.extend(leaf_keys) + # Direct config/PDK parameters have no backend leaf key. Keep the + # warning useful by identifying the canonical parameter instead + # of silently dropping the divergence or inventing a key. + diverging.extend(leaf_keys or [dotted]) return diverging, compared diff --git a/test/cli/commands/test_effective_config.py b/test/cli/commands/test_effective_config.py index d035086c..876a6cd7 100644 --- a/test/cli/commands/test_effective_config.py +++ b/test/cli/commands/test_effective_config.py @@ -1,6 +1,8 @@ import json from pathlib import Path +import pytest + from chipcompiler.cli import main as cli_main @@ -634,6 +636,111 @@ def test_run_set_overrides_invalid_manifest_value( parameters = flow_mocks.capture["create_kwargs"]["parameters"] assert parameters["frequency_max"] == 100.0 + @pytest.mark.parametrize( + ("param", "value", "expected"), + [ + ( + "place.random_seed", + "3001", + {"dreamplace": {"random_seed": 3001}}, + ), + ( + "route.RT.-enable_timing", + "1", + {"route": {"RT": {"-enable_timing": "1"}}}, + ), + ], + ) + def test_run_set_direct_config_parameter_in_manifest_project( + self, tmp_path, capsys, monkeypatch, flow_mocks, manifest_stubs, param, value, expected + ): + project_dir = _write_manifest_project( + manifest_stubs, + tmp_path, + monkeypatch, + 100, + ecc_toml=self._hybrid_toml(project_dir=tmp_path / "proj", frequency=False), + ) + + rc = cli_main.run( + [ + "run", + "--project", + str(project_dir), + "--from", + "Synth", + "--to", + "Synth", + "--set", + f"{param}={value}", + ] + ) + + assert rc == 0 + assert flow_mocks.capture["create_kwargs"]["parameters"]["config_overrides"] == expected + + def test_run_set_direct_config_parameter_warns_on_different_toml_value( + self, tmp_path, capsys, monkeypatch, flow_mocks, manifest_stubs + ): + project_dir = _write_manifest_project( + manifest_stubs, + tmp_path, + monkeypatch, + 100, + ecc_toml=self._hybrid_toml(project_dir=tmp_path / "proj", frequency=False) + + "\n[params.place]\nrandom_seed = 3000\n", + ) + + rc = cli_main.run( + [ + "run", + "--project", + str(project_dir), + "--from", + "Synth", + "--to", + "Synth", + "--set", + "place.random_seed=3001", + "--plain", + ] + ) + + assert rc == 0 + warnings = [ + record + for record in manifest_stubs.records() + if record.get("warning") == "config_layer_diverged" + ] + assert len(warnings) == 1 + assert "place.random_seed" in warnings[0]["keys"] + + @pytest.mark.parametrize( + ("section", "key", "value"), + [ + ("[params.place]", "random_seed", "3001"), + ("[params.route.RT]", '"-enable_timing"', '"1"'), + ], + ) + def test_check_direct_config_param_override_in_manifest_project( + self, tmp_path, capsys, monkeypatch, manifest_stubs, section, key, value + ): + project_dir = tmp_path / "proj" + toml = self._hybrid_toml(project_dir, frequency=False) + toml += f"\n{section}\n{key} = {value}\n" + _write_manifest_project( + manifest_stubs, + tmp_path, + monkeypatch, + 100, + ecc_toml=toml, + ) + + rc = cli_main.run(["check", "--project", str(project_dir), "--plain"]) + + assert rc == 0 + assert manifest_stubs.records()[0]["status"] == "checked" + class TestManifestBoolTypeSafety: """project.json run_analysis is the first bool registry parameter: its diff --git a/test/cli/params/test_registry.py b/test/cli/params/test_registry.py index 16ad9c7d..5d78fc5a 100644 --- a/test/cli/params/test_registry.py +++ b/test/cli/params/test_registry.py @@ -1,11 +1,13 @@ import pytest +from chipcompiler.cli.project.effective_config import _diverging_lower_keys from chipcompiler.cli.project.params import ( PARAM_REGISTRY, ParamSchema, ResolvedParam, build_backend_overrides, build_config_overrides, + build_pdk_overrides, coerce_manifest_parameters, is_known_key, list_groups, @@ -399,6 +401,42 @@ def test_mapping_does_not_mutate_schema_defaults(self): build_backend_overrides([rp]) assert schema.default == original_default + @pytest.mark.parametrize("schema", PARAM_REGISTRY, ids=lambda schema: schema.param) + def test_every_schema_default_survives_all_mapping_paths(self, schema): + """Every registered parameter must be safe through resolution and projections.""" + resolved, errors = resolve_parameters(cli_overrides={schema.param: schema.default}) + assert errors == [] + rp = next(item for item in resolved if item.param == schema.param) + + backend = build_backend_overrides([rp]) + config = build_config_overrides([rp]) + pdk = build_pdk_overrides([rp]) + assert isinstance(backend, dict) + assert isinstance(config, dict) + assert isinstance(pdk, dict) + if schema.config_target is not None: + assert config + assert backend == {} + elif schema.pdk_target is not None: + assert pdk == {schema.pdk_target: schema.default} + assert backend == {} + else: + assert backend + + @pytest.mark.parametrize("schema", PARAM_REGISTRY, ids=lambda schema: schema.param) + def test_every_schema_is_safe_in_divergence_projection(self, schema): + diverging, compared = _diverging_lower_keys( + {schema.param: schema.default}, lambda _schema: (None, False) + ) + + assert diverging == [] + if schema.maps_to is None: + assert compared == set() + elif isinstance(schema.maps_to, str): + assert compared == {schema.maps_to} + else: + assert compared == {f"{parent}.{child}" for parent, child in schema.maps_to.items()} + class TestCliOverrides: def test_repeatable_set(self): From 9d23a1980de714e195c46015d738f3423c044fb4 Mon Sep 17 00:00:00 2001 From: Yell-walkalone <12112088@qq.com> Date: Mon, 14 Sep 2026 17:52:12 +0800 Subject: [PATCH 2/3] fix(ecc): drop stale STA pre-init from timing-driven routing The routing path and save_data still called release_sta/init_sta/ report_timing, an STA lifecycle that ecc-tools removed on 2026-07-29 (1a4440525) and that no longer exists on ECCToolsModule or the raw binding. Enabling route.RT.-enable_timing therefore crashed the routing step with AttributeError before the router started. iRT's timing mode is self-contained: RTInterface builds its own timing engine from the shared db config (lib paths, SDC set by load_data), so the Python-side pre-init is removed rather than ported. save_data's report_timing block was unreachable (every caller passed False) and is dropped together with the parameter; the unused is_rt_timing_enable wrapper follows. --- chipcompiler/tools/ecc/module.py | 13 ----------- chipcompiler/tools/ecc/runner.py | 40 ++++---------------------------- 2 files changed, 5 insertions(+), 48 deletions(-) diff --git a/chipcompiler/tools/ecc/module.py b/chipcompiler/tools/ecc/module.py index 78ddf590..9be4923c 100644 --- a/chipcompiler/tools/ecc/module.py +++ b/chipcompiler/tools/ecc/module.py @@ -1,5 +1,4 @@ #!/usr/bin/env python -import json import os import shutil from pathlib import Path @@ -569,18 +568,6 @@ def feature_route_read(self, json_path: str): def feature_route(self, json_path: str): self.ecc.feature_route(path=path_text(json_path)) - def is_rt_timing_enable(self, config: str): - if os.path.exists(config): - with open(config, encoding="utf-8") as f_reader: - json_data = json.load(f_reader) - # check if time enable - if ( - json_data is not None - and json_data.get("RT", {}).get("-enable_timing", "0") == "1" - ): - return True - return False - ######################################################################## # RCX api ######################################################################## diff --git a/chipcompiler/tools/ecc/runner.py b/chipcompiler/tools/ecc/runner.py index e5714012..188fcc92 100644 --- a/chipcompiler/tools/ecc/runner.py +++ b/chipcompiler/tools/ecc/runner.py @@ -391,7 +391,6 @@ def save_data( ecc_module: ECCToolsModule, *, feature_step: bool = True, - report_timing: bool = False, ) -> bool: """ module is ecc module from db engine, @@ -424,17 +423,6 @@ def save_data( ecc_module.report_summary(path=step.report.db or "") - if report_timing: - ecc_module.release_sta() - ecc_module.init_sta( - output_dir=(step.data.steps or {}).get("sta", ""), - top_module=workspace.design.top_module, - lib_paths=workspace.pdk.libs, - sdc_path=workspace.pdk.sdc, - ) - ecc_module.report_timing() - ecc_module.release_sta() - # update parameters db_json = json_read(step.feature.db or "") if len(db_json) > 0: @@ -573,24 +561,14 @@ def run_routing( if ecc_module is not None: sub_flow.update_step(step_name=EccSubFlowEnum.load_data.value, state=StateEnum.Success) - if ecc_module.is_rt_timing_enable( - config=workspace.config.get(f"{StepEnum.ROUTING.value}", "") - ): - ecc_module.release_sta() - ecc_module.init_sta( - output_dir=(step.data.steps or {}).get(StepEnum.ROUTING.value, ""), - top_module=workspace.design.top_module, - lib_paths=workspace.pdk.libs, - sdc_path=workspace.pdk.sdc, - ) - + # Timing-driven routing is self-contained in iRT: RTInterface builds its + # own timing engine from the shared db config (lib paths, SDC), so no + # Python-side STA lifecycle is needed before run_routing. ecc_module.run_routing(config=workspace.config.get(f"{StepEnum.ROUTING.value}", "")) sub_flow.update_step(step_name=EccSubFlowEnum.run_routing.value, state=StateEnum.Success) - reslut = save_data( - workspace=workspace, step=step, ecc_module=ecc_module, report_timing=False - ) + reslut = save_data(workspace=workspace, step=step, ecc_module=ecc_module) sub_flow.update_step(step_name=EccSubFlowEnum.save_data.value, state=StateEnum.Success) @@ -623,7 +601,6 @@ def run_drc(workspace: Workspace, step: EccStep, ecc_module: ECCToolsModule | No step=step, ecc_module=ecc_module, feature_step=False, - report_timing=False, ) if not reslut: return False @@ -670,7 +647,6 @@ def run_lvs(workspace: Workspace, step: EccStep, ecc_module: ECCToolsModule | No step=step, ecc_module=ecc_module, feature_step=False, - report_timing=False, ) if not reslut: return False @@ -703,9 +679,7 @@ def run_filler( sub_flow.update_step(step_name=EccSubFlowEnum.run_filler.value, state=StateEnum.Success) - reslut = save_data( - workspace=workspace, step=step, ecc_module=ecc_module, report_timing=False - ) + reslut = save_data(workspace=workspace, step=step, ecc_module=ecc_module) sub_flow.update_step(step_name=EccSubFlowEnum.save_data.value, state=StateEnum.Success) @@ -746,7 +720,6 @@ def run_pre_floorplan( step=step, ecc_module=ecc_module, feature_step=False, - report_timing=False, ) sub_flow.update_step(step_name=EccSubFlowEnum.save_data.value, state=StateEnum.Success) @@ -788,7 +761,6 @@ def run_post_floorplan( step=step, ecc_module=ecc_module, feature_step=False, - report_timing=False, ) sub_flow.update_step(step_name=EccSubFlowEnum.save_data.value, state=StateEnum.Success) @@ -893,7 +865,6 @@ def run_rcx(workspace: Workspace, step: EccStep, ecc_module: ECCToolsModule | No step=step, ecc_module=ecc_module, feature_step=False, - report_timing=False, ): workspace.logger.error("Failed to save RCX data") return False @@ -1023,7 +994,6 @@ def run_sta(workspace: Workspace, step: EccStep, ecc_module: ECCToolsModule | No step=step, ecc_module=ecc_module, feature_step=False, - report_timing=False, ) sub_flow.update_step(step_name=EccSubFlowEnum.save_data.value, state=StateEnum.Success) From 0d282feddf35790543119a44059bcae77ca1898b Mon Sep 17 00:00:00 2001 From: Yell-walkalone <12112088@qq.com> Date: Mon, 14 Sep 2026 17:52:24 +0800 Subject: [PATCH 3/3] feat(sizer): record native crash signal and first fatal log line Timing-opt failures now distinguish a fatal signal (signal=SIGABRT(6)) from a plain tool exit (exit_code=N) and surface the first native fatal banner from the step log (e.g. glibc buffer overflow), so tool crashes, external kills, and timeouts remain separable in the wrapper evidence. --- chipcompiler/tools/ecc_sizer/runner.py | 41 ++++++++++++- test/tools/ecc_sizer/test_runner.py | 83 ++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 2 deletions(-) diff --git a/chipcompiler/tools/ecc_sizer/runner.py b/chipcompiler/tools/ecc_sizer/runner.py index e495fc1a..71f45c29 100644 --- a/chipcompiler/tools/ecc_sizer/runner.py +++ b/chipcompiler/tools/ecc_sizer/runner.py @@ -1,6 +1,7 @@ import logging import os import shutil +import signal import subprocess from pathlib import Path @@ -20,6 +21,41 @@ def _has_staging_outputs(step: EccStep) -> bool: return os.path.exists(sizer_staging_def(step)) and os.path.exists(sizer_staging_verilog(step)) +# Native crash banners glibc/libstdc++ write to the inherited stderr before +# aborting (e.g. "*** buffer overflow detected ***: terminated"). A kill or +# timeout prints no such line, so the banner separates tool crashes from +# external termination. +_FATAL_LOG_MARKERS = ( + "*** ", + "terminate called after throwing", +) + + +def _termination_reason(returncode: int) -> str: + """Classify a native exit status as a fatal signal or a tool exit code.""" + if returncode >= 0: + return f"exit_code={returncode}" + signum = -returncode + try: + signame = signal.Signals(signum).name + except ValueError: + signame = "UNKNOWN" + return f"signal={signame}({signum})" + + +def _first_fatal_log_line(log_path: str) -> str: + """The first native fatal banner in the step log, or an empty string.""" + try: + with open(log_path, errors="replace") as log_file: + for line in log_file: + stripped = line.strip() + if stripped.startswith(_FATAL_LOG_MARKERS): + return stripped + except OSError: + pass + return "" + + def _published_paths(step: EccStep) -> list[Path]: output = step.output if not isinstance(output, EccOutput): @@ -127,10 +163,11 @@ def run_step( if result.returncode != 0 or not _has_staging_outputs(step): logger.error( - "Sizer failed for step %s: exit code=%d, staging present=%s", + "Sizer failed for step %s: %s, staging present=%s, fatal_log_line=%r", step.name, - result.returncode, + _termination_reason(result.returncode), _has_staging_outputs(step), + _first_fatal_log_line(log_path), ) sub_flow.update_step(step_name=run_sizer_step, state=StateEnum.Imcomplete) return StateEnum.Imcomplete diff --git a/test/tools/ecc_sizer/test_runner.py b/test/tools/ecc_sizer/test_runner.py index 4981bb79..99263b49 100644 --- a/test/tools/ecc_sizer/test_runner.py +++ b/test/tools/ecc_sizer/test_runner.py @@ -1,3 +1,4 @@ +import logging import os import subprocess from pathlib import Path @@ -166,6 +167,88 @@ def test_sizer_runner_marks_subflow_incomplete_when_outputs_are_missing( assert _subflow_states(step)["run sizer"] == StateEnum.Imcomplete.value +def test_sizer_runner_marks_subflow_incomplete_when_tool_is_signal_terminated( + tmp_path, + monkeypatch, + caplog, +): + from chipcompiler.tools.ecc_sizer import builder as sizer_builder + from chipcompiler.tools.ecc_sizer import runner as sizer_runner + + workspace = _workspace(tmp_path) + step = sizer_builder.build_step( + workspace=workspace, + step_name=StepEnum.TIMING_OPT.value, + input_def=Path("input.def"), + input_verilog=Path("input.v"), + ) + sizer_builder.build_step_space(step) + sizer_builder.build_step_config(workspace, step) + Path(step.log.file).write_text( + "Read 527000 / 527400 Instances\n" + "*** buffer overflow detected ***: terminated\n" + "trailing line\n", + encoding="utf-8", + ) + + monkeypatch.setattr(sizer_runner, "get_sizer_command", lambda: ["/fake/sizer"]) + monkeypatch.setattr(sizer_runner, "is_eda_exist", lambda: True) + monkeypatch.setattr(sizer_runner, "is_sizer_runtime_exist", lambda: True) + monkeypatch.setattr(sizer_runner, "is_dreamplace_exist", lambda: True) + monkeypatch.setattr( + subprocess, + "run", + lambda command, cwd, stdout, stderr, check: SimpleNamespace(returncode=-6), + ) + + with caplog.at_level(logging.ERROR, logger=sizer_runner.__name__): + assert sizer_runner.run_step(workspace, step) == StateEnum.Imcomplete + + assert _subflow_states(step)["run sizer"] == StateEnum.Imcomplete.value + assert not sizer_builder.sizer_staging_def(step).exists() + assert not sizer_builder.sizer_staging_verilog(step).exists() + failure = caplog.records[-1].getMessage() + assert "signal=SIGABRT(6)" in failure + assert "*** buffer overflow detected ***: terminated" in failure + + +def test_sizer_runner_reports_plain_exit_code_without_signal_or_fatal_line( + tmp_path, + monkeypatch, + caplog, +): + from chipcompiler.tools.ecc_sizer import builder as sizer_builder + from chipcompiler.tools.ecc_sizer import runner as sizer_runner + + workspace = _workspace(tmp_path) + step = sizer_builder.build_step( + workspace=workspace, + step_name=StepEnum.TIMING_OPT.value, + input_def=Path("input.def"), + input_verilog=Path("input.v"), + ) + sizer_builder.build_step_space(step) + sizer_builder.build_step_config(workspace, step) + + monkeypatch.setattr(sizer_runner, "get_sizer_command", lambda: ["/fake/sizer"]) + monkeypatch.setattr(sizer_runner, "is_eda_exist", lambda: True) + monkeypatch.setattr(sizer_runner, "is_sizer_runtime_exist", lambda: True) + monkeypatch.setattr(sizer_runner, "is_dreamplace_exist", lambda: True) + monkeypatch.setattr( + subprocess, + "run", + lambda command, cwd, stdout, stderr, check: SimpleNamespace(returncode=1), + ) + + with caplog.at_level(logging.ERROR, logger=sizer_runner.__name__): + assert sizer_runner.run_step(workspace, step) == StateEnum.Imcomplete + + failure = caplog.records[-1].getMessage() + assert "exit_code=1" in failure + assert "signal=" not in failure + assert "fatal_log_line=''" in failure + + def test_sizer_runner_inherits_captured_stdio_instead_of_truncating_step_log( tmp_path, monkeypatch,