On Tue, 2025-02-11 at 16:49 -0800, Stephen Brennan wrote:
> In order to handle duplicate variables in a data section, we'll need to
> compare variables against the others in their data section. To do this,
> we'll need to postpone adding the variables until they have all been
> identified and collected. Store all the necessary data to encode the
> VARs (and their DECL_TAGs) and then encode them just before the DATASEC
> containing them. No meaningful change in the output is intended, though
> the ordering of type IDs generated will likely differ.
>
> Signed-off-by: Stephen Brennan <stephen.s.brennan@xxxxxxxxxx>
> ---
I think this patch looks good,
but annotation index handling needs a small a fix,
see below.
Reviewed-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
[...]
> @@ -109,6 +109,20 @@ struct elf_functions {
> int suffix_cnt; /* number of .isra, .part etc */
> };
>
> +struct saved_annot {
> + const char *value;
> + int component_idx;
> +};
> +
> +struct saved_var {
> + const char *name;
> + uint32_t type;
Nit: maybe name this 'id' or 'var_id'?
> + struct saved_annot *annots;
> + uint32_t linkage;
> + uint32_t offset;
> + uint32_t size;
> +};
> +
[...]
> @@ -2359,30 +2428,34 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
> name, cu->name, addr);
> }
>
> - /* add a BTF_KIND_VAR in encoder->types */
> - id = btf_encoder__add_var(encoder, type, name, linkage);
> - if (id < 0) {
> - fprintf(stderr, "error: failed to encode variable '%s' at addr 0x%" PRIx64 "\n",
> - name, addr);
> - goto out;
> + /* Save the annotations */
> + annot_count = 0;
> + annots = NULL;
> + list_for_each_entry(annot, &var->annots, node) {
> + annot_count += 1;
> }
> + if (annot_count) {
> + int idx = 0;
>
> - list_for_each_entry(annot, &var->annots, node) {
> - int tag_type_id = btf_encoder__add_decl_tag(encoder, annot->value, id, annot->component_idx);
> - if (tag_type_id < 0) {
> - fprintf(stderr, "error: failed to encode tag '%s' to variable '%s' with component_idx %d\n",
> - annot->value, name, annot->component_idx);
> + annots = calloc(annot_count + 1, sizeof(*annots));
> + if (!annots) {
> + fprintf(stderr, "error: allocation failure\n");
> + err = -ENOMEM;
> goto out;
> }
> + list_for_each_entry(annot, &var->annots, node) {
> + annots[idx].value = annot->value;
> + annots[idx].component_idx = annot->component_idx;
> + }
I think 'idx++' is missing here.
> }
>
> /*
> - * Add the variable to the secinfo for the section it appears in.
> - * Later we will generate a BTF_VAR_DATASEC for all any section with
> - * an encoded variable.
> + * Store all the collected information for the variable. Later,
> + * we will deduplicate and output all.
> */
> - id = btf_encoder__add_var_secinfo(encoder, shndx, id, (uint32_t)addr, (uint32_t)size);
> - if (id < 0) {
> + err = btf_encoder__store_var(encoder, shndx, name, type, linkage,
> + (uint32_t)addr, (uint32_t)size, annots);
> + if (err < 0) {
> fprintf(stderr, "error: failed to encode section info for variable '%s' at addr 0x%" PRIx64 "\n",
> name, addr);
Free 'annots' in case of an error?
> goto out;
[...]
