On Fri, 2023-11-10 at 08:10 -0800, Andrii Nakryiko wrote:
> Instead of always printing numbers as either decimals (and in some
> cases, like for "imm=%llx", in hexadecimals), decide the form based on
> actual values. For numbers in a reasonably small range (currently,
> [0, U16_MAX] for unsigned values, and [S16_MIN, S16_MAX] for signed ones),
> emit them as decimals. In all other cases, even for signed values,
> emit them in hexadecimals.
>
> For large values hex form is often times way more useful: it's easier to
> see an exact difference between 0xffffffff80000000 and 0xffffffff7fffffff,
> than between 18446744071562067966 and 18446744071562067967, as one
> particular example.
>
> Small values representing small pointer offsets or application
> constants, on the other hand, are way more useful to be represented in
> decimal notation.
>
> Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
[...]
> @@ -576,14 +627,14 @@ static void print_reg_state(struct bpf_verifier_env *env, const struct bpf_reg_s
> tnum_is_const(reg->var_off)) {
> /* reg->off should be 0 for SCALAR_VALUE */
> verbose(env, "%s", t == SCALAR_VALUE ? "" : reg_type_str(env, t));
> - verbose(env, "%lld", reg->var_off.value + reg->off);
> + verbose_snum(env, reg->var_off.value + reg->off);
> return;
> }
> /*
> * _a stands for append, was shortened to avoid multiline statements below.
> * This macro is used to output a comma separated list of attributes.
> */
> -#define verbose_a(fmt, ...) ({ verbose(env, "%s" fmt, sep, __VA_ARGS__); sep = ","; })
> +#define verbose_a(fmt, ...) ({ verbose(env, "%s" fmt, sep, ##__VA_ARGS__); sep = ","; })
>
> verbose(env, "%s", reg_type_str(env, t));
> if (base_type(t) == PTR_TO_BTF_ID)
> @@ -608,8 +659,10 @@ static void print_reg_state(struct bpf_verifier_env *env, const struct bpf_reg_s
> verbose_a("r=%d", reg->range);
> if (tnum_is_const(reg->var_off)) {
> /* a pointer register with fixed offset */
> - if (reg->var_off.value)
> - verbose_a("imm=%llx", reg->var_off.value);
> + if (reg->var_off.value) {
> + verbose_a("imm=");
> + verbose_snum(env, reg->var_off.value);
> + }
Maybe use same verbose_{u,s}num() for reg->off and reg->range in this
function?
