2020-02-02 05:11:37

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 1/3] scripts/kallsyms: rename local variables in read_symbol()

I will use 'sym' for the point to struce sym_entry in the next commit.
Rename 'sym', 'stype' to 'name', 'type', which are more intuitive.

Signed-off-by: Masahiro Yamada <[email protected]>
---

scripts/kallsyms.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 94153732ec00..5c34edd98b3e 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -176,43 +176,43 @@ static void check_symbol_range(const char *sym, unsigned long long addr,

static int read_symbol(FILE *in, struct sym_entry *s)
{
- char sym[500], stype;
+ char name[500], type;
int rc;

- rc = fscanf(in, "%llx %c %499s\n", &s->addr, &stype, sym);
+ rc = fscanf(in, "%llx %c %499s\n", &s->addr, &type, name);
if (rc != 3) {
- if (rc != EOF && fgets(sym, 500, in) == NULL)
+ if (rc != EOF && fgets(name, 500, in) == NULL)
fprintf(stderr, "Read error or end of file.\n");
return -1;
}
- if (strlen(sym) >= KSYM_NAME_LEN) {
+ if (strlen(name) >= KSYM_NAME_LEN) {
fprintf(stderr, "Symbol %s too long for kallsyms (%zu >= %d).\n"
"Please increase KSYM_NAME_LEN both in kernel and kallsyms.c\n",
- sym, strlen(sym), KSYM_NAME_LEN);
+ name, strlen(name), KSYM_NAME_LEN);
return -1;
}

- if (is_ignored_symbol(sym, stype))
+ if (is_ignored_symbol(name, type))
return -1;

/* Ignore most absolute/undefined (?) symbols. */
- if (strcmp(sym, "_text") == 0)
+ if (strcmp(name, "_text") == 0)
_text = s->addr;

- check_symbol_range(sym, s->addr, text_ranges, ARRAY_SIZE(text_ranges));
- check_symbol_range(sym, s->addr, &percpu_range, 1);
+ check_symbol_range(name, s->addr, text_ranges, ARRAY_SIZE(text_ranges));
+ check_symbol_range(name, s->addr, &percpu_range, 1);

/* include the type field in the symbol name, so that it gets
* compressed together */
- s->len = strlen(sym) + 1;
+ s->len = strlen(name) + 1;
s->sym = malloc(s->len + 1);
if (!s->sym) {
fprintf(stderr, "kallsyms failure: "
"unable to allocate required amount of memory\n");
exit(EXIT_FAILURE);
}
- strcpy(sym_name(s), sym);
- s->sym[0] = stype;
+ strcpy(sym_name(s), name);
+ s->sym[0] = type;

s->percpu_absolute = 0;

--
2.17.1


2020-02-02 05:11:37

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[]

kallsyms_token_table[] only contains ASCII characters. It should be
char instead of u8.

Signed-off-by: Masahiro Yamada <[email protected]>
---

kernel/kallsyms.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 136ce049c4ad..53f84f685841 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -44,7 +44,7 @@ __attribute__((weak, section(".rodata")));
extern const unsigned long kallsyms_relative_base
__attribute__((weak, section(".rodata")));

-extern const u8 kallsyms_token_table[] __weak;
+extern const char kallsyms_token_table[] __weak;
extern const u16 kallsyms_token_index[] __weak;

extern const unsigned int kallsyms_markers[] __weak;
@@ -58,7 +58,8 @@ static unsigned int kallsyms_expand_symbol(unsigned int off,
char *result, size_t maxlen)
{
int len, skipped_first = 0;
- const u8 *tptr, *data;
+ const char *tptr;
+ const u8 *data;

/* Get the compressed symbol length from the first symbol byte. */
data = &kallsyms_names[off];
--
2.17.1

2020-02-02 05:11:37

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 2/3] scripts/kallsyms: change table to store (strcut sym_entry *)

The symbol table is extended every 10000 addition by using realloc(),
where data copy might occur to the new buffer.

To decrease the amount of possible data copy, let's change the table
to store the pointer.

The symbol type + symbol name part is appended at the end of
(struct sym_entry), and allocated together with the struct body.

Signed-off-by: Masahiro Yamada <[email protected]>
---

scripts/kallsyms.c | 121 ++++++++++++++++++++++++---------------------
1 file changed, 65 insertions(+), 56 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 5c34edd98b3e..a566d8201b56 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -33,8 +33,8 @@ struct sym_entry {
unsigned long long addr;
unsigned int len;
unsigned int start_pos;
- unsigned char *sym;
unsigned int percpu_absolute;
+ unsigned char sym[0];
};

struct addr_range {
@@ -55,7 +55,7 @@ static struct addr_range percpu_range = {
"__per_cpu_start", "__per_cpu_end", -1ULL, 0
};

-static struct sym_entry *table;
+static struct sym_entry **table;
static unsigned int table_size, table_cnt;
static int all_symbols;
static int absolute_percpu;
@@ -174,49 +174,55 @@ static void check_symbol_range(const char *sym, unsigned long long addr,
}
}

-static int read_symbol(FILE *in, struct sym_entry *s)
+static struct sym_entry *read_symbol(FILE *in)
{
char name[500], type;
+ unsigned long long addr;
+ unsigned int len;
+ struct sym_entry *sym;
int rc;

- rc = fscanf(in, "%llx %c %499s\n", &s->addr, &type, name);
+ rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name);
if (rc != 3) {
if (rc != EOF && fgets(name, 500, in) == NULL)
fprintf(stderr, "Read error or end of file.\n");
- return -1;
+ return NULL;
}
if (strlen(name) >= KSYM_NAME_LEN) {
fprintf(stderr, "Symbol %s too long for kallsyms (%zu >= %d).\n"
"Please increase KSYM_NAME_LEN both in kernel and kallsyms.c\n",
name, strlen(name), KSYM_NAME_LEN);
- return -1;
+ return NULL;
}

if (is_ignored_symbol(name, type))
- return -1;
+ return NULL;

/* Ignore most absolute/undefined (?) symbols. */
if (strcmp(name, "_text") == 0)
- _text = s->addr;
+ _text = addr;

- check_symbol_range(name, s->addr, text_ranges, ARRAY_SIZE(text_ranges));
- check_symbol_range(name, s->addr, &percpu_range, 1);
+ check_symbol_range(name, addr, text_ranges, ARRAY_SIZE(text_ranges));
+ check_symbol_range(name, addr, &percpu_range, 1);

/* include the type field in the symbol name, so that it gets
* compressed together */
- s->len = strlen(name) + 1;
- s->sym = malloc(s->len + 1);
- if (!s->sym) {
+
+ len = strlen(name) + 1;
+
+ sym = malloc(sizeof(*sym) + len);
+ if (!sym) {
fprintf(stderr, "kallsyms failure: "
"unable to allocate required amount of memory\n");
exit(EXIT_FAILURE);
}
- strcpy(sym_name(s), name);
- s->sym[0] = type;
-
- s->percpu_absolute = 0;
+ sym->addr = addr;
+ sym->len = len;
+ sym->sym[0] = type;
+ memcpy(sym_name(sym), name, len);
+ sym->percpu_absolute = 0;

- return 0;
+ return sym;
}

static int symbol_in_range(const struct sym_entry *s,
@@ -268,12 +274,12 @@ static void shrink_table(void)

pos = 0;
for (i = 0; i < table_cnt; i++) {
- if (symbol_valid(&table[i])) {
+ if (symbol_valid(table[i])) {
if (pos != i)
table[pos] = table[i];
pos++;
} else {
- free(table[i].sym);
+ free(table[i]);
}
}
table_cnt = pos;
@@ -287,7 +293,15 @@ static void shrink_table(void)

static void read_map(FILE *in)
{
+ struct sym_entry *sym;
+
while (!feof(in)) {
+ sym = read_symbol(in);
+ if (!sym)
+ continue;
+
+ sym->start_pos = table_cnt;
+
if (table_cnt >= table_size) {
table_size += 10000;
table = realloc(table, sizeof(*table) * table_size);
@@ -296,10 +310,8 @@ static void read_map(FILE *in)
exit (1);
}
}
- if (read_symbol(in, &table[table_cnt]) == 0) {
- table[table_cnt].start_pos = table_cnt;
- table_cnt++;
- }
+
+ table[table_cnt++] = sym;
}
}

@@ -387,27 +399,27 @@ static void write_src(void)
int overflow;

if (!absolute_percpu) {
- offset = table[i].addr - relative_base;
+ offset = table[i]->addr - relative_base;
overflow = (offset < 0 || offset > UINT_MAX);
- } else if (symbol_absolute(&table[i])) {
- offset = table[i].addr;
+ } else if (symbol_absolute(table[i])) {
+ offset = table[i]->addr;
overflow = (offset < 0 || offset > INT_MAX);
} else {
- offset = relative_base - table[i].addr - 1;
+ offset = relative_base - table[i]->addr - 1;
overflow = (offset < INT_MIN || offset >= 0);
}
if (overflow) {
fprintf(stderr, "kallsyms failure: "
"%s symbol value %#llx out of range in relative mode\n",
- symbol_absolute(&table[i]) ? "absolute" : "relative",
- table[i].addr);
+ symbol_absolute(table[i]) ? "absolute" : "relative",
+ table[i]->addr);
exit(EXIT_FAILURE);
}
printf("\t.long\t%#x\n", (int)offset);
- } else if (!symbol_absolute(&table[i])) {
- output_address(table[i].addr);
+ } else if (!symbol_absolute(table[i])) {
+ output_address(table[i]->addr);
} else {
- printf("\tPTR\t%#llx\n", table[i].addr);
+ printf("\tPTR\t%#llx\n", table[i]->addr);
}
}
printf("\n");
@@ -437,12 +449,12 @@ static void write_src(void)
if ((i & 0xFF) == 0)
markers[i >> 8] = off;

