Re: [PATCH v3 10/21] objtool: Ignore jumps to the end of the function for checksum runs

[sashiko-bot@kernel.org]

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 <jpoimboe@kernel.org>

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)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1778642120.git.jpoimboe@kernel.org?part=10