[1/3] objtool: Separate prefix code from stack validation code
Commit Message
Simplify the prefix code and make it a standalone feature.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
tools/objtool/check.c | 88 ++++++++++++++++++++++++-------------------
1 file changed, 50 insertions(+), 38 deletions(-)
Comments
On Wed, Apr 12, 2023 at 01:26:13PM -0700, Josh Poimboeuf wrote:
> Simplify the prefix code and make it a standalone feature.
The main thing being that you moved it all after
validate_reachable_instructions() ?
> +static int add_prefix_symbols(struct objtool_file *file)
> +{
> + struct section *sec;
> + struct symbol *func;
> + int ret, warnings = 0;
> +
> + for_each_sec(file, sec) {
> + if (!(sec->sh.sh_flags & SHF_EXECINSTR))
> + continue;
> +
> + list_for_each_entry(func, &sec->symbol_list, list) {
One of the other patches did a sec_for_each_symbol() thing.
> + if (func->type != STT_FUNC)
> + continue;
> +
> + add_prefix_symbol(file, func);
> + }
> + }
> +
> + return warnings;
> +}
On Thu, Apr 13, 2023 at 11:30:31AM +0200, Peter Zijlstra wrote:
> On Wed, Apr 12, 2023 at 01:26:13PM -0700, Josh Poimboeuf wrote:
> > Simplify the prefix code and make it a standalone feature.
>
> The main thing being that you moved it all after
> validate_reachable_instructions() ?
The main thing being that I extricated it from validate_symbol() so it's
a proper independent feature.
> > +static int add_prefix_symbols(struct objtool_file *file)
> > +{
> > + struct section *sec;
> > + struct symbol *func;
> > + int ret, warnings = 0;
> > +
> > + for_each_sec(file, sec) {
> > + if (!(sec->sh.sh_flags & SHF_EXECINSTR))
> > + continue;
> > +
> > + list_for_each_entry(func, &sec->symbol_list, list) {
>
> One of the other patches did a sec_for_each_symbol() thing.
Too many patch sets floating around ;-)
@@ -4120,54 +4120,61 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
return false;
}
-static int add_prefix_symbol(struct objtool_file *file, struct symbol *func,
- struct instruction *insn)
+static int add_prefix_symbol(struct objtool_file *file, struct symbol *func)
{
- if (!opts.prefix)
- return 0;
+ struct instruction *insn, *prev;
- for (;;) {
- struct instruction *prev = prev_insn_same_sec(file, insn);
- u64 offset;
+ insn = find_insn(file, func->sec, func->offset);
+ if (!insn)
+ return -1;
- if (!prev)
- break;
+ for (prev = prev_insn_same_sec(file, insn);
+ prev;
+ prev = prev_insn_same_sec(file, prev)) {
+ u64 offset;
if (prev->type != INSN_NOP)
- break;
+ return -1;
offset = func->offset - prev->offset;
- if (offset >= opts.prefix) {
- if (offset == opts.prefix) {
- /*
- * Since the sec->symbol_list is ordered by
- * offset (see elf_add_symbol()) the added
- * symbol will not be seen by the iteration in
- * validate_section().
- *
- * Hence the lack of list_for_each_entry_safe()
- * there.
- *
- * The direct concequence is that prefix symbols
- * don't get visited (because pointless), except
- * for the logic in ignore_unreachable_insn()
- * that needs the terminating insn to be visited
- * otherwise it will report the hole.
- *
- * Hence mark the first instruction of the
- * prefix symbol as visisted.
- */
- prev->visited |= VISITED_BRANCH;
- elf_create_prefix_symbol(file->elf, func, opts.prefix);
- }
- break;
- }
- insn = prev;
+
+ if (offset > opts.prefix)
+ return -1;
+
+ if (offset < opts.prefix)
+ continue;
+
+ elf_create_prefix_symbol(file->elf, func, opts.prefix);
+ break;
}
+ if (!prev)
+ return -1;
+
return 0;
}
+static int add_prefix_symbols(struct objtool_file *file)
+{
+ struct section *sec;
+ struct symbol *func;
+ int ret, warnings = 0;
+
+ for_each_sec(file, sec) {
+ if (!(sec->sh.sh_flags & SHF_EXECINSTR))
+ continue;
+
+ list_for_each_entry(func, &sec->symbol_list, list) {
+ if (func->type != STT_FUNC)
+ continue;
+
+ add_prefix_symbol(file, func);
+ }
+ }
+
+ return warnings;
+}
+
static int validate_symbol(struct objtool_file *file, struct section *sec,
struct symbol *sym, struct insn_state *state)
{
@@ -4186,8 +4193,6 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
if (!insn || insn->ignore || insn->visited)
return 0;
- add_prefix_symbol(file, sym, insn);
-
state->uaccess = sym->uaccess_safe;
ret = validate_branch(file, insn_func(insn), insn, *state);
@@ -4744,6 +4749,13 @@ int check(struct objtool_file *file)
warnings += ret;
}
+ if (opts.prefix) {
+ ret = add_prefix_symbols(file);
+ if (ret < 0)
+ return ret;
+ warnings += ret;
+ }
+
if (opts.ibt) {
ret = create_ibt_endbr_seal_sections(file);
if (ret < 0)