> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 9449f6f50f18..30f8c426d145 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -2147,16 +2147,9 @@ int btf__find_str(struct btf *btf, const char *s)
> * - > 0 offset into string section, on success;
> * - < 0, on error.
> */
> -int btf__add_str(struct btf *btf, const char *s)
> +static int btf_add_local_str(struct btf *btf, const char *s)
> {
> - int off;
> - int err;
> -
> - if (btf->base_btf) {
> - off = btf__find_str(btf->base_btf, s);
> - if (off != -ENOENT)
> - return off;
> - }
> + int off, err;
>
> err = btf_ensure_modifiable(btf);
> if (err)
> @@ -2167,10 +2160,24 @@ int btf__add_str(struct btf *btf, const char *s)
> return libbpf_err(off);
>
> btf->hdr.str_len = strset__data_size(btf->strs_set);
> + btf->strs_deduped = false;
The refactoring left the API doc comment attached to the new static
helper btf_add_local_str(), so the public btf__add_str() (btf.c:2168) now
has no documentation at all, and its declaration at tools/lib/bpf/btf.h:202
is bare without doxygen. The comment is also no longer descriptive of what
it now documents: the distinguishing property of btf_add_local_str() versus
btf__add_str() is that it always adds to this BTF's own string section and
never reuses a string already present in base_btf, which is exactly what
makes the new transfer mode able to keep split-only strings out of the base.
Should the comment be moved back onto btf__add_str() and btf_add_local_str()
given a one-liner about bypassing the base string table?
The new btf->strs_deduped = false sits in btf_add_local_str(), which is on
the path of every public btf__add_str() call (and therefore of btf__add_int,
btf__add_var, btf__add_field, btf__add_enum_value, btf__add_func_param,
btf__add_datasec, btf__add_locsec, btf_add_composite, btf_add_decl_tag,
btf_add_ref_kind, btf_add_enum_common, and more), not just the new transfer
path.
Before this commit nothing in tools/lib/bpf ever cleared strs_deduped. The
complete set of writers post-commit is: btf_ensure_modifiable() sets it true
when hdr.str_len == 0 or when !base_btf && str_len == 1, end of
btf_dedup_strings() sets it true, plus the two new clears here and in
btf_compact_strings().
For BTF built from scratch, btf__new_empty() (str_len == 1) and
btf__new_empty_split() (str_len == 0) both make btf_ensure_modifiable() set
strs_deduped = true, so pre-commit btf_dedup_strings() returned 0 at its
early-return check and the string table was never rebuilt. Post-commit the
first added string clears the flag, so btf__dedup() now always rebuilds the
string table for such BTF: strings are reordered and unreferenced strings
are dropped. Any offset a caller previously obtained from the public
btf__add_str() and stashed outside a BTF type's name field is therefore
silently invalidated across btf__dedup().
This is not needed by the commit: both btf_compact_strings() call sites
already force btf->strs_deduped = false themselves at btf.c:4536, so the
transfer path does not depend on btf_add_local_str() clearing it.
All in-tree consumers were checked and are safe: bpf_linker's
finalize_btf() (tools/lib/bpf/linker.c:2860) runs finalize_btf_ext() first
and then passes opts.btf_ext = linker->btf_ext to btf__dedup(), so the
offsets produced at linker.c:1968, 2697, 2703, 2740 and 2952 all get
remapped; bpftool merge_btf_files() (tools/bpf/bpftool/btf.c:880) and
resolve_btfids finalize_btf() (tools/bpf/resolve_btfids/main.c:1578) hold no
external string offsets. So there is no in-tree breakage, but this is an
unadvertised behaviour change to a public API plus a full string-table
rebuild on every static link, and the commit message does not mention
btf__add_str() or strs_deduped at all.
Is the clearing of strs_deduped here intentional for non-transfer use of
btf__add_str(), or should it only be cleared in btf_compact_strings() where
the transfer path already sets it?
>
> return btf->start_str_off + off;
> }
>
> +int btf__add_str(struct btf *btf, const char *s)
> +{
> + int off;
> +
> + if (btf->base_btf) {
> + off = btf__find_str(btf->base_btf, s);
> + if (off != -ENOENT)
> + return off;
> + }
> +
> + return btf_add_local_str(btf, s);
> +}
[ ... ]
---
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/33537080133
