On Fri, Mar 1, 2024 at 9:44 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Fri, 2024-03-01 at 09:34 -0800, Andrii Nakryiko wrote: > [...] > > > As I mentioned in offline conversation, I wonder if it's better and > > less error-prone to do linked register processing in backtrack_insn() > > not just for conditional jumps, for all instructions? Whenever we > > currently do bpf_set_reg(), we can first check if there are linked > > registers and they contain a register we are about to set precise. If > > that's the case, set all of them precise. > > > > That would make it unnecessary to have this "before BPF_X|BPF_K" > > checks, I think. > > It should not be a problem to do bt_set_equal_scalars() at the > beginning of backtrack_insn(). > Same way, I can put the after call at the end of backtrack_insn(). > Is this what you have in mind? Not exactly. It was more a proposal to change the current use of bt_set_reg() with bt_set_linked_regs(), which would take into account linked registers. And do it throughout the entire backtrack_insn(), regardless of specific instruction being backtracked. I think that would eliminate the need to have bt_set_equal_scalars() before instruction as well. > > > It might be sufficient to process just conditional jumps given today's > > use of linked registers, but is there any downside to doing it across > > all instructions? Are you worried about regression in number of states > > due to precision? Or performance? > > Changing position for bt_set_equal_scalars() calls should not affect > anything semantically at the moment. Changes to backtracking state > would be done only if some linked registers are present in 'hist' and > that would be true only for conditional jumps. > Maybe some more CPU cycles but I don't think that would be noticeable.
