Message-Id: <20240723063258.2240610-2-zhengyejian@huaweicloud.com>
Date: Tue, 23 Jul 2024 14:32:54 +0800
From: Zheng Yejian <zhengyejian@...weicloud.com>
To: masahiroy@...nel.org,
	peterz@...radead.org,
	rostedt@...dmis.org,
	mhiramat@...nel.org,
	mark.rutland@....com,
	mpe@...erman.id.au,
	npiggin@...il.com,
	christophe.leroy@...roup.eu,
	naveen.n.rao@...ux.ibm.com,
	tglx@...utronix.de,
	mingo@...hat.com,
	bp@...en8.de,
	dave.hansen@...ux.intel.com,
	hpa@...or.com,
	mcgrof@...nel.org,
	mathieu.desnoyers@...icios.com,
	nathan@...nel.org,
	nicolas@...sle.eu,
	ojeda@...nel.org,
	akpm@...ux-foundation.org,
	surenb@...gle.com,
	pasha.tatashin@...een.com,
	kent.overstreet@...ux.dev,
	james.clark@....com,
	jpoimboe@...nel.org
Cc: x86@...nel.org,
	linux-kernel@...r.kernel.org,
	linux-trace-kernel@...r.kernel.org,
	linuxppc-dev@...ts.ozlabs.org,
	linux-modules@...r.kernel.org,
	linux-kbuild@...r.kernel.org,
	bpf@...r.kernel.org,
	zhengyejian@...weicloud.com
Subject: [PATCH v2 1/5] kallsyms: Emit symbol at the holes in the text

When a weak type function is overridden, its symbol will be removed
from the symbol table, but its code will not be removed. Besides,
due to lacking of size for kallsyms, kernel compute function size by
substracting its symbol address from its next symbol address (see
kallsyms_lookup_size_offset()). These will cause that size of some
function is computed to be larger than it actually is, just because
symbol of its following weak function is removed.

This issue also causes multiple __fentry__ locations to be counted in
the same function scope, and eventually causes ftrace_location() to find
wrong __fentry__ location. It was reported in
Link: https://lore.kernel.org/all/20240607115211.734845-1-zhengyejian1@huawei.com/

Peter suggested to change scipts/kallsyms.c to emit readily
identifiable symbol names for all the weak junk.

So in this patch:
1. Pass size info to scripts/kallsyms  (see mksysmap());
2. Traverse sorted function symbols, if one function address plus its
   size less than next function address, it means there's a hole, with
   Masahiro's suggestion, then emit a symbol there of which type and
   name are both empty to represent the hole.

