Skip to content

dtdoctor: fix broken diagnosis paths and add test coverage - #117239

Draft
kartben wants to merge 5 commits into
zephyrproject-rtos:mainfrom
kartben:dtdoctor-tests
Draft

dtdoctor: fix broken diagnosis paths and add test coverage#117239
kartben wants to merge 5 commits into
zephyrproject-rtos:mainfrom
kartben:dtdoctor-tests

Conversation

@kartben

@kartben kartben commented Aug 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member
  • Fix several issues in the DT Doctor SCA scripts that prevented diagnoses from being emitted (toolchain error regexes, alias reporting, select/imply matching, crash when ZEPHYR_BASE is unset)
  • Add a test suite under tests/misc/dtdoctor/: unit tests covering error detection and both diagnosis paths, plus an integration test (harness: ctest) that builds the documented way, with -DZEPHYR_SCA_VARIANT=dtdoctor, and exercises the tool end to end with real failing compile and link commands

@kartben
kartben force-pushed the dtdoctor-tests branch 7 times, most recently from 2596d6c to cd5e44d Compare August 24, 2026 21:19
@kartben kartben mentioned this pull request Aug 25, 2026
Draft
@kartben
kartben force-pushed the dtdoctor-tests branch 4 times, most recently from f9f5734 to 52ecd7d Compare August 25, 2026 15:38
kartben added 3 commits August 25, 2026 18:03
@kartben
scripts: dts: dtdoctor: fix toolchain error detection regexes
8ba9c4d 
The lld pattern required a literal '(' that lld never prints and only
captured the ordinal digits, which the analyzer then silently rejects,
so lld link errors were never diagnosed. Capture the full symbol
instead.

The gcc pattern only matched the file-scope "undeclared here" spelling
and missed the function-scope "undeclared (first use in this
function)" one. Match on "undeclared" alone; clang messages place the
symbol after "undeclared" so they cannot cross-match.

g++ uses a different spelling entirely, "was not declared in this
scope", which no pattern matched even though the SCA variant also
wraps the C++ compiler; add a dedicated pattern for it.

Also fix a stale script name in the module docstring.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
@kartben
scripts: dts: dtdoctor: fix analyzer diagnosis paths
a52ab9d 
Fix several defects that prevented parts of the diagnosis from ever
being emitted:

- Alias references were looked up via getattr(edt, "aliases", {}),
  but EDT has no such attribute (aliases is a Node property), so the
  alias report was dead code. Use node.aliases directly, and drop the
  similar getattr indirection for chosen nodes.
- The select/imply scan discarded the condition, where the DT_HAS
  reference actually appears, and rendered expressions unprefixed
  while matching against a CONFIG_-prefixed target, so it never
  matched. Match the rendered condition and suggest the defining
  symbol along with the other symbols in the condition.
- Matching used plain substring tests, so DT_HAS_FOO_ENABLED also
  matched DT_HAS_FOO_ENABLED_<X> symbols, producing false-positive
  suggestions. Use word-boundary matching.
- setup_kconfig() crashed with TypeError when ZEPHYR_BASE was unset;
  degrade to a "check driver Kconfig manually" hint instead.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
@kartben
tests: misc: dtdoctor: add unit tests for the DT Doctor scripts
a0f8aba 
Verify that the tool detects what it claims to, rather than matching
its output verbatim:

- wrapper: per-toolchain error detection (gcc file and function scope,
  UTF-8 quotes, g++, GNU ld, clang, lld), symbol de-duplication,
  return code passthrough, output replay, argv splitting, and one run
  with real subprocesses
- analyzer CLI: symbol and ordinal resolution, exit codes
- enabled-node diagnosis: Kconfig gating-option suggestions backed by
  small fixture Kconfig trees (depends on, select/imply conditions, no
  substring false positives), multi-compatible union, fallbacks
- disabled-node diagnosis: status source location, dependent nodes,
  chosen and alias references, remediation hint

EDT objects are built from inline DTS snippets with minimal fixture
bindings and pickled the same way gen_edt.py does. Ordinals and line
numbers are always computed, never hardcoded.

The suite runs as a ctest entry of a minimal test application
(harness: ctest), so twister covers it without a dedicated CI
workflow.

Assisted-by: Claude:opus-5
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
@kartben
kartben force-pushed the dtdoctor-tests branch 3 times, most recently from c1c9e92 to c9c8b2e Compare August 25, 2026 17:03
kartben added 2 commits August 25, 2026 21:31
@kartben
tests: misc: dtdoctor: add real-toolchain integration test
7e03837 
Exercise the DT Doctor SCA scripts end to end using the ctest harness:
the application is built the documented way, with
ZEPHYR_SCA_VARIANT=dtdoctor, and ctest then runs deliberately-failing
compile and link commands through the real SCA wrapper against the
build's edt.pickle and checks the resulting diagnosis.

The failing translation units use the real devicetree macros
(DEVICE_DT_GET() on the fixture nodes; the disabled node fails at
compile time since <zephyr/device.h> only declares symbols for okay
nodes, the enabled driver-less one at link time) and are compiled
with the application's own compile commands replayed from
compile_commands.json, so the whole chain is real: gen_defines.py
output, <devicetree.h> expansion, toolchain message, wrapper,
analyzer. The application builds one C++ file (CONFIG_CPP=y) so a
real C++ compile command is exported; the suite asserts g++'s
C++-only "was not declared in this scope" spelling, while clang++
shares the C message.

The scenario builds with both SDK toolchains (integration_toolchains:
zephyr/gnu and zephyr/llvm) so the gcc/g++/GNU ld and the clang/lld
error formats are all exercised. Also covered: the disabled-node
diagnosis (source location, chosen/alias references) and the
enabled-node headline against the real generated Kconfig environment.
A short README.rst describes the overall test structure.

Assisted-by: Claude:opus-5
Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
@kartben
MAINTAINERS: add dedicated DT Doctor area
7ffd85e 
Give the DT Doctor tooling (SCA wrapper, analyzer, CMake glue, docs and
the new test suite) a dedicated area so changes to any of these files
get assigned consistently, and associate the sca.dtdoctor test scenario
with it.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@kartben