On Thu, 2025-01-30 at 13:23 +0200, Dimitar Kanaliev wrote: [...] > static void coerce_reg_to_size_sx(struct bpf_reg_state *reg, int size) > { > - s64 init_s64_max, init_s64_min, s64_max, s64_min, u64_cval; > - u64 top_smax_value, top_smin_value; > - u64 num_bits = size * 8; > + u64 s = size * 8 - 1; > + u64 sign_mask = 1ULL << s; > + s64 smin_value, smax_value; > + u64 umax_value; > > - if (tnum_is_const(reg->var_off)) { > - u64_cval = reg->var_off.value; > - if (size == 1) > - reg->var_off = tnum_const((s8)u64_cval); > - else if (size == 2) > - reg->var_off = tnum_const((s16)u64_cval); > - else > - /* size == 4 */ > - reg->var_off = tnum_const((s32)u64_cval); > - > - u64_cval = reg->var_off.value; > - reg->smax_value = reg->smin_value = u64_cval; > - reg->umax_value = reg->umin_value = u64_cval; > - reg->s32_max_value = reg->s32_min_value = u64_cval; > - reg->u32_max_value = reg->u32_min_value = u64_cval; > + if (size >= 8) > return; > - } > > - top_smax_value = ((u64)reg->smax_value >> num_bits) << num_bits; > - top_smin_value = ((u64)reg->smin_value >> num_bits) << num_bits; > + reg->var_off = tnum_scast(reg->var_off, size); > > - if (top_smax_value != top_smin_value) > - goto out; > - > - /* find the s64_min and s64_min after sign extension */ > - if (size == 1) { > - init_s64_max = (s8)reg->smax_value; > - init_s64_min = (s8)reg->smin_value; > - } else if (size == 2) { > - init_s64_max = (s16)reg->smax_value; > - init_s64_min = (s16)reg->smin_value; > + if (reg->var_off.mask & sign_mask) { > + smin_value = -(1LL << s); > + smax_value = (1LL << s) - 1; > } else { > - init_s64_max = (s32)reg->smax_value; > - init_s64_min = (s32)reg->smin_value; > + smin_value = (s64)(reg->var_off.value); > + smax_value = (s64)(reg->var_off.value | reg->var_off.mask); Note the following code in __update_reg64_bounds(): static void __update_reg64_bounds(struct bpf_reg_state *reg) { /* min signed is max(sign bit) | min(other bits) */ reg->smin_value = max_t(s64, reg->smin_value, reg->var_off.value | (reg->var_off.mask & S64_MIN)); /* max signed is min(sign bit) | max(other bits) */ reg->smax_value = min_t(s64, reg->smax_value, reg->var_off.value | (reg->var_off.mask & S64_MAX)); reg->umin_value = max(reg->umin_value, reg->var_off.value); reg->umax_value = min(reg->umax_value, reg->var_off.value | reg->var_off.mask); } Is it possible to set {u,s}min/{u,s}max to {U,S}64_MIN/{U,S}64_MAX and rely on __update_reg64_bounds() for this computation? > } > > - s64_max = max(init_s64_max, init_s64_min); > - s64_min = min(init_s64_max, init_s64_min); > + reg->smin_value = smin_value; > + reg->smax_value = smax_value; > > - /* both of s64_max/s64_min positive or negative */ > - if ((s64_max >= 0) == (s64_min >= 0)) { > - reg->s32_min_value = reg->smin_value = s64_min; > - reg->s32_max_value = reg->smax_value = s64_max; > - reg->u32_min_value = reg->umin_value = s64_min; > - reg->u32_max_value = reg->umax_value = s64_max; > - reg->var_off = tnum_range(s64_min, s64_max); > - return; > - } > + reg->umin_value = reg->var_off.value; > + umax_value = reg->var_off.value | reg->var_off.mask; > + reg->umax_value = umax_value; > > -out: > - set_sext64_default_val(reg, size); After this commit the functions set_sext64_default_val() and set_sext32_default_val() are never called. > + reg->s32_min_value = (s32)smin_value; > + reg->s32_max_value = (s32)smax_value; > + reg->u32_min_value = (u32)reg->umin_value; > + reg->u32_max_value = (u32)umax_value; > } [...]