2020-04-28 19:22:02

by Peter Zijlstra

Subject: [PATCH v2 09/14] objtool: Move the IRET hack into the arch decoder

From: Miroslav Benes <[email protected]>

Quoting Julien:

"And the other suggestion is my other email was that you don't even
need to add INSN_EXCEPTION_RETURN. You can keep IRET as
INSN_CONTEXT_SWITCH by default and x86 decoder lookups the symbol
conaining an iret. If it's a function symbol, it can just set the type
to INSN_OTHER so that it caries on to the next instruction after
having handled the stack_op."

Suggested-by: Julien Thierry <[email protected]>
Signed-off-by: Miroslav Benes <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
tools/objtool/arch.h | 1 -
tools/objtool/arch/x86/decode.c | 28 ++++++++++++++++++----------
tools/objtool/check.c | 11 -----------
tools/objtool/elf.c | 4 ++--
tools/objtool/elf.h | 2 +-
5 files changed, 21 insertions(+), 25 deletions(-)

--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -19,7 +19,6 @@ enum insn_type {
INSN_CALL,
INSN_CALL_DYNAMIC,
INSN_RETURN,
- INSN_EXCEPTION_RETURN,
INSN_CONTEXT_SWITCH,
INSN_BUG,
INSN_NOP,
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -94,6 +94,7 @@ int arch_decode_instruction(const struct
rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0,
modrm_reg = 0, sib = 0;
struct stack_op *op = NULL;
+ struct symbol *sym;

x86_64 = is_x86_64(elf);
if (x86_64 == -1)
@@ -469,17 +470,24 @@ int arch_decode_instruction(const struct
break;

case 0xcf: /* iret */
- *type = INSN_EXCEPTION_RETURN;
-
- ADD_OP(op) {
- /* add $40, %rsp */
- op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_SP;
- op->src.offset = 5*8;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
+ /*
+ * Handle sync_core(), which has an IRET to self.
+ * All other IRET are in STT_NONE entry code.
+ */
+ sym = find_symbol_containing(sec, offset);
+ if (sym && sym->type == STT_FUNC) {
+ ADD_OP(op) {
+ /* add $40, %rsp */
+ op->src.type = OP_SRC_ADD;
+ op->src.reg = CFI_SP;
+ op->src.offset = 5*8;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ }
+ break;
}
- break;
+
+ /* fallthrough */

case 0xca: /* retf */
case 0xcb: /* retf */
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2308,17 +2308,6 @@ static int validate_branch(struct objtoo

break;

- case INSN_EXCEPTION_RETURN:
- /*
- * This handles x86's sync_core() case, where we use an
- * IRET to self. All 'normal' IRET instructions are in
- * STT_NOTYPE entry symbols.
- */
- if (func)
- break;
-
- return 0;
-
case INSN_CONTEXT_SWITCH:
if (func && (!next_insn || !next_insn->hint)) {
WARN_FUNC("unsupported instruction in callable function",
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -61,7 +61,7 @@ static void rb_add(struct rb_root *tree,
rb_insert_color(node, tree);
}

-static struct rb_node *rb_find_first(struct rb_root *tree, const void *key,
+static struct rb_node *rb_find_first(const struct rb_root *tree, const void *key,
int (*cmp)(const void *key, const struct rb_node *))
{
struct rb_node *node = tree->rb_node;
@@ -189,7 +189,7 @@ struct symbol *find_func_by_offset(struc
return NULL;
}

-struct symbol *find_symbol_containing(struct section *sec, unsigned long offset)
+struct symbol *find_symbol_containing(const struct section *sec, unsigned long offset)
{
struct rb_node *node;

--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -124,7 +124,7 @@ struct section *find_section_by_name(con
struct symbol *find_func_by_offset(struct section *sec, unsigned long offset);
struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset);
struct symbol *find_symbol_by_name(const struct elf *elf, const char *name);
-struct symbol *find_symbol_containing(struct section *sec, unsigned long offset);
+struct symbol *find_symbol_containing(const struct section *sec, unsigned long offset);
struct rela *find_rela_by_dest(const struct elf *elf, struct section *sec, unsigned long offset);
struct rela *find_rela_by_dest_range(const struct elf *elf, struct section *sec,
unsigned long offset, unsigned int len);

2020-05-01 18:24:43

by tip-bot2 for Eric Biggers

Subject: [tip: objtool/core] objtool: Move the IRET hack into the arch decoder

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: b490f45362002fef57996388e395efc974b013f4
Gitweb: https://git.kernel.org/tip/b490f45362002fef57996388e395efc974b013f4
Author: Miroslav Benes <[email protected]>
AuthorDate: Fri, 24 Apr 2020 16:30:42 +02:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Thu, 30 Apr 2020 20:14:33 +02:00

objtool: Move the IRET hack into the arch decoder

Quoting Julien:

"And the other suggestion is my other email was that you don't even
need to add INSN_EXCEPTION_RETURN. You can keep IRET as
INSN_CONTEXT_SWITCH by default and x86 decoder lookups the symbol
conaining an iret. If it's a function symbol, it can just set the type
to INSN_OTHER so that it caries on to the next instruction after
having handled the stack_op."

Suggested-by: Julien Thierry <[email protected]>
Signed-off-by: Miroslav Benes <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Miroslav Benes <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
tools/objtool/arch.h | 1 -
tools/objtool/arch/x86/decode.c | 28 ++++++++++++++++++----------
tools/objtool/check.c | 11 -----------
tools/objtool/elf.c | 4 ++--
tools/objtool/elf.h | 2 +-
5 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index 25dd4a9..cd118eb 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -19,7 +19,6 @@ enum insn_type {
INSN_CALL,
INSN_CALL_DYNAMIC,
INSN_RETURN,
- INSN_EXCEPTION_RETURN,
INSN_CONTEXT_SWITCH,
INSN_BUG,
INSN_NOP,
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index e26bedb..d7b5d10 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -94,6 +94,7 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec,
rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0,
modrm_reg = 0, sib = 0;
struct stack_op *op = NULL;
+ struct symbol *sym;

x86_64 = is_x86_64(elf);
if (x86_64 == -1)
@@ -469,17 +470,24 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec,
break;

case 0xcf: /* iret */
- *type = INSN_EXCEPTION_RETURN;
-
- ADD_OP(op) {
- /* add $40, %rsp */
- op->src.type = OP_SRC_ADD;
- op->src.reg = CFI_SP;
- op->src.offset = 5*8;
- op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_SP;
+ /*
+ * Handle sync_core(), which has an IRET to self.
+ * All other IRET are in STT_NONE entry code.
+ */
+ sym = find_symbol_containing(sec, offset);
+ if (sym && sym->type == STT_FUNC) {
+ ADD_OP(op) {
+ /* add $40, %rsp */
+ op->src.type = OP_SRC_ADD;
+ op->src.reg = CFI_SP;
+ op->src.offset = 5*8;
+ op->dest.type = OP_DEST_REG;
+ op->dest.reg = CFI_SP;
+ }
+ break;
}
- break;
+
+ /* fallthrough */

case 0xca: /* retf */
case 0xcb: /* retf */
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4f3db2f..d822858 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2320,17 +2320,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,

break;

- case INSN_EXCEPTION_RETURN:
- /*
- * This handles x86's sync_core() case, where we use an
- * IRET to self. All 'normal' IRET instructions are in
- * STT_NOTYPE entry symbols.
- */
- if (func)
- break;
-
- return 0;
-
case INSN_CONTEXT_SWITCH:
if (func && (!next_insn || !next_insn->hint)) {
WARN_FUNC("unsupported instruction in callable function",
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 453b723..b6349ca 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -61,7 +61,7 @@ static void rb_add(struct rb_root *tree, struct rb_node *node,
rb_insert_color(node, tree);
}

-static struct rb_node *rb_find_first(struct rb_root *tree, const void *key,
+static struct rb_node *rb_find_first(const struct rb_root *tree, const void *key,
int (*cmp)(const void *key, const struct rb_node *))
{
struct rb_node *node = tree->rb_node;
@@ -189,7 +189,7 @@ struct symbol *find_func_by_offset(struct section *sec, unsigned long offset)
return NULL;
}

-struct symbol *find_symbol_containing(struct section *sec, unsigned long offset)
+struct symbol *find_symbol_containing(const struct section *sec, unsigned long offset)
{
struct rb_node *node;

diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index 5e76ac3..9d6bb07 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -124,7 +124,7 @@ struct section *find_section_by_name(const struct elf *elf, const char *name);
struct symbol *find_func_by_offset(struct section *sec, unsigned long offset);
struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset);
struct symbol *find_symbol_by_name(const struct elf *elf, const char *name);
-struct symbol *find_symbol_containing(struct section *sec, unsigned long offset);
+struct symbol *find_symbol_containing(const struct section *sec, unsigned long offset);
struct rela *find_rela_by_dest(const struct elf *elf, struct section *sec, unsigned long offset);
struct rela *find_rela_by_dest_range(const struct elf *elf, struct section *sec,
unsigned long offset, unsigned int len);