On Sun, 2023-11-19 at 18:09 -0800, Alexei Starovoitov wrote:
[...]
> > + register unsigned i asm("r2");
> > + register __u8 *p asm("r1");
>
> I suspect this is fragile.
> The compiler will use r2 for 'i' if 'i' is actually there,
> but if it can optimize 'i' and 'p' away the r1 and r2 may be used
> for something else.
> The "register" keyword is not mandatory. Unlike "volatile".
[...]
> > + if (a != 0 && a != 1 && a != 11 && a != 101 && a != 111 &&
> > + b != 0 && b != 1 && b != 11 && b != 101 && b != 111)
> > + asm volatile ("r0 /= 0;" ::: "r0");
> > + /* Instruction for match in __msg spec. */
> > + asm volatile ("*(u8 *)(r1 + 0) = r2;" :: "r"(p), "r"(i) : "memory");
>
> Feels even more fragile. Not sure what gcc will do.
> Can 'i' be checked as run-time value ?
> If it passes the verifier and after bpf_prog_run the 'i' is equal
> to expected value we're good, no?
Runtime check should work, thank you for this suggestion.
I'll remove unnecessary 'asm' blocks and use __retval instead
('r0 /= 0' will remain).
