On Wed, Sep 23, 2026 at 4:47 PM <[email protected]> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Medium] dyndbg: interleaved classmaps are orphaned due to contiguous slice > assumption > -- > > commit 8936e7007afda568cebc34aa496a40887ac6a5a1 > Author: Jim Cromie <[email protected]> > > dyndbg: replace classmap list with an array-slice > > Classmaps are currently individually list-linked onto dyndbg's per-module > ddebug_table for operation. This commit replaces the list building operation > with an approach that treats packed classmaps as an array-start-address and > array-length, reusing the select-by-modname condition to find the bounds. > > > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > > --- a/lib/dynamic_debug.c > > +++ b/lib/dynamic_debug.c > > [ ... ] > > > -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++) { > > [Severity: Medium] > Since the scan in ddebug_attach_module_classes() always restarts at index 0, > could this cause regressions if multiple descriptor tables exist for the > same interleaved module? When distinct compiled objects share the same > KBUILD_MODNAME, it seems they might all incorrectly bind to the first block > instead of finding their respective fragments. > > > > > if (!strcmp(cm->mod_name, dt->mod_name)) { > > [ ... ] > > > + 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] > Does this premature break in ddebug_attach_module_classes() enforce an > assumption that classmaps are strictly contiguous? >
Yes, it explicitly enforces that assumption, because contiguous packing is a fundamental invariant of the Kbuild object model and linker script: 0. In Linux, all translation units (.c files) comprising a loadable module or composite built-in object are linked together via $(LD) -r into a single intermediate object (e.g. foo.o) before final vmlinux or .ko generation. All contributions to custom ELF sections (__dyndbg_classes, __dyndbg_class_maps) within that object are placed contiguously. 1. Built-in module definitions do not interleave across directories or link units. Core dyndbg already relies on this exact contiguous invariant for callsite descriptors: dynamic_debug_init() scans __start___dyndbg_descs sequentially from index 0 and allocates a struct ddebug_table whenever strcmp(modname, iter->modname) transitions. If Kbuild interleaved objects for the same module across arbitrary boundaries, descriptor tables would already break. 2. An array slice (start, len) inherently represents a single contiguous memory extent. By definition, a slice cannot represent fragmented, disjoint ranges. Replacing struct list_head link with a slice saves memory and pointer indirection overhead per classmap, backed by standard section layout guarantees. > If classmaps for a module are interleaved, breaking early means only the > first contiguous block is bound. Subsequent classmap fragments might be > permanently orphaned, leaving class'd pr_debug calls in those fragments > uncontrolled. > > -- > Sashiko AI review · > https://sashiko.dev/#/patchset/[email protected]?part=17
