On Sat, 26 Aug 2023 at 00:25, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Tue, Aug 22, 2023 at 4:06 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Tue, Aug 22, 2023 at 3:54 PM Kumar Kartikeya Dwivedi > > <memxor@xxxxxxxxx> wrote: > > > > > > > > > I suppose we could switch to the ' if (!(LHS <op> RHS)) bpf_throw(); ' > > > sequence in C, force volatile load for LHS and __builtin_constant_p > > > for RHS to get the same behavior. Emitting these redundant checks is > > > definitely a bit weird just to emit BTF. > > > > I guess we can try > > #define bpf_assert(LHS, OP, RHS) if (!(LHS OP RHS)) bpf_throw(); > > with barrier_var(LHS) and __builtin_constant_p(RHS) and > > keep things completely in C, > > but there is no guarantee that the compiler will not convert == to !=, > > swap lhs and rhs, etc. > > Maybe we can have both asm and C style macros, then recommend C to start > > and switch to asm if things are dodgy. > > Feels like dangerous ambiguity. > > This seems similar to the issue I had with > __attribute__((cleanup(some_kfunc)))) not emitting BTF info for that > some_kfunc? See bpf_for_each(), seems like just adding > `(void)bpf_iter_##type##_destroy` makes Clang emit BTF info. > > It would be nice to have this fixed for cleanup() attribute and asm, > of course. But this is a simple work around. Good to know, this is cleaner than my solution. But I am planning to switch to the direct C approach, so it should not be needed anymore.