- printf("\t.byte 0x%02x", table[i].len);
- for (k = 0; k < table[i].len; k++)
- printf(", 0x%02x", table[i].sym[k]);
+ printf("\t.byte 0x%02x", table[i]->len);
+ for (k = 0; k < table[i]->len; k++)
+ printf(", 0x%02x", table[i]->sym[k]);
printf("\n");

- off += table[i].len + 1;
+ off += table[i]->len + 1;
}
printf("\n");

@@ -496,7 +508,7 @@ static void build_initial_tok_table(void)
unsigned int i;

for (i = 0; i < table_cnt; i++)
- learn_symbol(table[i].sym, table[i].len);
+ learn_symbol(table[i]->sym, table[i]->len);
}

static unsigned char *find_token(unsigned char *str, int len,
@@ -520,15 +532,15 @@ static void compress_symbols(const unsigned char *str, int idx)

for (i = 0; i < table_cnt; i++) {

- len = table[i].len;
- p1 = table[i].sym;
+ len = table[i]->len;
+ p1 = table[i]->sym;

/* find the token on the symbol */
p2 = find_token(p1, len, str);
if (!p2) continue;

/* decrease the counts for this symbol's tokens */
- forget_symbol(table[i].sym, len);
+ forget_symbol(table[i]->sym, len);

size = len;

@@ -547,10 +559,10 @@ static void compress_symbols(const unsigned char *str, int idx)

} while (p2);

- table[i].len = len;
+ table[i]->len = len;

/* increase the counts for this symbol's new tokens */
- learn_symbol(table[i].sym, len);
+ learn_symbol(table[i]->sym, len);
}
}

