Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dyndbg: __UNIQUE_ID(_ddebug) prefix change breaks objtool klp diff --
commit 317df06d413f02902ba4470f9dd1320a81d15cee Author: Jim Cromie <[email protected]> dyndbg,module: make proper substructs in _ddebug_info This commit refactors struct _ddebug_info to group descriptor and class map vectors into struct _ddebug_descs and struct _ddebug_class_maps substructs, embeds struct _ddebug_info into struct ddebug_table, and renames the linker sections accordingly. > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > index 36b7d341076db..b05dc843a1d8e 100644 > --- a/include/linux/dynamic_debug.h > +++ b/include/linux/dynamic_debug.h [ ... ] > @@ -245,7 +257,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > * macro. > */ > #define _dynamic_func_call_cls(cls, fmt, func, ...) \ > - __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, > ##__VA_ARGS__) > + __dynamic_func_call_cls(__UNIQUE_ID(_ddebug), cls, fmt, func, > ##__VA_ARGS__) > #define _dynamic_func_call(fmt, func, ...) \ > _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__) > > @@ -255,7 +267,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > * with precisely the macro's varargs. > */ > #define _dynamic_func_call_cls_no_desc(cls, fmt, func, ...) \ > - __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt, \ > + __dynamic_func_call_cls_no_desc(__UNIQUE_ID(_ddebug), cls, fmt, \ > func, ##__VA_ARGS__) [Severity: Medium] Does changing __UNIQUE_ID(ddebug) to __UNIQUE_ID(_ddebug) in _dynamic_func_call_cls() and _dynamic_func_call_cls_no_desc() break livepatch builds with objtool klp diff? In include/linux/compiler.h, __UNIQUE_ID() already appends an underscore after __UNIQUE_ID_: /* Format: __UNIQUE_ID_<name>_<__COUNTER__> */ #define __UNIQUE_ID(name) \ __PASTE(__UNIQUE_ID_, \ __PASTE(name, \ __PASTE(_, __COUNTER__))) Passing _ddebug changes the generated static struct _ddebug descriptor symbol names from __UNIQUE_ID_ddebug_<N> to __UNIQUE_ID__ddebug_<N> (with a double underscore). When CONFIG_JUMP_LABEL is enabled and a livepatched module function contains a dynamic debug callsite, tools/objtool/klp-diff.c checks for the single underscore prefix in validate_special_section_klp_reloc(): if (strstr(reloc->sym->name, "__UNIQUE_ID_ddebug_")) { WARN("%s: disabling unsupported pr_debug()", code_sym); ret = 1; continue; } ERROR("%s+0x%lx: unsupported static branch key %s. Use static_key_enabled() instead", code_sym, code_offset, reloc->sym->name); return -1; With the extra underscore in __UNIQUE_ID__ddebug_<N>, strstr() will no longer match and validate_special_section_klp_reloc() will fail with an unsupported static branch key error. Should this remain __UNIQUE_ID(ddebug), or should validate_special_section_klp_reloc() in tools/objtool/klp-diff.c be updated to match? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=19
