Re: [PATCH v11 1/4] kallsyms: embed source file:line info in kernel stack traces

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] kallsyms: collect_symbol_starts() excludes STT_NOTYPE and
  unsized symbols
- [Medium] kallsyms: collect_symbol_starts() includes SHN_ABS and
  mapping symbols
- [Low] scripts: missing libelf in pkg-config query for gen_lineinfo in
  Makefile
--

commit 5d77a3194e4ba642dd6e711d224cf8351955c65e
Author: Sasha Levin <sashal@kernel.org>

kallsyms: embed source file:line info in kernel stack traces

This commit adds CONFIG_KALLSYMS_LINEINFO and the scripts/gen_lineinfo host
tool to extract DWARF .debug_line mappings from vmlinux and embed source
file:line annotations into kernel stack traces.

> diff --git a/scripts/Makefile b/scripts/Makefile
> index d7fbb7ca098b1..94b09cf7c1839 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -4,6 +4,7 @@
>  # the kernel for the build process.
>  
>  hostprogs-always-y				+= kallsyms
> +hostprogs-always-$(CONFIG_KALLSYMS_LINEINFO)		+= gen_lineinfo
>  hostprogs-always-$(BUILD_C_RECORDMCOUNT)		+= recordmcount
>  hostprogs-always-$(CONFIG_BUILDTIME_TABLE_SORT)		+= sorttable
>  hostprogs-always-$(CONFIG_ASN1)				+= asn1_compiler
> @@ -40,6 +41,8 @@ HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
>  HOSTCFLAGS_sign-file.o = $(shell $(HOSTPKG_CONFIG) --cflags libcrypto 2> /dev/null)
>  HOSTCFLAGS_sign-file.o += -I$(srctree)/tools/include/uapi/
>  HOSTLDLIBS_sign-file = $(shell $(HOSTPKG_CONFIG) --libs libcrypto 2> /dev/null || echo -lcrypto)
> +HOSTCFLAGS_gen_lineinfo.o = $(shell $(HOSTPKG_CONFIG) --cflags libdw 2> /dev/null)
> +HOSTLDLIBS_gen_lineinfo = $(shell $(HOSTPKG_CONFIG) --libs libdw 2> /dev/null || echo -ldw -lelf)
>  
>  ifdef CONFIG_UNWINDER_ORC
>  ifeq ($(ARCH),x86_64)

[Severity: Low]
Should HOSTCFLAGS_gen_lineinfo.o and HOSTLDLIBS_gen_lineinfo query
pkg-config for both libdw and libelf?

scripts/gen_lineinfo.c directly calls libelf and gelf functions such as
elf_begin() and gelf_getsym().  In elfutils, libdw.pc lists libelf under
Requires.private and outputs only -ldw for --libs, so when pkg-config
succeeds the fallback is skipped and linking gen_lineinfo fails with
undefined libelf references on linkers using --no-copy-dt-needed-entries.

> diff --git a/scripts/gen_lineinfo.c b/scripts/gen_lineinfo.c
> new file mode 100644
> index 0000000000000..1b347d07b7ad5
> --- /dev/null
> +++ b/scripts/gen_lineinfo.c

[ ... ]

