Re: [PATCH bpf-next v3 5/8] selftests/bpf: utility function to get program disassembly after jit

[Eduard Zingerman <eddyz87@gmail.com>]

On Wed, 2024-08-21 at 11:07 -0700, Alexei Starovoitov wrote:

[...]

> > +static const char *lookup_symbol(void *data, uint64_t ref_value, uint64_t *ref_type,
> > +                                uint64_t ref_pc, const char **ref_name)
> > +{
> > +       struct local_labels *labels = data;
> > +       uint64_t type = *ref_type;
> > +       int i;
> > +
> > +       *ref_type = LLVMDisassembler_ReferenceType_InOut_None;
> > +       *ref_name = NULL;
> > +       if (type != LLVMDisassembler_ReferenceType_In_Branch)
> > +               return NULL;
> > +       /* Depending on labels->print_phase either discover local labels or
> > +        * return a name assigned with local jump target:
> > +        * - if print_phase is true and ref_value is in labels->pcs,
> > +        *   return corresponding labels->name.
> > +        * - if print_phase is false, save program-local jump targets
> > +        *   in labels->pcs;
> > +        */
> > +       if (labels->print_phase) {
> > +               for (i = 0; i < labels->cnt; ++i)
> > +                       if (labels->pcs[i] == ref_value)
> > +                               return labels->names[i];
> > +       } else {
> > +               if (labels->cnt < MAX_LOCAL_LABELS && ref_value < labels->prog_len)
> > +                       labels->pcs[labels->cnt++] = ref_value;
> > +       }
> > +       return NULL;
> > +}
> 
> bpftool should probably adopt similar logic
> just to be consistent?

Makes sense, will prepare patch for bpftool.

[...]

> > +       qsort(labels.pcs, labels.cnt, sizeof(*labels.pcs), cmp_u32);
> > +       for (i = 0; i < labels.cnt; ++i)
> > +               /* use (i % 100) to avoid format truncation warning */
> > +               snprintf(labels.names[i], sizeof(labels.names[i]), "L%d", i % 100);
> 
> 100 here and names[..][4] are a bit of magic.
> Pls add some #define and comments to clarify in the follow up.

Will do.

> Overall it looks to be a great improvement to selftests.
> Applied.
> 
> Pls add necessary packages to bpf CI.

Thank you!