On Thu, Apr 6, 2023 at 11:10 AM Dave Marchevsky <davemarchevsky@xxxxxxxx> wrote: > > On 4/6/23 12:45 PM, Yonghong Song wrote: > > Currently, the verifier does not handle '<const> <cond_op> <non_const>' well. > > For example, > > ... > > 10: (79) r1 = *(u64 *)(r10 -16) ; R1_w=scalar() R10=fp0 > > 11: (b7) r2 = 0 ; R2_w=0 > > 12: (2d) if r2 > r1 goto pc+2 > > 13: (b7) r0 = 0 > > 14: (95) exit > > 15: (65) if r1 s> 0x1 goto pc+3 > > 16: (0f) r0 += r1 > > ... > > At insn 12, verifier decides both true and false branch are possible, but > > actually only false branch is possible. > > > > Currently, the verifier already supports patterns '<non_const> <cond_op> <const>. > > Add support for patterns '<const> <cond_op> <non_const>' in a similar way. > > > > Also fix selftest 'verifier_bounds_mix_sign_unsign/bounds checks mixing signed and unsigned, variant 10' > > due to this change. > > > > Signed-off-by: Yonghong Song <yhs@xxxxxx> > > --- > > I still think there's a refactoring opportunity here, but I see your comments > on the related thread in v1 of this series, and don't think it's a blocker > to find cleanest refactor. Agreed, but current implementation is not wrong, so: Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > Acked-by: Dave Marchevsky <davemarchevsky@xxxxxx>
