Messages in this thread Patch in this message | | | Subject | Re: [PATCH 2/7] objtool: Allow branches within the same alternative. | From | Julien Thierry <> | Date | Thu, 2 Apr 2020 13:03:30 +0100 |
| |
Hi Alexandre,
I ran into the same issue for the arm64 work: https://lkml.org/lkml/2020/1/9/656
Your solution seems nicer however.
On 4/2/20 9:22 AM, Alexandre Chartre wrote: > Currently objtool prevents any branch to an alternative. While preventing > branching from the outside to the middle of an alternative makes perfect > sense, branching within the same alternative should be allowed. To do so, > identify each alternative and check that a branch to an alternative comes > from the same alternative. > > Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com> > --- > tools/objtool/check.c | 19 +++++++++++++------ > tools/objtool/check.h | 3 ++- > 2 files changed, 15 insertions(+), 7 deletions(-) > > diff --git a/tools/objtool/check.c b/tools/objtool/check.c > index 708562fb89e1..214809ac2776 100644 > --- a/tools/objtool/check.c > +++ b/tools/objtool/check.c > @@ -712,7 +712,9 @@ static int handle_group_alt(struct objtool_file *file, > struct instruction *orig_insn, > struct instruction **new_insn) > { > + static unsigned int alt_group_next_index = 1; > struct instruction *last_orig_insn, *last_new_insn, *insn, *fake_jump = NULL; > + unsigned int alt_group = alt_group_next_index++; > unsigned long dest_off; > > last_orig_insn = NULL; > @@ -721,7 +723,7 @@ static int handle_group_alt(struct objtool_file *file, > if (insn->offset >= special_alt->orig_off + special_alt->orig_len) > break; > > - insn->alt_group = true; > + insn->alt_group = alt_group; > last_orig_insn = insn; > } > > @@ -1942,6 +1944,7 @@ static int validate_sibling_call(struct instruction *insn, struct insn_state *st > * tools/objtool/Documentation/stack-validation.txt. > */ > static int validate_branch(struct objtool_file *file, struct symbol *func, > + struct instruction *from,
Maybe instead of passing a new instruction pointer, just the alt_group could be passed? 0 Meaning it was not in an alt group.
> struct instruction *first, struct insn_state state) > { > struct alternative *alt; > @@ -1953,7 +1956,9 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, > insn = first; > sec = insn->sec; > > - if (insn->alt_group && list_empty(&insn->alts)) { > + if (insn->alt_group && > + (!from || from->alt_group != insn->alt_group) && > + list_empty(&insn->alts)) {
This would become
if (insn->alt_group != alt_group && list_empty(&insn->alts))
And the recursive validate_branch() calls would just take insn->alt_group as parameter (and the calls in validate_functions() and validate_unwind_hints() would take 0).
Any opinions on that?
> WARN_FUNC("don't know how to handle branch to middle of alternative instruction group", > sec, insn->offset); > return 1; > @@ -2035,7 +2040,8 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, > if (alt->skip_orig) > skip_orig = true; > > - ret = validate_branch(file, func, alt->insn, state); > + ret = validate_branch(file, func, > + NULL, alt->insn, state); > if (ret) { > if (backtrace) > BT_FUNC("(alt)", insn); > @@ -2105,7 +2111,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func, > return ret; > > } else if (insn->jump_dest) { > - ret = validate_branch(file, func, > + ret = validate_branch(file, func, insn, > insn->jump_dest, state); > if (ret) { > if (backtrace) > @@ -2236,7 +2242,8 @@ static int validate_unwind_hints(struct objtool_file *file) > > for_each_insn(file, insn) { > if (insn->hint && !insn->visited) { > - ret = validate_branch(file, insn->func, insn, state); > + ret = validate_branch(file, insn->func, > + NULL, insn, state); > if (ret && backtrace) > BT_FUNC("<=== (hint)", insn); > warnings += ret; > @@ -2377,7 +2384,7 @@ static int validate_functions(struct objtool_file *file) > > state.uaccess = func->uaccess_safe; > > - ret = validate_branch(file, func, insn, state); > + ret = validate_branch(file, func, NULL, insn, state); > if (ret && backtrace) > BT_FUNC("<=== (func)", insn); > warnings += ret; > diff --git a/tools/objtool/check.h b/tools/objtool/check.h > index 6d875ca6fce0..cffb23d81782 100644 > --- a/tools/objtool/check.h > +++ b/tools/objtool/check.h > @@ -33,7 +33,8 @@ struct instruction { > unsigned int len; > enum insn_type type; > unsigned long immediate; > - bool alt_group, dead_end, ignore, hint, save, restore, ignore_alts; > + unsigned int alt_group; > + bool dead_end, ignore, hint, save, restore, ignore_alts; > bool retpoline_safe; > u8 visited; > struct symbol *call_dest; >
Cheers,
-- Julien Thierry
| |