Messages in this thread Patch in this message | | | Date | Fri, 19 Jan 2018 17:42:29 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH 18/35] objtool: Another static block fail |
| |
On Thu, Jan 18, 2018 at 02:48:18PM +0100, Peter Zijlstra wrote: > @@ -1216,7 +1218,8 @@ static bool __grow_static_block(struct o > > switch (insn->type) { > case INSN_JUMP_UNCONDITIONAL: > - /* mark this instruction, terminate this section */ > + /* follow the jump, mark this instruction, terminate this section */ > + jmp_grow_static_block(file, insn->jump_dest);
Hmm we should only do this when the jump is backwards, if its forwards we should just mark the destination instruction and let the main iteration loop take it.
> insn->static_jump_dest = true; > return false; >
something like the below merged in maybe; I'm going to look at this again later, my brain is completely useless today :/
Josh, how would you feel about adding the bits required to make objtool a full disassembler? That would make it far easier to visualse the state and I don't think its _that_ much more, all the difficult bits are already done afaict, all we need is infrastructure to print the already fully decoded instruction.
--- --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1210,16 +1210,16 @@ static int read_retpoline_hints(struct o static void jmp_grow_static_block(struct objtool_file *file, struct instruction *insn); static bool __grow_static_block(struct objtool_file *file, - struct instruction *insn, bool ign_bt) + struct instruction *insn) { - /* if a jump can come in here, terminate */ - if (insn->branch_target && !ign_bt) + /* if a !static jump can come in here, terminate */ + if (insn->branch_target && !insn->static_jump_dest) return false; switch (insn->type) { case INSN_JUMP_UNCONDITIONAL: /* follow the jump, mark this instruction, terminate this section */ - jmp_grow_static_block(file, insn->jump_dest); + jmp_grow_static_block(file, insn); insn->static_jump_dest = true; return false; @@ -1243,27 +1243,43 @@ static bool __grow_static_block(struct o static void jmp_grow_static_block(struct objtool_file *file, struct instruction *insn) { - bool ignore = true; + struct instruction *dest = insn->jump_dest; + bool static_block = true; - /* !jump_dest */ - if (!insn) + if (!dest) return; /* more than a single site jumps here, can't be certain */ - if (insn->branch_target > 1) + if (dest->branch_target > 1) return; - for (; &insn->list != &file->insn_list; - insn = list_next_entry(insn, list)) { - + if (dest->offset > insn->offset) { /* - * Per definition the first insn of a jump is a branch target, - * don't terminate because of that. + * fwd jump, the main iteration will still get there. + * make sure it continues the section there. */ - if (!__grow_static_block(file, insn, ignore)) - break; + dest->static_jump_dest = true; + return; + } - ignore = false; + /* backwards jump, we need to revisit the instructions */ + for (; static_block && dest != insn; dest = list_next_entry(dest, list)) { + + static_block = __grow_static_block(file, dest); + + if (insn->type == INSN_CALL) { + struct symbol *func = insn->call_dest; + if (!func) + continue; + + /* + * we flipped the call to static, update the stats. + */ + if (insn->static_jump_dest) { + func->non_static_call--; + func->static_call++; + } + } } } @@ -1276,7 +1292,7 @@ static int grow_static_blocks(struct obj for_each_insn(file, insn) { if (static_block || insn->static_jump_dest) - static_block = __grow_static_block(file, insn, !static_block); + static_block = __grow_static_block(file, insn); if (insn->type == INSN_CALL) { func = insn->call_dest; @@ -1284,9 +1300,9 @@ static int grow_static_blocks(struct obj continue; if (static_block) - func->static_call = true; + func->static_call++; else - func->non_static_call = true; + func->non_static_call++; } } @@ -1301,7 +1317,7 @@ static int grow_static_blocks(struct obj /* static && !non_static -- only static callers */ func_for_each_insn(file, func, insn) { - if (!__grow_static_block(file, insn, false)) + if (!__grow_static_block(file, insn)) break; } } --- a/tools/objtool/elf.h +++ b/tools/objtool/elf.h @@ -61,7 +61,7 @@ struct symbol { unsigned char bind, type; unsigned long offset; unsigned int len; - bool static_call, non_static_call; + unsigned int static_call, non_static_call; }; struct rela {
| |