> +static void collect_symbol_starts(Elf *elf, unsigned long long text_addr)
> +{
> +	Elf_Scn *scn = NULL;
> +	GElf_Shdr shdr;
> +
> +	while ((scn = elf_nextscn(elf, scn)) != NULL) {
> +		Elf_Data *data;
> +		size_t nsyms;
> +
> +		if (!gelf_getshdr(scn, &shdr))
> +			continue;
> +		if (shdr.sh_type != SHT_SYMTAB || !shdr.sh_entsize)
> +			continue;
> +
> +		data = elf_getdata(scn, NULL);
> +		if (!data)
> +			continue;
> +
> +		nsyms = shdr.sh_size / shdr.sh_entsize;
> +		for (size_t i = 0; i < nsyms; i++) {
> +			GElf_Sym sym;
> +			unsigned long long raw;
> +
> +			if (!gelf_getsym(data, i, &sym))
> +				continue;
> +			switch (GELF_ST_TYPE(sym.st_info)) {
> +			case STT_FUNC:
> +			case STT_NOTYPE:
> +				break;
> +			default:
> +				continue;
> +			}
> +			if (sym.st_value < text_addr)
> +				continue;
> +			if (text_end_addr && sym.st_value >= text_end_addr)
> +				continue;

[Severity: Medium]
Can unfiltered SHN_ABS, mapping ($x, $d, $a, $t) and local (.L*) symbols in
collect_symbol_starts() corrupt the symbol extents in text_starts[]?

Any STT_NOTYPE symbol whose value falls in [_text, _etext) is added to
text_starts[] without checking sym.st_shndx or symbol names.  When the loop
at the end of collect_symbol_starts() infers sizes for unsized symbols from
the distance to the next entry (while leaving the final entry at size 0 if
unsized), internal mapping or .L* labels truncate the inferred extent of the
enclosing routine, and mapping or SHN_ABS symbols receive non-zero sizes of
their own.  starts_new_sequence() -> symbol_extent_at() then misclassifies
DWARF sequence boundaries in resolve_cu_row_groups(), dropping valid opening
line rows or keeping bogus extents.

> +
> +			raw = sym.st_value - text_addr;
> +			if (raw > UINT_MAX)
> +				continue;
> +
> +			if (num_sym_starts >= sym_starts_capacity) {
> +				sym_starts_capacity = sym_starts_capacity ?
> +					sym_starts_capacity * 2 : 16384;
> +				sym_starts = xrealloc(sym_starts,
> +						      sym_starts_capacity *
> +						      sizeof(*sym_starts));
> +			}
> +			if (num_text_starts >= text_starts_capacity) {
> +				text_starts_capacity = text_starts_capacity ?
> +					text_starts_capacity * 2 : 16384;
> +				text_starts = xrealloc(text_starts,
> +						       text_starts_capacity *
> +						       sizeof(*text_starts));
> +			}
> +			text_starts[num_text_starts].offset = (unsigned int)raw;
> +			text_starts[num_text_starts].size =
> +				sym.st_size > UINT_MAX ? UINT_MAX :
> +				(unsigned int)sym.st_size;
> +			num_text_starts++;
> +
> +			if (GELF_ST_TYPE(sym.st_info) != STT_FUNC)
> +				continue;
> +
> +			sym_starts[num_sym_starts].offset = (unsigned int)raw;
> +			sym_starts[num_sym_starts].size =
> +				sym.st_size > UINT_MAX ? UINT_MAX :
> +				(unsigned int)sym.st_size;
> +			num_sym_starts++;
> +		}
> +	}
> +
> +	sort_starts(sym_starts, &num_sym_starts);
> +	sort_starts(text_starts, &num_text_starts);
> +
> +	/*
> +	 * An unsized label reaches the next symbol, and a sequence beginning
> +	 * there cannot run past it.
> +	 */
> +	for (unsigned int i = 1; i < num_text_starts; i++) {
> +		if (!text_starts[i - 1].size)
> +			text_starts[i - 1].size = text_starts[i].offset -
> +						  text_starts[i - 1].offset;
> +	}
> +}

[Severity: Medium]
Does excluding STT_NOTYPE and unsized symbols from sym_starts[] leave stack
frames in those symbols without lineinfo annotations?

At runtime, __sprint_symbol() resolves kallsyms symbols regardless of whether
they are STT_FUNC or STT_NOTYPE, and kallsyms_lookup_lineinfo() rejects any
entry below sym_start - _text.  However, sym_starts[] excludes STT_NOTYPE
symbols (such as Clang .cold split subfunctions, SYM_CODE_START assembly
routines, SYM_INNER_LABEL entry points like entry_SYSCALL_64_after_hwframe,
and CONFIG_PROPELLER_CLANG basic-block symbols) and never backfills zero
st_size values.  As a result, synthesize_symbol_starts() does not copy
covering rows to their start addresses and deduplicate() can drop entries
at their start when the file and line match the previous entry, causing
kallsyms_lookup_lineinfo() to fail the min_offset boundary check.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106132432.248387-1-sashal@kernel.org?part=1