Re: [PATCH 01/13] tests: Check each error when merging a dts file

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 10616 bytes --]

On Sun, Nov 01, 2026 at 09:36:55AM -0700, Simon Glass wrote:
> When the parser merges the top-level fragments of a dts file, it can
> report that a label or path is not found, or that a label-relative
> reference is not supported in a plugin. Most of these errors are not
> covered by any test, and the two tests which do reach them only check
> that dtc fails, not the message or where it is reported. The merge
> logic is about to move out of the grammar, so this needs better tests.
> 
> Add a test file for each error which is not covered, and check the
> message and its position for all of them, including the two existing
> ones. Also check that merging carries on after an error, so that later
> errors are reported too, that a reference error is still reported when
> the parser finds another error, and that a labelled reference within a
> plugin adds the label, since nothing covers these either.
> 
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Simon Glass <sjg@chromium.org>

Couple of nits, otherwise LGTM.

> ---
> 
>  tests/merge-delete-missing.dts     |  9 +++++++
>  tests/merge-errors.dts             | 14 ++++++++++
>  tests/merge-first-ref-relative.dts | 10 ++++++++
>  tests/merge-first-ref.dts          |  9 +++++++
>  tests/merge-labelled-plugin.dts    | 12 +++++++++
>  tests/merge-labelled-relative.dts  | 14 ++++++++++
>  tests/merge-omit-missing.dts       |  9 +++++++
>  tests/merge-parse-error.dts        | 17 +++++++++++++
>  tests/merge-path-ref-missing.dts   | 11 ++++++++
>  tests/merge-path-relative.dts      | 12 +++++++++
>  tests/run_tests.sh                 | 41 ++++++++++++++++++++++++++++++
>  11 files changed, 158 insertions(+)
>  create mode 100644 tests/merge-delete-missing.dts
>  create mode 100644 tests/merge-errors.dts
>  create mode 100644 tests/merge-first-ref-relative.dts
>  create mode 100644 tests/merge-first-ref.dts
>  create mode 100644 tests/merge-labelled-plugin.dts
>  create mode 100644 tests/merge-labelled-relative.dts
>  create mode 100644 tests/merge-omit-missing.dts
>  create mode 100644 tests/merge-parse-error.dts
>  create mode 100644 tests/merge-path-ref-missing.dts
>  create mode 100644 tests/merge-path-relative.dts
> 
> diff --git a/tests/merge-delete-missing.dts b/tests/merge-delete-missing.dts
> new file mode 100644
> index 0000000..b36e39e
> --- /dev/null
> +++ b/tests/merge-delete-missing.dts
> @@ -0,0 +1,9 @@
> +/dts-v1/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* Try to delete a node using a non-existent label */
> +/delete-node/ &nosuchnode;
> diff --git a/tests/merge-errors.dts b/tests/merge-errors.dts
> new file mode 100644
> index 0000000..6d3969f
> --- /dev/null
> +++ b/tests/merge-errors.dts
> @@ -0,0 +1,14 @@
> +/dts-v1/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* Each error is reported, not just the first */

I'm a bit confused by this comment - I only see one error in this
file.

> +&nosuchnode1 {
> +	prop;
> +};
> +
> +/delete-node/ &nosuchnode2;
> +/omit-if-no-ref/ &nosuchnode3;
> diff --git a/tests/merge-first-ref-relative.dts b/tests/merge-first-ref-relative.dts
> new file mode 100644
> index 0000000..103cd1f
> --- /dev/null
> +++ b/tests/merge-first-ref-relative.dts
> @@ -0,0 +1,10 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/* A plugin cannot start with a label-relative reference */

That's slightly misleading, insn't it?  A plugin can't have _any_
label relative references unless they resolve within the plugin.

