Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] ddebug_attach_module_classes() assumes all classmaps for a given module are perfectly contiguous, prematurely breaking the loop and causing disjoint classmaps for built-in modules with duplicate names (e.g., "main") to be ignored and incorrectly shared. --
commit 0adb4bd721cea60d2371ac3aa8cc0fb11eab6469 Author: Jim Cromie <[email protected]> dyndbg: replace classmap list with an array-slice This commit replaces the linked list used to manage classmaps with an array slice representation. It modifies ddebug_attach_module_classes() to find a contiguous subrange of classmaps for a module and stores its start and length, removing the list_head from struct ddebug_class_map. > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index d4cce0f4f197d..7f03b331d185b 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c [ ... ] > @@ -1147,30 +1149,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: High] Does this logic correctly handle disjoint classmaps for built-in modules with duplicate names (such as "main")? Since di->classes is initialized to __start___dyndbg_classes in dynamic_debug_init() and is never advanced, ddebug_add_module() processes the classmaps array from the beginning each time. Because built-in modules with duplicate names are not contiguous in memory, dynamic_debug_init() creates multiple ddebug_table structures for them. When the second "main" chunk is processed, wouldn't this search from the beginning, find the first module's classmaps, and then prematurely exit the loop here when it reaches the end of that first contiguous block? Could this cause the second module's actual classmaps to be permanently ignored, resulting in subsequent duplicate module names incorrectly using the wrong classmaps for resolution and control? > } > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=16
