Re: [RFC bpf-next 9/9] selftests/bpf: add selftests for indirect jumps

[Anton Protopopov <a.s.protopopov@gmail.com>]

On 25/06/18 02:59PM, Alexei Starovoitov wrote:
> On Wed, Jun 18, 2025 at 1:19 PM Anton Protopopov
> <a.s.protopopov@gmail.com> wrote:
> >
> > On 25/06/18 09:43AM, Alexei Starovoitov wrote:
> > > On Wed, Jun 18, 2025 at 9:30 AM Anton Protopopov
> > > <a.s.protopopov@gmail.com> wrote:
> > > >
> > > > On 25/06/18 09:01AM, Alexei Starovoitov wrote:
> > > > > On Wed, Jun 18, 2025 at 7:43 AM Anton Protopopov
> > > > > <a.s.protopopov@gmail.com> wrote:
> > > > > >
> > > > > > On 25/06/17 08:24PM, Alexei Starovoitov wrote:
> > > > > > > On Sun, Jun 15, 2025 at 1:55 AM Anton Protopopov
> > > > > > > <a.s.protopopov@gmail.com> wrote:
> > > > > > > > +SEC("syscall")
> > > > > > > > +int two_towers(struct simple_ctx *ctx)
> > > > > > > > +{
> > > > > > > > +       switch (ctx->x) {
> > > > > > > >
> > > > > > >
> > > > > > > Not sure why you went with switch() statements everywhere.
> > > > > > > Please add few tests with explicit indirect goto
> > > > > > > like interpreter does: goto *jumptable[insn->code];
> > > > > >
> > > > > > This requires to patch libbpf a bit more, as some meta-info
> > > > > > accompanying this instruction should be emitted, like LLVM does with
> > > > > > jump_table_sizes. And this probably should be a different section,
> > > > > > such that it doesn't conflict with LLVM/GCC. I thought to add this
> > > > > > later, but will try to add to the next version.
> > > > >
> > > > > Hmm. I'm not sure why llvm should handle explicit indirect goto
> > > > > any different than the one generated from switch.
> > > > > The generated bpf.o should be the same.
> > > >
> > > > For a switch statement LLVM will create a jump table
> > > > and create the {,.rel}.llvm_jump_table_sizes tables.
> > > >
> > > > For a direct goto *, say
> > > >
> > > >     static const void *table[] = {
> > > >             &&l1, &&l2, &&l3, &&l4, &&l5,
> > > >     };
> > > >     if (index > ARRAY_SIZE(table))
> > > >             return 0;
> > > >     goto *table[index];
> > > >
> > > > it will not generate {,.rel}.llvm_jump_table_sizes. I wonder, does
> > > > LLVM emit the size of `table`? (If no, then some assembly needed to
> > > > emit it.) In any case it should be easy to add this case, but still
> > > > it is a bit of coding, thus a bit different case.)
> > >
> > > It's controlled by -emit-jump-table-sizes-section flag.
> > > I haven't looked at pending llvm/bpf diff, but it should be possible
> > > to standardize. Emit it for both or for none.
> > > My preference would be for _none_.
> > >
> > > Not sure why you made libbpf rely on that section name.
> > > Relocations against text can be in other rodata sections.
> > > Normal behavior for x86 and other backends.
> >
> > So, those sections are just an easier way to find jump table sizes.
> > The other way is as was described by Yonghong in [1] (parse
> > .rel.rodata, follow each symbol to its section, find offset, then
> > find each gotox instruction, map it to a load, then one can find that
> > the load is from a jump table, etc.). Just to be sure, is the latter by
> > your opinion the better way (because it doesn't depend on emitting
> > tables?)?
> >
> > Those tables are _not_ generated for the code I've listed above.
> > However, in this case I can get the size of the table directly from
> > the symtab.
> 
> Since Yonghong's diff did:
> bool BPFAsmPrinter::doInitialization(Module &M) {
> 
> EmitJumpTableSizesSection = true;
> 
> and llvm did not emit jump table for explicit 'goto *table[index]'
> I suspect it will be hard to fix.
> Meaning libbpf cannot rely on a special section name.
> So it makes sense not to force this mode in llvm
> (especially since no other backend does it) and do generic
> detection in libbpf. It will work for both explicit gotox and
> switch generated at the end.

Ok, got it, thanks for the explanation.