Re: [PATCH 5/6] Restore phandle references from __local_fixups__ node

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

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

On Wed, Aug 20, 2025 at 03:11:31PM +0200, Uwe Kleine-König wrote:
> The __local_fixups__ node contains information about phandles. Parse it
> to improve the result when decompiling a device tree blob.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
>  dtc.c        |  2 ++
>  dtc.h        |  2 ++
>  livetree.c   | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  treesource.c | 33 ++++++++++++++++++++++++++++++
>  4 files changed, 94 insertions(+)
> 
> diff --git a/dtc.c b/dtc.c
> index 63a6c85da5cf..9f90b373967d 100644
> --- a/dtc.c
> +++ b/dtc.c
> @@ -343,6 +343,8 @@ int main(int argc, char *argv[])
>  	if (generate_symbols)
>  		generate_label_tree(dti, "__symbols__", true);
>  
> +	local_fixup_phandles(dti, "__local_fixups__");
> +
>  	if (generate_fixups) {
>  		generate_fixups_tree(dti, "__fixups__");
>  		generate_local_fixups_tree(dti, "__local_fixups__");
> diff --git a/dtc.h b/dtc.h
> index f97f3c29e2d4..d07a583441f6 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -343,6 +343,7 @@ void generate_labels_from_tree(struct dt_info *dti, const char *name);
>  void generate_label_tree(struct dt_info *dti, const char *name, bool allocph);
>  void generate_fixups_tree(struct dt_info *dti, const char *name);
>  void generate_local_fixups_tree(struct dt_info *dti, const char *name);
> +void local_fixup_phandles(struct dt_info *dti, const char *name);
>  
>  /* Checks */
>  
> @@ -358,6 +359,7 @@ struct dt_info *dt_from_blob(const char *fname);
>  
>  /* Tree source */
>  
> +void add_phandle_marker(struct dt_info *dti, struct property *prop, unsigned int offset);
>  void dt_to_source(FILE *f, struct dt_info *dti);
>  struct dt_info *dt_from_source(const char *f);
>  
> diff --git a/livetree.c b/livetree.c
> index 8aafc9aa5cd6..3dbef9071017 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -1160,3 +1160,60 @@ void generate_local_fixups_tree(struct dt_info *dti, const char *name)
>  			"Warning: Preexisting data in %s malformed, some content could not be added.\n",
>  			name);
>  }
> +
> +static void local_fixup_phandles_node(struct dt_info *dti, struct node *lf, struct node *n)
> +{
> +	struct property *lfp;
> +	struct node *lfsubnode;
> +
> +	for_each_property(lf, lfp) {
> +		struct property *p = get_property(n, lfp->name);
> +		fdt32_t *offsets = (fdt32_t *)lfp->val.val;
> +		size_t i;
> +
> +		if (!p) {
> +			if (quiet < 1)
> +				fprintf(stderr, "Warning: Property %s in %s referenced in __local_fixups__ missing\n",
> +					lfp->name, n->fullpath);
> +			continue;
> +		}
> +
> +		/*
> +		 * Each property in the __local_fixups__ tree is a concatenation
> +		 * of offsets, so it must be a multiple of sizeof(fdt32_t).
> +		 */
> +		if (lfp->val.len % sizeof(fdt32_t)) {
> +			if (quiet < 1)
> +				fprintf(stderr, "Warning: property %s in /__local_fixups__%s malformed\n",
> +					lfp->name, n->fullpath);
> +			continue;
> +		}
> +
> +		for (i = 0; i < lfp->val.len / sizeof(fdt32_t); i++)
> +			add_phandle_marker(dti, p, dtb_ld32(offsets + i));
> +	}
> +
> +	for_each_child(lf, lfsubnode) {
> +		struct node *subnode = get_subnode(n, lfsubnode->name);
> +
> +		if (!subnode) {
> +			if (quiet < 1)
> +				fprintf(stderr, "Warning: node %s/%s referenced in __local_fixups__ missing\n",
> +					lfsubnode->name, n->fullpath);
> +			continue;
> +		}
> +
> +		local_fixup_phandles_node(dti, lfsubnode, subnode);
> +	}
> +}
> +
> +void local_fixup_phandles(struct dt_info *dti, const char *name)
> +{
> +	struct node *an;
> +
> +	an = get_subnode(dti->dt, name);
> +	if (!an)
> +		return;
> +
> +	local_fixup_phandles_node(dti, an, dti->dt);
> +}
> diff --git a/treesource.c b/treesource.c
> index 576495902f0d..5b8d7a679519 100644
> --- a/treesource.c
> +++ b/treesource.c
> @@ -183,6 +183,39 @@ static void add_string_markers(struct property *prop, unsigned int offset, int l
>  		mi = add_marker(mi, TYPE_STRING, offset + l, NULL);
>  }
>  
> +void add_phandle_marker(struct dt_info *dti, struct property *prop, unsigned int offset)
> +{
> +	cell_t phandle;
> +	struct node *refn;
> +	char *ref;
> +
> +	if (prop->val.len < offset + 4) {
> +		if (quiet < 1)
> +			fprintf(stderr,
> +				"Warning: property %s too short to contain a phandle at offset %u\n",
> +				prop->name, offset);

You print the error, but don't exit the function.

> +
> +	}
> +
> +	phandle = dtb_ld32(prop->val.val + offset);
> +	refn = get_node_by_phandle(dti->dt, phandle);
> +
> +	if (!refn) {
> +		if (quiet < 1)
> +			fprintf(stderr,
> +				"Warning: node referenced by phandle 0x%x in property %s not found\n",
> +				phandle, prop->name);
> +		return;
> +	}
> +
> +	if (refn->labels)
> +		ref = refn->labels->label;
> +	else
> +		ref = refn->fullpath;

The new marker will point to data in different structures by
reference, which makes me worried about lifetimes.  But this is not
new, and probably not the only hairy lifetime issue in dtc.

> +
> +	add_marker(&prop->val.markers, REF_PHANDLE, offset, ref);
> +}
> +
>  static enum markertype guess_value_type(struct property *prop, unsigned int offset, int len)
>  {
>  	const char *p = prop->val.val + offset;

-- 
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 --]