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!
