Re: [PATCH bpf-next v4 10/12] resolve_btfids: add --btf_link to fill in .BTF.link records

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2905 bytes --]

> resolve_btfids: add --btf_link to fill in .BTF.link records

TL;DR: Not a bug: patch_btf_link() duplicates most of patch_btfids()'s ELF
open/find-section/update code, so each --btf_link reopens and rewrites the
file; suggests a shared helper or a single ELF handle.

> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c

[ ... ]

> +static int patch_btf_link(const char *elf_path, const struct btf_link *link)
> +{

[ ... ]

> +	elf_version(EV_CURRENT);
> +	fd = open(elf_path, O_RDWR);
> +	if (fd < 0) {
> +		pr_err("FAILED to open %s: %s\n", elf_path, strerror(errno));
> +		goto out_data;
> +	}
> +	elf = elf_begin(fd, ELF_C_RDWR_MMAP, NULL);
> +	if (!elf) {
> +		pr_err("FAILED cannot create ELF descriptor: %s\n", elf_errmsg(-1));
> +		goto out_close;
> +	}
> +	elf_flagelf(elf, ELF_C_SET, ELF_F_LAYOUT);

[ ... ]

> +	if (elf_getshdrstrndx(elf, &shdrstrndx)) {
> +		pr_err("FAILED cannot get shdr str ndx\n");
> +		goto out_elf;
> +	}
> +	while ((scn = elf_nextscn(elf, scn))) {
> +		if (gelf_getshdr(scn, &sh) != &sh) {
> +			pr_err("FAILED to get section header\n");
> +			goto out_elf;
> +		}
> +		name = elf_strptr(elf, shdrstrndx, sh.sh_name);
> +		if (name && !strcmp(name, section))
> +			break;
> +	}

[ ... ]

> +	elf_flagdata(data, ELF_C_SET, ELF_F_DIRTY);
> +	if (elf_update(elf, ELF_C_WRITE) < 0) {
> +		pr_err("FAILED to update ELF file %s: %s\n", elf_path, elf_errmsg(-1));
> +		goto out_elf;
> +	}

[ ... ]

> @@ -1773,21 +1974,28 @@ int main(int argc, const char **argv)
[ ... ]
> -	if (btfids_path)
> -		return patch_btfids(btfids_path, obj.path);
> +	if (btfids_path) {
> +		err = patch_btfids(btfids_path, obj.path);
> +		for (i = 0; !err && i < btf_links.cnt; i++)
> +			err = patch_btf_link(obj.path, &btf_links.links[i]);
> +		goto out;
> +	}

This isn't a bug, but patch_btf_link() repeats most of what patch_btfids()
does just above it: elf_version(), open(O_RDWR), elf_begin(ELF_C_RDWR_MMAP),
elf_flagelf(ELF_F_LAYOUT), the elf_getshdrstrndx() plus elf_nextscn() /
gelf_getshdr() / elf_strptr() / strcmp() loop that finds a section by name,
and the elf_flagdata(ELF_F_DIRTY) / elf_update() / elf_end() / close()
teardown.  Only the section name and what gets written into data->d_buf
differ.

With the loop in main(), a --patch_btfids run with N --btf_link options
opens, maps and rewrites the same ELF file N + 1 times.

Could the two share a small helper that opens the ELF and returns the
Elf_Data of a named section, or could the .BTF.link records be written while
.BTF_ids is being patched, using the same ELF handle?


---
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

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/36938681172