This line was touched by commit f235541699bc ("export.h: allow for
per-symbol configurable EXPORT_SYMBOL()"), but the commit log did
not explain why.
CONFIG_TRIM_UNUSED_KSYMS works for me without defined(__KERNEL__).
Signed-off-by: Masahiro Yamada <[email protected]>
---
include/linux/export.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/export.h b/include/linux/export.h
index fd8711ed9ac4..cdd98a0d918c 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -20,7 +20,7 @@ extern struct module __this_module;
#ifdef CONFIG_MODULES
-#if defined(__KERNEL__) && !defined(__GENKSYMS__)
+#if !defined(__GENKSYMS__)
#ifdef CONFIG_MODVERSIONS
/* Mark the CRC weak since genksyms apparently decides not to
* generate a checksums for some symbols */
--
2.17.1
Arnd Bergmann reported false-positive modpost warnings detected by
his randconfig testing of linux-next:
https://lkml.org/lkml/2019/9/6/619
Actually, this happens under the combination of CONFIG_MODVERSIONS=y
and CONFIG_TRIM_UNUSED_KSYMS=y since commit 15bfc2348d54 ("modpost:
check for static EXPORT_SYMBOL* functions").
For example, arch/arm/config/multi_v7_defconfig + CONFIG_MODVERSIONS=y
+ CONFIG_TRIM_UNUSED_KSYMS=y produces the following false-positives:
WARNING: "__lshrdi3" [vmlinux] is a static (unknown)
WARNING: "__ashrdi3" [vmlinux] is a static (unknown)
WARNING: "__aeabi_lasr" [vmlinux] is a static (unknown)
WARNING: "__aeabi_llsr" [vmlinux] is a static (unknown)
WARNING: "ftrace_set_clr_event" [vmlinux] is a static (unknown)
WARNING: "__muldi3" [vmlinux] is a static (unknown)
WARNING: "__aeabi_ulcmp" [vmlinux] is a static (unknown)
WARNING: "__ucmpdi2" [vmlinux] is a static (unknown)
WARNING: "__aeabi_lmul" [vmlinux] is a static (unknown)
WARNING: "__bswapsi2" [vmlinux] is a static (unknown)
WARNING: "__bswapdi2" [vmlinux] is a static (unknown)
WARNING: "__ashldi3" [vmlinux] is a static (unknown)
WARNING: "__aeabi_llsl" [vmlinux] is a static (unknown)
The root cause of the problem is not in the modpost, but in the
implementation of CONFIG_TRIM_UNUSED_KSYMS.
If there is at least one untrimmed symbol in the file, genksyms is
invoked to calculate CRC of *all* the symbols in that file even if
some of them have been trimmed due to no caller existing.
As a result, .tmp_*.ver files contain CRC of trimmed symbols, thus
unneeded __crc* symbols are added to objects. It has been harmless
until recently.
Since commit 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL*
functions"), it is harmful because the bogus __crc* symbols make
modpost call sym_update_crc(), and then new_symbol(), but there is
no one that clears the ->is_static member.
I gave Fixes to the first commit that uncovered the issue, but the
potential problem has long existed since commit f235541699bc
("export.h: allow for per-symbol configurable EXPORT_SYMBOL()").
Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
Reported-by: Arnd Bergmann <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---
include/linux/export.h | 42 ++++++++++++++-----------------------
scripts/genksyms/keywords.c | 6 +-----
2 files changed, 17 insertions(+), 31 deletions(-)
diff --git a/include/linux/export.h b/include/linux/export.h
index cdd98a0d918c..7d8c112a8b61 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -18,9 +18,6 @@ extern struct module __this_module;
#define THIS_MODULE ((struct module *)0)
#endif
-#ifdef CONFIG_MODULES
-
-#if !defined(__GENKSYMS__)
#ifdef CONFIG_MODVERSIONS
/* Mark the CRC weak since genksyms apparently decides not to
* generate a checksums for some symbols */
@@ -74,6 +71,12 @@ struct kernel_symbol {
};
#endif
+#ifdef __GENKSYMS__
+
+#define ___EXPORT_SYMBOL(sym, sec) __GENKSYMS_EXPORT_SYMBOL(sym)
+
+#else
+
/* For every exported symbol, place a struct in the __ksymtab section */
#define ___EXPORT_SYMBOL(sym, sec) \
extern typeof(sym) sym; \
@@ -83,7 +86,9 @@ struct kernel_symbol {
= #sym; \
__KSYMTAB_ENTRY(sym, sec)
-#if defined(__DISABLE_EXPORTS)
+#endif
+
+#if !defined(CONFIG_MODULES) || defined(__DISABLE_EXPORTS)
/*
* Allow symbol exports to be disabled completely so that C code may
@@ -117,37 +122,22 @@ struct kernel_symbol {
#define __cond_export_sym_0(sym, sec) /* nothing */
#else
-#define __EXPORT_SYMBOL ___EXPORT_SYMBOL
-#endif
-#define EXPORT_SYMBOL(sym) \
- __EXPORT_SYMBOL(sym, "")
+#define __EXPORT_SYMBOL(sym, sec) ___EXPORT_SYMBOL(sym, sec)
-#define EXPORT_SYMBOL_GPL(sym) \
- __EXPORT_SYMBOL(sym, "_gpl")
-
-#define EXPORT_SYMBOL_GPL_FUTURE(sym) \
- __EXPORT_SYMBOL(sym, "_gpl_future")
+#endif /* CONFIG_MODULES */
+#define EXPORT_SYMBOL(sym) __EXPORT_SYMBOL(sym, "")
+#define EXPORT_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_gpl")
+#define EXPORT_SYMBOL_GPL_FUTURE(sym) __EXPORT_SYMBOL(sym, "_gpl_future")
#ifdef CONFIG_UNUSED_SYMBOLS
-#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused")
-#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl")
+#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused")
+#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl")
#else
#define EXPORT_UNUSED_SYMBOL(sym)
#define EXPORT_UNUSED_SYMBOL_GPL(sym)
#endif
-#endif /* __GENKSYMS__ */
-
-#else /* !CONFIG_MODULES... */
-
-#define EXPORT_SYMBOL(sym)
-#define EXPORT_SYMBOL_GPL(sym)
-#define EXPORT_SYMBOL_GPL_FUTURE(sym)
-#define EXPORT_UNUSED_SYMBOL(sym)
-#define EXPORT_UNUSED_SYMBOL_GPL(sym)
-
-#endif /* CONFIG_MODULES */
#endif /* !__ASSEMBLY__ */
#endif /* _LINUX_EXPORT_H */
diff --git a/scripts/genksyms/keywords.c b/scripts/genksyms/keywords.c
index c586d32dd2c3..7a85c4e21175 100644
--- a/scripts/genksyms/keywords.c
+++ b/scripts/genksyms/keywords.c
@@ -3,11 +3,7 @@ static struct resword {
const char *name;
int token;
} keywords[] = {
- { "EXPORT_SYMBOL", EXPORT_SYMBOL_KEYW },
- { "EXPORT_SYMBOL_GPL", EXPORT_SYMBOL_KEYW },
- { "EXPORT_SYMBOL_GPL_FUTURE", EXPORT_SYMBOL_KEYW },
- { "EXPORT_UNUSED_SYMBOL", EXPORT_SYMBOL_KEYW },
- { "EXPORT_UNUSED_SYMBOL_GPL", EXPORT_SYMBOL_KEYW },
+ { "__GENKSYMS_EXPORT_SYMBOL", EXPORT_SYMBOL_KEYW },
{ "__asm", ASM_KEYW },
{ "__asm__", ASM_KEYW },
{ "__attribute", ATTRIBUTE_KEYW },
--
2.17.1
On Mon, Sep 9, 2019 at 12:53 PM Masahiro Yamada
<[email protected]> wrote:
> Since commit 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL*
> functions"), it is harmful because the bogus __crc* symbols make
> modpost call sym_update_crc(), and then new_symbol(), but there is
> no one that clears the ->is_static member.
>
> I gave Fixes to the first commit that uncovered the issue, but the
> potential problem has long existed since commit f235541699bc
> ("export.h: allow for per-symbol configurable EXPORT_SYMBOL()").
>
> Fixes: 15bfc2348d54 ("modpost: check for static EXPORT_SYMBOL* functions")
> Reported-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
Tested-by: Arnd Bergmann <[email protected]>
Thanks for providing a proper fix!
Arnd
Hi Nicolas,
On Mon, Sep 9, 2019 at 10:48 PM Nicolas Pitre <[email protected]> wrote:
>
> On Mon, 9 Sep 2019, Masahiro Yamada wrote:
>
> > This line was touched by commit f235541699bc ("export.h: allow for
> > per-symbol configurable EXPORT_SYMBOL()"), but the commit log did
> > not explain why.
> >
> > CONFIG_TRIM_UNUSED_KSYMS works for me without defined(__KERNEL__).
>
> I'm pretty sure it was needed back then so not to interfere with users
> of this file. My fault for not documenting it.
Hmm, I did not see a problem in my quick build test.
Do you remember which file was causing the problem?
--
Best Regards
Masahiro Yamada
On Mon, 9 Sep 2019, Masahiro Yamada wrote:
> Hi Nicolas,
>
> On Mon, Sep 9, 2019 at 10:48 PM Nicolas Pitre <[email protected]> wrote:
> >
> > On Mon, 9 Sep 2019, Masahiro Yamada wrote:
> >
> > > This line was touched by commit f235541699bc ("export.h: allow for
> > > per-symbol configurable EXPORT_SYMBOL()"), but the commit log did
> > > not explain why.
> > >
> > > CONFIG_TRIM_UNUSED_KSYMS works for me without defined(__KERNEL__).
> >
> > I'm pretty sure it was needed back then so not to interfere with users
> > of this file. My fault for not documenting it.
>
> Hmm, I did not see a problem in my quick build test.
>
> Do you remember which file was causing the problem?
If you build commit 7ec925701f5f with CONFIG_TRIM_UNUSED_KSYMS=y and the
defined(__KERNEL__) test removed then you'll get:
HOSTCC scripts/mod/modpost.o
In file included from scripts/mod/modpost.c:24:
scripts/mod/../../include/linux/export.h:81:10: fatal error: linux/kconfig.h: No such file or directory
Nicolas
On Mon, 9 Sep 2019, Masahiro Yamada wrote:
> This line was touched by commit f235541699bc ("export.h: allow for
> per-symbol configurable EXPORT_SYMBOL()"), but the commit log did
> not explain why.
>
> CONFIG_TRIM_UNUSED_KSYMS works for me without defined(__KERNEL__).
I'm pretty sure it was needed back then so not to interfere with users
of this file. My fault for not documenting it.
Nicolas
On Tue, 10 Sep 2019, Masahiro Yamada wrote:
> On Tue, Sep 10, 2019 at 1:06 AM Nicolas Pitre <[email protected]> wrote:
> >
> > On Mon, 9 Sep 2019, Masahiro Yamada wrote:
> >
> > > Hi Nicolas,
> > >
> > > On Mon, Sep 9, 2019 at 10:48 PM Nicolas Pitre <[email protected]> wrote:
> > > >
> > > > On Mon, 9 Sep 2019, Masahiro Yamada wrote:
> > > >
> > > > > This line was touched by commit f235541699bc ("export.h: allow for
> > > > > per-symbol configurable EXPORT_SYMBOL()"), but the commit log did
> > > > > not explain why.
> > > > >
> > > > > CONFIG_TRIM_UNUSED_KSYMS works for me without defined(__KERNEL__).
> > > >
> > > > I'm pretty sure it was needed back then so not to interfere with users
> > > > of this file. My fault for not documenting it.
> > >
> > > Hmm, I did not see a problem in my quick build test.
> > >
> > > Do you remember which file was causing the problem?
> >
> > If you build commit 7ec925701f5f with CONFIG_TRIM_UNUSED_KSYMS=y and the
> > defined(__KERNEL__) test removed then you'll get:
> >
> > HOSTCC scripts/mod/modpost.o
> > In file included from scripts/mod/modpost.c:24:
> > scripts/mod/../../include/linux/export.h:81:10: fatal error: linux/kconfig.h: No such file or directory
> >
> >
> > Nicolas
>
>
> Thanks for explaining this.
>
> It is not the case any more.
>
>
> I will reword the commit message as follows:
>
> ------------------------>8---------------------------------------
> export.h: remove defined(__KERNEL__), which is no longer needed
>
> The conditional define(__KERNEL__) was added by commit f235541699bc
> ("export.h: allow for per-symbol configurable EXPORT_SYMBOL()").
>
> It was needed at that time to avoid the build error of modpost
> with CONFIG_TRIM_UNUSED_KSYMS=y.
>
> Since commit b2c5cdcfd4bc ("modpost: remove symbol prefix support"),
> modpost no longer includes linux/export.h, thus the define(__KERNEL__)
> is unneeded.
> ------------------------>8---------------------------------------
>
Acked-by: Nicolas Pitre <[email protected]>
Nicolas
On Tue, Sep 10, 2019 at 1:06 AM Nicolas Pitre <[email protected]> wrote:
>
> On Mon, 9 Sep 2019, Masahiro Yamada wrote:
>
> > Hi Nicolas,
> >
> > On Mon, Sep 9, 2019 at 10:48 PM Nicolas Pitre <[email protected]> wrote:
> > >
> > > On Mon, 9 Sep 2019, Masahiro Yamada wrote:
> > >
> > > > This line was touched by commit f235541699bc ("export.h: allow for
> > > > per-symbol configurable EXPORT_SYMBOL()"), but the commit log did
> > > > not explain why.
> > > >
> > > > CONFIG_TRIM_UNUSED_KSYMS works for me without defined(__KERNEL__).
> > >
> > > I'm pretty sure it was needed back then so not to interfere with users
> > > of this file. My fault for not documenting it.
> >
> > Hmm, I did not see a problem in my quick build test.
> >
> > Do you remember which file was causing the problem?
>
> If you build commit 7ec925701f5f with CONFIG_TRIM_UNUSED_KSYMS=y and the
> defined(__KERNEL__) test removed then you'll get:
>
> HOSTCC scripts/mod/modpost.o
> In file included from scripts/mod/modpost.c:24:
> scripts/mod/../../include/linux/export.h:81:10: fatal error: linux/kconfig.h: No such file or directory
>
>
> Nicolas
Thanks for explaining this.
It is not the case any more.
I will reword the commit message as follows:
------------------------>8---------------------------------------
export.h: remove defined(__KERNEL__), which is no longer needed
The conditional define(__KERNEL__) was added by commit f235541699bc
("export.h: allow for per-symbol configurable EXPORT_SYMBOL()").
It was needed at that time to avoid the build error of modpost
with CONFIG_TRIM_UNUSED_KSYMS=y.
Since commit b2c5cdcfd4bc ("modpost: remove symbol prefix support"),
modpost no longer includes linux/export.h, thus the define(__KERNEL__)
is unneeded.
------------------------>8---------------------------------------
--
Best Regards
Masahiro Yamada
On Mon, Sep 9, 2019 at 7:53 PM Masahiro Yamada
<[email protected]> wrote:
>
> This line was touched by commit f235541699bc ("export.h: allow for
> per-symbol configurable EXPORT_SYMBOL()"), but the commit log did
> not explain why.
>
> CONFIG_TRIM_UNUSED_KSYMS works for me without defined(__KERNEL__).
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
Both applied to linux-kbuild.
> include/linux/export.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/export.h b/include/linux/export.h
> index fd8711ed9ac4..cdd98a0d918c 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -20,7 +20,7 @@ extern struct module __this_module;
>
> #ifdef CONFIG_MODULES
>
> -#if defined(__KERNEL__) && !defined(__GENKSYMS__)
> +#if !defined(__GENKSYMS__)
> #ifdef CONFIG_MODVERSIONS
> /* Mark the CRC weak since genksyms apparently decides not to
> * generate a checksums for some symbols */
> --
> 2.17.1
>
--
Best Regards
Masahiro Yamada