Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The `is_mapping_sym()` function relies on an overly broad substring match (`strchr(sym->name, '$')`), which risks falsely classifying any unannotated (`STT_NOTYPE`) symbol containing a `$` as an ARM64 mapping symbol. --
commit fb925cb10f7a9c6ab8ecefca87d3cd3b4e4da879 Author: Josh Poimboeuf <[email protected]> objtool/klp: Don't correlate arm64 mapping symbols This commit updates the livepatch diff tool to skip the correlation of arm64 mapping symbols. These symbols mark transitions between code and data but are not standard symbols that require correlation. > diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c > index eb21f3bf3120b..e1d4d94c9d77c 100644 > --- a/tools/objtool/klp-diff.c > +++ b/tools/objtool/klp-diff.c > @@ -501,6 +501,7 @@ static bool dont_correlate(struct symbol *sym) > is_prefix_func(sym) || > is_uncorrelated_static_local(sym) || > is_local_label(sym) || > + is_mapping_sym(sym) || Does this code risk falsely classifying user-defined or macro-generated assembly labels as mapping symbols? Looking at the definition: tools/objtool/include/objtool/elf.h:is_mapping_sym() { return is_notype_sym(sym) && strchr(sym->name, '$'); } Because it relies on a broad substring match, could this exclude valid unannotated STT_NOTYPE symbols containing a $ from correlation across all architectures? Triggering a livepatching failure would require an unusual edge case, but would it be safer to use a stricter match for standard arm64 mapping symbol formats like $x or $d? > is_string_sec(sym->sec) || > is_anonymous_rodata(sym) || [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=17
