On Thu, 2025-10-23 at 19:26 +0800, KaFai Wan wrote:
[...]
> > @@ -16173,6 +16173,25 @@ static int is_pkt_ptr_branch_taken(struct
> > bpf_reg_state *dst_reg,
> > static int is_branch_taken(struct bpf_reg_state *reg1, struct bpf_reg_state
> > *reg2,
> > u8 opcode, bool is_jmp32)
> > {
> > + if (reg1 == reg2) {
> > + switch (opcode) {
> > + case BPF_JGE:
> > + case BPF_JLE:
> > + case BPF_JSGE:
> > + case BPF_JSLE:
> > + case BPF_JEQ:
> > + case BPF_JSET:
>
> Others are fine, but BPF_JSET on the same register could be 0 (if value is 0).
> And it's unknown to take the branch if 0 within the range.
Right, missed that one.
>
> > + return 1;
> > + case BPF_JGT:
> > + case BPF_JLT:
> > + case BPF_JSGT:
> > + case BPF_JSLT:
> > + case BPF_JNE:
> > + return 0;
> > + default:
> > + return -1;
> > + }
> > + }
> >
> > But that's too much code for an artificial case.
> > Idk, either way is fine with me.
>
> There is is_scalar_branch_taken() in is_branch_taken(), I missed it. I'll a)
> check the opcode one by one in is_scalar_branch_taken(), and b) keep this
> patch
> for unknown BPF_JSET branch.
Sounds good to me. Note that the logic is correct for both scalar and
non-scalar cases, so I don't think we have to constrain it to
is_scalar_branch_taken() (don't think there is a need to check if
pointer comparisons are allowed, as no new information is inferred
from comparisons with self).
