Re: [RFC][PATCH 13/17] objtool/x86: Add arch_is_offset_insn()

[Nikolay Borisov]

On 9.08.23 г. 10:12 ч., Peter Zijlstra wrote:
> Add a little wrappery to identify the magic symbols that are actually
> inside another instruction -- yay for variable length instruction
> encoding.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> ---
>   tools/objtool/arch/x86/decode.c      |    6 ++++++
>   tools/objtool/check.c                |   13 ++++++++++---
>   tools/objtool/include/objtool/arch.h |    1 +
>   tools/objtool/include/objtool/elf.h  |    1 +
>   4 files changed, 18 insertions(+), 3 deletions(-)
>
> --- a/tools/objtool/arch/x86/decode.c
> +++ b/tools/objtool/arch/x86/decode.c
> @@ -826,3 +826,9 @@ bool arch_is_rethunk(struct symbol *sym)
>   {
>   	return !strcmp(sym->name, "__x86_return_thunk");
>   }
> +
> +bool arch_is_offset_insn(struct symbol *sym)
> +{
> +	return !strcmp(sym->name, "zen_return_thunk") ||
> +	       !strcmp(sym->name, "srso_safe_ret");
> +}
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -459,8 +459,7 @@ static int decode_instructions(struct ob
>   			 * Both zen_return_thunk() and srso_safe_ret() are embedded inside
>   			 * another instruction and objtool doesn't grok that. Skip validating them.
>   			 */
> -			if (!strcmp(func->name, "zen_return_thunk") ||
> -			    !strcmp(func->name, "srso_safe_ret") || func->alias != func)
> +			if (func->offset_insn || func->alias != func)
>   				continue;
>   
>   			if (!find_insn(file, sec, func->offset)) {
> @@ -1303,6 +1302,11 @@ __weak bool arch_is_rethunk(struct symbo
>   	return false;
>   }
>   
> +__weak bool arch_is_offset_insn(struct symbol *sym)
> +{
> +	return false;
> +}
> +
>   static struct reloc *insn_reloc(struct objtool_file *file, struct instruction *insn)
>   {
>   	struct reloc *reloc;
> @@ -1588,7 +1592,7 @@ static int add_jump_destinations(struct
>   			 * middle of another instruction.  Objtool only
>   			 * knows about the outer instruction.
>   			 */
> -			if (sym && !strcmp(sym->name, "zen_return_thunk")) {
> +			if (sym && sym->offset_insn) {
>   				add_return_call(file, insn, false);
>   				continue;
>   			}
> @@ -2507,6 +2511,9 @@ static int classify_symbols(struct objto
>   		if (arch_is_rethunk(func))
>   			func->return_thunk = true;
>   
> +		if (arch_is_offset_insn(func))
> +			func->offset_insn = true;
> +
>   		if (arch_ftrace_match(func->name))
>   			func->fentry = true;

nit: Why go through this if when one can simply do:

func->foo = arch_is_foo(bar) ?

>   
> --- a/tools/objtool/include/objtool/arch.h
> +++ b/tools/objtool/include/objtool/arch.h
> @@ -90,6 +90,7 @@ int arch_decode_hint_reg(u8 sp_reg, int
>   
>   bool arch_is_retpoline(struct symbol *sym);
>   bool arch_is_rethunk(struct symbol *sym);
> +bool arch_is_offset_insn(struct symbol *sym);
>   
>   int arch_rewrite_retpolines(struct objtool_file *file);
>   
> --- a/tools/objtool/include/objtool/elf.h
> +++ b/tools/objtool/include/objtool/elf.h
> @@ -66,6 +66,7 @@ struct symbol {
>   	u8 fentry            : 1;
>   	u8 profiling_func    : 1;
>   	u8 warned	     : 1;
> +	u8 offset_insn       : 1;
>   	struct list_head pv_target;
>   	struct reloc *relocs;
>   };