On Tue, Nov 21, 2023 at 8:20 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Mon, 2023-11-20 at 16:22 -0800, Andrii Nakryiko wrote:
> > Similar to special handling of STACK_ZERO, when reading 1/2/4 bytes from
> > stack from slot that has register spilled into it and that register has
> > a constant value zero, preserve that zero and mark spilled register as
> > precise for that. This makes spilled const zero register and STACK_ZERO
> > cases equivalent in their behavior.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > ---
>
> Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
>
> [...]
>
> > + if (spill_cnt == size &&
> > + tnum_is_const(reg->var_off) && reg->var_off.value == 0) {
> > + __mark_reg_const_zero(&state->regs[dst_regno]);
> > + /* this IS register fill, so keep insn_flags */
> > + } else if (zero_cnt == size) {
> > + /* similarly to mark_reg_stack_read(), preserve zeroes */
> > + __mark_reg_const_zero(&state->regs[dst_regno]);
> > + insn_flags = 0; /* not restoring original register state */
>
> nit: In case if there would be v3, could you please
> leave a comment here, something like below:
>
> when check_stack_write_fixed_off() puts STACK_ZERO marks
> for writes, not aligned on register boundary, it marks source
> registers precise. Thus, additional precision propagation is
> necessary in this case and insn_flags could be cleared.
>
> or something along these lines?
>
Hm... this seems misleading, precision propagation of original
register on write is orthogonal to this. The real reason why we clear
insn_flags is because there is no *register* fill here. There might
not be any spilled register at all here, or a completely irrelevant
spilled register. This instruction is basically `r1 = 0;`, though
obfuscated as stack dereference. So from the backtracking side of
things, there is no stack access here.
> > + } else {
> > + mark_reg_unknown(env, state->regs, dst_regno);
> > + insn_flags = 0; /* not restoring original register state */
> > + }
> > }
> > state->regs[dst_regno].live |= REG_LIVE_WRITTEN;
> > } else if (dst_regno >= 0) {
>
>
>
