Messages in this thread | | | Date | Thu, 1 Jul 2021 10:35:34 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v2 02/24] objtool: Introduce CFI hash |
| |
On Wed, Jun 30, 2021 at 02:45:57PM +0200, Miroslav Benes wrote: > > @@ -2725,15 +2820,24 @@ static int validate_branch(struct objtoo > > > > if (insn->visited & visited) > > return 0; > > - } > > + } else > > + nr_visited++; > > just a coding style > > } else { > nr_visited++; > } > > > @@ -3192,6 +3304,13 @@ int check(struct objtool_file *file) > > warnings += ret; > > } > > > > + if (stats) { > > + printf("nr_visited: %ld\n", nr_visited); > > Could we make this nr_insns_visited?
Both done.
> > + printf("nr_cfi: %ld\n", nr_cfi); > > + printf("nr_cfi_reused: %ld\n", nr_cfi_reused); > > + printf("nr_cfi_cache: %ld\n", nr_cfi_cache); > > + } > > + > > +++ b/tools/objtool/orc_gen.c > > @@ -13,13 +13,19 @@ > > #include <objtool/warn.h> > > #include <objtool/endianness.h> > > > > -static int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi) > > +static int init_orc_entry(struct orc_entry *orc, struct cfi_state *cfi, > > + struct instruction *insn) > > { > > - struct instruction *insn = container_of(cfi, struct instruction, cfi); > > struct cfi_reg *bp = &cfi->regs[CFI_BP]; > > > > memset(orc, 0, sizeof(*orc)); > > > > + if (!cfi) { > > + orc->end = 0; > > + orc->sp_reg = ORC_REG_UNDEFINED; > > + return 0; > > + } > > + > > This looks strange. You access cfi a couple of lines earlier (bp cfi_reg > initialization).
That's a relative address compute, it doesn't actually dereference the pointer.
> Is it even possible to have cfi == NULL here? The second call site below > has a check and the first one should not happen (insn->cfip should be > always set if I am not missing something).
I can happen for instructions that are not visited, like NOP fillers between functions. Remove the check and try to build a file :-)
| |