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

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

On 25/11/02 09:13PM, bot+bpf-ci@kernel.org wrote:
> 
> ```
> commit b44690c09995d34f7665c7d687b359d41a6ab79f
> Author: Anton Protopopov <a.s.protopopov@gmail.com>
> 
> libbpf: support llvm-generated indirect jumps
> 
> This commit adds support for LLVM-generated indirect jumps in BPF v4
> instruction set. It handles jump table metadata from the .jumptables
> ELF section and creates BPF_MAP_TYPE_INSN_ARRAY maps for them.
> 
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index fbe74686c..ed14090a9 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> 
> [ ... ]
> 
> > @@ -6144,6 +6192,157 @@ static void poison_kfunc_call(struct bpf_program *prog, int relo_idx,
> >  	insn->imm = POISON_CALL_KFUNC_BASE + ext_idx;
> >  }
> >
> > +static int find_jt_map(struct bpf_object *obj, struct bpf_program *prog, int sym_off)
> > +{
> 
> [ ... ]
> 
> > +static int create_jt_map(struct bpf_object *obj, struct bpf_program *prog, struct reloc_desc *relo)
> > +{
> > +	const __u32 jt_entry_size = 8;
> > +	int sym_off = relo->sym_off;
> > +	int jt_size = relo->sym_size;
> > +	__u32 max_entries = jt_size / jt_entry_size;
> > +	__u32 value_size = sizeof(struct bpf_insn_array_value);
> > +	struct bpf_insn_array_value val = {};
> > +	int subprog_idx;
> > +	int map_fd, err;
> > +	__u64 insn_off;
> > +	__u64 *jt;
> > +	__u32 i;
> > +
> > +	map_fd = find_jt_map(obj, prog, sym_off);
> > +	if (map_fd >= 0)
> > +		return map_fd;
> > +
> > +	if (sym_off % jt_entry_size) {
> > +		pr_warn("map '.jumptables': jumptable start %d should be multiple of %u\n",
> > +			sym_off, jt_entry_size);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (jt_size % jt_entry_size) {
> > +		pr_warn("map '.jumptables': jumptable size %d should be multiple of %u\n",
> > +			jt_size, jt_entry_size);
> > +		return -EINVAL;
> > +	}
> > +
> > +	map_fd = bpf_map_create(BPF_MAP_TYPE_INSN_ARRAY, ".jumptables",
> > +				4, value_size, max_entries, NULL);
> > +	if (map_fd < 0)
> > +		return map_fd;
> > +
> > +	if (!obj->jumptables_data) {
> > +		pr_warn("map '.jumptables': ELF file is missing jump table data\n");
> > +		err = -EINVAL;
> > +		goto err_close;
> > +	}
> > +	if (sym_off + jt_size > obj->jumptables_data_sz) {
> > +		pr_warn("map '.jumptables': jumptables_data size is %zd, trying to access %d\n",
> > +			obj->jumptables_data_sz, sym_off + jt_size);
> > +		err = -EINVAL;
> > +		goto err_close;
> > +	}
> > +
> > +	subprog_idx = -1; /* main program */
> > +	if (relo->insn_idx < 0 || relo->insn_idx >= prog->insns_cnt) {
> > +		pr_warn("map '.jumptables': invalid instruction index %d\n", relo->insn_idx);
> > +		err = -EINVAL;
> > +		goto err_close;
> > +	}
> > +	if (prog->subprogs)
> > +		subprog_idx = find_subprog_idx(prog, relo->insn_idx);
> > +
> > +	jt = (__u64 *)(obj->jumptables_data + sym_off);
> > +	for (i = 0; i < max_entries; i++) {
> > +		/*
> > +		 * The offset should be made to be relative to the beginning of
> > +		 * the main function, not the subfunction.
> > +		 */
> > +		insn_off = jt[i]/sizeof(struct bpf_insn);
> > +		if (subprog_idx >= 0) {
> > +			insn_off -= prog->subprogs[subprog_idx].sec_insn_off;
> > +			insn_off += prog->subprogs[subprog_idx].sub_insn_off;
> > +		} else {
> > +			insn_off -= prog->sec_insn_off;
> > +		}
> > +
> > +		/*
> > +		 * LLVM-generated jump tables contain u64 records, however
> > +		 * should contain values that fit in u32.
> > +		 */
> > +		if (insn_off > UINT32_MAX) {
> > +			pr_warn("map '.jumptables': invalid jump table value 0x%llx at offset %d\n",
> > +				(long long)jt[i], sym_off + i);
>                                                       ^^^^^^^^^^^^
> 
> Does this error message print the correct offset? Since jt is a pointer
> to __u64, the array access jt[i] is at byte offset "sym_off + i * 8",
> not "sym_off + i". All the other error messages in create_jt_map report
> byte offsets and sizes (sym_off, jt_size, sym_off + jt_size), so this
> one should probably be "sym_off + i * jt_entry_size" for consistency.

Is there a way to run this AI as part of any PR to
kernel-patches/bpf, not only those coming from the mailing list?
Maybe for a selected commit?

Also, how deterministinc it is?  Will it generate different comments
for a given patch for different runs?

> > +			err = -EINVAL;
> > +			goto err_close;
> > +		}
> > +
> > +		val.orig_off = insn_off;
> > +		err = bpf_map_update_elem(map_fd, &i, &val, 0);
> > +		if (err)
> > +			goto err_close;
> > +	}
> > +
> > +	err = bpf_map_freeze(map_fd);
> > +	if (err)
> > +		goto err_close;
> > +
> > +	err = add_jt_map(obj, prog, sym_off, map_fd);
> > +	if (err)
> > +		goto err_close;
> > +
> > +	return map_fd;
> > +
> > +err_close:
> > +	close(map_fd);
> > +	return err;
> > +}
> 
> [ ... ]
> 
> 
> ```
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> In-Reply-To-Subject: `libbpf: support llvm-generated indirect jumps`
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19018051915