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 <[email protected]> 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/[email protected]?part=21
