Re: [PATCH v10 bpf-next 08/11] libbpf: support llvm-generated indirect jumps

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

On 25/11/04 10:31AM, Eduard Zingerman wrote:
> On Mon, 2025-11-03 at 21:26 -0800, Yonghong Song wrote:
> 
> [...]
> 
> > > > +		if (subprog_idx >= 0) {
> > > > +			insn_off -= prog->subprogs[subprog_idx].sec_insn_off;
> > > > +			insn_off += prog->subprogs[subprog_idx].sub_insn_off;
> > > I'd like to reiterate my point about relocation related warnings [1]:
> > > 
> > >    > I'm seeing the following messages when rebuilding bpf_gotox using
> > >    > llvm main, where Yonghong added __BPF_FEATURE_GOTOX.
> > >    >
> > >    >     CLNG-BPF [test_progs-cpuv4] bpf_gotox.bpf.o
> > >    >     GEN-SKEL [test_progs-cpuv4] bpf_gotox.skel.h
> > >    >   libbpf: elf: skipping relo section(13) .rel.jumptables for section(6) .jumptables
> > >    >   libbpf: elf: skipping relo section(13) .rel.jumptables for section(6) .jumptables
> > > 
> > > In the context of Yonghong's reply [2].
> > > 
> > > I inserted some debug prints and confirm that these relocations are
> > > generated for basic block labels, e.g.:
> > > 
> > >    .S file corresponding to shortened bpf_gotox.c:
> > >        ...
> > >               gotox r1
> > >       .Ltmp25:
> > >       .Ltmp26:
> > >       .Ltmp27:                                # Block address taken
> > >       LBB0_5:                                 # %l1
> > >               #DEBUG_LABEL: one_jump_two_maps:l1
> > >               .loc    0 36 10 is_stmt 1               # progs/bpf_gotox.c:36:10
> > >       .Ltmp28:
> > >               w1 = *(u32 *)(r10 - 4)
> > >        ...
> > >               .section        .jumptables,"",@progbits
> > >      BPF.JT.0.0:
> > >              .quad   LBB0_5
> > > 
> > >    objdump --symbols, corresponding to same shortened bpf_gotox.c:
> > > 
> > >      Symbol table '.symtab' contains 18 entries:
> > >         Num:    Value          Size Type    Bind   Vis       Ndx Name
> > >           ...
> > >           2: 0000000000000000     0 SECTION LOCAL  DEFAULT     3 syscall
> > > 
> > >    objdump --relocations, corresponding to same shortened bpf_gotox.c:
> > > 
> > >      Relocation section '.rel.jumptables' at offset 0xde8 contains 4 entries:
> > >           Offset             Info             Type               Symbol's Value  Symbol's Name
> > >       0000000000000000  0000000200000002 R_BPF_64_ABS64         0000000000000000 syscall
> > >       ...
> > > 
> > > Here the first entry corresponds to LBB0_5 symbol, specifically:
> > > 
> > >                           Relocation type (R_BPF_64_ABS64).
> > >                               vvvvvvvv
> > >     0000000000000000  0000000200000002 R_BPF_64_ABS64         0000000000000000 syscall
> > >     ^^^^^^^^^^^^^^^^  ^^^^^^^^                                ^^^^^^^^^^^^^^^^
> > >     Offset at which   Section                                 Given that relocation type is
> > >     to apply the      index                                   R_BPF_64_ABS64, this is the value
> > >     relocation,       for 'syscall'.                          which has to be written at offset.
> > >     first jumptables                                          (See [3]).
> > >     record.
> > > 
> > > Given above, I conclude that:
> > > 
> > > - [to Anton] libbpf has to apply the relocations from .rel.jumptables
> > >    in order to assign correct the sec_insn_off for records in the jump
> > >    table.  Right now we imply that each record in the table corresponds
> > >    to a section where jump table is referenced from, but that is not
> > >    true.
> > > 
> > > - [to Yonghong] LLVM should generate a different relocation kind,
> > >    or a different "Symbol's Value", otherwise applying relocations as
> > >    instructed in [3] will lead to zeroes in the jump table:
> > >    
> > >    > In another case, R_BPF_64_ABS64 relocation type is used for normal
> > >    > 64-bit data. The actual to-be-relocated data is stored at
> > >    > r_offset and the read/write data bitsize is 64 (8 bytes). The
> > >    > relocation can be resolved with the symbol value plus implicit
> > >    > addend.
> > 
> > I think the following llvm patch
> >     https://github.com/llvm/llvm-project/pull/166301
> > should avoid jumptable relocation. The idea is originated from one of
> > Eduard's idea which put the label difference in the jump table entry.
> > Please give a try. I think it should work.
> 
> I tried Yonghong's llvm change, and it solves the issue.
> Relocations are no longer generated for .jumptables section.
> So, nothing to change on the libbpf side.

Thanks Eduard and Yonghong! It does work, I've removed the check from libbpf.

One additional change I had to do is to fix verifier_gotox not
to generate .jumptables.rel:

        SEC("socket")                                                   \
        OUTCOME                                                         \
        __naked void jump_table_ ## NAME(void)                          \
        {                                                               \
                asm volatile ("                                         \
                .pushsection .jumptables,\"\",@progbits;                \
        jt0_%=:                                                         \
-               .quad ret0_%=;                                          \
-               .quad ret1_%=;                                          \
+               .quad ret0_%= - socket;                                 \
+               .quad ret1_%= - socket;                                 \
                .size jt0_%=, 16;                                       \
                .global jt0_%=;                                         \
                .popsection;                                            \

(And alike in other subtests, of course.)

> [...]