@@ -606,8 +618,8 @@ static void insert_real_symbols_in_table(void)
unsigned int i, j, c;

for (i = 0; i < table_cnt; i++) {
- for (j = 0; j < table[i].len; j++) {
- c = table[i].sym[j];
+ for (j = 0; j < table[i]->len; j++) {
+ c = table[i]->sym[j];
best_table[c][0]=c;
best_table_len[c]=1;
}
@@ -660,13 +672,10 @@ static int may_be_linker_script_provide_symbol(const struct sym_entry *se)

static int compare_symbols(const void *a, const void *b)
{
- const struct sym_entry *sa;
- const struct sym_entry *sb;
+ const struct sym_entry *sa = *(const struct sym_entry **)a;
+ const struct sym_entry *sb = *(const struct sym_entry **)b;
int wa, wb;

- sa = a;
- sb = b;
-
/* sort by address first */
if (sa->addr > sb->addr)
return 1;
@@ -697,7 +706,7 @@ static int compare_symbols(const void *a, const void *b)

static void sort_symbols(void)
{
- qsort(table, table_cnt, sizeof(struct sym_entry), compare_symbols);
+ qsort(table, table_cnt, sizeof(table[0]), compare_symbols);
}

static void make_percpus_absolute(void)
@@ -705,14 +714,14 @@ static void make_percpus_absolute(void)
unsigned int i;

for (i = 0; i < table_cnt; i++)
- if (symbol_in_range(&table[i], &percpu_range, 1)) {
+ if (symbol_in_range(table[i], &percpu_range, 1)) {
/*
* Keep the 'A' override for percpu symbols to
* ensure consistent behavior compared to older
* versions of this tool.
*/
- table[i].sym[0] = 'A';
- table[i].percpu_absolute = 1;
+ table[i]->sym[0] = 'A';
+ table[i]->percpu_absolute = 1;
}
}

@@ -722,12 +731,12 @@ static void record_relative_base(void)
unsigned int i;

for (i = 0; i < table_cnt; i++)
- if (!symbol_absolute(&table[i])) {
+ if (!symbol_absolute(table[i])) {
/*
* The table is sorted by address.
* Take the first non-absolute symbol value.
*/
- relative_base = table[i].addr;
+ relative_base = table[i]->addr;
return;
}
}
--
2.17.1

2020-02-03 01:34:25

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[]

On Sun, 2 Feb 2020 14:09:22 +0900
Masahiro Yamada <[email protected]> wrote:

> kallsyms_token_table[] only contains ASCII characters. It should be
> char instead of u8.

Indeed.

Reviewed-by: Masami Hiramatsu <[email protected]>

Thank you,

>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> kernel/kallsyms.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index 136ce049c4ad..53f84f685841 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -44,7 +44,7 @@ __attribute__((weak, section(".rodata")));
> extern const unsigned long kallsyms_relative_base
> __attribute__((weak, section(".rodata")));
>
> -extern const u8 kallsyms_token_table[] __weak;
> +extern const char kallsyms_token_table[] __weak;
> extern const u16 kallsyms_token_index[] __weak;
>
> extern const unsigned int kallsyms_markers[] __weak;
> @@ -58,7 +58,8 @@ static unsigned int kallsyms_expand_symbol(unsigned int off,
> char *result, size_t maxlen)
> {
> int len, skipped_first = 0;
> - const u8 *tptr, *data;
> + const char *tptr;
> + const u8 *data;
>
> /* Get the compressed symbol length from the first symbol byte. */
> data = &kallsyms_names[off];
> --
> 2.17.1
>


--
Masami Hiramatsu <[email protected]>

2020-02-03 04:19:28

by Justin Capella

[permalink] [raw]
Subject: Re: [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[]

unsigned char maybe? I understand printable chars unlikely to cause
any trouble and probably a non issue, but the domain is different for
char, and while I don't know of supported implementations where sizeof
char isn't a byte, I think it's a possibility and perhaps why u8 was
in use?

2020-02-03 07:29:37

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 3/3] kallsyms: fix type of kallsyms_token_table[]

On Mon, Feb 3, 2020 at 12:26 PM Justin Capella <[email protected]> wrote:
>
> unsigned char maybe? I understand printable chars unlikely to cause
> any trouble and probably a non issue, but the domain is different for
> char, and while I don't know of supported implementations where sizeof
> char isn't a byte, I think it's a possibility and perhaps why u8 was
> in use?


As I said in the commit description,
this only contains ASCII, which is 7-bit code.

We do not care the signed-ness of char.


kallsyms_names[] has been u8 since the
following old commit:


commit e10392112d315c45f054c22c862e3a7ae27d17d4
Author: Paulo Marques <[email protected]>
Date: Mon Oct 18 17:59:03 2004 -0700

[PATCH] kallsyms data size reduction / lookup speedup



Probably, this is just coding mistake.



--
Best Regards
Masahiro Yamada