Re: [PATCH v8 21/43] dyndbg: replace classmap list with an array-slice

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The array-slice logic incorrectly assigns the first contiguous block of classmaps to all disjoint instances of a split built-in module, silently ignoring subsequent classmap blocks.
--

commit 83893b53c8dd08c74af1a0d4c13885f9854d8f97
Author: Jim Cromie <jim.cromie@gmail.com>

dyndbg: replace classmap list with an array-slice

This commit replaces the linked list used to track a module's classmaps
with a contiguous array-slice approach. It updates
ddebug_attach_module_classes() to find the start and end of a module's
classmaps within the compiled section based on contiguous packing and
module name.

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index e0e3cadd82bd9..470314e4810eb 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
[ ... ]
> @@ -1153,30 +1155,34 @@ static const struct proc_ops proc_fops = {
>  	.proc_write = ddebug_proc_write
>  };
>  
> -static void ddebug_attach_module_classes(struct ddebug_table *dt,
> -					 struct ddebug_class_map *classes,
> -					 int num_classes)
> +static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
>  {
>  	struct ddebug_class_map *cm;
> -	int i, j, ct = 0;
> +	int i, nc = 0;
>  
> -	for (cm = classes, i = 0; i < num_classes; i++, cm++) {
> +	/*
> +	 * Find this module's classmaps in a subrange/wholerange of
> +	 * the builtin/modular classmap vector/section.  Save the start
> +	 * and length of the subrange at its edges.
> +	 */
> +	for (cm = di->classes, i = 0; i < di->num_classes; i++, cm++) {
>  
>  		if (!strcmp(cm->mod_name, dt->mod_name)) {
> -
> -			v2pr_info("class[%d]: module:%s base:%d len:%d ty:%d\n", i,
> -				  cm->mod_name, cm->base, cm->length, cm->map_type);
> -
> -			for (j = 0; j < cm->length; j++)
> -				v3pr_info(" %d: %d %s\n", j + cm->base, j,
> -					  cm->class_names[j]);
> -
> -			list_add(&cm->link, &dt->maps);
> -			ct++;
> +			if (!nc) {
> +				v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
> +					  i, cm->mod_name, cm->base, cm->length, cm->map_type);
> +				dt->classes = cm;
> +			}
> +			nc++;
> +		} else if (nc) {
> +			/* end of matching classmaps */
> +			break;
>  		}
>  	}

[Severity: Medium]
When there are disjoint built-in modules sharing the same name (due to Kbuild
naming collisions), dynamic_debug_init() creates a separate ddebug_table for
each disjoint block of descriptors.

Does this logic incorrectly assign the first contiguous block of classmaps
to all disjoint instances of a split built-in module?

Since this loop always starts scanning from the beginning of di->classes and
breaks on the first mismatch after a match, it seems subsequent calls for the
same module name will redundantly receive the exact same first block of
classmaps.

Could this permanently orphan the actual classmaps belonging to the subsequent
blocks, potentially leading to unknown class_id warnings or broken pr_debug
class filtering?

> -	if (ct)
> -		vpr_info("module:%s attached %d classes\n", dt->mod_name, ct);
> +	if (nc) {
> +		dt->num_classes = nc;
> +		vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905-dd-cmap-part2-clean-v8-0-a4cc0674f6fd@gmail.com?part=21