Suggested-by: Peter Zijlstra <peterz@...radead.org>
Suggested-by: Masahiro Yamada <masahiroy@...nel.org>
Signed-off-by: Zheng Yejian <zhengyejian@...weicloud.com>
---
 scripts/kallsyms.c      | 94 +++++++++++++++++++++++++++++++++++++++--
 scripts/link-vmlinux.sh |  4 +-
 scripts/mksysmap        |  2 +-
 3 files changed, 94 insertions(+), 6 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 47978efe4797..cf64c20a8292 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -34,6 +34,7 @@
 
 struct sym_entry {
 	unsigned long long addr;
+	unsigned long long size;
 	unsigned int len;
 	unsigned int seq;
 	unsigned int start_pos;
@@ -72,7 +73,6 @@ static int token_profit[0x10000];
 static unsigned char best_table[256][2];
 static unsigned char best_table_len[256];
 
-
 static void usage(void)
 {
 	fprintf(stderr, "Usage: kallsyms [--all-symbols] [--absolute-percpu] "
@@ -128,8 +128,16 @@ static struct sym_entry *read_symbol(FILE *in, char **buf, size_t *buf_len)
 	size_t len;
 	ssize_t readlen;
 	struct sym_entry *sym;
+	unsigned long long size = 0;
 
 	errno = 0;
+	/*
+	 * Example of expected symbol format:
+	 * 1. symbol with size info:
+	 *    ffffffff81000070 00000000000001d7 T __startup_64
+	 * 2. symbol without size info:
+	 *    0000000002a00000 A text_size
+	 */
 	readlen = getline(buf, buf_len, in);
 	if (readlen < 0) {
 		if (errno) {
@@ -143,9 +151,24 @@ static struct sym_entry *read_symbol(FILE *in, char **buf, size_t *buf_len)
 		(*buf)[readlen - 1] = 0;
 
 	addr = strtoull(*buf, &p, 16);
+	if (*buf == p || *p++ != ' ') {
+		fprintf(stderr, "line format error: unable to parse address\n");
+		exit(EXIT_FAILURE);
+	}
+
+	if (*p == '0') {
+		char *str = p;
+
+		size = strtoull(str, &p, 16);
+		if (str == p || *p++ != ' ') {
+			fprintf(stderr, "line format error: unable to parse size\n");
+			exit(EXIT_FAILURE);
+		}
+	}
 
-	if (*buf == p || *p++ != ' ' || !isascii((type = *p++)) || *p++ != ' ') {
-		fprintf(stderr, "line format error\n");
+	type = *p++;
+	if (!isascii(type) || *p++ != ' ') {
+		fprintf(stderr, "line format error: unable to parse type\n");
 		exit(EXIT_FAILURE);
 	}
 
@@ -180,6 +203,7 @@ static struct sym_entry *read_symbol(FILE *in, char **buf, size_t *buf_len)
 		exit(EXIT_FAILURE);
 	}
 	sym->addr = addr;
+	sym->size = size;
 	sym->len = len;
 	sym->sym[0] = type;
 	strcpy(sym_name(sym), name);
@@ -788,6 +812,69 @@ static void sort_symbols(void)
 	qsort(table, table_cnt, sizeof(table[0]), compare_symbols);
 }
 
+static bool has_hole(const struct sym_entry *se1, const struct sym_entry *se2)
+{
+	char type = se1->sym[0];
+
+	/* Only check text symbol or weak symbol */
+	if (type != 't' && type != 'T' &&
+	    type != 'w' && type != 'W')
+		return 0;
+	/* Symbol without size has no hole */
+	if (!se1->size)
+		return 0;
+	return se1->addr + se1->size < se2->addr;
+}
+
+static struct sym_entry *gen_hole_symbol(const struct sym_entry *se)
+{
+	struct sym_entry *sym;
+
+	/* Use empty symbol type/name as a special case to represent the hole */
+	sym = malloc(sizeof(*sym) + 1);
+	if (!sym) {
+		fprintf(stderr, "unable to allocate memory for hole symbol\n");
+		exit(EXIT_FAILURE);
+	}
+	sym->addr = se->addr + se->size;
+	sym->size = 0;
+	sym->len = 1;
+	sym->sym[0] = '\0';
+	sym->percpu_absolute = 0;
+	return sym;
+}
+
+static void emit_hole_symbols(void)
+{
+	unsigned int i, j, nr_emit;
+	unsigned int new_cnt;
+
+	nr_emit = 0;
+	for (i = 0; i < table_cnt - 1; i++) {
+		if (has_hole(table[i], table[i+1]))
+			nr_emit++;
+	}
+	if (!nr_emit)
+		return;
+
+	new_cnt = table_cnt + nr_emit;
+	table = realloc(table, sizeof(*table) * new_cnt);
+	if (!table) {
+		fprintf(stderr, "unable to allocate memory for emitting hole symbols\n");
+		exit(EXIT_FAILURE);
+	}
+
+	for (i = table_cnt - 1, j = new_cnt - 1; i >= 0; i--, j--) {
+		if ((i != table_cnt - 1) && has_hole(table[i], table[i+1]))
+			table[j--] = gen_hole_symbol(table[i]);
+		if (j != i)
+			table[j] = table[i];
+		else
+			break;
+	}
+	table_cnt = new_cnt;
+}
+
 static void make_percpus_absolute(void)
 {
 	unsigned int i;
@@ -847,6 +934,7 @@ int main(int argc, char **argv)
 	if (absolute_percpu)
 		make_percpus_absolute();
 	sort_symbols();
+	emit_hole_symbols();
 	if (base_relative)
 		record_relative_base();
 	optimize_token_table();
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 518c70b8db50..8e1373902bfe 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -189,11 +189,11 @@ kallsyms_step()
 }
 
 # Create map file with all symbols from ${1}
-# See mksymap for additional details
+# See mksysmap for additional details
 mksysmap()
 {
 	info NM ${2}
-	${NM} -n "${1}" | sed -f "${srctree}/scripts/mksysmap" > "${2}"
+	${NM} -nS "${1}" | sed -f "${srctree}/scripts/mksysmap" > "${2}"
 }
 
 sorttable()
diff --git a/scripts/mksysmap b/scripts/mksysmap
index c12723a04655..7a4415f21143 100755
--- a/scripts/mksysmap
+++ b/scripts/mksysmap
@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 #
 # sed script to filter out symbols that are not needed for System.map,
-# or not suitable for kallsyms. The input should be 'nm -n <file>'.
+# or not suitable for kallsyms. The input should be 'nm -nS <file>'.
 #
 # System.map is used by module-init tools and some debugging
 # tools to retrieve the actual addresses of symbols in the kernel.
-- 
2.25.1

Powered by blists - more mailing lists