2023-08-09 09:01:49

by Peter Zijlstra

[permalink] [raw]
Subject: [RFC][PATCH 13/17] objtool/x86: Add arch_is_offset_insn()

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) <[email protected]>
---
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;

--- 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;
};




2023-08-09 10:37:46

by Nikolay Borisov

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



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) <[email protected]>
> ---
> 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;
> };
>
>

2023-08-09 15:28:44

by Josh Poimboeuf

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

On Wed, Aug 09, 2023 at 09:12:31AM +0200, 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) <[email protected]>
> ---
> 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");
> +}

arch_is_embedded_in_insn()?

--
Josh