On Thu, 2023-11-02 at 17:08 -0700, Andrii Nakryiko wrote: > This change doesn't seem to have any effect on selftests and production > BPF object files, but we preemptively try to make it more robust. > > First, "learn sign from signed bounds" comment is misleading, as we are > learning not just sign, but also values. > > Second, we simplify the check for determining whether entire range is > positive or negative similarly to other checks added earlier, using > appropriate u32/u64 cast and single comparisons. As explain in comments > in __reg64_deduce_bounds(), the checks are equivalent. > > Last but not least, smin/smax and s32_min/s32_max reassignment based on > min/max of both umin/umax and smin/smax (and 32-bit equivalents) is hard > to explain and justify. We are updating unsigned bounds from signed > bounds, why would we update signed bounds at the same time? This might > be correct, but it's far from obvious why and the code or comments don't > try to justify this. Given we've added a separate deduction of signed > bounds from unsigned bounds earlier, this seems at least redundant, if > not just wrong. > > In short, we remove doubtful pieces, and streamline the rest to follow > the logic and approach of the rest of reg_bounds_sync() checks. > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx> [...]