On Wed, Nov 3, 2021 at 4:55 AM Lorenz Bauer <lmb@xxxxxxxxxxxxxx> wrote:
> #pragma clang loop unroll(full)
> for (int b = 1 << 10; b >= 4; b >>= 1) {
> if (start + b > end) {
> continue;
> }
>
> // If we do 8 byte reads, we have to handle overflows which is
> slower than 4 byte reads.
> for (int i = 0; i < b; i += 4) {
> csum += *(uint32_t *)(start + i);
> }
>
> start += b;
> }
> if (start + 2 <= end) {
> csum += *(uint16_t *)(start);
> start += 2;
> }
> if (start + 1 <= end) {
> csum += *(start);
> }
Thanks for flagging!
Could you craft a test case that we can use a repro and future
test case?
> fp-88=map_value fp-96=mmmmmmmm fp-104=map_value fp-112=inv fp-120=fp
...
> I've bisected the problem to commit 3e8ce29850f1 ("bpf: Prevent
> pointer mismatch in bpf_timer_init.") The commit seems unrelated to
> loop processing though (it does touch the verifier however). Either I
> got the bisection wrong or there is something subtle going on.
I stared at that commit and the example asm.
I suspect the bisect went wrong.
Could you try reverting a single
commit 354e8f1970f8 ("bpf: Support <8-byte scalar spill and refill")
?
The above fp-112=inv means that the verifier is tracking scalar spill.
That could be the reason for bounded loop logic seeing different
stack state on every iteration.
But the asm snippet doesn't have the store to stack at [fp-112]
location, so it could be a red herring.
Are you using the same llvm during bisect?
The commit 354e8f1970f8 should be harmless
(when commit f30d4968e9ae ("bpf: Do not reject when the stack read
size is different from the tracked scalar size"))
is also applied. That fix is in bpf tree only, so far.
The tracking of 8-byte spill is the most useful with the latest llvm
that was taught to use 8-byte aligned stack for such spills.
Without being able to repro it's hard to investigate much further.
