From: Masahiro Yamada <masahiroy@kernel.org> To: linux-kbuild@vger.kernel.org Cc: linux-s390@vger.kernel.org, Nicolas Schier <nicolas@fjasle.eu>, Peter Zijlstra <peterz@infradead.org>, Masahiro Yamada <masahiroy@kernel.org>, Nick Desaulniers <ndesaulniers@google.com>, linux-kernel@vger.kernel.org, Nathan Chancellor <nathan@kernel.org>, clang-built-linux@googlegroups.com, Sami Tolvanen <samitolvanen@google.com>, linuxppc-dev@lists.ozlabs.org, Ard Biesheuvel <ardb@kernel.org>, linux-modules@vger.kernel.org Subject: [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost Date: Mon, 9 May 2022 04:06:27 +0900 [thread overview] Message-ID: <20220508190631.2386038-11-masahiroy@kernel.org> (raw) In-Reply-To: <20220508190631.2386038-1-masahiroy@kernel.org> The 'static' specifier and EXPORT_SYMBOL() are an odd combination. Commit 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions") tried to detect it, but this check has false negatives. Here is the sample code. Makefile: obj-y += foo1.o foo2.o foo1.c: #include <linux/export.h> static void foo(void) {} EXPORT_SYMBOL(foo); foo2.c: void foo(void) {} foo1.c exports the static symbol 'foo', but modpost cannot catch it because it is fooled by foo2.c, which has a global symbol with the same name. s->is_static is cleared if a global symbol with the same name is found somewhere, but EXPORT_SYMBOL() and the global symbol do not necessarily belong to the same compilation unit. This check should be done per compilation unit, but I do not know how to do it in modpost. modpost runs against vmlinux.o or modules, which merges multiple objects, then forgets their origin. It is true modpost gets access to the lists of all the member objects (.vmlinux.objs and *.mod), but it is impossible to parse individual objects in modpost; they might be LLVM IR instead of ELF when CONFIG_LTO_CLANG=y. Add a simple bash script to parse the output from ${NM}. This works for CONFIG_LTO_CLANG=y because llvm-nm can dump symbols of LLVM bitcode. Revert 15bfc2348d54. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Changes in v4: - New patch scripts/Makefile.build | 4 ++++ scripts/check-local-export | 48 ++++++++++++++++++++++++++++++++++++++ scripts/mod/modpost.c | 28 +--------------------- 3 files changed, 53 insertions(+), 27 deletions(-) create mode 100755 scripts/check-local-export diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 838ea5e83174..c2a173b3fd60 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -244,9 +244,12 @@ cmd_gen_ksymdeps = \ $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd endif +cmd_check_local_export = $(srctree)/scripts/check-local-export $@ + define rule_cc_o_c $(call cmd_and_fixdep,cc_o_c) $(call cmd,gen_ksymdeps) + $(call cmd,check_local_export) $(call cmd,checksrc) $(call cmd,checkdoc) $(call cmd,gen_objtooldep) @@ -257,6 +260,7 @@ endef define rule_as_o_S $(call cmd_and_fixdep,as_o_S) $(call cmd,gen_ksymdeps) + $(call cmd,check_local_export) $(call cmd,gen_objtooldep) $(call cmd,gen_symversions_S) endef diff --git a/scripts/check-local-export b/scripts/check-local-export new file mode 100755 index 000000000000..d1721fa63057 --- /dev/null +++ b/scripts/check-local-export @@ -0,0 +1,48 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0-only +# +# Copyright (C) 2022 Masahiro Yamada + +set -e +set -o pipefail + +declare -A symbol_types +declare -a export_symbols + +exit_code=0 + +while read value type name +do + # to avoid error for clang LTO; $name may be empty + if [[ $value = -* && -z $name ]]; then + continue + fi + + # The first field (value) may be empty. If so, fix it up. + if [[ -z $name ]]; then + name=${type} + type=${value} + fi + + # save (name, type) in the associative array + symbol_types[$name]=$type + + # append the exported symbol to the array + if [[ $name == __ksymtab_* ]]; then + export_symbols+=(${name#__ksymtab_}) + fi +done < <(${NM} ${1} 2>/dev/null) + +# Catch error in the process substitution +wait $! + +for name in "${export_symbols[@]}" +do + # nm(3) says "If lowercase, the symbol is usually local" + if [[ ${symbol_types[$name]} =~ [a-z] ]]; then + echo "$@: error: local symbol '${name}' was exported" >&2 + exit_code=1 + fi +done + +exit ${exit_code} diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 018527d96680..fa73ddb6a6cf 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -212,7 +212,6 @@ struct symbol { unsigned int crc; bool crc_valid; bool weak; - bool is_static; /* true if symbol is not global */ bool is_gpl_only; /* exported by EXPORT_SYMBOL_GPL */ char name[]; }; @@ -242,7 +241,7 @@ static struct symbol *alloc_symbol(const char *name) memset(s, 0, sizeof(*s)); strcpy(s->name, name); - s->is_static = true; + return s; } @@ -875,20 +874,6 @@ static void read_symbols(const char *modname) sym_get_data(&info, sym)); } - // check for static EXPORT_SYMBOL_* functions && global vars - for (sym = info.symtab_start; sym < info.symtab_stop; sym++) { - unsigned char bind = ELF_ST_BIND(sym->st_info); - - if (bind == STB_GLOBAL || bind == STB_WEAK) { - struct symbol *s = - find_symbol(remove_dot(info.strtab + - sym->st_name)); - - if (s) - s->is_static = false; - } - } - check_sec_ref(mod, modname, &info); if (!mod->is_vmlinux) { @@ -1318,7 +1303,6 @@ static void read_dump(const char *fname) mod->from_dump = true; } s = sym_add_exported(symname, mod, gpl_only); - s->is_static = false; sym_set_crc(s, crc); sym_update_namespace(symname, namespace); } @@ -1383,7 +1367,6 @@ int main(int argc, char **argv) char *missing_namespace_deps = NULL; char *dump_write = NULL, *files_source = NULL; int opt; - int n; LIST_HEAD(dump_lists); struct dump_list *dl, *dl2; @@ -1459,15 +1442,6 @@ int main(int argc, char **argv) if (sec_mismatch_count && !sec_mismatch_warn_only) error("Section mismatches detected.\n" "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n"); - for (n = 0; n < SYMBOL_HASH_SIZE; n++) { - struct symbol *s; - - for (s = symbolhash[n]; s; s = s->next) { - if (s->is_static) - error("\"%s\" [%s] is a static EXPORT_SYMBOL\n", - s->name, s->module->name); - } - } if (nr_unresolved > MAX_UNRESOLVED_REPORTS) warn("suppressed %u unresolved symbol warnings because there were too many)\n", -- 2.32.0
WARNING: multiple messages have this Message-ID (diff)
From: Masahiro Yamada <masahiroy@kernel.org> To: linux-kbuild@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Nathan Chancellor <nathan@kernel.org>, Nick Desaulniers <ndesaulniers@google.com>, Nicolas Schier <nicolas@fjasle.eu>, Peter Zijlstra <peterz@infradead.org>, linux-modules@vger.kernel.org, linux-s390@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, clang-built-linux@googlegroups.com, Ard Biesheuvel <ardb@kernel.org>, Sami Tolvanen <samitolvanen@google.com>, Masahiro Yamada <masahiroy@kernel.org> Subject: [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost Date: Mon, 9 May 2022 04:06:27 +0900 [thread overview] Message-ID: <20220508190631.2386038-11-masahiroy@kernel.org> (raw) In-Reply-To: <20220508190631.2386038-1-masahiroy@kernel.org> The 'static' specifier and EXPORT_SYMBOL() are an odd combination. Commit 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions") tried to detect it, but this check has false negatives. Here is the sample code. Makefile: obj-y += foo1.o foo2.o foo1.c: #include <linux/export.h> static void foo(void) {} EXPORT_SYMBOL(foo); foo2.c: void foo(void) {} foo1.c exports the static symbol 'foo', but modpost cannot catch it because it is fooled by foo2.c, which has a global symbol with the same name. s->is_static is cleared if a global symbol with the same name is found somewhere, but EXPORT_SYMBOL() and the global symbol do not necessarily belong to the same compilation unit. This check should be done per compilation unit, but I do not know how to do it in modpost. modpost runs against vmlinux.o or modules, which merges multiple objects, then forgets their origin. It is true modpost gets access to the lists of all the member objects (.vmlinux.objs and *.mod), but it is impossible to parse individual objects in modpost; they might be LLVM IR instead of ELF when CONFIG_LTO_CLANG=y. Add a simple bash script to parse the output from ${NM}. This works for CONFIG_LTO_CLANG=y because llvm-nm can dump symbols of LLVM bitcode. Revert 15bfc2348d54. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Changes in v4: - New patch scripts/Makefile.build | 4 ++++ scripts/check-local-export | 48 ++++++++++++++++++++++++++++++++++++++ scripts/mod/modpost.c | 28 +--------------------- 3 files changed, 53 insertions(+), 27 deletions(-) create mode 100755 scripts/check-local-export diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 838ea5e83174..c2a173b3fd60 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -244,9 +244,12 @@ cmd_gen_ksymdeps = \ $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd endif +cmd_check_local_export = $(srctree)/scripts/check-local-export $@ + define rule_cc_o_c $(call cmd_and_fixdep,cc_o_c) $(call cmd,gen_ksymdeps) + $(call cmd,check_local_export) $(call cmd,checksrc) $(call cmd,checkdoc) $(call cmd,gen_objtooldep) @@ -257,6 +260,7 @@ endef define rule_as_o_S $(call cmd_and_fixdep,as_o_S) $(call cmd,gen_ksymdeps) + $(call cmd,check_local_export) $(call cmd,gen_objtooldep) $(call cmd,gen_symversions_S) endef diff --git a/scripts/check-local-export b/scripts/check-local-export new file mode 100755 index 000000000000..d1721fa63057 --- /dev/null +++ b/scripts/check-local-export @@ -0,0 +1,48 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0-only +# +# Copyright (C) 2022 Masahiro Yamada + +set -e +set -o pipefail + +declare -A symbol_types +declare -a export_symbols + +exit_code=0 + +while read value type name +do + # to avoid error for clang LTO; $name may be empty + if [[ $value = -* && -z $name ]]; then + continue + fi + + # The first field (value) may be empty. If so, fix it up. + if [[ -z $name ]]; then + name=${type} + type=${value} + fi + + # save (name, type) in the associative array + symbol_types[$name]=$type + + # append the exported symbol to the array + if [[ $name == __ksymtab_* ]]; then + export_symbols+=(${name#__ksymtab_}) + fi +done < <(${NM} ${1} 2>/dev/null) + +# Catch error in the process substitution +wait $! + +for name in "${export_symbols[@]}" +do + # nm(3) says "If lowercase, the symbol is usually local" + if [[ ${symbol_types[$name]} =~ [a-z] ]]; then + echo "$@: error: local symbol '${name}' was exported" >&2 + exit_code=1 + fi +done + +exit ${exit_code} diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 018527d96680..fa73ddb6a6cf 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -212,7 +212,6 @@ struct symbol { unsigned int crc; bool crc_valid; bool weak; - bool is_static; /* true if symbol is not global */ bool is_gpl_only; /* exported by EXPORT_SYMBOL_GPL */ char name[]; }; @@ -242,7 +241,7 @@ static struct symbol *alloc_symbol(const char *name) memset(s, 0, sizeof(*s)); strcpy(s->name, name); - s->is_static = true; + return s; } @@ -875,20 +874,6 @@ static void read_symbols(const char *modname) sym_get_data(&info, sym)); } - // check for static EXPORT_SYMBOL_* functions && global vars - for (sym = info.symtab_start; sym < info.symtab_stop; sym++) { - unsigned char bind = ELF_ST_BIND(sym->st_info); - - if (bind == STB_GLOBAL || bind == STB_WEAK) { - struct symbol *s = - find_symbol(remove_dot(info.strtab + - sym->st_name)); - - if (s) - s->is_static = false; - } - } - check_sec_ref(mod, modname, &info); if (!mod->is_vmlinux) { @@ -1318,7 +1303,6 @@ static void read_dump(const char *fname) mod->from_dump = true; } s = sym_add_exported(symname, mod, gpl_only); - s->is_static = false; sym_set_crc(s, crc); sym_update_namespace(symname, namespace); } @@ -1383,7 +1367,6 @@ int main(int argc, char **argv) char *missing_namespace_deps = NULL; char *dump_write = NULL, *files_source = NULL; int opt; - int n; LIST_HEAD(dump_lists); struct dump_list *dl, *dl2; @@ -1459,15 +1442,6 @@ int main(int argc, char **argv) if (sec_mismatch_count && !sec_mismatch_warn_only) error("Section mismatches detected.\n" "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n"); - for (n = 0; n < SYMBOL_HASH_SIZE; n++) { - struct symbol *s; - - for (s = symbolhash[n]; s; s = s->next) { - if (s->is_static) - error("\"%s\" [%s] is a static EXPORT_SYMBOL\n", - s->name, s->module->name); - } - } if (nr_unresolved > MAX_UNRESOLVED_REPORTS) warn("suppressed %u unresolved symbol warnings because there were too many)\n", -- 2.32.0
next prev parent reply other threads:[~2022-05-08 19:16 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 01/14] modpost: remove left-over cross_compile declaration Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 17:08 ` Nick Desaulniers 2022-05-09 17:08 ` Nick Desaulniers 2022-05-12 4:35 ` Masahiro Yamada 2022-05-12 4:35 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 02/14] modpost: change the license of EXPORT_SYMBOL to bool type Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-12 4:36 ` Masahiro Yamada 2022-05-12 4:36 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 17:19 ` Nick Desaulniers 2022-05-09 17:19 ` Nick Desaulniers 2022-05-10 6:55 ` Masahiro Yamada 2022-05-10 6:55 ` Masahiro Yamada 2022-05-11 18:47 ` Nick Desaulniers 2022-05-11 18:47 ` Nick Desaulniers 2022-05-11 19:27 ` Masahiro Yamada 2022-05-11 19:27 ` Masahiro Yamada 2022-05-11 19:51 ` Jeff Johnson 2022-05-11 19:51 ` Jeff Johnson 2022-05-11 20:01 ` Nick Desaulniers 2022-05-11 20:01 ` Nick Desaulniers 2022-05-08 19:06 ` [PATCH v4 04/14] modpost: add sym_find_with_module() helper Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 05/14] modpost: extract symbol versions from *.cmd files Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 21:52 ` Sami Tolvanen 2022-05-09 21:52 ` Sami Tolvanen 2022-05-08 19:06 ` [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 17:50 ` Nick Desaulniers 2022-05-09 17:50 ` Nick Desaulniers 2022-05-10 13:03 ` Masahiro Yamada 2022-05-10 13:03 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 07/14] kbuild: stop merging *.symversions Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 08/14] genksyms: adjust the output format to modpost Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 09/14] kbuild: do not create *.prelink.o for Clang LTO or IBT Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 23:13 ` Sami Tolvanen 2022-05-09 23:13 ` Sami Tolvanen 2022-05-08 19:06 ` Masahiro Yamada [this message] 2022-05-08 19:06 ` [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost Masahiro Yamada 2022-05-09 18:05 ` Nick Desaulniers 2022-05-09 18:05 ` Nick Desaulniers 2022-05-10 13:53 ` Masahiro Yamada 2022-05-10 13:53 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 11/14] kbuild: make built-in.a rule robust against too long argument error Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 12/14] kbuild: make *.mod " Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 13/14] kbuild: add cmd_and_savecmd macro Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-08 19:06 ` [PATCH v4 14/14] kbuild: rebuild multi-object modules when objtool is updated Masahiro Yamada 2022-05-08 19:06 ` Masahiro Yamada 2022-05-09 4:24 ` [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada 2022-05-09 4:24 ` Masahiro Yamada 2022-05-09 22:13 ` Nathan Chancellor 2022-05-09 22:13 ` Nathan Chancellor 2022-05-10 6:55 ` Masahiro Yamada 2022-05-10 6:55 ` Masahiro Yamada 2022-05-09 22:19 ` Nathan Chancellor 2022-05-09 22:19 ` Nathan Chancellor
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220508190631.2386038-11-masahiroy@kernel.org \ --to=masahiroy@kernel.org \ --cc=ardb@kernel.org \ --cc=clang-built-linux@googlegroups.com \ --cc=linux-kbuild@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-modules@vger.kernel.org \ --cc=linux-s390@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=nathan@kernel.org \ --cc=ndesaulniers@google.com \ --cc=nicolas@fjasle.eu \ --cc=peterz@infradead.org \ --cc=samitolvanen@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.