[PATCH dwarves v4 0/4] Emit global variables in BTF

[PATCH dwarves v4 0/4] Emit global variables in BTF

[Stephen Brennan]

Hi all,

This is v4 of the series which adds global variables to pahole's generated BTF.

Since v3:

1. Gathered Alan's Reviewed-by + Tested-by, and Jiri's Acked-by.
2. Consistently start shndx loops at 1, and use size_t.
3. Since patch 1 of v3 was already applied, I dropped it out of this series.

v3: https://lore.kernel.org/dwarves/20241002235253.487251-1-stephen.s.brennan@oracle.com/
v2: https://lore.kernel.org/dwarves/20240920081903.13473-1-stephen.s.brennan@oracle.com/
v1: https://lore.kernel.org/dwarves/20240912190827.230176-1-stephen.s.brennan@oracle.com/

Thanks everyone for your review, tests, and consideration!
Stephen

Stephen Brennan (4):
  btf_encoder: stop indexing symbols for VARs
  btf_encoder: explicitly check addr/size for u32 overflow
  btf_encoder: allow encoding VARs from many sections
  pahole: add global_var BTF feature

 btf_encoder.c      | 340 +++++++++++++++++++++------------------------
 btf_encoder.h      |   1 +
 dwarves.h          |   1 +
 man-pages/pahole.1 |   7 +-
 pahole.c           |   3 +-
 5 files changed, 167 insertions(+), 185 deletions(-)

-- 
2.43.5

[PATCH dwarves v4 1/4] btf_encoder: stop indexing symbols for VARs

[Stephen Brennan]

Currently we index symbols from the percpu ELF section, and when
processing DWARF variables for inclusion, we check whether the variable
matches an existing symbol. The matched symbol is used for three
purposes:

1. When no symbol of the same address is found, the variable is skipped.
   This can occur because the symbol name was an invalid BTF
   identifier, and so it did not get indexed. Or more commonly, it can
   be because the variable is not stored in the per-cpu section, and
   thus was not indexed.
2. If the symbol offset is 0, then we compare the DWARF variable's name
   against the symbol name to filter out "special" DWARF variables.
3. We use the symbol size in the DATASEC entry for the variable.

For 1, we don't need the symbol table: we can simply check the DWARF
variable name directly, and we can use the variable address to determine
the ELF section it is contained in. For 3, we also don't need the symbol
table: we can use the variable's size information from DWARF. Issue 2 is
more complicated, but thanks to the addition of the "artificial" and
"top_level" flags, many of the "special" DWARF variables can be directly
filtered out, and the few remaining problematic variables can be
filtered by name from a kernel-specific list of patterns.

This allows the symbol table index to be removed. The benefit of
removing this index is twofold. First, handling variable addresses is
simplified, since we don't need to know whether the file is ET_REL.
Second, this will make it easier to output variables that aren't just
percpu, since we won't need to index variables from all ELF sections.

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---
 btf_encoder.c | 253 ++++++++++++++++++++------------------------------
 1 file changed, 99 insertions(+), 154 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 652a945..61e9ece 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -93,16 +93,11 @@ struct elf_function {
 	struct btf_encoder_func_state state;
 };
 
-struct var_info {
-	uint64_t    addr;
-	const char *name;
-	uint32_t    sz;
-};
-
 struct elf_secinfo {
 	uint64_t    addr;
 	const char *name;
 	uint64_t    sz;
+	uint32_t    type;
 };
 
 /*
@@ -125,17 +120,11 @@ struct btf_encoder {
 			  gen_floats,
 			  skip_encoding_decl_tag,
 			  tag_kfuncs,
-			  is_rel,
 			  gen_distilled_base;
 	uint32_t	  array_index_id;
 	struct elf_secinfo *secinfo;
 	size_t             seccnt;
-	struct {
-		struct var_info *vars;
-		int		var_cnt;
-		int		allocated;
-		uint32_t	shndx;
-	} percpu;
+	size_t             percpu_shndx;
 	int                encode_vars;
 	struct {
 		struct elf_function *entries;
@@ -2098,111 +2087,18 @@ int btf_encoder__encode(struct btf_encoder *encoder)
 	return err;
 }
 
-static int percpu_var_cmp(const void *_a, const void *_b)
-{
-	const struct var_info *a = _a;
-	const struct var_info *b = _b;
-
-	if (a->addr == b->addr)
-		return 0;
-	return a->addr < b->addr ? -1 : 1;
-}
-
-static bool btf_encoder__percpu_var_exists(struct btf_encoder *encoder, uint64_t addr, uint32_t *sz, const char **name)
-{
-	struct var_info key = { .addr = addr };
-	const struct var_info *p = bsearch(&key, encoder->percpu.vars, encoder->percpu.var_cnt,
-					   sizeof(encoder->percpu.vars[0]), percpu_var_cmp);
-	if (!p)
-		return false;
-
-	*sz = p->sz;
-	*name = p->name;
-	return true;
-}
-
-static int btf_encoder__collect_percpu_var(struct btf_encoder *encoder, GElf_Sym *sym, size_t sym_sec_idx)
-{
-	const char *sym_name;
-	uint64_t addr;
-	uint32_t size;
-
-	/* compare a symbol's shndx to determine if it's a percpu variable */
-	if (sym_sec_idx != encoder->percpu.shndx)
-		return 0;
-	if (elf_sym__type(sym) != STT_OBJECT)
-		return 0;
-
-	addr = elf_sym__value(sym);
 
