On Thu, May 14, 2026 at 10:30:20PM +0000, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Medium] Missing parentheses in the jump destination check causes objtool > to silently ignore all missing jump targets when branch validation is > disabled, instead of only jumps to the end of the function. > -- > > commit aa068d4053e8b92ff17082c7a023821e72f5d625 > Author: Josh Poimboeuf <[email protected]> > > objtool: Ignore jumps to the end of the function for checksum runs > > Sometimes Clang arm64 code jumps to the end of the function for UB. > This commit ignores these jumps to the end of the function for > checksum runs to avoid errors. > > > diff --git a/tools/objtool/check.c b/tools/objtool/check.c > > index 10b18cf9c3608..73451aef68029 100644 > > --- a/tools/objtool/check.c > > +++ b/tools/objtool/check.c > > [ ... ] > > > @@ -1593,10 +1609,14 @@ static int add_jump_destinations(struct > > objtool_file *file) > > /* > > * GCOV/KCOV dead code can jump to the end of > > * the function/section. > > + * > > + * Clang on arm64 also does this sometimes for > > + * undefined behavior. > > */ > > - if (file->ignore_unreachables && func && > > - dest_sec == insn->sec && > > - dest_off == func->offset + func->len) > > + if (!validate_branch_enabled() || > > + (file->ignore_unreachables && func && > > + dest_sec == insn->sec && > > + dest_off == func->offset + func->len)) > > continue; > > Does this condition silently ignore all missing jump targets when branch > validation is disabled? > > Because of the || without proper parenthetical grouping around the first > two terms, if !validate_branch_enabled() is true (which it is for > --checksum or --noinstr runs), the rest of the condition short-circuits. > > This appears to cause objtool to skip the error reporting and silently > leave the jump destination as NULL for any unresolvable jump in the binary, > instead of only those jumping to the end of the function. > > Should the logic be grouped to restrict the exception to the end of the > function? > > if ((!validate_branch_enabled() || > file->ignore_unreachables) && > func && > dest_sec == insn->sec && > dest_off == func->offset + func->len)
Yes, we should tighten that up. -- Josh