> +&{label/node} {
> +	prop;
> +};
> +
> +/* Later fragments are still merged */
> +/delete-node/ &nosuchnode;
> diff --git a/tests/merge-first-ref.dts b/tests/merge-first-ref.dts
> new file mode 100644
> index 0000000..8f23a44
> --- /dev/null
> +++ b/tests/merge-first-ref.dts
> @@ -0,0 +1,9 @@
> +/dts-v1/;
> +
> +/* Only a plugin can start with a reference */
> +&nosuchnode {
> +	prop;
> +};
> +
> +/* Later fragments are still merged */
> +/delete-node/ &nosuchnode2;
> diff --git a/tests/merge-labelled-plugin.dts b/tests/merge-labelled-plugin.dts
> new file mode 100644
> index 0000000..3849cc5
> --- /dev/null
> +++ b/tests/merge-labelled-plugin.dts
> @@ -0,0 +1,12 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* Add a label to a node defined in the plugin itself */
> +newlabel: &label {
> +	prop = "value";
> +};
> diff --git a/tests/merge-labelled-relative.dts b/tests/merge-labelled-relative.dts
> new file mode 100644
> index 0000000..00369e3
> --- /dev/null
> +++ b/tests/merge-labelled-relative.dts
> @@ -0,0 +1,14 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	label: node {
> +		subnode {
> +		};
> +	};
> +};
> +
> +/* A plugin cannot use a label-relative reference, even if it is found */
> +newlabel: &{label/subnode} {
> +	prop;
> +};
> diff --git a/tests/merge-omit-missing.dts b/tests/merge-omit-missing.dts
> new file mode 100644
> index 0000000..0357648
> --- /dev/null
> +++ b/tests/merge-omit-missing.dts
> @@ -0,0 +1,9 @@
> +/dts-v1/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* Try to mark a node using a non-existent path */
> +/omit-if-no-ref/ &{/nosuchnode};
> diff --git a/tests/merge-parse-error.dts b/tests/merge-parse-error.dts
> new file mode 100644
> index 0000000..c39318b
> --- /dev/null
> +++ b/tests/merge-parse-error.dts
> @@ -0,0 +1,17 @@
> +/dts-v1/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* Reported even though the parser finds another error below */
> +&nosuchnode {
> +	prop;
> +};
> +
> +/ {
> +	node {
> +		prop = <(1/0)>;
> +	};
> +};
> diff --git a/tests/merge-path-ref-missing.dts b/tests/merge-path-ref-missing.dts
> new file mode 100644
> index 0000000..ad2e35e
> --- /dev/null
> +++ b/tests/merge-path-ref-missing.dts
> @@ -0,0 +1,11 @@
> +/dts-v1/;
> +
> +/ {
> +	node {
> +	};
> +};
> +
> +/* Try to redefine a node using a non-existent path */
> +&{/nosuchnode} {
> +	prop;
> +};
> diff --git a/tests/merge-path-relative.dts b/tests/merge-path-relative.dts
> new file mode 100644
> index 0000000..94a0b38
> --- /dev/null
> +++ b/tests/merge-path-relative.dts
> @@ -0,0 +1,12 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +	label: node {
> +	};
> +};
> +
> +/* A plugin cannot use a label-relative path reference */
> +&{label/node} {
> +	prop;
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 5bc7817..e0504c0 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -765,6 +765,47 @@ dtc_tests () {
>      run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references -- -I dts -O dtb "$SRCDIR/nonexist-node-ref.dts"
>      run_sh_test "$SRCDIR/dtc-checkfails.sh" phandle_references -- -I dts -O dtb "$SRCDIR/nonexist-label-ref.dts"
>      run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb "$SRCDIR/nonexist-node-ref2.dts"
> +
> +    # Check each error when merging the fragments of a dts file, with its
> +    # position
> +    run_wrap_error_msg_test "test_label_ref.dts:7.8-15 Label or path handle not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/test_label_ref.dts"
> +    run_wrap_error_msg_test "nonexist-node-ref2.dts:9.1-12 Label or path nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/nonexist-node-ref2.dts"
> +    run_wrap_error_msg_test "merge-first-ref.dts:4.13-6.3 Label or path nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref.dts"
> +    run_wrap_error_msg_test "merge-first-ref.dts:9.15-27 Label or path nosuchnode2 not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref.dts"
> +    run_wrap_error_msg_test "merge-first-ref-relative.dts:5.15-7.3 Label-relative reference label/node not supported in plugin" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref-relative.dts"
> +    run_wrap_error_msg_test "merge-first-ref-relative.dts:10.15-26 Label or path nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-first-ref-relative.dts"
> +    run_wrap_error_msg_test "merge-labelled-relative.dts:12.1-10 Label-relative reference label/subnode not supported in plugin" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-labelled-relative.dts"
> +    run_wrap_error_msg_test "merge-path-relative.dts:10.1-14 Label-relative reference label/node not supported in plugin" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-path-relative.dts"
> +    run_wrap_error_msg_test "merge-path-ref-missing.dts:9.1-15 Label or path /nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-path-ref-missing.dts"
> +    run_wrap_error_msg_test "merge-delete-missing.dts:9.15-26 Label or path nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-delete-missing.dts"
> +    run_wrap_error_msg_test "merge-omit-missing.dts:9.18-32 Label or path /nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-omit-missing.dts"
> +    run_wrap_error_msg_test "merge-errors.dts:9.1-13 Label or path nosuchnode1 not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-errors.dts"
> +    run_wrap_error_msg_test "merge-errors.dts:13.15-27 Label or path nosuchnode2 not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-errors.dts"
> +    run_wrap_error_msg_test "merge-errors.dts:14.18-30 Label or path nosuchnode3 not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-errors.dts"
> +    run_wrap_error_msg_test "merge-parse-error.dts:9.1-12 Label or path nosuchnode not found" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-parse-error.dts"
> +    run_wrap_error_msg_test "merge-parse-error.dts:15.12-15 Division by zero" \
> +	$DTC -I dts -O dtb -o /dev/null "$SRCDIR/merge-parse-error.dts"
> +
> +    # A labelled reference to a node in a plugin adds the label to it
> +    run_dtc_test -@ -I dts -O dtb -o merge-labelled-plugin.test.dtb \
> +	"$SRCDIR/merge-labelled-plugin.dts"
> +    run_fdtget_test "/node" merge-labelled-plugin.test.dtb /__symbols__ newlabel
> +    run_fdtget_test "value" merge-labelled-plugin.test.dtb /node prop
>      check_tests "$SRCDIR/bad-name-property.dts" name_properties
>  
>      check_tests "$SRCDIR/bad-ncells.dts" address_cells_is_cell size_cells_is_cell interrupts_extended_is_cell
> -- 
> 2.43.0
> 
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]