-	size = elf_sym__size(sym);
-	if (!size)
-		return 0; /* ignore zero-sized symbols */
-
-	sym_name = elf_sym__name(sym, encoder->symtab);
-	if (!btf_name_valid(sym_name)) {
-		dump_invalid_symbol("Found symbol of invalid name when encoding btf",
-				    sym_name, encoder->verbose, encoder->force);
-		if (encoder->force)
-			return 0;
-		return -1;
-	}
-
-	if (encoder->verbose)
-		printf("Found per-CPU symbol '%s' at address 0x%" PRIx64 "\n", sym_name, addr);
-
-	/* Make sure addr is section-relative. For kernel modules (which are
-	 * ET_REL files) this is already the case. For vmlinux (which is an
-	 * ET_EXEC file) we need to subtract the section address.
-	 */
-	if (!encoder->is_rel)
-		addr -= encoder->secinfo[encoder->percpu.shndx].addr;
-
-	if (encoder->percpu.var_cnt == encoder->percpu.allocated) {
-		struct var_info *new;
-
-		new = reallocarray_grow(encoder->percpu.vars,
-					&encoder->percpu.allocated,
-					sizeof(*encoder->percpu.vars));
-		if (!new) {
-			fprintf(stderr, "Failed to allocate memory for variables\n");
-			return -1;
-		}
-		encoder->percpu.vars = new;
-	}
-	encoder->percpu.vars[encoder->percpu.var_cnt].addr = addr;
-	encoder->percpu.vars[encoder->percpu.var_cnt].sz = size;
-	encoder->percpu.vars[encoder->percpu.var_cnt].name = sym_name;
-	encoder->percpu.var_cnt++;
-
-	return 0;
-}
-
-static int btf_encoder__collect_symbols(struct btf_encoder *encoder, bool collect_percpu_vars)
+static int btf_encoder__collect_symbols(struct btf_encoder *encoder)
 {
-	Elf32_Word sym_sec_idx;
+	uint32_t sym_sec_idx;
 	uint32_t core_id;
 	GElf_Sym sym;
 
-	/* cache variables' addresses, preparing for searching in symtab. */
-	encoder->percpu.var_cnt = 0;
-
-	/* search within symtab for percpu variables */
 	elf_symtab__for_each_symbol_index(encoder->symtab, core_id, sym, sym_sec_idx) {
-		if (collect_percpu_vars && btf_encoder__collect_percpu_var(encoder, &sym, sym_sec_idx))
-			return -1;
 		if (btf_encoder__collect_function(encoder, &sym))
 			return -1;
 	}
 
-	if (collect_percpu_vars) {
-		if (encoder->percpu.var_cnt)
-			qsort(encoder->percpu.vars, encoder->percpu.var_cnt, sizeof(encoder->percpu.vars[0]), percpu_var_cmp);
-
-		if (encoder->verbose)
-			printf("Found %d per-CPU variables!\n", encoder->percpu.var_cnt);
-	}
-
 	if (encoder->functions.cnt) {
 		qsort(encoder->functions.entries, encoder->functions.cnt, sizeof(encoder->functions.entries[0]),
 		      functions_cmp);
@@ -2224,15 +2120,57 @@ static bool ftype__has_arg_names(const struct ftype *ftype)
 	return true;
 }
 
+static size_t get_elf_section(struct btf_encoder *encoder, uint64_t addr)
+{
+	/* Start at index 1 to ignore initial SHT_NULL section */
+	for (size_t i = 1; i < encoder->seccnt; i++) {
+		/* Variables are only present in PROGBITS or NOBITS (.bss) */
+		if (encoder->secinfo[i].type == SHT_PROGBITS ||
+		    encoder->secinfo[i].type == SHT_NOBITS)
+			continue;
+
+		if (encoder->secinfo[i].addr <= addr &&
+		    (addr - encoder->secinfo[i].addr) < encoder->secinfo[i].sz)
+			return i;
+	}
+	return 0;
+}
+
+/*
+ * Filter out variables / symbol names with common prefixes and no useful
+ * values. Prefixes should be added sparingly, and it should be objectively
+ * obvious that they are not useful.
+ */
+static bool filter_variable_name(const char *name)
+{
+	static const struct { char *s; size_t len; } skip[] = {
+		#define X(str) {str, sizeof(str) - 1}
+		X("__UNIQUE_ID"),
+		X("__tpstrtab_"),
+		X("__exitcall_"),
+		X("__func_stack_frame_non_standard_")
+		#undef X
+	};
+	int i;
+
+	if (*name != '_')
+		return false;
+
+	for (i = 0; i < ARRAY_SIZE(skip); i++) {
+		if (strncmp(name, skip[i].s, skip[i].len) == 0)
+			return true;
+	}
+	return false;
+}
+
 static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 {
 	struct cu *cu = encoder->cu;
 	uint32_t core_id;
 	struct tag *pos;
 	int err = -1;
-	struct elf_secinfo *pcpu_scn = &encoder->secinfo[encoder->percpu.shndx];
 
-	if (encoder->percpu.shndx == 0 || !encoder->symtab)
+	if (encoder->percpu_shndx == 0 || !encoder->symtab)
 		return 0;
 
 	if (encoder->verbose)
@@ -2240,59 +2178,69 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 
 	cu__for_each_variable(cu, core_id, pos) {
 		struct variable *var = tag__variable(pos);
-		uint32_t size, type, linkage;
-		const char *name, *dwarf_name;
+		uint32_t type, linkage;
+		const char *name;
 		struct llvm_annotation *annot;
 		const struct tag *tag;
+		size_t shndx, size;
 		uint64_t addr;
 		int id;
 
+		/* Skip incomplete (non-defining) declarations */
 		if (var->declaration && !var->spec)
 			continue;
 
-		/* percpu variables are allocated in global space */
-		if (variable__scope(var) != VSCOPE_GLOBAL && !var->spec)
+		/*
+		 * top_level: indicates that the variable is declared at the top
+		 *   level of the CU, and thus it is globally scoped.
+		 * artificial: indicates that the variable is a compiler-generated
+		 *   "fake" variable that doesn't appear in the source.
+		 * scope: set by pahole to indicate the type of storage the
+		 *   variable has. GLOBAL indicates it is stored in static
+		 *   memory (as opposed to a stack variable or register)
+		 *
+		 * Some variables are "top_level" but not GLOBAL:
+		 *   e.g. current_stack_pointer, which is a register variable,
+		 *   despite having global CU-declarations. We don't want that,
+		 *   since no code could actually find this variable.
+		 * Some variables are GLOBAL but not top_level:
+		 *   e.g. function static variables
+		 */
+		if (!var->top_level || var->artificial || var->scope != VSCOPE_GLOBAL)
 			continue;
 
 		/* addr has to be recorded before we follow spec */
 		addr = var->ip.addr;
-		dwarf_name = variable__name(var);
 
-		/* Make sure addr is section-relative. DWARF, unlike ELF,
-		 * always contains virtual symbol addresses, so subtract
-		 * the section address unconditionally.
-		 */
-		if (addr < pcpu_scn->addr || addr >= pcpu_scn->addr + pcpu_scn->sz)
+		/* Get the ELF section info for the variable */
+		shndx = get_elf_section(encoder, addr);
+		if (shndx != encoder->percpu_shndx)
 			continue;
-		addr -= pcpu_scn->addr;
 
-		if (!btf_encoder__percpu_var_exists(encoder, addr, &size, &name))
-			continue; /* not a per-CPU variable */
+		/* Convert addr to section relative */
+		addr -= encoder->secinfo[shndx].addr;
 
-		/* A lot of "special" DWARF variables (e.g, __UNIQUE_ID___xxx)
-		 * have addr == 0, which is the same as, say, valid
-		 * fixed_percpu_data per-CPU variable. To distinguish between
-		 * them, additionally compare DWARF and ELF symbol names. If
-		 * DWARF doesn't provide proper name, pessimistically assume
-		 * bad variable.
-		 *
-		 * Examples of such special variables are:
-		 *
-		 *  1. __ADDRESSABLE(sym), which are forcely emitted as symbols.
-		 *  2. __UNIQUE_ID(prefix), which are introduced to generate unique ids.
-		 *  3. __exitcall(fn), functions which are labeled as exit calls.
-		 *
-		 *  This is relevant only for vmlinux image, as for kernel
-		 *  modules per-CPU data section has non-zero offset so all
-		 *  per-CPU symbols have non-zero values.
-		 */
-		if (var->ip.addr == 0) {
-			if (!dwarf_name || strcmp(dwarf_name, name))
+		/* DWARF specification reference should be followed, because
+		 * information like the name & type may not be present on var */
+		if (var->spec)
+			var = var->spec;
+
+		name = variable__name(var);
+		if (!name)
+			continue;
+
+		/* Check for invalid BTF names */
+		if (!btf_name_valid(name)) {
+			dump_invalid_symbol("Found invalid variable name when encoding btf",
+					    name, encoder->verbose, encoder->force);
+			if (encoder->force)
 				continue;
+			else
+				return -1;
 		}
 
-		if (var->spec)
-			var = var->spec;
+		if (filter_variable_name(name))
+			continue;
 
 		if (var->ip.tag.type == 0) {
 			fprintf(stderr, "error: found variable '%s' in CU '%s' that has void type\n",
@@ -2304,9 +2252,10 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 		}
 
 		tag = cu__type(cu, var->ip.tag.type);
-		if (tag__size(tag, cu) == 0) {
+		size = tag__size(tag, cu);
+		if (size == 0) {
 			if (encoder->verbose)
-				fprintf(stderr, "Ignoring zero-sized per-CPU variable '%s'...\n", dwarf_name ?: "<missing name>");
+				fprintf(stderr, "Ignoring zero-sized per-CPU variable '%s'...\n", name);
 			continue;
 		}
 
@@ -2388,8 +2337,6 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 			goto out_delete;
 		}
 
-		encoder->is_rel = ehdr.e_type == ET_REL;
-
 		switch (ehdr.e_ident[EI_DATA]) {
 		case ELFDATA2LSB:
 			btf__set_endianness(encoder->btf, BTF_LITTLE_ENDIAN);
@@ -2430,15 +2377,16 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 			encoder->secinfo[shndx].addr = shdr.sh_addr;
 			encoder->secinfo[shndx].sz = shdr.sh_size;
 			encoder->secinfo[shndx].name = secname;
+			encoder->secinfo[shndx].type = shdr.sh_type;
 
 			if (strcmp(secname, PERCPU_SECTION) == 0)
-				encoder->percpu.shndx = shndx;
+				encoder->percpu_shndx = shndx;
 		}
 
-		if (!encoder->percpu.shndx && encoder->verbose)
+		if (!encoder->percpu_shndx && encoder->verbose)
 			printf("%s: '%s' doesn't have '%s' section\n", __func__, cu->filename, PERCPU_SECTION);
 
-		if (btf_encoder__collect_symbols(encoder, encoder->encode_vars & BTF_VAR_PERCPU))
+		if (btf_encoder__collect_symbols(encoder))
 			goto out_delete;
 
 		if (encoder->verbose)
@@ -2480,9 +2428,6 @@ void btf_encoder__delete(struct btf_encoder *encoder)
 	encoder->functions.allocated = encoder->functions.cnt = 0;
 	free(encoder->functions.entries);
 	encoder->functions.entries = NULL;
-	encoder->percpu.allocated = encoder->percpu.var_cnt = 0;
-	free(encoder->percpu.vars);
-	encoder->percpu.vars = NULL;
 
 	free(encoder);
 }
-- 
2.43.5

[PATCH dwarves v4 2/4] btf_encoder: explicitly check addr/size for u32 overflow

[Stephen Brennan]

The addr is a uint64_t, and depending on the size of a data section,
there's no guarantee that it fits into a uint32_t, even after
subtracting out the section start address. Similarly, the variable size
is a size_t which could exceed a uint32_t. Check both for overflow, and
if found, skip the variable with an error message. Use explicit casts
when we cast to uint32_t so it's plain to see that this is happening.

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---
 btf_encoder.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 61e9ece..5586cd8 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -2253,9 +2253,16 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 
 		tag = cu__type(cu, var->ip.tag.type);
 		size = tag__size(tag, cu);
-		if (size == 0) {
+		if (size == 0 || size > UINT32_MAX) {
 			if (encoder->verbose)
-				fprintf(stderr, "Ignoring zero-sized per-CPU variable '%s'...\n", name);
+				fprintf(stderr, "Ignoring %s-sized per-CPU variable '%s'...\n",
+					size == 0 ? "zero" : "over", name);
+			continue;
+		}
+		if (addr > UINT32_MAX) {
+			if (encoder->verbose)
+				fprintf(stderr, "Ignoring variable '%s' - its offset %zu doesn't fit in a u32\n",
+					name, addr);
 			continue;
 		}
 
@@ -2288,7 +2295,7 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 		 * add a BTF_VAR_SECINFO in encoder->percpu_secinfo, which will be added into
 		 * encoder->types later when we add BTF_VAR_DATASEC.
 		 */
-		id = btf_encoder__add_var_secinfo(encoder, id, addr, size);
+		id = btf_encoder__add_var_secinfo(encoder, id, (uint32_t)addr, (uint32_t)size);
 		if (id < 0) {
 			fprintf(stderr, "error: failed to encode section info for variable '%s' at addr 0x%" PRIx64 "\n",
 			        name, addr);
-- 
2.43.5

[PATCH dwarves v4 3/4] btf_encoder: allow encoding VARs from many sections

[Stephen Brennan]

Currently we maintain one buffer of DATASEC entries that describe the
offsets for variables in the percpu ELF section. In order to make it
possible to output all global variables, we'll need to output a DATASEC
for each ELF section containing variables, and we'll need to control
whether or not to encode variables on a per-section basis.

With this change, the ability to emit VARs from multiple sections is
technically present, but not enabled, so pahole still only emits percpu
variables. A subsequent change will enable emitting all global
variables.

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---
 btf_encoder.c | 87 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 53 insertions(+), 34 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 5586cd8..838a0b1 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -98,6 +98,8 @@ struct elf_secinfo {
 	const char *name;
 	uint64_t    sz;
 	uint32_t    type;
+	bool        include;
+	struct gobuffer secinfo;
 };
 
 /*
@@ -107,7 +109,6 @@ struct btf_encoder {
 	struct list_head  node;
 	struct btf        *btf;
 	struct cu         *cu;
-	struct gobuffer   percpu_secinfo;
 	const char	  *source_filename;
 	const char	  *filename;
 	struct elf_symtab *symtab;
@@ -124,7 +125,6 @@ struct btf_encoder {
 	uint32_t	  array_index_id;
 	struct elf_secinfo *secinfo;
 	size_t             seccnt;
-	size_t             percpu_shndx;
 	int                encode_vars;
 	struct {
 		struct elf_function *entries;
@@ -784,46 +784,56 @@ static int32_t btf_encoder__add_var(struct btf_encoder *encoder, uint32_t type,
 	return id;
 }
 
-static int32_t btf_encoder__add_var_secinfo(struct btf_encoder *encoder, uint32_t type,
-				     uint32_t offset, uint32_t size)
+static int32_t btf_encoder__add_var_secinfo(struct btf_encoder *encoder, size_t shndx,
+					    uint32_t type, uint32_t offset, uint32_t size)
 {
 	struct btf_var_secinfo si = {
 		.type = type,
 		.offset = offset,
 		.size = size,
 	};
-	return gobuffer__add(&encoder->percpu_secinfo, &si, sizeof(si));
+	return gobuffer__add(&encoder->secinfo[shndx].secinfo, &si, sizeof(si));
 }
 
 int32_t btf_encoder__add_encoder(struct btf_encoder *encoder, struct btf_encoder *other)
 {
-	struct gobuffer *var_secinfo_buf = &other->percpu_secinfo;
-	size_t sz = gobuffer__size(var_secinfo_buf);
-	uint16_t nr_var_secinfo = sz / sizeof(struct btf_var_secinfo);
-	uint32_t type_id;
-	uint32_t next_type_id = btf__type_cnt(encoder->btf);
-	int32_t i, id;
-	struct btf_var_secinfo *vsi;
-
+	size_t shndx;
 	if (encoder == other)
 		return 0;
 
 	btf_encoder__add_saved_funcs(other);
 
-	for (i = 0; i < nr_var_secinfo; i++) {
-		vsi = (struct btf_var_secinfo *)var_secinfo_buf->entries + i;
-		type_id = next_type_id + vsi->type - 1; /* Type ID starts from 1 */
-		id = btf_encoder__add_var_secinfo(encoder, type_id, vsi->offset, vsi->size);
-		if (id < 0)
-			return id;
+	for (shndx = 1; shndx < other->seccnt; shndx++) {
+		struct gobuffer *var_secinfo_buf = &other->secinfo[shndx].secinfo;
+		size_t sz = gobuffer__size(var_secinfo_buf);
+		uint16_t nr_var_secinfo = sz / sizeof(struct btf_var_secinfo);
+		uint32_t type_id;
+		uint32_t next_type_id = btf__type_cnt(encoder->btf);
+		int32_t i, id;
+		struct btf_var_secinfo *vsi;
+
+		if (strcmp(encoder->secinfo[shndx].name, other->secinfo[shndx].name)) {
+			fprintf(stderr, "mismatched ELF sections at index %zu: \"%s\", \"%s\"\n",
+				shndx, encoder->secinfo[shndx].name, other->secinfo[shndx].name);
+			return -1;
+		}
+
+		for (i = 0; i < nr_var_secinfo; i++) {
+			vsi = (struct btf_var_secinfo *)var_secinfo_buf->entries + i;
+			type_id = next_type_id + vsi->type - 1; /* Type ID starts from 1 */
+			id = btf_encoder__add_var_secinfo(encoder, shndx, type_id, vsi->offset, vsi->size);
+			if (id < 0)
+				return id;
+		}
 	}
 
 	return btf__add_btf(encoder->btf, other->btf);
 }
 
-static int32_t btf_encoder__add_datasec(struct btf_encoder *encoder, const char *section_name)
+static int32_t btf_encoder__add_datasec(struct btf_encoder *encoder, size_t shndx)
 {
-	struct gobuffer *var_secinfo_buf = &encoder->percpu_secinfo;
+	struct gobuffer *var_secinfo_buf = &encoder->secinfo[shndx].secinfo;
+	const char *section_name = encoder->secinfo[shndx].name;
 	struct btf *btf = encoder->btf;
 	size_t sz = gobuffer__size(var_secinfo_buf);
 	uint16_t nr_var_secinfo = sz / sizeof(struct btf_var_secinfo);
@@ -2032,12 +2042,14 @@ int btf_encoder__encode(struct btf_encoder *encoder)
 {
 	bool should_tag_kfuncs;
 	int err;
+	size_t shndx;
 
 	/* for single-threaded case, saved funcs are added here */
 	btf_encoder__add_saved_funcs(encoder);
 
-	if (gobuffer__size(&encoder->percpu_secinfo) != 0)
-		btf_encoder__add_datasec(encoder, PERCPU_SECTION);
+	for (shndx = 1; shndx < encoder->seccnt; shndx++)
+		if (gobuffer__size(&encoder->secinfo[shndx].secinfo))
+			btf_encoder__add_datasec(encoder, shndx);
 
 	/* Empty file, nothing to do, so... done! */
 	if (btf__type_cnt(encoder->btf) == 1)
@@ -2170,7 +2182,7 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 	struct tag *pos;
 	int err = -1;
 
-	if (encoder->percpu_shndx == 0 || !encoder->symtab)
+	if (!encoder->symtab)
 		return 0;
 
 	if (encoder->verbose)
@@ -2214,7 +2226,7 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 
 		/* Get the ELF section info for the variable */
 		shndx = get_elf_section(encoder, addr);
-		if (shndx != encoder->percpu_shndx)
+		if (!shndx || shndx >= encoder->seccnt || !encoder->secinfo[shndx].include)
 			continue;
 
 		/* Convert addr to section relative */
@@ -2255,7 +2267,7 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 		size = tag__size(tag, cu);
 		if (size == 0 || size > UINT32_MAX) {
 			if (encoder->verbose)
-				fprintf(stderr, "Ignoring %s-sized per-CPU variable '%s'...\n",
+				fprintf(stderr, "Ignoring %s-sized variable '%s'...\n",
 					size == 0 ? "zero" : "over", name);
 			continue;
 		}
@@ -2292,13 +2304,14 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 		}
 
 		/*
-		 * add a BTF_VAR_SECINFO in encoder->percpu_secinfo, which will be added into
-		 * encoder->types later when we add BTF_VAR_DATASEC.
+		 * 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.
 		 */
-		id = btf_encoder__add_var_secinfo(encoder, id, (uint32_t)addr, (uint32_t)size);
+		id = btf_encoder__add_var_secinfo(encoder, shndx, id, (uint32_t)addr, (uint32_t)size);
 		if (id < 0) {
 			fprintf(stderr, "error: failed to encode section info for variable '%s' at addr 0x%" PRIx64 "\n",
-			        name, addr);
+				name, addr);
 			goto out;
 		}
 	}
@@ -2376,6 +2389,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 			goto out_delete;
 		}
 
+		bool found_percpu = false;
 		for (shndx = 0; shndx < encoder->seccnt; shndx++) {
 			const char *secname = NULL;
 			Elf_Scn *sec = elf_section_by_idx(cu->elf, &shdr, shndx, &secname);
@@ -2386,11 +2400,14 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 			encoder->secinfo[shndx].name = secname;
 			encoder->secinfo[shndx].type = shdr.sh_type;
 
-			if (strcmp(secname, PERCPU_SECTION) == 0)
-				encoder->percpu_shndx = shndx;
+			if (strcmp(secname, PERCPU_SECTION) == 0) {
+				found_percpu = true;
+				if (encoder->encode_vars & BTF_VAR_PERCPU)
+					encoder->secinfo[shndx].include = true;
+			}
 		}
 
-		if (!encoder->percpu_shndx && encoder->verbose)
+		if (!found_percpu && encoder->verbose)
 			printf("%s: '%s' doesn't have '%s' section\n", __func__, cu->filename, PERCPU_SECTION);
 
 		if (btf_encoder__collect_symbols(encoder))
@@ -2418,12 +2435,14 @@ void btf_encoder__delete_func(struct elf_function *func)
 void btf_encoder__delete(struct btf_encoder *encoder)
 {
 	int i;
+	size_t shndx;
 
 	if (encoder == NULL)
 		return;
 
 	btf_encoders__delete(encoder);
-	__gobuffer__delete(&encoder->percpu_secinfo);
+	for (shndx = 0; shndx < encoder->seccnt; shndx++)
+		__gobuffer__delete(&encoder->secinfo[shndx].secinfo);
 	zfree(&encoder->filename);
 	zfree(&encoder->source_filename);
 	btf__free(encoder->btf);
-- 
2.43.5

[PATCH dwarves v4 4/4] pahole: add global_var BTF feature

[Stephen Brennan]

So far, pahole has only encoded type information for percpu variables.
However, there are several reasons why type information for all global
variables would be useful in the kernel:

1. Runtime kernel debuggers like drgn could use the BTF to introspect
kernel data structures without needing to install heavyweight DWARF.

2. BPF programs using the "__ksym" annotation could declare the
variables using the correct type, rather than "void".

It makes sense to introduce a feature for this in pahole so that these
capabilities can be explored in the kernel. The feature is non-default:
when using "--btf-features=default", it is disabled. It must be
explicitly requested, e.g. with "--btf-features=+global_var".

Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Tested-by: Alan Maguire <alan.maguire@oracle.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---
 btf_encoder.c      | 5 +++++
 btf_encoder.h      | 1 +
 dwarves.h          | 1 +
 man-pages/pahole.1 | 7 +++++--
 pahole.c           | 3 ++-
 5 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 838a0b1..201a48c 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -2348,6 +2348,8 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 		encoder->encode_vars = 0;
 		if (!conf_load->skip_encoding_btf_vars)
 			encoder->encode_vars |= BTF_VAR_PERCPU;
+		if (conf_load->encode_btf_global_vars)
+			encoder->encode_vars |= BTF_VAR_GLOBAL;
 
 		GElf_Ehdr ehdr;
 
@@ -2400,6 +2402,9 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 			encoder->secinfo[shndx].name = secname;
 			encoder->secinfo[shndx].type = shdr.sh_type;
 
+			if (encoder->encode_vars & BTF_VAR_GLOBAL)
+				encoder->secinfo[shndx].include = true;
+
 			if (strcmp(secname, PERCPU_SECTION) == 0) {
 				found_percpu = true;
 				if (encoder->encode_vars & BTF_VAR_PERCPU)
diff --git a/btf_encoder.h b/btf_encoder.h
index 91e7947..824963b 100644
--- a/btf_encoder.h
+++ b/btf_encoder.h
@@ -20,6 +20,7 @@ struct list_head;
 enum btf_var_option {
 	BTF_VAR_NONE = 0,
 	BTF_VAR_PERCPU = 1,
+	BTF_VAR_GLOBAL = 2,
 };
 
 struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filename, struct btf *base_btf, bool verbose, struct conf_load *conf_load);
diff --git a/dwarves.h b/dwarves.h
index 0fede91..fef881f 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -92,6 +92,7 @@ struct conf_load {
 	bool			btf_gen_optimized;
 	bool			skip_encoding_btf_inconsistent_proto;
 	bool			skip_encoding_btf_vars;
+	bool			encode_btf_global_vars;
 	bool			btf_gen_floats;
 	bool			btf_encode_force;
 	bool			reproducible_build;
diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
index b3e6632..7c1a69a 100644
--- a/man-pages/pahole.1
+++ b/man-pages/pahole.1
@@ -238,7 +238,9 @@ the debugging information.
 
 .TP
 .B \-\-skip_encoding_btf_vars
-Do not encode VARs in BTF.
+By default, VARs are encoded only for percpu variables. When specified, this
+option prevents encoding any VARs. Note that this option can be overridden
+by the feature "global_var".
 
 .TP
 .B \-\-skip_encoding_btf_decl_tag
@@ -304,7 +306,7 @@ Encode BTF using the specified feature list, or specify 'default' for all standa
 	encode_force       Ignore invalid symbols when encoding BTF; for example
 	                   if a symbol has an invalid name, it will be ignored
 	                   and BTF encoding will continue.
-	var                Encode variables using BTF_KIND_VAR in BTF.
+	var                Encode percpu variables using BTF_KIND_VAR in BTF.
 	float              Encode floating-point types in BTF.
 	decl_tag           Encode declaration tags using BTF_KIND_DECL_TAG.
 	type_tag           Encode type tags using BTF_KIND_TYPE_TAG.
@@ -329,6 +331,7 @@ Supported non-standard features (not enabled for 'default')
 	                   the associated base BTF to support later relocation
 	                   of split BTF with a possibly changed base, storing
 	                   it in a .BTF.base ELF section.
+	global_var         Encode all global variables using BTF_KIND_VAR in BTF.
 .fi
 
 So for example, specifying \-\-btf_encode=var,enum64 will result in a BTF encoding that (as well as encoding basic BTF information) will contain variables and enum64 values.
diff --git a/pahole.c b/pahole.c
index b21a7f2..9f0dc59 100644
--- a/pahole.c
+++ b/pahole.c
@@ -1301,6 +1301,7 @@ struct btf_feature {
 	BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false),
 	BTF_NON_DEFAULT_FEATURE(reproducible_build, reproducible_build, false),
 	BTF_NON_DEFAULT_FEATURE(distilled_base, btf_gen_distilled_base, false),
+	BTF_NON_DEFAULT_FEATURE(global_var, encode_btf_global_vars, false),
 };
 
 #define BTF_MAX_FEATURE_STR	1024
@@ -1733,7 +1734,7 @@ static const struct argp_option pahole__options[] = {
 	{
 		.name = "skip_encoding_btf_vars",
 		.key  = ARGP_skip_encoding_btf_vars,
-		.doc  = "Do not encode VARs in BTF."
+		.doc  = "Do not encode any VARs in BTF [if this is not specified, only percpu variables are encoded. To encode global variables too, use --encode_btf_global_vars]."
 	},
 	{
 		.name = "btf_encode_force",
-- 
2.43.5

Re: [PATCH dwarves v4 0/4] Emit global variables in BTF

[Arnaldo Carvalho de Melo]

On Fri, Oct 04, 2024 at 10:26:24AM -0700, Stephen Brennan wrote:
> Hi all,
> 
> This is v4 of the series which adds global variables to pahole's generated BTF.
> 
> Since v3:
> 
> 1. Gathered Alan's Reviewed-by + Tested-by, and Jiri's Acked-by.
> 2. Consistently start shndx loops at 1, and use size_t.
> 3. Since patch 1 of v3 was already applied, I dropped it out of this series.
> 
> v3: https://lore.kernel.org/dwarves/20241002235253.487251-1-stephen.s.brennan@oracle.com/
> v2: https://lore.kernel.org/dwarves/20240920081903.13473-1-stephen.s.brennan@oracle.com/
> v1: https://lore.kernel.org/dwarves/20240912190827.230176-1-stephen.s.brennan@oracle.com/
> 
> Thanks everyone for your review, tests, and consideration!

Looks ok, I run the existing regression tests:

acme@x1:~/git/pahole$ tests/tests 
  1: Validation of BTF encoding of functions; this may take some time: Ok
  2: Pretty printing of files using DWARF type information: Ok
  3: Parallel reproducible DWARF Loading/Serial BTF encoding: Ok
/home/acme/git/pahole
acme@x1:~/git/pahole$

And now I'm building a kernel with clang + Thin LTO + Rust enabled in
the kernel to test other fixes I have merged and doing that with your
patch series.

Its all in the next branch and will move to master later today or
tomorrow when I finish the clang+LTO+Rust tests.

- Arnaldo

> Stephen
> 
> Stephen Brennan (4):
>   btf_encoder: stop indexing symbols for VARs
>   btf_encoder: explicitly check addr/size for u32 overflow
>   btf_encoder: allow encoding VARs from many sections
>   pahole: add global_var BTF feature
> 
>  btf_encoder.c      | 340 +++++++++++++++++++++------------------------
>  btf_encoder.h      |   1 +
>  dwarves.h          |   1 +
>  man-pages/pahole.1 |   7 +-
>  pahole.c           |   3 +-
>  5 files changed, 167 insertions(+), 185 deletions(-)
> 
> -- 
> 2.43.5

Re: [PATCH dwarves v4 0/4] Emit global variables in BTF

[Arnaldo Carvalho de Melo]

On Fri, Oct 04, 2024 at 05:58:48PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Oct 04, 2024 at 10:26:24AM -0700, Stephen Brennan wrote:
> > Hi all,
> > 
> > This is v4 of the series which adds global variables to pahole's generated BTF.
> > 
> > Since v3:
> > 
> > 1. Gathered Alan's Reviewed-by + Tested-by, and Jiri's Acked-by.
> > 2. Consistently start shndx loops at 1, and use size_t.
> > 3. Since patch 1 of v3 was already applied, I dropped it out of this series.
> > 
> > v3: https://lore.kernel.org/dwarves/20241002235253.487251-1-stephen.s.brennan@oracle.com/
> > v2: https://lore.kernel.org/dwarves/20240920081903.13473-1-stephen.s.brennan@oracle.com/
> > v1: https://lore.kernel.org/dwarves/20240912190827.230176-1-stephen.s.brennan@oracle.com/
> > 
> > Thanks everyone for your review, tests, and consideration!
> 
> Looks ok, I run the existing regression tests:
> 
> acme@x1:~/git/pahole$ tests/tests 
>   1: Validation of BTF encoding of functions; this may take some time: Ok
>   2: Pretty printing of files using DWARF type information: Ok
>   3: Parallel reproducible DWARF Loading/Serial BTF encoding: Ok
> /home/acme/git/pahole
> acme@x1:~/git/pahole$
> 
> And now I'm building a kernel with clang + Thin LTO + Rust enabled in
> the kernel to test other fixes I have merged and doing that with your
> patch series.
> 
> Its all in the next branch and will move to master later today or
> tomorrow when I finish the clang+LTO+Rust tests.

Ah, please consider looking at the test/ scripts and try to write a
simple test that will encode global vars from the running kernel and
then use bpftool to dump them and look for some of the well known kernel
global variables being encoded to match expectations, so that we have
this feature continuously tested vai tests/tests.

If now the increase in size due to global vars is of N%, please consider
checking if that is off by some unreasonable margin, etc.

Thanks!

- Arnaldo

Re: [PATCH dwarves v4 0/4] Emit global variables in BTF

[Andrii Nakryiko]

On Fri, Oct 4, 2024 at 2:21 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> On Fri, Oct 04, 2024 at 10:26:24AM -0700, Stephen Brennan wrote:
> > Hi all,
> >
> > This is v4 of the series which adds global variables to pahole's generated BTF.
> >
> > Since v3:
> >
> > 1. Gathered Alan's Reviewed-by + Tested-by, and Jiri's Acked-by.
> > 2. Consistently start shndx loops at 1, and use size_t.
> > 3. Since patch 1 of v3 was already applied, I dropped it out of this series.
> >
> > v3: https://lore.kernel.org/dwarves/20241002235253.487251-1-stephen.s.brennan@oracle.com/
> > v2: https://lore.kernel.org/dwarves/20240920081903.13473-1-stephen.s.brennan@oracle.com/
> > v1: https://lore.kernel.org/dwarves/20240912190827.230176-1-stephen.s.brennan@oracle.com/
> >
> > Thanks everyone for your review, tests, and consideration!
>
> Looks ok, I run the existing regression tests:
>
> acme@x1:~/git/pahole$ tests/tests
>   1: Validation of BTF encoding of functions; this may take some time: Ok
>   2: Pretty printing of files using DWARF type information: Ok
>   3: Parallel reproducible DWARF Loading/Serial BTF encoding: Ok
> /home/acme/git/pahole
> acme@x1:~/git/pahole$
>
> And now I'm building a kernel with clang + Thin LTO + Rust enabled in
> the kernel to test other fixes I have merged and doing that with your
> patch series.
>
> Its all in the next branch and will move to master later today or
> tomorrow when I finish the clang+LTO+Rust tests.

pahole-staging testing in libbpf CI started failing recently, can you
please double-check and see if this was caused by these changes? They
seem to be related to encoding BTF for per-CPU global variables, so
might be relevant ([0] for full run logs)

  #33      btf_dump:FAIL
  libbpf: extern (var ksym) 'bpf_prog_active': not found in kernel BTF
  libbpf: failed to load object 'kfunc_call_test_subprog'
  libbpf: failed to load BPF skeleton 'kfunc_call_test_subprog': -22
  test_subprog:FAIL:skel unexpected error: -22
  #126/17  kfunc_call/subprog:FAIL
  test_subprog_lskel:FAIL:skel unexpected error: -2
  #126/18  kfunc_call/subprog_lskel:FAIL
  #126     kfunc_call:FAIL
  test_ksyms_module_lskel:FAIL:test_ksyms_module_lskel__open_and_load
unexpected error: -2
  #135/1   ksyms_module/lskel:FAIL
  libbpf: extern (var ksym) 'bpf_testmod_ksym_percpu': not found in kernel BTF
  libbpf: failed to load object 'test_ksyms_module'
  libbpf: failed to load BPF skeleton 'test_ksyms_module': -22
  test_ksyms_module_libbpf:FAIL:test_ksyms_module__open unexpected error: -22
  #135/2   ksyms_module/libbpf:FAIL


  [0] https://github.com/libbpf/libbpf/actions/runs/11204199648/job/31142297399#step:4:12480

>
> - Arnaldo
>
> > Stephen
> >
> > Stephen Brennan (4):
> >   btf_encoder: stop indexing symbols for VARs
> >   btf_encoder: explicitly check addr/size for u32 overflow
> >   btf_encoder: allow encoding VARs from many sections
> >   pahole: add global_var BTF feature
> >
> >  btf_encoder.c      | 340 +++++++++++++++++++++------------------------
> >  btf_encoder.h      |   1 +
> >  dwarves.h          |   1 +
> >  man-pages/pahole.1 |   7 +-
> >  pahole.c           |   3 +-
> >  5 files changed, 167 insertions(+), 185 deletions(-)
> >
> > --
> > 2.43.5
>

Re: [PATCH dwarves v4 0/4] Emit global variables in BTF

[Stephen Brennan]

Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> On Fri, Oct 4, 2024 at 2:21 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>
>> On Fri, Oct 04, 2024 at 10:26:24AM -0700, Stephen Brennan wrote:
>> > Hi all,
>> >
>> > This is v4 of the series which adds global variables to pahole's generated BTF.
>> >
>> > Since v3:
>> >
>> > 1. Gathered Alan's Reviewed-by + Tested-by, and Jiri's Acked-by.
>> > 2. Consistently start shndx loops at 1, and use size_t.
>> > 3. Since patch 1 of v3 was already applied, I dropped it out of this series.
>> >
>> > v3: https://lore.kernel.org/dwarves/20241002235253.487251-1-stephen.s.brennan@oracle.com/
>> > v2: https://lore.kernel.org/dwarves/20240920081903.13473-1-stephen.s.brennan@oracle.com/
>> > v1: https://lore.kernel.org/dwarves/20240912190827.230176-1-stephen.s.brennan@oracle.com/
>> >
>> > Thanks everyone for your review, tests, and consideration!
>>
>> Looks ok, I run the existing regression tests:
>>
>> acme@x1:~/git/pahole$ tests/tests
>>   1: Validation of BTF encoding of functions; this may take some time: Ok
>>   2: Pretty printing of files using DWARF type information: Ok
>>   3: Parallel reproducible DWARF Loading/Serial BTF encoding: Ok
>> /home/acme/git/pahole
>> acme@x1:~/git/pahole$
>>
>> And now I'm building a kernel with clang + Thin LTO + Rust enabled in
>> the kernel to test other fixes I have merged and doing that with your
>> patch series.
>>
>> Its all in the next branch and will move to master later today or
>> tomorrow when I finish the clang+LTO+Rust tests.
>
> pahole-staging testing in libbpf CI started failing recently, can you
> please double-check and see if this was caused by these changes? They
> seem to be related to encoding BTF for per-CPU global variables, so
> might be relevant ([0] for full run logs)
>
>   #33      btf_dump:FAIL
>   libbpf: extern (var ksym) 'bpf_prog_active': not found in kernel BTF
>   libbpf: failed to load object 'kfunc_call_test_subprog'
>   libbpf: failed to load BPF skeleton 'kfunc_call_test_subprog': -22
>   test_subprog:FAIL:skel unexpected error: -22
>   #126/17  kfunc_call/subprog:FAIL
>   test_subprog_lskel:FAIL:skel unexpected error: -2
>   #126/18  kfunc_call/subprog_lskel:FAIL
>   #126     kfunc_call:FAIL
>   test_ksyms_module_lskel:FAIL:test_ksyms_module_lskel__open_and_load
> unexpected error: -2
>   #135/1   ksyms_module/lskel:FAIL
>   libbpf: extern (var ksym) 'bpf_testmod_ksym_percpu': not found in kernel BTF
>   libbpf: failed to load object 'test_ksyms_module'
>   libbpf: failed to load BPF skeleton 'test_ksyms_module': -22
>   test_ksyms_module_libbpf:FAIL:test_ksyms_module__open unexpected error: -22
>   #135/2   ksyms_module/libbpf:FAIL
>
>
>   [0] https://github.com/libbpf/libbpf/actions/runs/11204199648/job/31142297399#step:4:12480

Hi Andrii,

Thanks for the report.

The error: "'bpf_prog_active' not found in kernel BTF" sounds like it's
related to a bug that was present in v4 of this patch series:

https://lore.kernel.org/dwarves/ZwPob57HKYbfNpOH@x1/T/#t

Basically due to poor testing of a small refactor on my part, pahole
failed to emit almost all of the variables for BTF, so it would very
likely cause this error. And I think this broken commit may have been
hanging around in the git repository for the weekend, maybe Arnaldo can
confirm whether or not it was fixed up.

I cannot see the git SHA for the pahole branch which was used in this CI
run, so I can't say for sure. But I do see that the "tmp.master" branch
is now fixed up, so a re-run would verify whether this is the root
cause.

Thanks,
Stephen

Re: [PATCH dwarves v4 0/4] Emit global variables in BTF

[Arnaldo Carvalho de Melo]

On Mon, Oct 07, 2024 at 10:24:01AM -0700, Stephen Brennan wrote:
> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> > On Fri, Oct 4, 2024 at 2:21 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >>
> >> On Fri, Oct 04, 2024 at 10:26:24AM -0700, Stephen Brennan wrote:
> >> > Hi all,
> >> >
> >> > This is v4 of the series which adds global variables to pahole's generated BTF.
> >> >
> >> > Since v3:
> >> >
> >> > 1. Gathered Alan's Reviewed-by + Tested-by, and Jiri's Acked-by.
> >> > 2. Consistently start shndx loops at 1, and use size_t.
> >> > 3. Since patch 1 of v3 was already applied, I dropped it out of this series.
> >> >
> >> > v3: https://lore.kernel.org/dwarves/20241002235253.487251-1-stephen.s.brennan@oracle.com/
> >> > v2: https://lore.kernel.org/dwarves/20240920081903.13473-1-stephen.s.brennan@oracle.com/
> >> > v1: https://lore.kernel.org/dwarves/20240912190827.230176-1-stephen.s.brennan@oracle.com/
> >> >
> >> > Thanks everyone for your review, tests, and consideration!
> >>
> >> Looks ok, I run the existing regression tests:
> >>
> >> acme@x1:~/git/pahole$ tests/tests
> >>   1: Validation of BTF encoding of functions; this may take some time: Ok
> >>   2: Pretty printing of files using DWARF type information: Ok
> >>   3: Parallel reproducible DWARF Loading/Serial BTF encoding: Ok
> >> /home/acme/git/pahole
> >> acme@x1:~/git/pahole$
> >>
> >> And now I'm building a kernel with clang + Thin LTO + Rust enabled in
> >> the kernel to test other fixes I have merged and doing that with your
> >> patch series.
> >>
> >> Its all in the next branch and will move to master later today or
> >> tomorrow when I finish the clang+LTO+Rust tests.
> >
> > pahole-staging testing in libbpf CI started failing recently, can you
> > please double-check and see if this was caused by these changes? They
> > seem to be related to encoding BTF for per-CPU global variables, so
> > might be relevant ([0] for full run logs)
> >
> >   #33      btf_dump:FAIL
> >   libbpf: extern (var ksym) 'bpf_prog_active': not found in kernel BTF
> >   libbpf: failed to load object 'kfunc_call_test_subprog'
> >   libbpf: failed to load BPF skeleton 'kfunc_call_test_subprog': -22
> >   test_subprog:FAIL:skel unexpected error: -22
> >   #126/17  kfunc_call/subprog:FAIL
> >   test_subprog_lskel:FAIL:skel unexpected error: -2
> >   #126/18  kfunc_call/subprog_lskel:FAIL
> >   #126     kfunc_call:FAIL
> >   test_ksyms_module_lskel:FAIL:test_ksyms_module_lskel__open_and_load
> > unexpected error: -2
> >   #135/1   ksyms_module/lskel:FAIL
> >   libbpf: extern (var ksym) 'bpf_testmod_ksym_percpu': not found in kernel BTF
> >   libbpf: failed to load object 'test_ksyms_module'
> >   libbpf: failed to load BPF skeleton 'test_ksyms_module': -22
> >   test_ksyms_module_libbpf:FAIL:test_ksyms_module__open unexpected error: -22
> >   #135/2   ksyms_module/libbpf:FAIL
> >
> >
> >   [0] https://github.com/libbpf/libbpf/actions/runs/11204199648/job/31142297399#step:4:12480
> 
> Hi Andrii,
> 
> Thanks for the report.
> 
> The error: "'bpf_prog_active' not found in kernel BTF" sounds like it's
> related to a bug that was present in v4 of this patch series:
> 
> https://lore.kernel.org/dwarves/ZwPob57HKYbfNpOH@x1/T/#t
> 
> Basically due to poor testing of a small refactor on my part, pahole
> failed to emit almost all of the variables for BTF, so it would very
> likely cause this error. And I think this broken commit may have been
> hanging around in the git repository for the weekend, maybe Arnaldo can
> confirm whether or not it was fixed up.
> 
> I cannot see the git SHA for the pahole branch which was used in this CI
> run, so I can't say for sure. But I do see that the "tmp.master" branch
> is now fixed up, so a re-run would verify whether this is the root
> cause.

right, that is a piece of info I sometimes miss, the SHA used for the
test run, but today's test is in progress and should have the fix for
the inverted logic, we'll see...


- Arnaldo

Re: [PATCH dwarves v4 0/4] Emit global variables in BTF

[Arnaldo Carvalho de Melo]

On Mon, Oct 07, 2024 at 03:48:16PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Oct 07, 2024 at 10:24:01AM -0700, Stephen Brennan wrote:
> > Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> > > On Fri, Oct 4, 2024 at 2:21 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > >>
> > >> On Fri, Oct 04, 2024 at 10:26:24AM -0700, Stephen Brennan wrote:
> > >> > Hi all,
> > >> >
> > >> > This is v4 of the series which adds global variables to pahole's generated BTF.
> > >> >
> > >> > Since v3:
> > >> >
> > >> > 1. Gathered Alan's Reviewed-by + Tested-by, and Jiri's Acked-by.
> > >> > 2. Consistently start shndx loops at 1, and use size_t.
> > >> > 3. Since patch 1 of v3 was already applied, I dropped it out of this series.
> > >> >
> > >> > v3: https://lore.kernel.org/dwarves/20241002235253.487251-1-stephen.s.brennan@oracle.com/
> > >> > v2: https://lore.kernel.org/dwarves/20240920081903.13473-1-stephen.s.brennan@oracle.com/
> > >> > v1: https://lore.kernel.org/dwarves/20240912190827.230176-1-stephen.s.brennan@oracle.com/
> > >> >
> > >> > Thanks everyone for your review, tests, and consideration!
> > >>
> > >> Looks ok, I run the existing regression tests:
> > >>
> > >> acme@x1:~/git/pahole$ tests/tests
> > >>   1: Validation of BTF encoding of functions; this may take some time: Ok
> > >>   2: Pretty printing of files using DWARF type information: Ok
> > >>   3: Parallel reproducible DWARF Loading/Serial BTF encoding: Ok
> > >> /home/acme/git/pahole
> > >> acme@x1:~/git/pahole$
> > >>
> > >> And now I'm building a kernel with clang + Thin LTO + Rust enabled in
> > >> the kernel to test other fixes I have merged and doing that with your
> > >> patch series.
> > >>
> > >> Its all in the next branch and will move to master later today or
> > >> tomorrow when I finish the clang+LTO+Rust tests.
> > >
> > > pahole-staging testing in libbpf CI started failing recently, can you
> > > please double-check and see if this was caused by these changes? They
> > > seem to be related to encoding BTF for per-CPU global variables, so
> > > might be relevant ([0] for full run logs)
> > >
> > >   #33      btf_dump:FAIL
> > >   libbpf: extern (var ksym) 'bpf_prog_active': not found in kernel BTF
> > >   libbpf: failed to load object 'kfunc_call_test_subprog'
> > >   libbpf: failed to load BPF skeleton 'kfunc_call_test_subprog': -22
> > >   test_subprog:FAIL:skel unexpected error: -22
> > >   #126/17  kfunc_call/subprog:FAIL
> > >   test_subprog_lskel:FAIL:skel unexpected error: -2
> > >   #126/18  kfunc_call/subprog_lskel:FAIL
> > >   #126     kfunc_call:FAIL
> > >   test_ksyms_module_lskel:FAIL:test_ksyms_module_lskel__open_and_load
> > > unexpected error: -2
> > >   #135/1   ksyms_module/lskel:FAIL
> > >   libbpf: extern (var ksym) 'bpf_testmod_ksym_percpu': not found in kernel BTF
> > >   libbpf: failed to load object 'test_ksyms_module'
> > >   libbpf: failed to load BPF skeleton 'test_ksyms_module': -22
> > >   test_ksyms_module_libbpf:FAIL:test_ksyms_module__open unexpected error: -22
> > >   #135/2   ksyms_module/libbpf:FAIL
> > >
> > >
> > >   [0] https://github.com/libbpf/libbpf/actions/runs/11204199648/job/31142297399#step:4:12480
> > 
> > Hi Andrii,
> > 
> > Thanks for the report.
> > 
> > The error: "'bpf_prog_active' not found in kernel BTF" sounds like it's
> > related to a bug that was present in v4 of this patch series:
> > 
> > https://lore.kernel.org/dwarves/ZwPob57HKYbfNpOH@x1/T/#t
> > 
> > Basically due to poor testing of a small refactor on my part, pahole
> > failed to emit almost all of the variables for BTF, so it would very
> > likely cause this error. And I think this broken commit may have been
> > hanging around in the git repository for the weekend, maybe Arnaldo can
> > confirm whether or not it was fixed up.
> > 
> > I cannot see the git SHA for the pahole branch which was used in this CI
> > run, so I can't say for sure. But I do see that the "tmp.master" branch
> > is now fixed up, so a re-run would verify whether this is the root
> > cause.
> 
> right, that is a piece of info I sometimes miss, the SHA used for the
> test run, but today's test is in progress and should have the fix for
> the inverted logic, we'll see...

https://github.com/libbpf/libbpf/actions/runs/11221662157/job/31192457160

Passed, so and here as well:

acme@x1:~/git/pahole$ tests/tests 
  1: Validation of BTF encoding of functions; this may take some time: Ok
  2: Pretty printing of files using DWARF type information: Ok
  3: Parallel reproducible DWARF Loading/Serial BTF encoding: Ok
acme@x1:~/git/pahole$

- Arnaldo

Re: [PATCH dwarves v4 0/4] Emit global variables in BTF

[Andrii Nakryiko]

On Mon, Oct 7, 2024 at 12:00 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Mon, Oct 07, 2024 at 03:48:16PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Mon, Oct 07, 2024 at 10:24:01AM -0700, Stephen Brennan wrote:
> > > Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
> > > > On Fri, Oct 4, 2024 at 2:21 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > >>
> > > >> On Fri, Oct 04, 2024 at 10:26:24AM -0700, Stephen Brennan wrote:
> > > >> > Hi all,
> > > >> >
> > > >> > This is v4 of the series which adds global variables to pahole's generated BTF.
> > > >> >
> > > >> > Since v3:
> > > >> >
> > > >> > 1. Gathered Alan's Reviewed-by + Tested-by, and Jiri's Acked-by.
> > > >> > 2. Consistently start shndx loops at 1, and use size_t.
> > > >> > 3. Since patch 1 of v3 was already applied, I dropped it out of this series.
> > > >> >
> > > >> > v3: https://lore.kernel.org/dwarves/20241002235253.487251-1-stephen.s.brennan@oracle.com/
> > > >> > v2: https://lore.kernel.org/dwarves/20240920081903.13473-1-stephen.s.brennan@oracle.com/
> > > >> > v1: https://lore.kernel.org/dwarves/20240912190827.230176-1-stephen.s.brennan@oracle.com/
> > > >> >
> > > >> > Thanks everyone for your review, tests, and consideration!
> > > >>
> > > >> Looks ok, I run the existing regression tests:
> > > >>
> > > >> acme@x1:~/git/pahole$ tests/tests
> > > >>   1: Validation of BTF encoding of functions; this may take some time: Ok
> > > >>   2: Pretty printing of files using DWARF type information: Ok
> > > >>   3: Parallel reproducible DWARF Loading/Serial BTF encoding: Ok
> > > >> /home/acme/git/pahole
> > > >> acme@x1:~/git/pahole$
> > > >>
> > > >> And now I'm building a kernel with clang + Thin LTO + Rust enabled in
> > > >> the kernel to test other fixes I have merged and doing that with your
> > > >> patch series.
> > > >>
> > > >> Its all in the next branch and will move to master later today or
> > > >> tomorrow when I finish the clang+LTO+Rust tests.
> > > >
> > > > pahole-staging testing in libbpf CI started failing recently, can you
> > > > please double-check and see if this was caused by these changes? They
> > > > seem to be related to encoding BTF for per-CPU global variables, so
> > > > might be relevant ([0] for full run logs)
> > > >
> > > >   #33      btf_dump:FAIL
> > > >   libbpf: extern (var ksym) 'bpf_prog_active': not found in kernel BTF
> > > >   libbpf: failed to load object 'kfunc_call_test_subprog'
> > > >   libbpf: failed to load BPF skeleton 'kfunc_call_test_subprog': -22
> > > >   test_subprog:FAIL:skel unexpected error: -22
> > > >   #126/17  kfunc_call/subprog:FAIL
> > > >   test_subprog_lskel:FAIL:skel unexpected error: -2
> > > >   #126/18  kfunc_call/subprog_lskel:FAIL
> > > >   #126     kfunc_call:FAIL
> > > >   test_ksyms_module_lskel:FAIL:test_ksyms_module_lskel__open_and_load
> > > > unexpected error: -2
> > > >   #135/1   ksyms_module/lskel:FAIL
> > > >   libbpf: extern (var ksym) 'bpf_testmod_ksym_percpu': not found in kernel BTF
> > > >   libbpf: failed to load object 'test_ksyms_module'
> > > >   libbpf: failed to load BPF skeleton 'test_ksyms_module': -22
> > > >   test_ksyms_module_libbpf:FAIL:test_ksyms_module__open unexpected error: -22
> > > >   #135/2   ksyms_module/libbpf:FAIL
> > > >
> > > >
> > > >   [0] https://github.com/libbpf/libbpf/actions/runs/11204199648/job/31142297399#step:4:12480
> > >
> > > Hi Andrii,
> > >
> > > Thanks for the report.
> > >
> > > The error: "'bpf_prog_active' not found in kernel BTF" sounds like it's
> > > related to a bug that was present in v4 of this patch series:
> > >
> > > https://lore.kernel.org/dwarves/ZwPob57HKYbfNpOH@x1/T/#t
> > >
> > > Basically due to poor testing of a small refactor on my part, pahole
> > > failed to emit almost all of the variables for BTF, so it would very
> > > likely cause this error. And I think this broken commit may have been
> > > hanging around in the git repository for the weekend, maybe Arnaldo can
> > > confirm whether or not it was fixed up.
> > >
> > > I cannot see the git SHA for the pahole branch which was used in this CI
> > > run, so I can't say for sure. But I do see that the "tmp.master" branch
> > > is now fixed up, so a re-run would verify whether this is the root
> > > cause.
> >
> > right, that is a piece of info I sometimes miss, the SHA used for the
> > test run, but today's test is in progress and should have the fix for
> > the inverted logic, we'll see...
>
> https://github.com/libbpf/libbpf/actions/runs/11221662157/job/31192457160
>
> Passed, so and here as well:

Ok, great! Seems like I was either too slow or too fast with reporting
this, depending how you look at this :)

>
> acme@x1:~/git/pahole$ tests/tests
>   1: Validation of BTF encoding of functions; this may take some time: Ok
>   2: Pretty printing of files using DWARF type information: Ok
>   3: Parallel reproducible DWARF Loading/Serial BTF encoding: Ok
> acme@x1:~/git/pahole$
>
> - Arnaldo