* [PATCH 01/10] objtool: Move header sync-check ealier in build
2020-03-25 8:41 [PATCH 00/10] Objtool updates for easier portability Julien Thierry
@ 2020-03-25 8:41 ` Julien Thierry
2020-03-25 8:41 ` [PATCH 02/10] objtool: check: Remove redundant checks on operand type Julien Thierry
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Julien Thierry @ 2020-03-25 8:41 UTC (permalink / raw)
To: linux-kernel; +Cc: jpoimboe, peterz, raphael.gault, Julien Thierry
Currently, the check of tools files against kernel equivalent is only
done after every object file has been built. This means one might fix
build issues against outdated headers without seeing a warning about
this.
Check headers before any object is built. Also, make it part of a
FORCE'd recipe so every attempt to build objtool will report the
outdated headers (if any).
Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
tools/objtool/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index ee08aeff30a1..519af6ec4eee 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -43,10 +43,10 @@ export srctree OUTPUT CFLAGS SRCARCH AWK
include $(srctree)/tools/build/Makefile.include
$(OBJTOOL_IN): fixdep FORCE
+ @$(CONFIG_SHELL) ./sync-check.sh
@$(MAKE) $(build)=objtool
$(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
- @$(CONFIG_SHELL) ./sync-check.sh
$(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 02/10] objtool: check: Remove redundant checks on operand type
2020-03-25 8:41 [PATCH 00/10] Objtool updates for easier portability Julien Thierry
2020-03-25 8:41 ` [PATCH 01/10] objtool: Move header sync-check ealier in build Julien Thierry
@ 2020-03-25 8:41 ` Julien Thierry
2020-03-25 8:41 ` [PATCH 03/10] objtool: check: Clean instruction state before each function validation Julien Thierry
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Julien Thierry @ 2020-03-25 8:41 UTC (permalink / raw)
To: linux-kernel; +Cc: jpoimboe, peterz, raphael.gault, Julien Thierry
POP operations are already in code path where destination operand
is OP_DEST_REG. There is no need to check the operand type again.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
tools/objtool/check.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index a2c62bc82907..44a3abbb0b0b 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1730,15 +1730,13 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
case OP_SRC_POP:
case OP_SRC_POPF:
- if (!state->drap && op->dest.type == OP_DEST_REG &&
- op->dest.reg == cfa->base) {
+ if (!state->drap && op->dest.reg == cfa->base) {
/* pop %rbp */
cfa->base = CFI_SP;
}
if (state->drap && cfa->base == CFI_BP_INDIRECT &&
- op->dest.type == OP_DEST_REG &&
op->dest.reg == state->drap_reg &&
state->drap_offset == -state->stack_size) {
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 03/10] objtool: check: Clean instruction state before each function validation
2020-03-25 8:41 [PATCH 00/10] Objtool updates for easier portability Julien Thierry
2020-03-25 8:41 ` [PATCH 01/10] objtool: Move header sync-check ealier in build Julien Thierry
2020-03-25 8:41 ` [PATCH 02/10] objtool: check: Remove redundant checks on operand type Julien Thierry
@ 2020-03-25 8:41 ` Julien Thierry
2020-03-25 8:41 ` [PATCH 04/10] objtool: check: Ignore empty alternative groups Julien Thierry
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Julien Thierry @ 2020-03-25 8:41 UTC (permalink / raw)
To: linux-kernel; +Cc: jpoimboe, peterz, raphael.gault, Julien Thierry
When a function fails its validation, it might leave a stale state
that will be used for the validation of other functions. That would
cause false warnings on potentially valid functions.
Reset the instruction state before the validation of each individual
function.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
tools/objtool/check.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 44a3abbb0b0b..0ccf6882d8ce 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2406,13 +2406,6 @@ static int validate_functions(struct objtool_file *file)
struct insn_state state;
int ret, warnings = 0;
- clear_insn_state(&state);
-
- state.cfa = initial_func_cfi.cfa;
- memcpy(&state.regs, &initial_func_cfi.regs,
- CFI_NUM_REGS * sizeof(struct cfi_reg));
- state.stack_size = initial_func_cfi.cfa.offset;
-
for_each_sec(file, sec) {
list_for_each_entry(func, &sec->symbol_list, list) {
if (func->type != STT_FUNC)
@@ -2431,6 +2424,12 @@ static int validate_functions(struct objtool_file *file)
if (!insn || insn->ignore || insn->visited)
continue;
+ clear_insn_state(&state);
+ state.cfa = initial_func_cfi.cfa;
+ memcpy(&state.regs, &initial_func_cfi.regs,
+ CFI_NUM_REGS * sizeof(struct cfi_reg));
+ state.stack_size = initial_func_cfi.cfa.offset;
+
state.uaccess = func->uaccess_safe;
ret = validate_branch(file, func, insn, state);
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 04/10] objtool: check: Ignore empty alternative groups
2020-03-25 8:41 [PATCH 00/10] Objtool updates for easier portability Julien Thierry
` (2 preceding siblings ...)
2020-03-25 8:41 ` [PATCH 03/10] objtool: check: Clean instruction state before each function validation Julien Thierry
@ 2020-03-25 8:41 ` Julien Thierry
2020-03-25 8:41 ` [PATCH 05/10] objtool: check: Remove check preventing branches within alternative Julien Thierry
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Julien Thierry @ 2020-03-25 8:41 UTC (permalink / raw)
To: linux-kernel; +Cc: jpoimboe, peterz, raphael.gault, Julien Thierry
Atlernative section can contain entries for alternatives with no
instructions. Objtool will currently crash when handling such an entry.
Just skip that entry, but still give a warning to discourage useless
entries.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
tools/objtool/check.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0ccf6882d8ce..75ebaa0a6216 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -917,6 +917,12 @@ static int add_special_section_alts(struct objtool_file *file)
}
if (special_alt->group) {
+ if (!special_alt->orig_len) {
+ WARN_FUNC("empty alternative entry",
+ orig_insn->sec, orig_insn->offset);
+ continue;
+ }
+
ret = handle_group_alt(file, special_alt, orig_insn,
&new_insn);
if (ret)
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 05/10] objtool: check: Remove check preventing branches within alternative
2020-03-25 8:41 [PATCH 00/10] Objtool updates for easier portability Julien Thierry
` (3 preceding siblings ...)
2020-03-25 8:41 ` [PATCH 04/10] objtool: check: Ignore empty alternative groups Julien Thierry
@ 2020-03-25 8:41 ` Julien Thierry
2020-03-25 8:41 ` [PATCH 06/10] objtool: check: Use arch specific values in restore_reg() Julien Thierry
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Julien Thierry @ 2020-03-25 8:41 UTC (permalink / raw)
To: linux-kernel; +Cc: jpoimboe, peterz, raphael.gault, Julien Thierry
While jumping from outside an alternative region to the middle of an
alternative region is very likely wrong, jumping from an alternative
region into the same region is valid. It is a common pattern on arm64.
The first pattern is unlikely to happen in practice and checking only
for this adds a lot of complexity.
Just remove the current check.
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
tools/objtool/check.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 75ebaa0a6216..b8c288c02c99 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2015,12 +2015,6 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
insn = first;
sec = insn->sec;
- if (insn->alt_group && list_empty(&insn->alts)) {
- WARN_FUNC("don't know how to handle branch to middle of alternative instruction group",
- sec, insn->offset);
- return 1;
- }
-
while (1) {
next_insn = next_insn_same_sec(file, insn);
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 06/10] objtool: check: Use arch specific values in restore_reg()
2020-03-25 8:41 [PATCH 00/10] Objtool updates for easier portability Julien Thierry
` (4 preceding siblings ...)
2020-03-25 8:41 ` [PATCH 05/10] objtool: check: Remove check preventing branches within alternative Julien Thierry
@ 2020-03-25 8:41 ` Julien Thierry
2020-03-25 8:42 ` [PATCH 07/10] objtool: check: Allow save/restore hint in non standard function symbols Julien Thierry
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Julien Thierry @ 2020-03-25 8:41 UTC (permalink / raw)
To: linux-kernel; +Cc: jpoimboe, peterz, raphael.gault, Julien Thierry
Initial register state is set up by arch specific code. Use the value
the arch code has set when restoring registers from the stack.
Suggested-by: Raphael Gault <raphael.gault@arm.com>
Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
tools/objtool/check.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index b8c288c02c99..7bf4dbc2e31f 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1501,8 +1501,8 @@ static void save_reg(struct insn_state *state, unsigned char reg, int base,
static void restore_reg(struct insn_state *state, unsigned char reg)
{
- state->regs[reg].base = CFI_UNDEFINED;
- state->regs[reg].offset = 0;
+ state->regs[reg].base = initial_func_cfi.regs[reg].base;
+ state->regs[reg].offset = initial_func_cfi.regs[reg].offset;
}
/*
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 07/10] objtool: check: Allow save/restore hint in non standard function symbols
2020-03-25 8:41 [PATCH 00/10] Objtool updates for easier portability Julien Thierry
` (5 preceding siblings ...)
2020-03-25 8:41 ` [PATCH 06/10] objtool: check: Use arch specific values in restore_reg() Julien Thierry
@ 2020-03-25 8:42 ` Julien Thierry
2020-03-25 8:42 ` [PATCH 08/10] objtool: Add abstraction for computation of symbols offsets Julien Thierry
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Julien Thierry @ 2020-03-25 8:42 UTC (permalink / raw)
To: linux-kernel; +Cc: jpoimboe, peterz, raphael.gault, Julien Thierry
The kernel code base provides CODE_SYM_START/END to declare assembly
code sequences that don't follow function standard calling conventions.
As non-C/non-standard code, these sequences can very much benefit from
unwind hints. However, when a restore unwind_hint is used in a
non-function code sequence, objtool will crash when looking for the
corresponding save hint.
Record the code symbol an instruction belongs to and look for save hints
belonging to the same code symbol as the restore hint.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
tools/objtool/check.c | 27 +++++++++++++++++++--------
tools/objtool/check.h | 1 +
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 7bf4dbc2e31f..90d3db00352d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -236,7 +236,7 @@ static void clear_insn_state(struct insn_state *state)
static int decode_instructions(struct objtool_file *file)
{
struct section *sec;
- struct symbol *func;
+ struct symbol *sym;
unsigned long offset;
struct instruction *insn;
int ret;
@@ -276,18 +276,22 @@ static int decode_instructions(struct objtool_file *file)
list_add_tail(&insn->list, &file->insn_list);
}
- list_for_each_entry(func, &sec->symbol_list, list) {
- if (func->type != STT_FUNC || func->alias != func)
+ list_for_each_entry(sym, &sec->symbol_list, list) {
+ if ((sym->type != STT_FUNC && sym->type != STT_NOTYPE) ||
+ sym->alias != sym)
continue;
- if (!find_insn(file, sec, func->offset)) {
+ if (!find_insn(file, sec, sym->offset)) {
WARN("%s(): can't find starting instruction",
- func->name);
+ sym->name);
return -1;
}
- func_for_each_insn(file, func, insn)
- insn->func = func;
+ func_for_each_insn(file, sym, insn) {
+ insn->code_sym = sym;
+ if (sym->type == STT_FUNC)
+ insn->func = sym;
+ }
}
}
@@ -771,6 +775,7 @@ static int handle_group_alt(struct objtool_file *file,
fake_jump->type = INSN_JUMP_UNCONDITIONAL;
fake_jump->jump_dest = list_next_entry(last_orig_insn, list);
fake_jump->func = orig_insn->func;
+ fake_jump->code_sym = orig_insn->code_sym;
}
if (!special_alt->new_len) {
@@ -2043,9 +2048,15 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
if (insn->restore) {
struct instruction *save_insn, *i;
+ if (!insn->code_sym) {
+ WARN_FUNC("restore instruction doesn't belong to any symbol",
+ insn->sec, insn->offset);
+ return 1;
+ }
+
i = insn;
save_insn = NULL;
- func_for_each_insn_continue_reverse(file, func, i) {
+ func_for_each_insn_continue_reverse(file, insn->code_sym, i) {
if (i->save) {
save_insn = i;
break;
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index 6d875ca6fce0..0cfdad839b76 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -42,6 +42,7 @@ struct instruction {
struct rela *jump_table;
struct list_head alts;
struct symbol *func;
+ struct symbol *code_sym;
struct stack_op stack_op;
struct insn_state state;
struct orc_entry orc;
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 08/10] objtool: Add abstraction for computation of symbols offsets
2020-03-25 8:41 [PATCH 00/10] Objtool updates for easier portability Julien Thierry
` (6 preceding siblings ...)
2020-03-25 8:42 ` [PATCH 07/10] objtool: check: Allow save/restore hint in non standard function symbols Julien Thierry
@ 2020-03-25 8:42 ` Julien Thierry
2020-03-25 8:42 ` [PATCH 09/10] objtool: Split generic and arch specific CFI definitions Julien Thierry
2020-03-25 8:42 ` [PATCH 10/10] objtool: Support multiple stack_op per instruction Julien Thierry
9 siblings, 0 replies; 11+ messages in thread
From: Julien Thierry @ 2020-03-25 8:42 UTC (permalink / raw)
To: linux-kernel; +Cc: jpoimboe, peterz, raphael.gault, Julien Thierry
From: Raphael Gault <raphael.gault@arm.com>
The jump destination and relocation offset used previously are only
reliable on x86_64 architecture. We abstract these computations by calling
arch-dependent implementations.
Signed-off-by: Raphael Gault <raphael.gault@arm.com>
[J.T. Remove superfluous comment, replace other addend offsets
with arch_dest_rela_offset]
Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
tools/objtool/arch.h | 6 ++++++
tools/objtool/arch/x86/decode.c | 11 +++++++++++
tools/objtool/check.c | 18 ++++++++++--------
3 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index ced3765c4f44..a9a50a25ca66 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -66,6 +66,8 @@ struct stack_op {
struct op_src src;
};
+struct instruction;
+
void arch_initial_func_cfi_state(struct cfi_state *state);
int arch_decode_instruction(struct elf *elf, struct section *sec,
@@ -75,4 +77,8 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
bool arch_callee_saved_reg(unsigned char reg);
+unsigned long arch_jump_destination(struct instruction *insn);
+
+unsigned long arch_dest_rela_offset(int addend);
+
#endif /* _ARCH_H */
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index a62e032863a8..7ce8650cf085 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -11,6 +11,7 @@
#include "../../../arch/x86/lib/inat.c"
#include "../../../arch/x86/lib/insn.c"
+#include "../../check.h"
#include "../../elf.h"
#include "../../arch.h"
#include "../../warn.h"
@@ -66,6 +67,16 @@ bool arch_callee_saved_reg(unsigned char reg)
}
}
+unsigned long arch_dest_rela_offset(int addend)
+{
+ return addend + 4;
+}
+
+unsigned long arch_jump_destination(struct instruction *insn)
+{
+ return insn->offset + insn->len + insn->immediate;
+}
+
int arch_decode_instruction(struct elf *elf, struct section *sec,
unsigned long offset, unsigned int maxlen,
unsigned int *len, enum insn_type *type,
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 90d3db00352d..7b80a4a6e53d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -590,13 +590,14 @@ static int add_jump_destinations(struct objtool_file *file)
insn->len);
if (!rela) {
dest_sec = insn->sec;
- dest_off = insn->offset + insn->len + insn->immediate;
+ dest_off = arch_jump_destination(insn);
} else if (rela->sym->type == STT_SECTION) {
dest_sec = rela->sym->sec;
- dest_off = rela->addend + 4;
+ dest_off = arch_dest_rela_offset(rela->addend);
} else if (rela->sym->sec->idx) {
dest_sec = rela->sym->sec;
- dest_off = rela->sym->sym.st_value + rela->addend + 4;
+ dest_off = rela->sym->sym.st_value +
+ arch_dest_rela_offset(rela->addend);
} else if (strstr(rela->sym->name, "_indirect_thunk_")) {
/*
* Retpoline jumps are really dynamic jumps in
@@ -686,7 +687,7 @@ static int add_call_destinations(struct objtool_file *file)
rela = find_rela_by_dest_range(insn->sec, insn->offset,
insn->len);
if (!rela) {
- dest_off = insn->offset + insn->len + insn->immediate;
+ dest_off = arch_jump_destination(insn);
insn->call_dest = find_func_by_offset(insn->sec, dest_off);
if (!insn->call_dest)
insn->call_dest = find_symbol_by_offset(insn->sec, dest_off);
@@ -709,13 +710,14 @@ static int add_call_destinations(struct objtool_file *file)
}
} else if (rela->sym->type == STT_SECTION) {
+ dest_off = arch_dest_rela_offset(rela->addend);
insn->call_dest = find_func_by_offset(rela->sym->sec,
- rela->addend+4);
+ dest_off);
if (!insn->call_dest) {
- WARN_FUNC("can't find call dest symbol at %s+0x%x",
+ WARN_FUNC("can't find call dest symbol at %s+0x%lx",
insn->sec, insn->offset,
rela->sym->sec->name,
- rela->addend + 4);
+ dest_off);
return -1;
}
} else
@@ -827,7 +829,7 @@ static int handle_group_alt(struct objtool_file *file,
if (!insn->immediate)
continue;
- dest_off = insn->offset + insn->len + insn->immediate;
+ dest_off = arch_jump_destination(insn);
if (dest_off == special_alt->new_off + special_alt->new_len) {
if (!fake_jump) {
WARN("%s: alternative jump to end of section",
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 09/10] objtool: Split generic and arch specific CFI definitions
2020-03-25 8:41 [PATCH 00/10] Objtool updates for easier portability Julien Thierry
` (7 preceding siblings ...)
2020-03-25 8:42 ` [PATCH 08/10] objtool: Add abstraction for computation of symbols offsets Julien Thierry
@ 2020-03-25 8:42 ` Julien Thierry
2020-03-25 8:42 ` [PATCH 10/10] objtool: Support multiple stack_op per instruction Julien Thierry
9 siblings, 0 replies; 11+ messages in thread
From: Julien Thierry @ 2020-03-25 8:42 UTC (permalink / raw)
To: linux-kernel; +Cc: jpoimboe, peterz, raphael.gault, Julien Thierry
Some CFI definitions used by generic objtool code have no reason to vary
from one architecture to another. Move those definition to generic code
and keep a separate per arch header to provide architecture specific CFI
definitions.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
tools/objtool/Makefile | 3 ++-
tools/objtool/arch/x86/include/cfi_regs.h | 25 +++++++++++++++++++++++
tools/objtool/cfi.h | 21 ++-----------------
3 files changed, 29 insertions(+), 20 deletions(-)
create mode 100644 tools/objtool/arch/x86/include/cfi_regs.h
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 519af6ec4eee..11b7dc5d18fe 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -29,7 +29,8 @@ all: $(OBJTOOL)
INCLUDES := -I$(srctree)/tools/include \
-I$(srctree)/tools/arch/$(HOSTARCH)/include/uapi \
- -I$(srctree)/tools/arch/$(SRCARCH)/include
+ -I$(srctree)/tools/arch/$(SRCARCH)/include \
+ -I$(srctree)/tools/objtool/arch/$(SRCARCH)/include
WARNINGS := $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -Wno-packed
CFLAGS := -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) $(LIBELF_FLAGS)
LDFLAGS += $(LIBELF_LIBS) $(LIBSUBCMD) $(KBUILD_HOSTLDFLAGS)
diff --git a/tools/objtool/arch/x86/include/cfi_regs.h b/tools/objtool/arch/x86/include/cfi_regs.h
new file mode 100644
index 000000000000..79bc517efba8
--- /dev/null
+++ b/tools/objtool/arch/x86/include/cfi_regs.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _OBJTOOL_CFI_REGS_H
+#define _OBJTOOL_CFI_REGS_H
+
+#define CFI_AX 0
+#define CFI_DX 1
+#define CFI_CX 2
+#define CFI_BX 3
+#define CFI_SI 4
+#define CFI_DI 5
+#define CFI_BP 6
+#define CFI_SP 7
+#define CFI_R8 8
+#define CFI_R9 9
+#define CFI_R10 10
+#define CFI_R11 11
+#define CFI_R12 12
+#define CFI_R13 13
+#define CFI_R14 14
+#define CFI_R15 15
+#define CFI_RA 16
+#define CFI_NUM_REGS 17
+
+#endif /* _OBJTOOL_CFI_REGS_H */
diff --git a/tools/objtool/cfi.h b/tools/objtool/cfi.h
index 4427bf8ed686..1a3e7b807994 100644
--- a/tools/objtool/cfi.h
+++ b/tools/objtool/cfi.h
@@ -6,30 +6,13 @@
#ifndef _OBJTOOL_CFI_H
#define _OBJTOOL_CFI_H
+#include "cfi_regs.h"
+
#define CFI_UNDEFINED -1
#define CFI_CFA -2
#define CFI_SP_INDIRECT -3
#define CFI_BP_INDIRECT -4
-#define CFI_AX 0
-#define CFI_DX 1
-#define CFI_CX 2
-#define CFI_BX 3
-#define CFI_SI 4
-#define CFI_DI 5
-#define CFI_BP 6
-#define CFI_SP 7
-#define CFI_R8 8
-#define CFI_R9 9
-#define CFI_R10 10
-#define CFI_R11 11
-#define CFI_R12 12
-#define CFI_R13 13
-#define CFI_R14 14
-#define CFI_R15 15
-#define CFI_RA 16
-#define CFI_NUM_REGS 17
-
struct cfi_reg {
int base;
int offset;
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 10/10] objtool: Support multiple stack_op per instruction
2020-03-25 8:41 [PATCH 00/10] Objtool updates for easier portability Julien Thierry
` (8 preceding siblings ...)
2020-03-25 8:42 ` [PATCH 09/10] objtool: Split generic and arch specific CFI definitions Julien Thierry
@ 2020-03-25 8:42 ` Julien Thierry
9 siblings, 0 replies; 11+ messages in thread
From: Julien Thierry @ 2020-03-25 8:42 UTC (permalink / raw)
To: linux-kernel; +Cc: jpoimboe, peterz, raphael.gault, Julien Thierry
Instruction sets can include more or less complex operations which might
not fit the currently defined set of stack_ops.
Combining more than one stack_op provides more flexibility to describe
the behaviour of an instruction. This also reduces the need to define
new stack_ops specific to a single instruction set.
Allow instruction decoders to generate multiple stack_op per
instruction.
Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
tools/objtool/arch.h | 4 +-
tools/objtool/arch/x86/decode.c | 13 +++++-
tools/objtool/check.c | 79 +++++++++++++++++++++------------
tools/objtool/check.h | 2 +-
4 files changed, 67 insertions(+), 31 deletions(-)
diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index a9a50a25ca66..f9883c431949 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -64,6 +64,7 @@ struct op_src {
struct stack_op {
struct op_dest dest;
struct op_src src;
+ struct list_head list;
};
struct instruction;
@@ -73,7 +74,8 @@ void arch_initial_func_cfi_state(struct cfi_state *state);
int arch_decode_instruction(struct elf *elf, struct section *sec,
unsigned long offset, unsigned int maxlen,
unsigned int *len, enum insn_type *type,
- unsigned long *immediate, struct stack_op *op);
+ unsigned long *immediate,
+ struct list_head *ops_list);
bool arch_callee_saved_reg(unsigned char reg);
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 7ce8650cf085..199b4084a13c 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -80,13 +80,15 @@ unsigned long arch_jump_destination(struct instruction *insn)
int arch_decode_instruction(struct elf *elf, struct section *sec,
unsigned long offset, unsigned int maxlen,
unsigned int *len, enum insn_type *type,
- unsigned long *immediate, struct stack_op *op)
+ unsigned long *immediate,
+ struct list_head *ops_list)
{
struct insn insn;
int x86_64, sign;
unsigned char op1, op2, rex = 0, rex_b = 0, rex_r = 0, rex_w = 0,
rex_x = 0, modrm = 0, modrm_mod = 0, modrm_rm = 0,
modrm_reg = 0, sib = 0;
+ struct stack_op *op;
x86_64 = is_x86_64(elf);
if (x86_64 == -1)
@@ -127,6 +129,10 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
if (insn.sib.nbytes)
sib = insn.sib.bytes[0];
+ op = calloc(1, sizeof(*op));
+ if (!op)
+ return -1;
+
switch (op1) {
case 0x1:
@@ -488,6 +494,11 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
*immediate = insn.immediate.nbytes ? insn.immediate.value : 0;
+ if (*type == INSN_STACK)
+ list_add_tail(&op->list, ops_list);
+ else
+ free(op);
+
return 0;
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 7b80a4a6e53d..592fbebf234a 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -259,6 +259,7 @@ static int decode_instructions(struct objtool_file *file)
}
memset(insn, 0, sizeof(*insn));
INIT_LIST_HEAD(&insn->alts);
+ INIT_LIST_HEAD(&insn->stack_ops);
clear_insn_state(&insn->state);
insn->sec = sec;
@@ -268,7 +269,7 @@ static int decode_instructions(struct objtool_file *file)
sec->len - offset,
&insn->len, &insn->type,
&insn->immediate,
- &insn->stack_op);
+ &insn->stack_ops);
if (ret)
goto err;
@@ -770,6 +771,7 @@ static int handle_group_alt(struct objtool_file *file,
}
memset(fake_jump, 0, sizeof(*fake_jump));
INIT_LIST_HEAD(&fake_jump->alts);
+ INIT_LIST_HEAD(&fake_jump->stack_ops);
clear_insn_state(&fake_jump->state);
fake_jump->sec = special_alt->new_sec;
@@ -1472,10 +1474,11 @@ static bool has_valid_stack_frame(struct insn_state *state)
return false;
}
-static int update_insn_state_regs(struct instruction *insn, struct insn_state *state)
+static int update_insn_state_regs(struct instruction *insn,
+ struct insn_state *state,
+ struct stack_op *op)
{
struct cfi_reg *cfa = &state->cfa;
- struct stack_op *op = &insn->stack_op;
if (cfa->base != CFI_SP)
return 0;
@@ -1565,9 +1568,9 @@ static void restore_reg(struct insn_state *state, unsigned char reg)
* 41 5d pop %r13
* c3 retq
*/
-static int update_insn_state(struct instruction *insn, struct insn_state *state)
+static int update_insn_state(struct instruction *insn, struct insn_state *state,
+ struct stack_op *op)
{
- struct stack_op *op = &insn->stack_op;
struct cfi_reg *cfa = &state->cfa;
struct cfi_reg *regs = state->regs;
@@ -1581,7 +1584,7 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
}
if (state->type == ORC_TYPE_REGS || state->type == ORC_TYPE_REGS_IRET)
- return update_insn_state_regs(insn, state);
+ return update_insn_state_regs(insn, state, op);
switch (op->dest.type) {
@@ -1918,6 +1921,42 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
return 0;
}
+static int handle_insn_ops(struct instruction *insn, struct insn_state *state)
+{
+ struct stack_op *op;
+
+ list_for_each_entry(op, &insn->stack_ops, list) {
+ int res;
+
+ res = update_insn_state(insn, state, op);
+ if (res)
+ return res;
+
+ if (op->dest.type == OP_DEST_PUSHF) {
+ if (!state->uaccess_stack) {
+ state->uaccess_stack = 1;
+ } else if (state->uaccess_stack >> 31) {
+ WARN_FUNC("PUSHF stack exhausted",
+ insn->sec, insn->offset);
+ return 1;
+ }
+ state->uaccess_stack <<= 1;
+ state->uaccess_stack |= state->uaccess;
+ }
+
+ if (op->src.type == OP_SRC_POPF) {
+ if (state->uaccess_stack) {
+ state->uaccess = state->uaccess_stack & 1;
+ state->uaccess_stack >>= 1;
+ if (state->uaccess_stack == 1)
+ state->uaccess_stack = 0;
+ }
+ }
+ }
+
+ return 0;
+}
+
static bool insn_state_match(struct instruction *insn, struct insn_state *state)
{
struct insn_state *state1 = &insn->state, *state2 = state;
@@ -2210,29 +2249,8 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
return 0;
case INSN_STACK:
- if (update_insn_state(insn, &state))
+ if (handle_insn_ops(insn, &state))
return 1;
-
- if (insn->stack_op.dest.type == OP_DEST_PUSHF) {
- if (!state.uaccess_stack) {
- state.uaccess_stack = 1;
- } else if (state.uaccess_stack >> 31) {
- WARN_FUNC("PUSHF stack exhausted", sec, insn->offset);
- return 1;
- }
- state.uaccess_stack <<= 1;
- state.uaccess_stack |= state.uaccess;
- }
-
- if (insn->stack_op.src.type == OP_SRC_POPF) {
- if (state.uaccess_stack) {
- state.uaccess = state.uaccess_stack & 1;
- state.uaccess_stack >>= 1;
- if (state.uaccess_stack == 1)
- state.uaccess_stack = 0;
- }
- }
-
break;
case INSN_STAC:
@@ -2477,12 +2495,17 @@ static void cleanup(struct objtool_file *file)
{
struct instruction *insn, *tmpinsn;
struct alternative *alt, *tmpalt;
+ struct stack_op *op, *tmpop;
list_for_each_entry_safe(insn, tmpinsn, &file->insn_list, list) {
list_for_each_entry_safe(alt, tmpalt, &insn->alts, list) {
list_del(&alt->list);
free(alt);
}
+ list_for_each_entry_safe(op, tmpop, &insn->stack_ops, list) {
+ list_del(&op->list);
+ free(op);
+ }
list_del(&insn->list);
hash_del(&insn->hash);
free(insn);
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index 0cfdad839b76..8e9a37b1c609 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -43,7 +43,7 @@ struct instruction {
struct list_head alts;
struct symbol *func;
struct symbol *code_sym;
- struct stack_op stack_op;
+ struct list_head stack_ops;
struct insn_state state;
struct orc_entry orc;
};
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread