* [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc
@ 2023-11-01 15:03 Masahiro Yamada
2023-11-01 15:03 ` [PATCH 1/7] modpost: move sym_name() to modpost.h Masahiro Yamada
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Masahiro Yamada @ 2023-11-01 15:03 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Greg Ungerer, Jack Brennen, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier
Greg Ungerer reports building with m68k-uclinux-gcc toolchain is broken:
https://lore.kernel.org/linux-kbuild/CAK7LNASQ_W5Yva5a4Xx8E2EYi-tN7x3OHgMFhK+93W+BiX1=9Q@mail.gmail.com/T/#m6ff0364f9ca8483c9f6d162619e5005833d1e887
Usually, we do not need to search for export symbols in the .symtab
section, but m68k-uclinux-gcc seems to be an exceptional case.
I do not know what makes it different from other toolchains.
Also, I do not know there exist other toolchains that work like that.
This series extends the symsearch feature in case we need to explicitly
search for export symbols.
Then, the last patch fixes the issue.
This series should be applicable for linux-next.
This series is too late for the current merge window, but I'd like
to fix the issue somehow by the next merge window.
Masahiro Yamada (7):
modpost: move sym_name() to modpost.h
modpost: add const qualifier to syminfo table
modpost: add table_size local variable to symsearch_find_nearest()
modpost: introduce a filtering feature to symsearch
modpost: prefer global symbols in symsearch_find_nearest()
modpost: add symsearch_find_with_name() helper function
modpost: look up the correct symbol in check_export_symbol()
scripts/mod/modpost.c | 39 ++++----
scripts/mod/modpost.h | 12 +++
scripts/mod/symsearch.c | 205 ++++++++++++++++++++++++++++------------
3 files changed, 180 insertions(+), 76 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/7] modpost: move sym_name() to modpost.h
2023-11-01 15:03 [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Masahiro Yamada
@ 2023-11-01 15:03 ` Masahiro Yamada
2023-11-01 15:03 ` [PATCH 2/7] modpost: add const qualifier to syminfo table Masahiro Yamada
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2023-11-01 15:03 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Greg Ungerer, Jack Brennen, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier
Move sym_name() to modpost.h so it can be used in other source files.
Also, add the 'const' qualifier to the function arguments.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
scripts/mod/modpost.c | 8 --------
scripts/mod/modpost.h | 9 +++++++++
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 973b5e5ae2dd..896ecfa8483f 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -710,14 +710,6 @@ static char *get_modinfo(struct elf_info *info, const char *tag)
return get_next_modinfo(info, tag, NULL);
}
-static const char *sym_name(struct elf_info *elf, Elf_Sym *sym)
-{
- if (sym)
- return elf->strtab + sym->st_name;
- else
- return "(unknown)";
-}
-
/*
* Check whether the 'string' argument matches one of the 'patterns',
* an array of shell wildcard patterns (glob).
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 1392afec118c..9834ac44846d 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -156,6 +156,15 @@ static inline unsigned int get_secindex(const struct elf_info *info,
return index;
}
+static inline const char *sym_name(const struct elf_info *elf,
+ const Elf_Sym *sym)
+{
+ if (sym)
+ return elf->strtab + sym->st_name;
+ else
+ return "(unknown)";
+}
+
/*
* If there's no name there, ignore it; likewise, ignore it if it's
* one of the magic symbols emitted used by current tools.
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/7] modpost: add const qualifier to syminfo table
2023-11-01 15:03 [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Masahiro Yamada
2023-11-01 15:03 ` [PATCH 1/7] modpost: move sym_name() to modpost.h Masahiro Yamada
@ 2023-11-01 15:03 ` Masahiro Yamada
2023-11-01 15:04 ` [PATCH 3/7] modpost: add table_size local variable to symsearch_find_nearest() Masahiro Yamada
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2023-11-01 15:03 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Greg Ungerer, Jack Brennen, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier
symsearch_find_nearest() does not modify the table.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
scripts/mod/symsearch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/mod/symsearch.c b/scripts/mod/symsearch.c
index aa4ed51f9960..00f0f9c354db 100644
--- a/scripts/mod/symsearch.c
+++ b/scripts/mod/symsearch.c
@@ -156,7 +156,7 @@ Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr,
{
unsigned int hi = elf->symsearch->table_size;
unsigned int lo = 0;
- struct syminfo *table = elf->symsearch->table;
+ const struct syminfo *table = elf->symsearch->table;
struct syminfo target;
target.addr = addr;
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/7] modpost: add table_size local variable to symsearch_find_nearest()
2023-11-01 15:03 [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Masahiro Yamada
2023-11-01 15:03 ` [PATCH 1/7] modpost: move sym_name() to modpost.h Masahiro Yamada
2023-11-01 15:03 ` [PATCH 2/7] modpost: add const qualifier to syminfo table Masahiro Yamada
@ 2023-11-01 15:04 ` Masahiro Yamada
2023-11-01 15:04 ` [PATCH 4/7] modpost: introduce a filtering feature to symsearch Masahiro Yamada
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2023-11-01 15:04 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Greg Ungerer, Jack Brennen, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier
Keep consistency with 'table', and make the conditional part slightly
shorter.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
scripts/mod/symsearch.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/mod/symsearch.c b/scripts/mod/symsearch.c
index 00f0f9c354db..97566aee0979 100644
--- a/scripts/mod/symsearch.c
+++ b/scripts/mod/symsearch.c
@@ -154,9 +154,10 @@ Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr,
unsigned int secndx, bool allow_negative,
Elf_Addr min_distance)
{
- unsigned int hi = elf->symsearch->table_size;
- unsigned int lo = 0;
const struct syminfo *table = elf->symsearch->table;
+ unsigned int table_size = elf->symsearch->table_size;
+ unsigned int hi = table_size;
+ unsigned int lo = 0;
struct syminfo target;
target.addr = addr;
@@ -183,8 +184,7 @@ Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr,
*/
Elf_Sym *result = NULL;
- if (allow_negative &&
- hi < elf->symsearch->table_size &&
+ if (allow_negative && hi < table_size &&
table[hi].section_index == secndx &&
table[hi].addr - addr <= min_distance) {
min_distance = table[hi].addr - addr;
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/7] modpost: introduce a filtering feature to symsearch
2023-11-01 15:03 [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Masahiro Yamada
` (2 preceding siblings ...)
2023-11-01 15:04 ` [PATCH 3/7] modpost: add table_size local variable to symsearch_find_nearest() Masahiro Yamada
@ 2023-11-01 15:04 ` Masahiro Yamada
2023-11-01 15:04 ` [PATCH 5/7] modpost: prefer global symbols in symsearch_find_nearest() Masahiro Yamada
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2023-11-01 15:04 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Greg Ungerer, Jack Brennen, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier
If adjacent table entries have the same section index and address,
symsearch_fixup() modifies the entries so the symbol lookup returns
the first symbol entry in the original .symtab section, but it may
not be the optimal result.
Add the filter() callback for more flexible symbol selection.
After the binary search is finished, a linear search begins to determine
the best symbol. Typically, the one found in the first iteration is the
closest, but the linear search continues as long as it sees another
symbol on the same distance. In each iteration, filter() is called to
determine if the current symbol should be taken.
Here are some useful scenarios:
- When multiple entries share the same section index and address,
filter() can be used to break a tie.
- When there is an unwanted symbol depending on the search context,
filter() can return false to skip it.
Currently, there is one hard-coded policy: if the target address falls
perfectly in the middle of the two neighbors, the lower address is
preferred. Let's move this preference to the filter function because it
is not directly related to the binary search algorithm.
This commit does not introduce any functional change, but more useful
filtering policies will be added in subsequent commits.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
scripts/mod/symsearch.c | 102 ++++++++++++++++++++++++++++++----------
1 file changed, 77 insertions(+), 25 deletions(-)
diff --git a/scripts/mod/symsearch.c b/scripts/mod/symsearch.c
index 97566aee0979..4549c5b0bb81 100644
--- a/scripts/mod/symsearch.c
+++ b/scripts/mod/symsearch.c
@@ -5,6 +5,8 @@
* to a given address.
*/
+#include <stdbool.h>
+
#include "modpost.h"
struct syminfo {
@@ -142,17 +144,11 @@ void symsearch_finish(struct elf_info *elf)
elf->symsearch = NULL;
}
-/*
- * Find the syminfo which is in secndx and "nearest" to addr.
- * allow_negative: allow returning a symbol whose address is > addr.
- * min_distance: ignore symbols which are further away than this.
- *
- * Returns a pointer into the symbol table for success.
- * Returns NULL if no legal symbol is found within the requested range.
- */
-Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr,
- unsigned int secndx, bool allow_negative,
- Elf_Addr min_distance)
+static Elf_Sym *symsearch_find(struct elf_info *elf, Elf_Addr addr,
+ unsigned int secndx, bool allow_negative,
+ Elf_Addr min_distance,
+ bool (*filter)(const Elf_Sym *, const Elf_Sym *, void *),
+ void *filter_data)
{
const struct syminfo *table = elf->symsearch->table;
unsigned int table_size = elf->symsearch->table_size;
@@ -178,22 +174,78 @@ Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr,
* entry in the array which comes before target, including the
* case where it perfectly matches the section and the address.
*
- * Note -- if the address we're looking up falls perfectly
- * in the middle of two symbols, this is written to always
- * prefer the symbol with the lower address.
+ * If there are multiple candidates, the filter() callback can be used
+ * to break a tie. filter() is provided with the current symbol and the
+ * best one so far. If it returns true, the current one is selected.
+ * Only a few iterations are expected, hence the linear search is fine.
*/
- Elf_Sym *result = NULL;
+ Elf_Addr distance;
+ Elf_Sym *best = NULL;
+ Elf_Sym *sym;
+ int i;
- if (allow_negative && hi < table_size &&
- table[hi].section_index == secndx &&
- table[hi].addr - addr <= min_distance) {
- min_distance = table[hi].addr - addr;
- result = &elf->symtab_start[table[hi].symbol_index];
+ /* Search to the left. */
+ for (i = hi - 1; i >= 0; i--) {
+ if (table[i].section_index != secndx)
+ break;
+
+ distance = addr - table[i].addr;
+ if (distance > min_distance)
+ break;
+
+ sym = &elf->symtab_start[table[i].symbol_index];
+ if (filter(sym, best, filter_data)) {
+ min_distance = distance;
+ best = sym;
+ }
}
- if (hi > 0 &&
- table[hi - 1].section_index == secndx &&
- addr - table[hi - 1].addr <= min_distance) {
- result = &elf->symtab_start[table[hi - 1].symbol_index];
+
+ if (!allow_negative)
+ return best;
+
+ /* Search to the right if allow_negative is true. */
+ for (i = hi; i < table_size; i++) {
+ if (table[i].section_index != secndx)
+ break;
+
+ distance = table[i].addr - addr;
+ if (distance > min_distance)
+ break;
+
+ sym = &elf->symtab_start[table[i].symbol_index];
+ if (filter(sym, best, filter_data)) {
+ min_distance = distance;
+ best = sym;
+ }
}
- return result;
+
+ return best;
+}
+
+/* Return true if sym1 is preferred over sym2. */
+static bool symsearch_nearest_filter(const Elf_Sym *sym1, const Elf_Sym *sym2,
+ void *data)
+{
+ /* If sym2 is NULL, this is the first occurrence, always take it. */
+ if (sym2 == NULL)
+ return true;
+
+ /* Prefer lower address. */
+ return sym1->st_value < sym2->st_value;
+}
+
+/*
+ * Find the syminfo which is in secndx and "nearest" to addr.
+ * allow_negative: allow returning a symbol whose address is > addr.
+ * min_distance: ignore symbols which are further away than this.
+ *
+ * Returns a pointer into the symbol table for success.
+ * Returns NULL if no legal symbol is found within the requested range.
+ */
+Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr,
+ unsigned int secndx, bool allow_negative,
+ Elf_Addr min_distance)
+{
+ return symsearch_find(elf, addr, secndx, allow_negative, min_distance,
+ symsearch_nearest_filter, NULL);
}
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/7] modpost: prefer global symbols in symsearch_find_nearest()
2023-11-01 15:03 [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Masahiro Yamada
` (3 preceding siblings ...)
2023-11-01 15:04 ` [PATCH 4/7] modpost: introduce a filtering feature to symsearch Masahiro Yamada
@ 2023-11-01 15:04 ` Masahiro Yamada
2023-11-01 15:04 ` [PATCH 6/7] modpost: add symsearch_find_with_name() helper function Masahiro Yamada
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2023-11-01 15:04 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Greg Ungerer, Jack Brennen, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier
When there are multiple symbols that share the same section index and
address, symsearch_find_nearest() returns the first occurrence in the
original .symtab section. We can add more rules to break a tie based
on symbol attributes.
Kallsyms does this; compare_symbols() in scripts/kallsyms.c first sorts
symbols by address, then by weakness and by underscore-prefixing in
order to provide users with the most desirable symbol.
This commit gives the following preference, in this order:
1. lower address
2. global symbol
3. no underscore prefix
If two symbols still tie, the first one encounterd in the linear search
is selected. This does not match the order in the original .symtab
section, but it is not a significant issue.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
scripts/mod/symsearch.c | 57 +++++++++++++++++------------------------
1 file changed, 24 insertions(+), 33 deletions(-)
diff --git a/scripts/mod/symsearch.c b/scripts/mod/symsearch.c
index 4549c5b0bb81..13464e4f4d72 100644
--- a/scripts/mod/symsearch.c
+++ b/scripts/mod/symsearch.c
@@ -20,9 +20,7 @@ struct syminfo {
* Entries in table are ascending, sorted first by section_index,
* then by addr, and last by symbol_index. The sorting by
* symbol_index is used to ensure predictable behavior when
- * multiple symbols are present with the same address; all
- * symbols past the first are effectively ignored, by eliding
- * them in symsearch_fixup().
+ * multiple symbols are present with the same address.
*/
struct symsearch {
unsigned int table_size;
@@ -97,32 +95,6 @@ static void symsearch_populate(struct elf_info *elf,
fatal("%s: size mismatch\n", __func__);
}
-/*
- * Do any fixups on the table after sorting.
- * For now, this just finds adjacent entries which have
- * the same section_index and addr, and it propagates
- * the first symbol_index over the subsequent entries,
- * so that only one symbol_index is seen for any given
- * section_index and addr. This ensures that whether
- * we're looking at an address from "above" or "below"
- * that we see the same symbol_index.
- * This does leave some duplicate entries in the table;
- * in practice, these are a small fraction of the
- * total number of entries, and they are harmless to
- * the binary search algorithm other than a few occasional
- * unnecessary comparisons.
- */
-static void symsearch_fixup(struct syminfo *table, unsigned int table_size)
-{
- /* Don't look at index 0, it will never change. */
- for (unsigned int i = 1; i < table_size; i++) {
- if (table[i].addr == table[i - 1].addr &&
- table[i].section_index == table[i - 1].section_index) {
- table[i].symbol_index = table[i - 1].symbol_index;
- }
- }
-}
-
void symsearch_init(struct elf_info *elf)
{
unsigned int table_size = symbol_count(elf);
@@ -134,8 +106,6 @@ void symsearch_init(struct elf_info *elf)
symsearch_populate(elf, elf->symsearch->table, table_size);
qsort(elf->symsearch->table, table_size,
sizeof(struct syminfo), syminfo_compare);
-
- symsearch_fixup(elf->symsearch->table, table_size);
}
void symsearch_finish(struct elf_info *elf)
@@ -226,12 +196,33 @@ static Elf_Sym *symsearch_find(struct elf_info *elf, Elf_Addr addr,
static bool symsearch_nearest_filter(const Elf_Sym *sym1, const Elf_Sym *sym2,
void *data)
{
+ struct elf_info *elf = data;
+ unsigned int bind1, bind2, unscores1, unscores2;
+
/* If sym2 is NULL, this is the first occurrence, always take it. */
if (sym2 == NULL)
return true;
/* Prefer lower address. */
- return sym1->st_value < sym2->st_value;
+ if (sym1->st_value < sym2->st_value)
+ return true;
+ if (sym1->st_value > sym2->st_value)
+ return false;
+
+ bind1 = ELF_ST_BIND(sym1->st_info);
+ bind2 = ELF_ST_BIND(sym2->st_info);
+
+ /* Prefer global symbol. */
+ if (bind1 == STB_GLOBAL && bind2 != STB_GLOBAL)
+ return true;
+ if (bind1 != STB_GLOBAL && bind2 == STB_GLOBAL)
+ return false;
+
+ /* Prefer less underscores. */
+ unscores1 = strspn(sym_name(elf, sym1), "_");
+ unscores2 = strspn(sym_name(elf, sym2), "_");
+
+ return unscores1 < unscores2;
}
/*
@@ -247,5 +238,5 @@ Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr,
Elf_Addr min_distance)
{
return symsearch_find(elf, addr, secndx, allow_negative, min_distance,
- symsearch_nearest_filter, NULL);
+ symsearch_nearest_filter, elf);
}
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 6/7] modpost: add symsearch_find_with_name() helper function
2023-11-01 15:03 [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Masahiro Yamada
` (4 preceding siblings ...)
2023-11-01 15:04 ` [PATCH 5/7] modpost: prefer global symbols in symsearch_find_nearest() Masahiro Yamada
@ 2023-11-01 15:04 ` Masahiro Yamada
2023-11-01 15:04 ` [PATCH 7/7] modpost: look up the correct symbol in check_export_symbol() Masahiro Yamada
2023-11-02 15:00 ` [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Greg Ungerer
7 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2023-11-01 15:04 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Greg Ungerer, Jack Brennen, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier
This helper function searches for a symbol with the provided name.
The symbol must be located in the specified section and within the
given distance from the target address.
In the expected use case, the min_distance is very small, so the
linear search will finish within a few iterations.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
scripts/mod/modpost.h | 3 +++
scripts/mod/symsearch.c | 44 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 9834ac44846d..43148b1a762b 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -186,6 +186,9 @@ void symsearch_finish(struct elf_info *elf);
Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr,
unsigned int secndx, bool allow_negative,
Elf_Addr min_distance);
+Elf_Sym *symsearch_find_with_name(struct elf_info *elf, Elf_Addr addr,
+ unsigned int secndx, bool allow_negative,
+ Elf_Addr min_distance, const char *name);
/* file2alias.c */
void handle_moddevtable(struct module *mod, struct elf_info *info,
diff --git a/scripts/mod/symsearch.c b/scripts/mod/symsearch.c
index 13464e4f4d72..9101bb9584a4 100644
--- a/scripts/mod/symsearch.c
+++ b/scripts/mod/symsearch.c
@@ -240,3 +240,47 @@ Elf_Sym *symsearch_find_nearest(struct elf_info *elf, Elf_Addr addr,
return symsearch_find(elf, addr, secndx, allow_negative, min_distance,
symsearch_nearest_filter, elf);
}
+
+struct name_filter_data {
+ struct elf_info *elf;
+ const char *name;
+};
+
+static bool symsearch_name_filter(const Elf_Sym *sym1, const Elf_Sym *sym2,
+ void *_data)
+{
+ struct name_filter_data *data = _data;
+ const char *name;
+
+ /* Check the symbol name. */
+ name = sym_name(data->elf, sym1);
+ if (strcmp(name, data->name))
+ return false;
+
+ /* If sym2 is NULL, this is the first occurrence, always take it. */
+ if (!sym2)
+ return true;
+
+ /* Prefer lower address. */
+ return sym1->st_value < sym2->st_value;
+}
+
+/*
+ * Find the symbol which is in secndx and has the given name, and is located
+ * close enough to the given address.
+ * allow_negative: allow returning a symbol whose address is > addr.
+ * min_distance: ignore symbols which are further away than this.
+ * name: the name of the symbol to search for.
+ *
+ * Returns a pointer into the symbol table for success.
+ * Returns NULL if no legal symbol is found within the requested range.
+ */
+Elf_Sym *symsearch_find_with_name(struct elf_info *elf, Elf_Addr addr,
+ unsigned int secndx, bool allow_negative,
+ Elf_Addr min_distance, const char *name)
+{
+ struct name_filter_data data = { .elf = elf, .name = name };
+
+ return symsearch_find(elf, addr, secndx, allow_negative, min_distance,
+ symsearch_name_filter, &data);
+}
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 7/7] modpost: look up the correct symbol in check_export_symbol()
2023-11-01 15:03 [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Masahiro Yamada
` (5 preceding siblings ...)
2023-11-01 15:04 ` [PATCH 6/7] modpost: add symsearch_find_with_name() helper function Masahiro Yamada
@ 2023-11-01 15:04 ` Masahiro Yamada
2023-11-02 15:00 ` [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Greg Ungerer
7 siblings, 0 replies; 9+ messages in thread
From: Masahiro Yamada @ 2023-11-01 15:04 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Greg Ungerer, Jack Brennen, Masahiro Yamada,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier
Greg Ungerer reported modpost produced false-positive
"local symbol '...' was exported" errors when m68k-uclinux-gcc is used.
I had assumed ELF_R_SYM(Elf_Rela::r_info) pointed to the exported symbol
itself if it is in the global scope. This assumption worked for many
toolchains, but as it turned out, it was not true for m68k-uclinux-gcc,
at least.
If the 'sym' argument passed to check_export_symbol() is not the
exported symbol, look up the correct one in the symbol table. It incurs
a search cost, but since we know its section index and address, we can
exploit the binary search.
Reported-by: Greg Ungerer <gerg@kernel.org>
Closes: https://lore.kernel.org/all/1fac9d12-2ec2-4ccb-bb81-34f3fc34789e@westnet.com.au/
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
scripts/mod/modpost.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 896ecfa8483f..ee67bc6d71ee 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1021,6 +1021,18 @@ static Elf_Sym *find_tosym(struct elf_info *elf, Elf_Addr addr, Elf_Sym *sym)
true, 20);
}
+static Elf_Sym *find_tosym_with_name(struct elf_info *elf, Elf_Addr addr,
+ Elf_Sym *sym, const char *name)
+{
+ /* If the supplied symbol has the expected name, return it. */
+ if (!strcmp(sym_name(elf, sym), name))
+ return sym;
+
+ /* Look up a symbol with the given name. */
+ return symsearch_find_with_name(elf, addr, get_secindex(elf, sym),
+ true, 20, name);
+}
+
static bool is_executable_section(struct elf_info *elf, unsigned int secndx)
{
if (secndx >= elf->num_sections)
@@ -1079,7 +1091,7 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
static void check_export_symbol(struct module *mod, struct elf_info *elf,
Elf_Addr faddr, const char *secname,
- Elf_Sym *sym)
+ Elf_Sym *sym, Elf_Addr taddr)
{
static const char *prefix = "__export_symbol_";
const char *label_name, *name, *data;
@@ -1096,6 +1108,14 @@ static void check_export_symbol(struct module *mod, struct elf_info *elf,
return;
}
+ name = label_name + strlen(prefix);
+ sym = find_tosym_with_name(elf, taddr, sym, name);
+ if (!sym) {
+ error("%s: could not find the the export symbol '%s'\n",
+ mod->name, name);
+ return;
+ }
+
if (ELF_ST_BIND(sym->st_info) != STB_GLOBAL &&
ELF_ST_BIND(sym->st_info) != STB_WEAK) {
error("%s: local symbol '%s' was exported\n", mod->name,
@@ -1103,13 +1123,6 @@ static void check_export_symbol(struct module *mod, struct elf_info *elf,
return;
}
- name = sym_name(elf, sym);
- if (strcmp(label_name + strlen(prefix), name)) {
- error("%s: .export_symbol section references '%s', but it does not seem to be an export symbol\n",
- mod->name, name);
- return;
- }
-
data = sym_get_data(elf, label); /* license */
if (!strcmp(data, "GPL")) {
is_gpl = true;
@@ -1156,7 +1169,7 @@ static void check_section_mismatch(struct module *mod, struct elf_info *elf,
const struct sectioncheck *mismatch;
if (module_enabled && elf->export_symbol_secndx == fsecndx) {
- check_export_symbol(mod, elf, faddr, tosec, sym);
+ check_export_symbol(mod, elf, faddr, tosec, sym, taddr);
return;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc
2023-11-01 15:03 [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Masahiro Yamada
` (6 preceding siblings ...)
2023-11-01 15:04 ` [PATCH 7/7] modpost: look up the correct symbol in check_export_symbol() Masahiro Yamada
@ 2023-11-02 15:00 ` Greg Ungerer
7 siblings, 0 replies; 9+ messages in thread
From: Greg Ungerer @ 2023-11-02 15:00 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild
Cc: linux-kernel, Jack Brennen, Nathan Chancellor, Nick Desaulniers,
Nicolas Schier
Hi Masahiro,
On 2/11/23 01:03, Masahiro Yamada wrote:
> Greg Ungerer reports building with m68k-uclinux-gcc toolchain is broken:
> https://lore.kernel.org/linux-kbuild/CAK7LNASQ_W5Yva5a4Xx8E2EYi-tN7x3OHgMFhK+93W+BiX1=9Q@mail.gmail.com/T/#m6ff0364f9ca8483c9f6d162619e5005833d1e887
>
> Usually, we do not need to search for export symbols in the .symtab
> section, but m68k-uclinux-gcc seems to be an exceptional case.
> I do not know what makes it different from other toolchains.
> Also, I do not know there exist other toolchains that work like that.
>
> This series extends the symsearch feature in case we need to explicitly
> search for export symbols.
>
> Then, the last patch fixes the issue.
>
> This series should be applicable for linux-next.
>
> This series is too late for the current merge window, but I'd like
> to fix the issue somehow by the next merge window.
Thanks for looking into this.
I can confirm this series fixes it for me (using linux-next).
Tested-by: Greg Ungerer <gerg@kernel.org>
Regards
Greg
>
>
> Masahiro Yamada (7):
> modpost: move sym_name() to modpost.h
> modpost: add const qualifier to syminfo table
> modpost: add table_size local variable to symsearch_find_nearest()
> modpost: introduce a filtering feature to symsearch
> modpost: prefer global symbols in symsearch_find_nearest()
> modpost: add symsearch_find_with_name() helper function
> modpost: look up the correct symbol in check_export_symbol()
>
> scripts/mod/modpost.c | 39 ++++----
> scripts/mod/modpost.h | 12 +++
> scripts/mod/symsearch.c | 205 ++++++++++++++++++++++++++++------------
> 3 files changed, 180 insertions(+), 76 deletions(-)
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-11-02 15:00 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-01 15:03 [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Masahiro Yamada
2023-11-01 15:03 ` [PATCH 1/7] modpost: move sym_name() to modpost.h Masahiro Yamada
2023-11-01 15:03 ` [PATCH 2/7] modpost: add const qualifier to syminfo table Masahiro Yamada
2023-11-01 15:04 ` [PATCH 3/7] modpost: add table_size local variable to symsearch_find_nearest() Masahiro Yamada
2023-11-01 15:04 ` [PATCH 4/7] modpost: introduce a filtering feature to symsearch Masahiro Yamada
2023-11-01 15:04 ` [PATCH 5/7] modpost: prefer global symbols in symsearch_find_nearest() Masahiro Yamada
2023-11-01 15:04 ` [PATCH 6/7] modpost: add symsearch_find_with_name() helper function Masahiro Yamada
2023-11-01 15:04 ` [PATCH 7/7] modpost: look up the correct symbol in check_export_symbol() Masahiro Yamada
2023-11-02 15:00 ` [PATCH 0/7] modpost: fix modpost errors for m68k-uclinux-gcc Greg Ungerer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox