On Wed, Oct 30, 2024 at 4:51 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote:
>
> Async callback state enqueing, while logically detached from parent
typo. enqueuing
> -static int get_prev_insn_idx(struct bpf_verifier_state *st, int i,
> - u32 *history)
> +static int get_prev_insn_idx(const struct bpf_verifier_env *env,
> + struct bpf_verifier_state *st,
> + int insn_idx, u32 hist_start, u32 *hist_endp)
> {
> - u32 cnt = *history;
> + u32 hist_end = *hist_endp;
> + u32 cnt = hist_end - hist_start;
>
> - if (i == st->first_insn_idx) {
> + if (insn_idx == st->first_insn_idx) {
> if (cnt == 0)
> return -ENOENT;
> - if (cnt == 1 && st->jmp_history[0].idx == i)
> + if (cnt == 1 && env->insn_hist[hist_end - 1].idx == insn_idx)
> return -ENOENT;
> }
I think the above bit would be easier to understand if it was
env->insn_hist[hist_start].
When cnt==1 it's the same as hist_end-1, but it took me more time
to grok that part. With [hist_start] would have been easier.
Not a big deal.
Another minor suggestion...
wouldn't it be cleaner to take hist_start/end from 'st' both
in get_prev_insn_idx() and in get_insn_hist_entry() ?
So that __mark_chain_precision() doesn't need to reach out into
details of 'st' just to pass hist_start/end values into other helpers.
