* [RFC -next] linux/linkage.h: fix symbol prefix handling @ 2013-03-07 11:44 James Hogan 2013-03-08 0:03 ` Rusty Russell 0 siblings, 1 reply; 15+ messages in thread From: James Hogan @ 2013-03-07 11:44 UTC (permalink / raw) To: Al Viro Cc: Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, Rusty Russell, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next, James Hogan The commit "consolidate cond_syscall and SYSCALL_ALIAS declarations" broke the build on blackfin and metag due to the following code: #ifndef SYMBOL_NAME #ifdef CONFIG_SYMBOL_PREFIX #define SYMBOL_NAME(x) CONFIG_SYMBOL_PREFIX ## x #else #define SYMBOL_NAME(x) x #endif #endif #define __SYMBOL_NAME(x) __stringify(SYMBOL_NAME(x)) __stringify literally stringifies CONFIG_SYMBOL_PREFIX ##x, so you get lines like this in the assembly output: .weak CONFIG_SYMBOL_PREFIXsys_quotactl .set CONFIG_SYMBOL_PREFIXsys_quotactl,CONFIG_SYMBOL_PREFIXsys_ni_syscall This is fixed by defining SYMBOL_PREFIX from the command line for c files in addition to assembly for architectures that set CONFIG_SYMBOL_PREFIX (scripts/Makefile.lib), and defining __SYMBOL_NAME as: #define __SYMBOL_NAME(x) __stringify(SYMBOL_PREFIX) #x We first have to ensure SYMBOL_PREFIX is defined (which avoids polluting the command lines for architectures that don't use symbol prefixes). Also the definition of SYMBOL_PREFIX in <linux/kernel.h> is removed as it conflicts, isn't used anywhere, and is defined as a string so differs from the assembly definition. Signed-off-by: James Hogan <james.hogan@imgtec.com> --- This is just one possible way of fixing it. It felt the cleanest out of the ways I tried, but feel free to suggest something better! include/linux/kernel.h | 7 ------- include/linux/linkage.h | 12 +++++------- scripts/Makefile.lib | 1 + 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 80d3687..e13e992 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -723,13 +723,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } /* Trap pasters of __FUNCTION__ at compile-time */ #define __FUNCTION__ (__func__) -/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */ -#ifdef CONFIG_SYMBOL_PREFIX -#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define SYMBOL_PREFIX "" -#endif - /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */ #ifdef CONFIG_FTRACE_MCOUNT_RECORD # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD diff --git a/include/linux/linkage.h b/include/linux/linkage.h index 829d66c..acb869b 100644 --- a/include/linux/linkage.h +++ b/include/linux/linkage.h @@ -15,14 +15,12 @@ #define asmlinkage CPP_ASMLINKAGE #endif -#ifndef SYMBOL_NAME -#ifdef CONFIG_SYMBOL_PREFIX -#define SYMBOL_NAME(x) CONFIG_SYMBOL_PREFIX ## x -#else -#define SYMBOL_NAME(x) x -#endif +/* This helps us to avoid #ifdef SYMBOL_PREFIX */ +#ifndef SYMBOL_PREFIX +#define SYMBOL_PREFIX #endif -#define __SYMBOL_NAME(x) __stringify(SYMBOL_NAME(x)) + +#define __SYMBOL_NAME(x) __stringify(SYMBOL_PREFIX) #x #ifndef cond_syscall #define cond_syscall(x) asm(".weak\t" __SYMBOL_NAME(x) \ diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 07125e6..f1cce6a 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -123,6 +123,7 @@ ifdef CONFIG_SYMBOL_PREFIX _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX)) _cpp_flags += $(_sym_flags) _a_flags += $(_sym_flags) +_c_flags += $(_sym_flags) endif -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-07 11:44 [RFC -next] linux/linkage.h: fix symbol prefix handling James Hogan @ 2013-03-08 0:03 ` Rusty Russell 2013-03-08 9:15 ` James Hogan 0 siblings, 1 reply; 15+ messages in thread From: Rusty Russell @ 2013-03-08 0:03 UTC (permalink / raw) To: Al Viro Cc: Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next, James Hogan James Hogan <james.hogan@imgtec.com> writes: > The commit "consolidate cond_syscall and SYSCALL_ALIAS declarations" > broke the build on blackfin and metag due to the following code: > > #ifndef SYMBOL_NAME > #ifdef CONFIG_SYMBOL_PREFIX > #define SYMBOL_NAME(x) CONFIG_SYMBOL_PREFIX ## x > #else > #define SYMBOL_NAME(x) x > #endif > #endif > #define __SYMBOL_NAME(x) __stringify(SYMBOL_NAME(x)) > > __stringify literally stringifies CONFIG_SYMBOL_PREFIX ##x, so you get > lines like this in the assembly output: > > .weak CONFIG_SYMBOL_PREFIXsys_quotactl > .set > CONFIG_SYMBOL_PREFIXsys_quotactl,CONFIG_SYMBOL_PREFIXsys_ni_syscall > > This is fixed by defining SYMBOL_PREFIX from the command line for c > files in addition to assembly for architectures that set > CONFIG_SYMBOL_PREFIX (scripts/Makefile.lib), and defining __SYMBOL_NAME > as: > > #define __SYMBOL_NAME(x) __stringify(SYMBOL_PREFIX) #x > > We first have to ensure SYMBOL_PREFIX is defined (which avoids polluting > the command lines for architectures that don't use symbol prefixes). > Also the definition of SYMBOL_PREFIX in <linux/kernel.h> is removed as > it conflicts, isn't used anywhere, and is defined as a string so differs > from the assembly definition. So now, if CONFIG_SYMBOL_PREFIX, SYMBOL_PREFIX is defined on the cmdline as a string. Otherwise it's empty (not the empty string?): > +/* This helps us to avoid #ifdef SYMBOL_PREFIX */ > +#ifndef SYMBOL_PREFIX > +#define SYMBOL_PREFIX > #endif > -#define __SYMBOL_NAME(x) __stringify(SYMBOL_NAME(x)) > + > +#define __SYMBOL_NAME(x) __stringify(SYMBOL_PREFIX) #x And why are you __stringify()ing a string? > #ifndef cond_syscall > #define cond_syscall(x) asm(".weak\t" __SYMBOL_NAME(x) \ > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 07125e6..f1cce6a 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -123,6 +123,7 @@ ifdef CONFIG_SYMBOL_PREFIX > _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX)) > _cpp_flags += $(_sym_flags) > _a_flags += $(_sym_flags) > +_c_flags += $(_sym_flags) > endif Confused, Rusty. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-08 0:03 ` Rusty Russell @ 2013-03-08 9:15 ` James Hogan 2013-03-11 6:35 ` Rusty Russell 0 siblings, 1 reply; 15+ messages in thread From: James Hogan @ 2013-03-08 9:15 UTC (permalink / raw) To: Rusty Russell Cc: Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next Hi Rusty, On 08/03/13 00:03, Rusty Russell wrote: > James Hogan <james.hogan@imgtec.com> writes: >> Also the definition of SYMBOL_PREFIX in <linux/kernel.h> is removed as >> it conflicts, isn't used anywhere, and is defined as a string so differs >> from the assembly definition. > > So now, if CONFIG_SYMBOL_PREFIX, SYMBOL_PREFIX is defined on the cmdline > as a string. Otherwise it's empty (not the empty string?): No, SYMBOL_PREFIX is now defined as a non-string, same as asm files, but the now unused definition in linux/kernel.h did define it as a string as it used CONFIG_SYMBOL_PREFIX which is a string. When I said "and is defined as a string" I was referring to the one in linux/kernel.h that this removes. Does that make sense? It's all a bit messy unfortunately (hence RFC). Cheers James ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-08 9:15 ` James Hogan @ 2013-03-11 6:35 ` Rusty Russell 2013-03-11 12:07 ` Stephen Rothwell 0 siblings, 1 reply; 15+ messages in thread From: Rusty Russell @ 2013-03-11 6:35 UTC (permalink / raw) To: James Hogan Cc: Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next James Hogan <james.hogan@imgtec.com> writes: > Hi Rusty, > > On 08/03/13 00:03, Rusty Russell wrote: >> James Hogan <james.hogan@imgtec.com> writes: >>> Also the definition of SYMBOL_PREFIX in <linux/kernel.h> is removed as >>> it conflicts, isn't used anywhere, and is defined as a string so differs >>> from the assembly definition. >> >> So now, if CONFIG_SYMBOL_PREFIX, SYMBOL_PREFIX is defined on the cmdline >> as a string. Otherwise it's empty (not the empty string?): > > No, SYMBOL_PREFIX is now defined as a non-string, same as asm files Ah, I misread this. It's stripping the quotes, not adding them: _sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX)) > Does that make sense? It's all a bit messy unfortunately (hence RFC). Yes, let's clear it up once and for all. Does this work? CONFIG_SYMBOL_PREFIX: cleanup. We have CONFIG_SYMBOL_PREFIX, which three archs define to the string "_". But Al Viro broke this in "consolidate cond_syscall and SYSCALL_ALIAS declarations", and he's not the first to do so. Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to prefix it so something. So various places define helpers which are defined to nothing if CONFIG_SYMBOL_PREFIX isn't set: 1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX. 2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym) 3) include/linux/export.h defines MODULE_SYMBOL_PREFIX. 4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7) 5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym) 6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX 7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version for pasting. (arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too). Let's solve this properly: 1) No more generic prefix, just CONFIG_SYMBOL_PREFIX_UNDERSCORE. 2) Make linux/export.h usable from asm. 3) Define VMLINUX_SYMBOL, VMLINUX_SYMBOL_NAME and VMLINUX_SYMBOL_PREFIX_STR. 4) Make everyone use them. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> diff --git a/Makefile b/Makefile index 5bd9f77..b2e3e1b 100644 --- a/Makefile +++ b/Makefile @@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN $(wildcard $(rm-files)) # Run depmod only if we have System.map and depmod is executable quiet_cmd_depmod = DEPMOD $(KERNELRELEASE) cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \ - $(KERNELRELEASE) "$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))" + $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_SYMBOL_PREFIX_UNDERSCORE))" # Create temporary dir for module support files # clean it up only when building all modules diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig index 600494c..cd0f7c7 100644 --- a/arch/blackfin/Kconfig +++ b/arch/blackfin/Kconfig @@ -1,6 +1,5 @@ -config SYMBOL_PREFIX - string - default "_" +config SYMBOL_PREFIX_UNDERSCORE + def_bool y config MMU def_bool n diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig index ae8551e..ec9924d 100644 --- a/arch/h8300/Kconfig +++ b/arch/h8300/Kconfig @@ -13,9 +13,8 @@ config H8300 select OLD_SIGSUSPEND3 select OLD_SIGACTION -config SYMBOL_PREFIX - string - default "_" +config SYMBOL_PREFIX_UNDERSCORE + def_bool y config MMU bool diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig index afc8973..e8719b4 100644 --- a/arch/metag/Kconfig +++ b/arch/metag/Kconfig @@ -1,6 +1,5 @@ -config SYMBOL_PREFIX - string - default "_" +config SYMBOL_PREFIX_UNDERSCORE + def_bool y config METAG def_bool y diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c index 3b9a284..00d20ad 100644 --- a/drivers/mtd/chips/gen_probe.c +++ b/drivers/mtd/chips/gen_probe.c @@ -204,14 +204,14 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map, struct cfi_private *cfi = map->fldrv_priv; __u16 type = primary?cfi->cfiq->P_ID:cfi->cfiq->A_ID; #ifdef CONFIG_MODULES - char probename[16+sizeof(MODULE_SYMBOL_PREFIX)]; + char probename[16+sizeof(VMLINUX_SYMBOL_PREFIX_STR)]; cfi_cmdset_fn_t *probe_function; - sprintf(probename, MODULE_SYMBOL_PREFIX "cfi_cmdset_%4.4X", type); + sprintf(probename, VMLINUX_SYMBOL_PREFIX_STR "cfi_cmdset_%4.4X", type); probe_function = __symbol_get(probename); if (!probe_function) { - request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1); + request_module(probename + sizeof(VMLINUX_SYMBOL_PREFIX_STR)-1); probe_function = __symbol_get(probename); } diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h index 4077b5d..80122d4 100644 --- a/include/asm-generic/unistd.h +++ b/include/asm-generic/unistd.h @@ -1,4 +1,5 @@ #include <uapi/asm-generic/unistd.h> +#include <linux/export.h> /* * These are required system calls, we should @@ -17,12 +18,7 @@ * but it doesn't work on all toolchains, so we just do it by hand */ #ifndef cond_syscall -#ifdef CONFIG_SYMBOL_PREFIX -#define __SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define __SYMBOL_PREFIX -#endif -#define cond_syscall(x) asm(".weak\t" __SYMBOL_PREFIX #x "\n\t" \ - ".set\t" __SYMBOL_PREFIX #x "," \ - __SYMBOL_PREFIX "sys_ni_syscall") +#define cond_syscall(x) asm(".weak\t" SYMBOL_NAME_STR(x) "\n\t" \ + ".set\t" SYMBOL_NAME_STR(x) "," \ + SYMBOL_NAME_STR(sys_ni_syscall)) #endif diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index afa12c7..eb58d2d 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -52,13 +52,7 @@ #define LOAD_OFFSET 0 #endif -#ifndef SYMBOL_PREFIX -#define VMLINUX_SYMBOL(sym) sym -#else -#define PASTE2(x,y) x##y -#define PASTE(x,y) PASTE2(x,y) -#define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym) -#endif +#include <linux/export.h> /* Align . to a 8 byte boundary equals to maximum function alignment. */ #define ALIGN_FUNCTION() . = ALIGN(8) diff --git a/include/linux/export.h b/include/linux/export.h index 696c0f4..c3c1cfc 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -7,15 +7,27 @@ * * If you feel the need to add #include <linux/foo.h> to this file * then you are doing something wrong and should go away silently. + * + * If you think the above arrogance just encourages more people to add + * random crap to this file, you're not alone. */ /* Some toolchains use a `_' prefix for all user symbols. */ -#ifdef CONFIG_SYMBOL_PREFIX -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX +#ifdef CONFIG_SYMBOL_PREFIX_UNDERSCORE +#define __VMLINUX_SYMBOL(x) _##x +#define __VMLINUX_SYMBOL_STR(x) "_" #x +#define VMLINUX_SYMBOL_PREFIX_STR "_" #else -#define MODULE_SYMBOL_PREFIX "" +#define __VMLINUX_SYMBOL(x) x +#define __VMLINUX_SYMBOL_STR(x) #x +#define VMLINUX_SYMBOL_PREFIX_STR "" #endif +/* Indirect, so macros are expanded before pasting. */ +#define VMLINUX_SYMBOL(x) __VMLINUX_SYMBOL(x) +#define VMLINUX_SYMBOL_STR(x) __VMLINUX_SYMBOL_STR(x) + +#ifndef __ASSEMBLY__ struct kernel_symbol { unsigned long value; @@ -51,7 +63,7 @@ extern struct module __this_module; __CRC_SYMBOL(sym, sec) \ static const char __kstrtab_##sym[] \ __attribute__((section("__ksymtab_strings"), aligned(1))) \ - = MODULE_SYMBOL_PREFIX #sym; \ + = VMLINUX_SYMBOL_STR(sym); \ static const struct kernel_symbol __ksymtab_##sym \ __used \ __attribute__((section("___ksymtab" sec "+" #sym), unused)) \ @@ -85,5 +97,6 @@ extern struct module __this_module; #define EXPORT_UNUSED_SYMBOL_GPL(sym) #endif /* CONFIG_MODULES */ +#endif /* !__ASSEMBLY__ */ #endif /* _LINUX_EXPORT_H */ diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 80d3687..e13e992 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -723,13 +723,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } /* Trap pasters of __FUNCTION__ at compile-time */ #define __FUNCTION__ (__func__) -/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */ -#ifdef CONFIG_SYMBOL_PREFIX -#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define SYMBOL_PREFIX "" -#endif - /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */ #ifdef CONFIG_FTRACE_MCOUNT_RECORD # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD diff --git a/include/linux/module.h b/include/linux/module.h index ead1b57..46f1ea0 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -190,7 +190,7 @@ extern int modules_disabled; /* for sysctl */ /* Get/put a kernel symbol (calls must be symmetric) */ void *__symbol_get(const char *symbol); void *__symbol_get_gpl(const char *symbol); -#define symbol_get(x) ((typeof(&x))(__symbol_get(MODULE_SYMBOL_PREFIX #x))) +#define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x)))) /* modules using other modules: kdb wants to see this. */ struct module_use { @@ -453,7 +453,7 @@ extern void __module_put_and_exit(struct module *mod, long code) #ifdef CONFIG_MODULE_UNLOAD unsigned long module_refcount(struct module *mod); void __symbol_put(const char *symbol); -#define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x) +#define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x)) void symbol_put_addr(void *addr); /* Sometimes we know we already have a refcount, and it's easier not diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S index 246b4c6..4a9a86d 100644 --- a/kernel/modsign_certificate.S +++ b/kernel/modsign_certificate.S @@ -1,15 +1,8 @@ -/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */ -#ifndef SYMBOL_PREFIX -#define ASM_SYMBOL(sym) sym -#else -#define PASTE2(x,y) x##y -#define PASTE(x,y) PASTE2(x,y) -#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym) -#endif +#include <linux/export.h> #define GLOBAL(name) \ - .globl ASM_SYMBOL(name); \ - ASM_SYMBOL(name): + .globl VMLINUX_SYMBOL(name); \ + VMLINUX_SYMBOL(name): .section ".init.data","aw" diff --git a/kernel/module.c b/kernel/module.c index 0925c9a..cfd4a3f 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1209,7 +1209,7 @@ static inline int check_modstruct_version(Elf_Shdr *sechdrs, /* Since this should be found in kernel (which can't be removed), * no locking is necessary. */ - if (!find_symbol(MODULE_SYMBOL_PREFIX "module_layout", NULL, + if (!find_symbol(VMLINUX_SYMBOL_STR(module_layout), NULL, &crc, true, false)) BUG(); return check_version(sechdrs, versindex, "module_layout", mod, crc, diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 07125e6..a373a1f 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -119,13 +119,6 @@ _c_flags += $(if $(patsubst n%,, \ $(CFLAGS_GCOV)) endif -ifdef CONFIG_SYMBOL_PREFIX -_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX)) -_cpp_flags += $(_sym_flags) -_a_flags += $(_sym_flags) -endif - - # If building the kernel in a separate objtree expand all occurrences # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/'). diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 3d569d6..d767681 100644 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -74,9 +74,8 @@ kallsyms() info KSYM ${2} local kallsymopt; - if [ -n "${CONFIG_SYMBOL_PREFIX}" ]; then - kallsymopt="${kallsymopt} \ - --symbol-prefix=${CONFIG_SYMBOL_PREFIX}" + if [ -n "${CONFIG_SYMBOL_PREFIX_UNDERSCORE}" ]; then + kallsymopt="${kallsymopt} --symbol-prefix=_" fi if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 78b30c1..6985021 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -18,14 +18,7 @@ #include "modpost.h" #include "../../include/generated/autoconf.h" #include "../../include/linux/license.h" - -/* Some toolchains use a `_' prefix for all user symbols. */ -#ifdef CONFIG_SYMBOL_PREFIX -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define MODULE_SYMBOL_PREFIX "" -#endif - +#include "../../include/linux/export.h" /* Are we using CONFIG_MODVERSIONS? */ int modversions = 0; @@ -562,7 +555,7 @@ static void parse_elf_finish(struct elf_info *info) static int ignore_undef_symbol(struct elf_info *info, const char *symname) { /* ignore __this_module, it will be resolved shortly */ - if (strcmp(symname, MODULE_SYMBOL_PREFIX "__this_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(__this_module)) == 0) return 1; /* ignore global offset table */ if (strcmp(symname, "_GLOBAL_OFFSET_TABLE_") == 0) @@ -583,8 +576,8 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname) return 0; } -#define CRC_PFX MODULE_SYMBOL_PREFIX "__crc_" -#define KSYMTAB_PFX MODULE_SYMBOL_PREFIX "__ksymtab_" +#define CRC_PFX VMLINUX_SYMBOL_STR(__crc_) +#define KSYMTAB_PFX VMLINUX_SYMBOL_STR(__ksymtab_) static void handle_modversions(struct module *mod, struct elf_info *info, Elf_Sym *sym, const char *symname) @@ -637,11 +630,11 @@ static void handle_modversions(struct module *mod, struct elf_info *info, } #endif - if (memcmp(symname, MODULE_SYMBOL_PREFIX, - strlen(MODULE_SYMBOL_PREFIX)) == 0) { + if (memcmp(symname, VMLINUX_SYMBOL_PREFIX_STR, + strlen(VMLINUX_SYMBOL_PREFIX_STR)) == 0) { mod->unres = alloc_symbol(symname + - strlen(MODULE_SYMBOL_PREFIX), + strlen(VMLINUX_SYMBOL_PREFIX_STR), ELF_ST_BIND(sym->st_info) == STB_WEAK, mod->unres); } @@ -652,9 +645,9 @@ static void handle_modversions(struct module *mod, struct elf_info *info, sym_add_exported(symname + strlen(KSYMTAB_PFX), mod, export); } - if (strcmp(symname, MODULE_SYMBOL_PREFIX "init_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(init_module)) == 0) mod->has_init = 1; - if (strcmp(symname, MODULE_SYMBOL_PREFIX "cleanup_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(cleanup_module)) == 0) mod->has_cleanup = 1; break; } ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-11 6:35 ` Rusty Russell @ 2013-03-11 12:07 ` Stephen Rothwell 2013-03-12 4:48 ` Rusty Russell 0 siblings, 1 reply; 15+ messages in thread From: Stephen Rothwell @ 2013-03-11 12:07 UTC (permalink / raw) To: Rusty Russell Cc: James Hogan, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next [-- Attachment #1: Type: text/plain, Size: 757 bytes --] Hi Rusty, On Mon, 11 Mar 2013 17:05:17 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote: > > diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig > index 600494c..cd0f7c7 100644 > --- a/arch/blackfin/Kconfig > +++ b/arch/blackfin/Kconfig > @@ -1,6 +1,5 @@ > -config SYMBOL_PREFIX > - string > - default "_" > +config SYMBOL_PREFIX_UNDERSCORE > + def_bool y Please, if you are going to do this, just have config SYMBOL_PREFIX_UNDERSCORE bool in one Kconfig file (probably arch/Kconfig) and then select it from the appropriate architecture Kconfig files. You could even put a help text on it to explain why a new architecture may need to select it. -- Cheers, Stephen Rothwell sfr@canb.auug.org.au [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-11 12:07 ` Stephen Rothwell @ 2013-03-12 4:48 ` Rusty Russell 2013-03-12 12:33 ` Stephen Rothwell 2013-03-12 12:36 ` James Hogan 0 siblings, 2 replies; 15+ messages in thread From: Rusty Russell @ 2013-03-12 4:48 UTC (permalink / raw) To: Stephen Rothwell Cc: James Hogan, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next Stephen Rothwell <sfr@canb.auug.org.au> writes: > Hi Rusty, > > On Mon, 11 Mar 2013 17:05:17 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote: >> >> diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig >> index 600494c..cd0f7c7 100644 >> --- a/arch/blackfin/Kconfig >> +++ b/arch/blackfin/Kconfig >> @@ -1,6 +1,5 @@ >> -config SYMBOL_PREFIX >> - string >> - default "_" >> +config SYMBOL_PREFIX_UNDERSCORE >> + def_bool y > > Please, if you are going to do this, just have > > config SYMBOL_PREFIX_UNDERSCORE > bool > > in one Kconfig file (probably arch/Kconfig) and then select it from the > appropriate architecture Kconfig files. You could even put a help text > on it to explain why a new architecture may need to select it. > > -- > Cheers, > Stephen Rothwell sfr@canb.auug.org.au OK. v2: Rename CONFIG_SYMBOL_PREFIX_UNDERSCORE to CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX, which is defined in arch/Kconfig and selected by the 3 archs which need it. Subject: CONFIG_SYMBOL_PREFIX: cleanup. We have CONFIG_SYMBOL_PREFIX, which three archs define to the string "_". But Al Viro broke this in "consolidate cond_syscall and SYSCALL_ALIAS declarations", and he's not the first to do so. Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to prefix it so something. So various places define helpers which are defined to nothing if CONFIG_SYMBOL_PREFIX isn't set: 1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX. 2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym) 3) include/linux/export.h defines MODULE_SYMBOL_PREFIX. 4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7) 5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym) 6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX 7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version for pasting. (arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too). Let's solve this properly: 1) No more generic prefix, just CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX. 2) Make linux/export.h usable from asm. 3) Define VMLINUX_SYMBOL, VMLINUX_SYMBOL_NAME and VMLINUX_SYMBOL_PREFIX_STR. 4) Make everyone use them. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> diff --git a/Makefile b/Makefile index a05ea42..9dc948d 100644 --- a/Makefile +++ b/Makefile @@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN $(wildcard $(rm-files)) # Run depmod only if we have System.map and depmod is executable quiet_cmd_depmod = DEPMOD $(KERNELRELEASE) cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \ - $(KERNELRELEASE) "$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))" + $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_SYMBOL_PREFIX_UNDERSCORE))" # Create temporary dir for module support files # clean it up only when building all modules diff --git a/arch/Kconfig b/arch/Kconfig index 5a1779c..12ba8f9 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -391,6 +391,12 @@ config MODULES_USE_ELF_REL Modules only use ELF REL relocations. Modules with ELF RELA relocations will give an error. +config HAVE_UNDERSCORE_SYMBOL_PREFIX + bool + help + Some architectures generate an _ in front of C symbols; things like + module loading and assembly files need to know about this. + # # ABI hall of shame # diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig index 600494c..314ee6a 100644 --- a/arch/blackfin/Kconfig +++ b/arch/blackfin/Kconfig @@ -1,7 +1,3 @@ -config SYMBOL_PREFIX - string - default "_" - config MMU def_bool n @@ -33,6 +29,7 @@ config BLACKFIN select ARCH_HAVE_CUSTOM_GPIO_H select ARCH_WANT_OPTIONAL_GPIOLIB select HAVE_UID16 + select HAVE_UNDERSCORE_SYMBOL_PREFIX select HAVE_VIRT_TO_BUS select ARCH_WANT_IPC_PARSE_VERSION select HAVE_GENERIC_HARDIRQS @@ -45,6 +42,7 @@ config BLACKFIN select HAVE_MOD_ARCH_SPECIFIC select MODULES_USE_ELF_RELA + config GENERIC_CSUM def_bool y diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig index ae8551e..ba043e3 100644 --- a/arch/h8300/Kconfig +++ b/arch/h8300/Kconfig @@ -12,10 +12,10 @@ config H8300 select MODULES_USE_ELF_RELA select OLD_SIGSUSPEND3 select OLD_SIGACTION + select HAVE_UNDERSCORE_SYMBOL_PREFIX -config SYMBOL_PREFIX - string - default "_" +config SYMBOL_PREFIX_UNDERSCORE + def_bool y config MMU bool diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig index afc8973..88272ce 100644 --- a/arch/metag/Kconfig +++ b/arch/metag/Kconfig @@ -1,6 +1,5 @@ -config SYMBOL_PREFIX - string - default "_" +config SYMBOL_PREFIX_UNDERSCORE + def_bool y config METAG def_bool y @@ -27,6 +26,7 @@ config METAG select HAVE_MOD_ARCH_SPECIFIC select HAVE_PERF_EVENTS select HAVE_SYSCALL_TRACEPOINTS + select HAVE_UNDERSCORE_SYMBOL_PREFIX select IRQ_DOMAIN select MODULES_USE_ELF_RELA select OF diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c index 3b9a284..00d20ad 100644 --- a/drivers/mtd/chips/gen_probe.c +++ b/drivers/mtd/chips/gen_probe.c @@ -204,14 +204,14 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map, struct cfi_private *cfi = map->fldrv_priv; __u16 type = primary?cfi->cfiq->P_ID:cfi->cfiq->A_ID; #ifdef CONFIG_MODULES - char probename[16+sizeof(MODULE_SYMBOL_PREFIX)]; + char probename[16+sizeof(VMLINUX_SYMBOL_PREFIX_STR)]; cfi_cmdset_fn_t *probe_function; - sprintf(probename, MODULE_SYMBOL_PREFIX "cfi_cmdset_%4.4X", type); + sprintf(probename, VMLINUX_SYMBOL_PREFIX_STR "cfi_cmdset_%4.4X", type); probe_function = __symbol_get(probename); if (!probe_function) { - request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1); + request_module(probename + sizeof(VMLINUX_SYMBOL_PREFIX_STR)-1); probe_function = __symbol_get(probename); } diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h index 4077b5d..80122d4 100644 --- a/include/asm-generic/unistd.h +++ b/include/asm-generic/unistd.h @@ -1,4 +1,5 @@ #include <uapi/asm-generic/unistd.h> +#include <linux/export.h> /* * These are required system calls, we should @@ -17,12 +18,7 @@ * but it doesn't work on all toolchains, so we just do it by hand */ #ifndef cond_syscall -#ifdef CONFIG_SYMBOL_PREFIX -#define __SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define __SYMBOL_PREFIX -#endif -#define cond_syscall(x) asm(".weak\t" __SYMBOL_PREFIX #x "\n\t" \ - ".set\t" __SYMBOL_PREFIX #x "," \ - __SYMBOL_PREFIX "sys_ni_syscall") +#define cond_syscall(x) asm(".weak\t" SYMBOL_NAME_STR(x) "\n\t" \ + ".set\t" SYMBOL_NAME_STR(x) "," \ + SYMBOL_NAME_STR(sys_ni_syscall)) #endif diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index afa12c7..eb58d2d 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -52,13 +52,7 @@ #define LOAD_OFFSET 0 #endif -#ifndef SYMBOL_PREFIX -#define VMLINUX_SYMBOL(sym) sym -#else -#define PASTE2(x,y) x##y -#define PASTE(x,y) PASTE2(x,y) -#define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym) -#endif +#include <linux/export.h> /* Align . to a 8 byte boundary equals to maximum function alignment. */ #define ALIGN_FUNCTION() . = ALIGN(8) diff --git a/include/linux/export.h b/include/linux/export.h index 696c0f4..0080e93 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -7,15 +7,27 @@ * * If you feel the need to add #include <linux/foo.h> to this file * then you are doing something wrong and should go away silently. + * + * If you think the above arrogance just encourages more people to add + * random crap to this file, you're not alone. */ /* Some toolchains use a `_' prefix for all user symbols. */ -#ifdef CONFIG_SYMBOL_PREFIX -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX +#ifdef CONFIG_SYMBOL_PREFIX_UNDERSCORE +#define __VMLINUX_SYMBOL(x) _##x +#define __VMLINUX_SYMBOL_STR(x) "_" #x +#define VMLINUX_SYMBOL_PREFIX_STR "_" #else -#define MODULE_SYMBOL_PREFIX "" +#define __VMLINUX_SYMBOL(x) x +#define __VMLINUX_SYMBOL_STR(x) #x +#define VMLINUX_SYMBOL_PREFIX_STR "" #endif +/* Indirect, so macros are expanded before pasting. */ +#define VMLINUX_SYMBOL(x) __VMLINUX_SYMBOL(x) +#define VMLINUX_SYMBOL_STR(x) __VMLINUX_SYMBOL_STR(x) + +#ifndef __ASSEMBLY__ struct kernel_symbol { unsigned long value; @@ -51,7 +63,7 @@ extern struct module __this_module; __CRC_SYMBOL(sym, sec) \ static const char __kstrtab_##sym[] \ __attribute__((section("__ksymtab_strings"), aligned(1))) \ - = MODULE_SYMBOL_PREFIX #sym; \ + = VMLINUX_SYMBOL_STR(sym); \ static const struct kernel_symbol __ksymtab_##sym \ __used \ __attribute__((section("___ksymtab" sec "+" #sym), unused)) \ @@ -85,5 +97,6 @@ extern struct module __this_module; #define EXPORT_UNUSED_SYMBOL_GPL(sym) #endif /* CONFIG_MODULES */ +#endif /* !__ASSEMBLY__ */ #endif /* _LINUX_EXPORT_H */ diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 80d3687..e13e992 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -723,13 +723,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } /* Trap pasters of __FUNCTION__ at compile-time */ #define __FUNCTION__ (__func__) -/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */ -#ifdef CONFIG_SYMBOL_PREFIX -#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define SYMBOL_PREFIX "" -#endif - /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */ #ifdef CONFIG_FTRACE_MCOUNT_RECORD # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD diff --git a/include/linux/module.h b/include/linux/module.h index ead1b57..46f1ea0 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -190,7 +190,7 @@ extern int modules_disabled; /* for sysctl */ /* Get/put a kernel symbol (calls must be symmetric) */ void *__symbol_get(const char *symbol); void *__symbol_get_gpl(const char *symbol); -#define symbol_get(x) ((typeof(&x))(__symbol_get(MODULE_SYMBOL_PREFIX #x))) +#define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x)))) /* modules using other modules: kdb wants to see this. */ struct module_use { @@ -453,7 +453,7 @@ extern void __module_put_and_exit(struct module *mod, long code) #ifdef CONFIG_MODULE_UNLOAD unsigned long module_refcount(struct module *mod); void __symbol_put(const char *symbol); -#define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x) +#define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x)) void symbol_put_addr(void *addr); /* Sometimes we know we already have a refcount, and it's easier not diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S index 246b4c6..4a9a86d 100644 --- a/kernel/modsign_certificate.S +++ b/kernel/modsign_certificate.S @@ -1,15 +1,8 @@ -/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */ -#ifndef SYMBOL_PREFIX -#define ASM_SYMBOL(sym) sym -#else -#define PASTE2(x,y) x##y -#define PASTE(x,y) PASTE2(x,y) -#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym) -#endif +#include <linux/export.h> #define GLOBAL(name) \ - .globl ASM_SYMBOL(name); \ - ASM_SYMBOL(name): + .globl VMLINUX_SYMBOL(name); \ + VMLINUX_SYMBOL(name): .section ".init.data","aw" diff --git a/kernel/module.c b/kernel/module.c index 0925c9a..cfd4a3f 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1209,7 +1209,7 @@ static inline int check_modstruct_version(Elf_Shdr *sechdrs, /* Since this should be found in kernel (which can't be removed), * no locking is necessary. */ - if (!find_symbol(MODULE_SYMBOL_PREFIX "module_layout", NULL, + if (!find_symbol(VMLINUX_SYMBOL_STR(module_layout), NULL, &crc, true, false)) BUG(); return check_version(sechdrs, versindex, "module_layout", mod, crc, diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 07125e6..a373a1f 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -119,13 +119,6 @@ _c_flags += $(if $(patsubst n%,, \ $(CFLAGS_GCOV)) endif -ifdef CONFIG_SYMBOL_PREFIX -_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX)) -_cpp_flags += $(_sym_flags) -_a_flags += $(_sym_flags) -endif - - # If building the kernel in a separate objtree expand all occurrences # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/'). diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 3d569d6..d767681 100644 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -74,9 +74,8 @@ kallsyms() info KSYM ${2} local kallsymopt; - if [ -n "${CONFIG_SYMBOL_PREFIX}" ]; then - kallsymopt="${kallsymopt} \ - --symbol-prefix=${CONFIG_SYMBOL_PREFIX}" + if [ -n "${CONFIG_SYMBOL_PREFIX_UNDERSCORE}" ]; then + kallsymopt="${kallsymopt} --symbol-prefix=_" fi if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 78b30c1..6985021 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -18,14 +18,7 @@ #include "modpost.h" #include "../../include/generated/autoconf.h" #include "../../include/linux/license.h" - -/* Some toolchains use a `_' prefix for all user symbols. */ -#ifdef CONFIG_SYMBOL_PREFIX -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define MODULE_SYMBOL_PREFIX "" -#endif - +#include "../../include/linux/export.h" /* Are we using CONFIG_MODVERSIONS? */ int modversions = 0; @@ -562,7 +555,7 @@ static void parse_elf_finish(struct elf_info *info) static int ignore_undef_symbol(struct elf_info *info, const char *symname) { /* ignore __this_module, it will be resolved shortly */ - if (strcmp(symname, MODULE_SYMBOL_PREFIX "__this_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(__this_module)) == 0) return 1; /* ignore global offset table */ if (strcmp(symname, "_GLOBAL_OFFSET_TABLE_") == 0) @@ -583,8 +576,8 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname) return 0; } -#define CRC_PFX MODULE_SYMBOL_PREFIX "__crc_" -#define KSYMTAB_PFX MODULE_SYMBOL_PREFIX "__ksymtab_" +#define CRC_PFX VMLINUX_SYMBOL_STR(__crc_) +#define KSYMTAB_PFX VMLINUX_SYMBOL_STR(__ksymtab_) static void handle_modversions(struct module *mod, struct elf_info *info, Elf_Sym *sym, const char *symname) @@ -637,11 +630,11 @@ static void handle_modversions(struct module *mod, struct elf_info *info, } #endif - if (memcmp(symname, MODULE_SYMBOL_PREFIX, - strlen(MODULE_SYMBOL_PREFIX)) == 0) { + if (memcmp(symname, VMLINUX_SYMBOL_PREFIX_STR, + strlen(VMLINUX_SYMBOL_PREFIX_STR)) == 0) { mod->unres = alloc_symbol(symname + - strlen(MODULE_SYMBOL_PREFIX), + strlen(VMLINUX_SYMBOL_PREFIX_STR), ELF_ST_BIND(sym->st_info) == STB_WEAK, mod->unres); } @@ -652,9 +645,9 @@ static void handle_modversions(struct module *mod, struct elf_info *info, sym_add_exported(symname + strlen(KSYMTAB_PFX), mod, export); } - if (strcmp(symname, MODULE_SYMBOL_PREFIX "init_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(init_module)) == 0) mod->has_init = 1; - if (strcmp(symname, MODULE_SYMBOL_PREFIX "cleanup_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(cleanup_module)) == 0) mod->has_cleanup = 1; break; } ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-12 4:48 ` Rusty Russell @ 2013-03-12 12:33 ` Stephen Rothwell 2013-03-13 0:00 ` Rusty Russell 2013-03-12 12:36 ` James Hogan 1 sibling, 1 reply; 15+ messages in thread From: Stephen Rothwell @ 2013-03-12 12:33 UTC (permalink / raw) To: Rusty Russell Cc: James Hogan, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next [-- Attachment #1: Type: text/plain, Size: 725 bytes --] Hi Rusty, Looks partly better. You seem to be using CONFIG_SYMBOL_PREFIX_UNDERSCORE but selecting CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX. One trivial comment below. Maybe this was an unfinished version of the patch? On Tue, 12 Mar 2013 15:18:15 +1030 Rusty Russell <rusty@rustcorp.com.au> wrote: > > diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig > index 600494c..314ee6a 100644 > --- a/arch/blackfin/Kconfig > +++ b/arch/blackfin/Kconfig > @@ -45,6 +42,7 @@ config BLACKFIN > select HAVE_MOD_ARCH_SPECIFIC > select MODULES_USE_ELF_RELA > > + > config GENERIC_CSUM > def_bool y Extra blank line added. -- Cheers, Stephen Rothwell sfr@canb.auug.org.au [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-12 12:33 ` Stephen Rothwell @ 2013-03-13 0:00 ` Rusty Russell 2013-03-13 6:31 ` Sam Ravnborg 0 siblings, 1 reply; 15+ messages in thread From: Rusty Russell @ 2013-03-13 0:00 UTC (permalink / raw) To: Stephen Rothwell Cc: James Hogan, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next Stephen Rothwell <sfr@canb.auug.org.au> writes: > Hi Rusty, > > Looks partly better. You seem to be using > CONFIG_SYMBOL_PREFIX_UNDERSCORE but selecting > CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX. One trivial comment below. > > Maybe this was an unfinished version of the patch? Indeed. It was crap.... now I've applied James' copious fixes, and actually, y'know, proof-read it. This is what I'll push into my fixes/ branch now, and if it survives a day in linux-next, will push to Linus. Thanks, Rusty. v2: Rename CONFIG_SYMBOL_PREFIX_UNDERSCORE to CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX, which is defined in arch/Kconfig and selected by the 3 archs which need it. v3: Clean up remnants of v1, s/VMLINUX_SYMBOL_NAME/VMLINUX_SYMBOL_STR/ in description, note that Al's commit is in linux-next only, remove whitespace-only hunk. CONFIG_SYMBOL_PREFIX: cleanup. We have CONFIG_SYMBOL_PREFIX, which three archs define to the string "_". But Al Viro broke this in "consolidate cond_syscall and SYSCALL_ALIAS declarations" (in linux-next), and he's not the first to do so. Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to prefix it so something. So various places define helpers which are defined to nothing if CONFIG_SYMBOL_PREFIX isn't set: 1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX. 2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym) 3) include/linux/export.h defines MODULE_SYMBOL_PREFIX. 4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7) 5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym) 6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX 7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version for pasting. (arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too). Let's solve this properly: 1) No more generic prefix, just CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX. 2) Make linux/export.h usable from asm. 3) Define VMLINUX_SYMBOL, VMLINUX_SYMBOL_STR and VMLINUX_SYMBOL_PREFIX_STR. 4) Make everyone use them. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Reviewed-by: James Hogan <james.hogan@imgtec.com> Tested-by: James Hogan <james.hogan@imgtec.com> (on metag) diff --git a/Makefile b/Makefile index a05ea42..10fb6c7 100644 --- a/Makefile +++ b/Makefile @@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN $(wildcard $(rm-files)) # Run depmod only if we have System.map and depmod is executable quiet_cmd_depmod = DEPMOD $(KERNELRELEASE) cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \ - $(KERNELRELEASE) "$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))" + $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_HAVE_SYMBOL_PREFIX_UNDERSCORE))" # Create temporary dir for module support files # clean it up only when building all modules diff --git a/arch/Kconfig b/arch/Kconfig index 5a1779c..12ba8f9 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -391,6 +391,12 @@ config MODULES_USE_ELF_REL Modules only use ELF REL relocations. Modules with ELF RELA relocations will give an error. +config HAVE_UNDERSCORE_SYMBOL_PREFIX + bool + help + Some architectures generate an _ in front of C symbols; things like + module loading and assembly files need to know about this. + # # ABI hall of shame # diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig index 600494c..314ee6a 100644 --- a/arch/blackfin/Kconfig +++ b/arch/blackfin/Kconfig @@ -1,7 +1,3 @@ -config SYMBOL_PREFIX - string - default "_" - config MMU def_bool n @@ -33,6 +29,7 @@ config BLACKFIN select ARCH_HAVE_CUSTOM_GPIO_H select ARCH_WANT_OPTIONAL_GPIOLIB select HAVE_UID16 + select HAVE_UNDERSCORE_SYMBOL_PREFIX select HAVE_VIRT_TO_BUS select ARCH_WANT_IPC_PARSE_VERSION select HAVE_GENERIC_HARDIRQS diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig index ae8551e..2333cf6 100644 --- a/arch/h8300/Kconfig +++ b/arch/h8300/Kconfig @@ -12,10 +12,7 @@ config H8300 select MODULES_USE_ELF_RELA select OLD_SIGSUSPEND3 select OLD_SIGACTION - -config SYMBOL_PREFIX - string - default "_" + select HAVE_UNDERSCORE_SYMBOL_PREFIX config MMU bool diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig index afc8973..2099617 100644 --- a/arch/metag/Kconfig +++ b/arch/metag/Kconfig @@ -1,7 +1,3 @@ -config SYMBOL_PREFIX - string - default "_" - config METAG def_bool y select EMBEDDED @@ -27,6 +23,7 @@ config METAG select HAVE_MOD_ARCH_SPECIFIC select HAVE_PERF_EVENTS select HAVE_SYSCALL_TRACEPOINTS + select HAVE_UNDERSCORE_SYMBOL_PREFIX select IRQ_DOMAIN select MODULES_USE_ELF_RELA select OF diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c index 3b9a284..00d20ad 100644 --- a/drivers/mtd/chips/gen_probe.c +++ b/drivers/mtd/chips/gen_probe.c @@ -204,14 +204,14 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map, struct cfi_private *cfi = map->fldrv_priv; __u16 type = primary?cfi->cfiq->P_ID:cfi->cfiq->A_ID; #ifdef CONFIG_MODULES - char probename[16+sizeof(MODULE_SYMBOL_PREFIX)]; + char probename[16+sizeof(VMLINUX_SYMBOL_PREFIX_STR)]; cfi_cmdset_fn_t *probe_function; - sprintf(probename, MODULE_SYMBOL_PREFIX "cfi_cmdset_%4.4X", type); + sprintf(probename, VMLINUX_SYMBOL_PREFIX_STR "cfi_cmdset_%4.4X", type); probe_function = __symbol_get(probename); if (!probe_function) { - request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1); + request_module(probename + sizeof(VMLINUX_SYMBOL_PREFIX_STR)-1); probe_function = __symbol_get(probename); } diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h index 4077b5d..15c0598 100644 --- a/include/asm-generic/unistd.h +++ b/include/asm-generic/unistd.h @@ -1,4 +1,5 @@ #include <uapi/asm-generic/unistd.h> +#include <linux/export.h> /* * These are required system calls, we should @@ -17,12 +18,7 @@ * but it doesn't work on all toolchains, so we just do it by hand */ #ifndef cond_syscall -#ifdef CONFIG_SYMBOL_PREFIX -#define __SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define __SYMBOL_PREFIX -#endif -#define cond_syscall(x) asm(".weak\t" __SYMBOL_PREFIX #x "\n\t" \ - ".set\t" __SYMBOL_PREFIX #x "," \ - __SYMBOL_PREFIX "sys_ni_syscall") +#define cond_syscall(x) asm(".weak\t" VMLINUX_SYMBOL_STR(x) "\n\t" \ + ".set\t" VMLINUX_SYMBOL_STR(x) "," \ + VMLINUX_SYMBOL_STR(sys_ni_syscall)) #endif diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index afa12c7..eb58d2d 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -52,13 +52,7 @@ #define LOAD_OFFSET 0 #endif -#ifndef SYMBOL_PREFIX -#define VMLINUX_SYMBOL(sym) sym -#else -#define PASTE2(x,y) x##y -#define PASTE(x,y) PASTE2(x,y) -#define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym) -#endif +#include <linux/export.h> /* Align . to a 8 byte boundary equals to maximum function alignment. */ #define ALIGN_FUNCTION() . = ALIGN(8) diff --git a/include/linux/export.h b/include/linux/export.h index 696c0f4..fc83b2a 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -7,15 +7,27 @@ * * If you feel the need to add #include <linux/foo.h> to this file * then you are doing something wrong and should go away silently. + * + * If you think the above arrogance just encourages more people to add + * random crap to this file, you're not alone. */ /* Some toolchains use a `_' prefix for all user symbols. */ -#ifdef CONFIG_SYMBOL_PREFIX -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX +#define __VMLINUX_SYMBOL(x) _##x +#define __VMLINUX_SYMBOL_STR(x) "_" #x +#define VMLINUX_SYMBOL_PREFIX_STR "_" #else -#define MODULE_SYMBOL_PREFIX "" +#define __VMLINUX_SYMBOL(x) x +#define __VMLINUX_SYMBOL_STR(x) #x +#define VMLINUX_SYMBOL_PREFIX_STR "" #endif +/* Indirect, so macros are expanded before pasting. */ +#define VMLINUX_SYMBOL(x) __VMLINUX_SYMBOL(x) +#define VMLINUX_SYMBOL_STR(x) __VMLINUX_SYMBOL_STR(x) + +#ifndef __ASSEMBLY__ struct kernel_symbol { unsigned long value; @@ -51,7 +63,7 @@ extern struct module __this_module; __CRC_SYMBOL(sym, sec) \ static const char __kstrtab_##sym[] \ __attribute__((section("__ksymtab_strings"), aligned(1))) \ - = MODULE_SYMBOL_PREFIX #sym; \ + = VMLINUX_SYMBOL_STR(sym); \ static const struct kernel_symbol __ksymtab_##sym \ __used \ __attribute__((section("___ksymtab" sec "+" #sym), unused)) \ @@ -85,5 +97,6 @@ extern struct module __this_module; #define EXPORT_UNUSED_SYMBOL_GPL(sym) #endif /* CONFIG_MODULES */ +#endif /* !__ASSEMBLY__ */ #endif /* _LINUX_EXPORT_H */ diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 80d3687..e13e992 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -723,13 +723,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } /* Trap pasters of __FUNCTION__ at compile-time */ #define __FUNCTION__ (__func__) -/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */ -#ifdef CONFIG_SYMBOL_PREFIX -#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define SYMBOL_PREFIX "" -#endif - /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */ #ifdef CONFIG_FTRACE_MCOUNT_RECORD # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD diff --git a/include/linux/module.h b/include/linux/module.h index ead1b57..46f1ea0 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -190,7 +190,7 @@ extern int modules_disabled; /* for sysctl */ /* Get/put a kernel symbol (calls must be symmetric) */ void *__symbol_get(const char *symbol); void *__symbol_get_gpl(const char *symbol); -#define symbol_get(x) ((typeof(&x))(__symbol_get(MODULE_SYMBOL_PREFIX #x))) +#define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x)))) /* modules using other modules: kdb wants to see this. */ struct module_use { @@ -453,7 +453,7 @@ extern void __module_put_and_exit(struct module *mod, long code) #ifdef CONFIG_MODULE_UNLOAD unsigned long module_refcount(struct module *mod); void __symbol_put(const char *symbol); -#define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x) +#define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x)) void symbol_put_addr(void *addr); /* Sometimes we know we already have a refcount, and it's easier not diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S index 246b4c6..4a9a86d 100644 --- a/kernel/modsign_certificate.S +++ b/kernel/modsign_certificate.S @@ -1,15 +1,8 @@ -/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */ -#ifndef SYMBOL_PREFIX -#define ASM_SYMBOL(sym) sym -#else -#define PASTE2(x,y) x##y -#define PASTE(x,y) PASTE2(x,y) -#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym) -#endif +#include <linux/export.h> #define GLOBAL(name) \ - .globl ASM_SYMBOL(name); \ - ASM_SYMBOL(name): + .globl VMLINUX_SYMBOL(name); \ + VMLINUX_SYMBOL(name): .section ".init.data","aw" diff --git a/kernel/module.c b/kernel/module.c index 0925c9a..cfd4a3f 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1209,7 +1209,7 @@ static inline int check_modstruct_version(Elf_Shdr *sechdrs, /* Since this should be found in kernel (which can't be removed), * no locking is necessary. */ - if (!find_symbol(MODULE_SYMBOL_PREFIX "module_layout", NULL, + if (!find_symbol(VMLINUX_SYMBOL_STR(module_layout), NULL, &crc, true, false)) BUG(); return check_version(sechdrs, versindex, "module_layout", mod, crc, diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 07125e6..a373a1f 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -119,13 +119,6 @@ _c_flags += $(if $(patsubst n%,, \ $(CFLAGS_GCOV)) endif -ifdef CONFIG_SYMBOL_PREFIX -_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX)) -_cpp_flags += $(_sym_flags) -_a_flags += $(_sym_flags) -endif - - # If building the kernel in a separate objtree expand all occurrences # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/'). diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 3d569d6..0149949 100644 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -74,9 +74,8 @@ kallsyms() info KSYM ${2} local kallsymopt; - if [ -n "${CONFIG_SYMBOL_PREFIX}" ]; then - kallsymopt="${kallsymopt} \ - --symbol-prefix=${CONFIG_SYMBOL_PREFIX}" + if [ -n "${CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX}" ]; then + kallsymopt="${kallsymopt} --symbol-prefix=_" fi if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 78b30c1..6985021 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -18,14 +18,7 @@ #include "modpost.h" #include "../../include/generated/autoconf.h" #include "../../include/linux/license.h" - -/* Some toolchains use a `_' prefix for all user symbols. */ -#ifdef CONFIG_SYMBOL_PREFIX -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define MODULE_SYMBOL_PREFIX "" -#endif - +#include "../../include/linux/export.h" /* Are we using CONFIG_MODVERSIONS? */ int modversions = 0; @@ -562,7 +555,7 @@ static void parse_elf_finish(struct elf_info *info) static int ignore_undef_symbol(struct elf_info *info, const char *symname) { /* ignore __this_module, it will be resolved shortly */ - if (strcmp(symname, MODULE_SYMBOL_PREFIX "__this_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(__this_module)) == 0) return 1; /* ignore global offset table */ if (strcmp(symname, "_GLOBAL_OFFSET_TABLE_") == 0) @@ -583,8 +576,8 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname) return 0; } -#define CRC_PFX MODULE_SYMBOL_PREFIX "__crc_" -#define KSYMTAB_PFX MODULE_SYMBOL_PREFIX "__ksymtab_" +#define CRC_PFX VMLINUX_SYMBOL_STR(__crc_) +#define KSYMTAB_PFX VMLINUX_SYMBOL_STR(__ksymtab_) static void handle_modversions(struct module *mod, struct elf_info *info, Elf_Sym *sym, const char *symname) @@ -637,11 +630,11 @@ static void handle_modversions(struct module *mod, struct elf_info *info, } #endif - if (memcmp(symname, MODULE_SYMBOL_PREFIX, - strlen(MODULE_SYMBOL_PREFIX)) == 0) { + if (memcmp(symname, VMLINUX_SYMBOL_PREFIX_STR, + strlen(VMLINUX_SYMBOL_PREFIX_STR)) == 0) { mod->unres = alloc_symbol(symname + - strlen(MODULE_SYMBOL_PREFIX), + strlen(VMLINUX_SYMBOL_PREFIX_STR), ELF_ST_BIND(sym->st_info) == STB_WEAK, mod->unres); } @@ -652,9 +645,9 @@ static void handle_modversions(struct module *mod, struct elf_info *info, sym_add_exported(symname + strlen(KSYMTAB_PFX), mod, export); } - if (strcmp(symname, MODULE_SYMBOL_PREFIX "init_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(init_module)) == 0) mod->has_init = 1; - if (strcmp(symname, MODULE_SYMBOL_PREFIX "cleanup_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(cleanup_module)) == 0) mod->has_cleanup = 1; break; } ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-13 0:00 ` Rusty Russell @ 2013-03-13 6:31 ` Sam Ravnborg 2013-03-13 9:21 ` James Hogan 2013-03-14 4:00 ` Rusty Russell 0 siblings, 2 replies; 15+ messages in thread From: Sam Ravnborg @ 2013-03-13 6:31 UTC (permalink / raw) To: Rusty Russell Cc: Stephen Rothwell, James Hogan, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next > actually, y'know, proof-read it. Hmm.. > + $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_HAVE_SYMBOL_PREFIX_UNDERSCORE))" > > +config HAVE_UNDERSCORE_SYMBOL_PREFIX HAVE_UNDERSCORE_... or HAVE_SYMBOL_... confusion. I prefer the HAVE_SYMBOL_... variant but no strong feelings.. > + * > + * If you think the above arrogance just encourages more people to add > + * random crap to this file, you're not alone. Kill this. > /* Some toolchains use a `_' prefix for all user symbols. */ > -#ifdef CONFIG_SYMBOL_PREFIX > -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX > +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX > +#define __VMLINUX_SYMBOL(x) _##x > +#define __VMLINUX_SYMBOL_STR(x) "_" #x > +#define VMLINUX_SYMBOL_PREFIX_STR "_" > #else > -#define MODULE_SYMBOL_PREFIX "" > +#define __VMLINUX_SYMBOL(x) x > +#define __VMLINUX_SYMBOL_STR(x) #x > +#define VMLINUX_SYMBOL_PREFIX_STR "" > #endif We know the prefix is an underscore. No benefits from defining VMLINUX_SYMBOL_PREFIX_STR. The config name even syas so. Skipping the above give us only one way to check for the prefix - today we mix the two. Sam ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-13 6:31 ` Sam Ravnborg @ 2013-03-13 9:21 ` James Hogan 2013-03-13 18:15 ` Sam Ravnborg 2013-03-14 4:00 ` Rusty Russell 1 sibling, 1 reply; 15+ messages in thread From: James Hogan @ 2013-03-13 9:21 UTC (permalink / raw) To: Sam Ravnborg Cc: Rusty Russell, Stephen Rothwell, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next Hi Sam, On 13/03/13 06:31, Sam Ravnborg wrote: >> /* Some toolchains use a `_' prefix for all user symbols. */ >> -#ifdef CONFIG_SYMBOL_PREFIX >> -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX >> +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX >> +#define __VMLINUX_SYMBOL(x) _##x >> +#define __VMLINUX_SYMBOL_STR(x) "_" #x >> +#define VMLINUX_SYMBOL_PREFIX_STR "_" >> #else >> -#define MODULE_SYMBOL_PREFIX "" >> +#define __VMLINUX_SYMBOL(x) x >> +#define __VMLINUX_SYMBOL_STR(x) #x >> +#define VMLINUX_SYMBOL_PREFIX_STR "" >> #endif > > We know the prefix is an underscore. No benefits from defining > VMLINUX_SYMBOL_PREFIX_STR. scripts/modpost.c uses the actual prefix string though: > if (memcmp(symname, MODULE_SYMBOL_PREFIX, > strlen(MODULE_SYMBOL_PREFIX)) == 0) { > mod->unres = > alloc_symbol(symname + > strlen(MODULE_SYMBOL_PREFIX), > ELF_ST_BIND(sym->st_info) == STB_WEAK, > mod->unres); See also my "module: fix symbol versioning with symbol prefixes" patch for an additional use in modpost. Having a MODULE_SYMBOL_PREFIX/VMLINUX_SYMBOL_PREFIX_STR seems like the simplest way to keep this code clean and readable without special cases. Cheers James ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-13 9:21 ` James Hogan @ 2013-03-13 18:15 ` Sam Ravnborg 0 siblings, 0 replies; 15+ messages in thread From: Sam Ravnborg @ 2013-03-13 18:15 UTC (permalink / raw) To: James Hogan Cc: Rusty Russell, Stephen Rothwell, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next On Wed, Mar 13, 2013 at 09:21:01AM +0000, James Hogan wrote: > Hi Sam, > > On 13/03/13 06:31, Sam Ravnborg wrote: > >> /* Some toolchains use a `_' prefix for all user symbols. */ > >> -#ifdef CONFIG_SYMBOL_PREFIX > >> -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX > >> +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX > >> +#define __VMLINUX_SYMBOL(x) _##x > >> +#define __VMLINUX_SYMBOL_STR(x) "_" #x > >> +#define VMLINUX_SYMBOL_PREFIX_STR "_" > >> #else > >> -#define MODULE_SYMBOL_PREFIX "" > >> +#define __VMLINUX_SYMBOL(x) x > >> +#define __VMLINUX_SYMBOL_STR(x) #x > >> +#define VMLINUX_SYMBOL_PREFIX_STR "" > >> #endif > > > > We know the prefix is an underscore. No benefits from defining > > VMLINUX_SYMBOL_PREFIX_STR. > > scripts/modpost.c uses the actual prefix string though: > > > if (memcmp(symname, MODULE_SYMBOL_PREFIX, > > strlen(MODULE_SYMBOL_PREFIX)) == 0) { > > mod->unres = > > alloc_symbol(symname + > > strlen(MODULE_SYMBOL_PREFIX), > > ELF_ST_BIND(sym->st_info) == STB_WEAK, > > mod->unres); > Allowing ourself to know this is "_" then we could make the above code look something like: if (have_symbol_prefix_underscore) { if (symname[0] == '_') { mod->unres = ... But I do not feel strong about this part. It is much more important to clean up so we use the same definition all over the kernel. Sam ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-13 6:31 ` Sam Ravnborg 2013-03-13 9:21 ` James Hogan @ 2013-03-14 4:00 ` Rusty Russell 2013-03-14 10:49 ` James Hogan 1 sibling, 1 reply; 15+ messages in thread From: Rusty Russell @ 2013-03-14 4:00 UTC (permalink / raw) To: Sam Ravnborg Cc: Stephen Rothwell, James Hogan, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next, Paul Gortmaker, linux-mtd Sam Ravnborg <sam@ravnborg.org> writes: >> actually, y'know, proof-read it. > Hmm.. >> + $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_HAVE_SYMBOL_PREFIX_UNDERSCORE))" >> >> +config HAVE_UNDERSCORE_SYMBOL_PREFIX > > HAVE_UNDERSCORE_... or HAVE_SYMBOL_... confusion. > I prefer the HAVE_SYMBOL_... variant but no strong feelings.. Thanks, fixed. (HAVE_UNDERSCORE_ won by grep majority vote, but I don't really care either). >> + * >> + * If you think the above arrogance just encourages more people to add >> + * random crap to this file, you're not alone. > Kill this. ?? It went in without my Ack, and it's a particularly offensive turd. I can replace it with a civilized comment, instead of being snarky though... >> /* Some toolchains use a `_' prefix for all user symbols. */ >> -#ifdef CONFIG_SYMBOL_PREFIX >> -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX >> +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX >> +#define __VMLINUX_SYMBOL(x) _##x >> +#define __VMLINUX_SYMBOL_STR(x) "_" #x >> +#define VMLINUX_SYMBOL_PREFIX_STR "_" >> #else >> -#define MODULE_SYMBOL_PREFIX "" >> +#define __VMLINUX_SYMBOL(x) x >> +#define __VMLINUX_SYMBOL_STR(x) #x >> +#define VMLINUX_SYMBOL_PREFIX_STR "" >> #endif > > We know the prefix is an underscore. No benefits from defining > VMLINUX_SYMBOL_PREFIX_STR. The config name even syas so. > Skipping the above give us only one way to check > for the prefix - today we mix the two. After testing, I think you're right. The two users are both slightly clearer after this (though mtd is a tiny bit less optimal). Note: James, your patch hunk now looks like this: - buf_printf(b, "\t{ %#8x, \"%s\" },\n", s->crc, s->name); + buf_printf(b, "\t{ %#8x, VMLINUX_SYMBOL_STR(%s) },\n", + s->crc, s->name); Here's version 3: From: Rusty Russell <rusty@rustcorp.com.au> Subject: CONFIG_SYMBOL_PREFIX: cleanup. We have CONFIG_SYMBOL_PREFIX, which three archs define to the string "_". But Al Viro broke this in "consolidate cond_syscall and SYSCALL_ALIAS declarations" (in linux-next), and he's not the first to do so. Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to prefix it so something. So various places define helpers which are defined to nothing if CONFIG_SYMBOL_PREFIX isn't set: 1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX. 2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym) 3) include/linux/export.h defines MODULE_SYMBOL_PREFIX. 4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7) 5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym) 6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX 7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version for pasting. (arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too). Let's solve this properly: 1) No more generic prefix, just CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX. 2) Make linux/export.h usable from asm. 3) Define VMLINUX_SYMBOL() and VMLINUX_SYMBOL_STR(). 4) Make everyone use them. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> diff --git a/Makefile b/Makefile index a05ea42..0b09ba5 100644 --- a/Makefile +++ b/Makefile @@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN $(wildcard $(rm-files)) # Run depmod only if we have System.map and depmod is executable quiet_cmd_depmod = DEPMOD $(KERNELRELEASE) cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \ - $(KERNELRELEASE) "$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))" + $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX))" # Create temporary dir for module support files # clean it up only when building all modules diff --git a/arch/Kconfig b/arch/Kconfig index 1455579..7b433a4 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -384,6 +384,12 @@ config MODULES_USE_ELF_REL Modules only use ELF REL relocations. Modules with ELF RELA relocations will give an error. +config HAVE_UNDERSCORE_SYMBOL_PREFIX + bool + help + Some architectures generate an _ in front of C symbols; things like + module loading and assembly files need to know about this. + # # ABI hall of shame # diff --git a/arch/blackfin/Kconfig b/arch/blackfin/Kconfig index c3f2e0b..453ebe4 100644 --- a/arch/blackfin/Kconfig +++ b/arch/blackfin/Kconfig @@ -1,7 +1,3 @@ -config SYMBOL_PREFIX - string - default "_" - config MMU def_bool n @@ -33,6 +29,7 @@ config BLACKFIN select ARCH_HAVE_CUSTOM_GPIO_H select ARCH_WANT_OPTIONAL_GPIOLIB select HAVE_UID16 + select HAVE_UNDERSCORE_SYMBOL_PREFIX select VIRT_TO_BUS select ARCH_WANT_IPC_PARSE_VERSION select HAVE_GENERIC_HARDIRQS diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig index 79250de..303e4f9 100644 --- a/arch/h8300/Kconfig +++ b/arch/h8300/Kconfig @@ -12,10 +12,7 @@ config H8300 select MODULES_USE_ELF_RELA select OLD_SIGSUSPEND3 select OLD_SIGACTION - -config SYMBOL_PREFIX - string - default "_" + select HAVE_UNDERSCORE_SYMBOL_PREFIX config MMU bool diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig index afc8973..2099617 100644 --- a/arch/metag/Kconfig +++ b/arch/metag/Kconfig @@ -1,7 +1,3 @@ -config SYMBOL_PREFIX - string - default "_" - config METAG def_bool y select EMBEDDED @@ -27,6 +23,7 @@ config METAG select HAVE_MOD_ARCH_SPECIFIC select HAVE_PERF_EVENTS select HAVE_SYSCALL_TRACEPOINTS + select HAVE_UNDERSCORE_SYMBOL_PREFIX select IRQ_DOMAIN select MODULES_USE_ELF_RELA select OF diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c index 3b9a284..74dbb6b 100644 --- a/drivers/mtd/chips/gen_probe.c +++ b/drivers/mtd/chips/gen_probe.c @@ -204,14 +204,16 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map, struct cfi_private *cfi = map->fldrv_priv; __u16 type = primary?cfi->cfiq->P_ID:cfi->cfiq->A_ID; #ifdef CONFIG_MODULES - char probename[16+sizeof(MODULE_SYMBOL_PREFIX)]; + char probename[sizeof(VMLINUX_SYMBOL_STR(cfi_cmdset_%4.4X))]; cfi_cmdset_fn_t *probe_function; - sprintf(probename, MODULE_SYMBOL_PREFIX "cfi_cmdset_%4.4X", type); + sprintf(probename, VMLINUX_SYMBOL_STR(cfi_cmdset_%4.4X), type); probe_function = __symbol_get(probename); if (!probe_function) { - request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1); + char modname[sizeof("cfi_cmdset_%4.4X")]; + sprintf(modname, "cfi_cmdset_%4.4X", type); + request_module(modname); probe_function = __symbol_get(probename); } diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h index 4077b5d..15c0598 100644 --- a/include/asm-generic/unistd.h +++ b/include/asm-generic/unistd.h @@ -1,4 +1,5 @@ #include <uapi/asm-generic/unistd.h> +#include <linux/export.h> /* * These are required system calls, we should @@ -17,12 +18,7 @@ * but it doesn't work on all toolchains, so we just do it by hand */ #ifndef cond_syscall -#ifdef CONFIG_SYMBOL_PREFIX -#define __SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define __SYMBOL_PREFIX -#endif -#define cond_syscall(x) asm(".weak\t" __SYMBOL_PREFIX #x "\n\t" \ - ".set\t" __SYMBOL_PREFIX #x "," \ - __SYMBOL_PREFIX "sys_ni_syscall") +#define cond_syscall(x) asm(".weak\t" VMLINUX_SYMBOL_STR(x) "\n\t" \ + ".set\t" VMLINUX_SYMBOL_STR(x) "," \ + VMLINUX_SYMBOL_STR(sys_ni_syscall)) #endif diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index afa12c7..eb58d2d 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -52,13 +52,7 @@ #define LOAD_OFFSET 0 #endif -#ifndef SYMBOL_PREFIX -#define VMLINUX_SYMBOL(sym) sym -#else -#define PASTE2(x,y) x##y -#define PASTE(x,y) PASTE2(x,y) -#define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym) -#endif +#include <linux/export.h> /* Align . to a 8 byte boundary equals to maximum function alignment. */ #define ALIGN_FUNCTION() . = ALIGN(8) diff --git a/include/linux/export.h b/include/linux/export.h index 696c0f4..412cd50 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -5,17 +5,24 @@ * to reduce the amount of pointless cruft we feed to gcc when only * exporting a simple symbol or two. * - * If you feel the need to add #include <linux/foo.h> to this file - * then you are doing something wrong and should go away silently. + * Try not to add #includes here. It slows compilation and makes kernel + * hackers place grumpy comments in header files. */ /* Some toolchains use a `_' prefix for all user symbols. */ -#ifdef CONFIG_SYMBOL_PREFIX -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX +#define __VMLINUX_SYMBOL(x) _##x +#define __VMLINUX_SYMBOL_STR(x) "_" #x #else -#define MODULE_SYMBOL_PREFIX "" +#define __VMLINUX_SYMBOL(x) x +#define __VMLINUX_SYMBOL_STR(x) #x #endif +/* Indirect, so macros are expanded before pasting. */ +#define VMLINUX_SYMBOL(x) __VMLINUX_SYMBOL(x) +#define VMLINUX_SYMBOL_STR(x) __VMLINUX_SYMBOL_STR(x) + +#ifndef __ASSEMBLY__ struct kernel_symbol { unsigned long value; @@ -51,7 +58,7 @@ extern struct module __this_module; __CRC_SYMBOL(sym, sec) \ static const char __kstrtab_##sym[] \ __attribute__((section("__ksymtab_strings"), aligned(1))) \ - = MODULE_SYMBOL_PREFIX #sym; \ + = VMLINUX_SYMBOL_STR(sym); \ static const struct kernel_symbol __ksymtab_##sym \ __used \ __attribute__((section("___ksymtab" sec "+" #sym), unused)) \ @@ -85,5 +92,6 @@ extern struct module __this_module; #define EXPORT_UNUSED_SYMBOL_GPL(sym) #endif /* CONFIG_MODULES */ +#endif /* !__ASSEMBLY__ */ #endif /* _LINUX_EXPORT_H */ diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 80d3687..e13e992 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -723,13 +723,6 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } /* Trap pasters of __FUNCTION__ at compile-time */ #define __FUNCTION__ (__func__) -/* This helps us to avoid #ifdef CONFIG_SYMBOL_PREFIX */ -#ifdef CONFIG_SYMBOL_PREFIX -#define SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define SYMBOL_PREFIX "" -#endif - /* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */ #ifdef CONFIG_FTRACE_MCOUNT_RECORD # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD diff --git a/include/linux/module.h b/include/linux/module.h index ead1b57..46f1ea0 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -190,7 +190,7 @@ extern int modules_disabled; /* for sysctl */ /* Get/put a kernel symbol (calls must be symmetric) */ void *__symbol_get(const char *symbol); void *__symbol_get_gpl(const char *symbol); -#define symbol_get(x) ((typeof(&x))(__symbol_get(MODULE_SYMBOL_PREFIX #x))) +#define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x)))) /* modules using other modules: kdb wants to see this. */ struct module_use { @@ -453,7 +453,7 @@ extern void __module_put_and_exit(struct module *mod, long code) #ifdef CONFIG_MODULE_UNLOAD unsigned long module_refcount(struct module *mod); void __symbol_put(const char *symbol); -#define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x) +#define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x)) void symbol_put_addr(void *addr); /* Sometimes we know we already have a refcount, and it's easier not diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S index 246b4c6..4a9a86d 100644 --- a/kernel/modsign_certificate.S +++ b/kernel/modsign_certificate.S @@ -1,15 +1,8 @@ -/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */ -#ifndef SYMBOL_PREFIX -#define ASM_SYMBOL(sym) sym -#else -#define PASTE2(x,y) x##y -#define PASTE(x,y) PASTE2(x,y) -#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym) -#endif +#include <linux/export.h> #define GLOBAL(name) \ - .globl ASM_SYMBOL(name); \ - ASM_SYMBOL(name): + .globl VMLINUX_SYMBOL(name); \ + VMLINUX_SYMBOL(name): .section ".init.data","aw" diff --git a/kernel/module.c b/kernel/module.c index 0925c9a..cfd4a3f 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1209,7 +1209,7 @@ static inline int check_modstruct_version(Elf_Shdr *sechdrs, /* Since this should be found in kernel (which can't be removed), * no locking is necessary. */ - if (!find_symbol(MODULE_SYMBOL_PREFIX "module_layout", NULL, + if (!find_symbol(VMLINUX_SYMBOL_STR(module_layout), NULL, &crc, true, false)) BUG(); return check_version(sechdrs, versindex, "module_layout", mod, crc, diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 07125e6..a373a1f 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -119,13 +119,6 @@ _c_flags += $(if $(patsubst n%,, \ $(CFLAGS_GCOV)) endif -ifdef CONFIG_SYMBOL_PREFIX -_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX)) -_cpp_flags += $(_sym_flags) -_a_flags += $(_sym_flags) -endif - - # If building the kernel in a separate objtree expand all occurrences # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/'). diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index 3d569d6..0149949 100644 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -74,9 +74,8 @@ kallsyms() info KSYM ${2} local kallsymopt; - if [ -n "${CONFIG_SYMBOL_PREFIX}" ]; then - kallsymopt="${kallsymopt} \ - --symbol-prefix=${CONFIG_SYMBOL_PREFIX}" + if [ -n "${CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX}" ]; then + kallsymopt="${kallsymopt} --symbol-prefix=_" fi if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 78b30c1..282decf 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -18,14 +18,7 @@ #include "modpost.h" #include "../../include/generated/autoconf.h" #include "../../include/linux/license.h" - -/* Some toolchains use a `_' prefix for all user symbols. */ -#ifdef CONFIG_SYMBOL_PREFIX -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX -#else -#define MODULE_SYMBOL_PREFIX "" -#endif - +#include "../../include/linux/export.h" /* Are we using CONFIG_MODVERSIONS? */ int modversions = 0; @@ -562,7 +555,7 @@ static void parse_elf_finish(struct elf_info *info) static int ignore_undef_symbol(struct elf_info *info, const char *symname) { /* ignore __this_module, it will be resolved shortly */ - if (strcmp(symname, MODULE_SYMBOL_PREFIX "__this_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(__this_module)) == 0) return 1; /* ignore global offset table */ if (strcmp(symname, "_GLOBAL_OFFSET_TABLE_") == 0) @@ -583,8 +576,8 @@ static int ignore_undef_symbol(struct elf_info *info, const char *symname) return 0; } -#define CRC_PFX MODULE_SYMBOL_PREFIX "__crc_" -#define KSYMTAB_PFX MODULE_SYMBOL_PREFIX "__ksymtab_" +#define CRC_PFX VMLINUX_SYMBOL_STR(__crc_) +#define KSYMTAB_PFX VMLINUX_SYMBOL_STR(__ksymtab_) static void handle_modversions(struct module *mod, struct elf_info *info, Elf_Sym *sym, const char *symname) @@ -637,14 +630,15 @@ static void handle_modversions(struct module *mod, struct elf_info *info, } #endif - if (memcmp(symname, MODULE_SYMBOL_PREFIX, - strlen(MODULE_SYMBOL_PREFIX)) == 0) { - mod->unres = - alloc_symbol(symname + - strlen(MODULE_SYMBOL_PREFIX), - ELF_ST_BIND(sym->st_info) == STB_WEAK, - mod->unres); - } +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX + if (symname[0] != '_') + break; + else + symname++; +#endif + mod->unres = alloc_symbol(symname, + ELF_ST_BIND(sym->st_info) == STB_WEAK, + mod->unres); break; default: /* All exported symbols */ @@ -652,9 +646,9 @@ static void handle_modversions(struct module *mod, struct elf_info *info, sym_add_exported(symname + strlen(KSYMTAB_PFX), mod, export); } - if (strcmp(symname, MODULE_SYMBOL_PREFIX "init_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(init_module)) == 0) mod->has_init = 1; - if (strcmp(symname, MODULE_SYMBOL_PREFIX "cleanup_module") == 0) + if (strcmp(symname, VMLINUX_SYMBOL_STR(cleanup_module)) == 0) mod->has_cleanup = 1; break; } ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-14 4:00 ` Rusty Russell @ 2013-03-14 10:49 ` James Hogan 2013-03-15 4:37 ` Rusty Russell 0 siblings, 1 reply; 15+ messages in thread From: James Hogan @ 2013-03-14 10:49 UTC (permalink / raw) To: Rusty Russell Cc: Sam Ravnborg, Stephen Rothwell, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next, Paul Gortmaker, linux-mtd On 14/03/13 04:00, Rusty Russell wrote: > From: Rusty Russell <rusty@rustcorp.com.au> > Subject: CONFIG_SYMBOL_PREFIX: cleanup. > > We have CONFIG_SYMBOL_PREFIX, which three archs define to the string > "_". But Al Viro broke this in "consolidate cond_syscall and > SYSCALL_ALIAS declarations" (in linux-next), and he's not the first to > do so. > > Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to > prefix it so something. So various places define helpers which are > defined to nothing if CONFIG_SYMBOL_PREFIX isn't set: > > 1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX. > 2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym) > 3) include/linux/export.h defines MODULE_SYMBOL_PREFIX. > 4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7) > 5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym) > 6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX > 7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if > CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version > for pasting. > > (arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too). > > Let's solve this properly: > 1) No more generic prefix, just CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX. > 2) Make linux/export.h usable from asm. > 3) Define VMLINUX_SYMBOL() and VMLINUX_SYMBOL_STR(). > 4) Make everyone use them. > > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Reviewed-by: James Hogan <james.hogan@imgtec.com> Tested-by: James Hogan <james.hogan@imgtec.com> (metag) The only other special case of symbol prefixing I'm aware of is in scripts/genksyms/genksyms.c. It makes the decision at runtime based on the --arch=$ARCH argument, and is the only use of the arch argument: > if ((strcmp(arch, "h8300") == 0) || (strcmp(arch, "blackfin") == 0) || > (strcmp(arch, "metag") == 0)) > mod_prefix = "_"; Does the patch below look reasonable in addition to your patch? (Note: I'm not sure if genksyms is only used internally or whether it's API should be kept stable?). Thanks James Subject: [PATCH] genksyms: pass symbol-prefix instead of arch Pass symbol-prefix to genksyms instead of arch, so that the decision what symbol prefix to use is kept in one place. Basically genksyms used to take a -a $ARCH argument and it used that to determine whether to add an underscore symbol prefix. It's now changed to take a -s $SYMBOL_PREFIX argument so that the caller decides whether a symbol prefix is required. The build system then uses CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX to determine whether to pass the argument. Signed-off-by: James Hogan <james.hogan@imgtec.com> --- scripts/Makefile.build | 3 ++- scripts/genksyms/genksyms.c | 18 +++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 0e801c3..d5d859c 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -211,7 +211,8 @@ $(obj)/%.i: $(src)/%.c FORCE cmd_gensymtypes = \ $(CPP) -D__GENKSYMS__ $(c_flags) $< | \ - $(GENKSYMS) $(if $(1), -T $(2)) -a $(ARCH) \ + $(GENKSYMS) $(if $(1), -T $(2)) \ + $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX)) \ $(if $(KBUILD_PRESERVE),-p) \ -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null)) diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c index d25e4a1..88632df 100644 --- a/scripts/genksyms/genksyms.c +++ b/scripts/genksyms/genksyms.c @@ -45,7 +45,6 @@ int in_source_file; static int flag_debug, flag_dump_defs, flag_reference, flag_dump_types, flag_preserve, flag_warnings; -static const char *arch = ""; static const char *mod_prefix = ""; static int errors; @@ -731,7 +730,7 @@ static void genksyms_usage(void) { fputs("Usage:\n" "genksyms [-adDTwqhV] > /path/to/.tmp_obj.ver\n" "\n" #ifdef __GNU_LIBRARY__ - " -a, --arch Select architecture\n" + " -s, --symbol-prefix Select symbol prefix\n" " -d, --debug Increment the debug level (repeatable)\n" " -D, --dump Dump expanded symbol defs (for debugging only)\n" " -r, --reference file Read reference symbols from a file\n" @@ -742,7 +741,7 @@ static void genksyms_usage(void) " -h, --help Print this message\n" " -V, --version Print the release version\n" #else /* __GNU_LIBRARY__ */ - " -a Select architecture\n" + " -s Select symbol prefix\n" " -d Increment the debug level (repeatable)\n" " -D Dump expanded symbol defs (for debugging only)\n" " -r file Read reference symbols from a file\n" @@ -763,7 +762,7 @@ int main(int argc, char **argv) #ifdef __GNU_LIBRARY__ struct option long_opts[] = { - {"arch", 1, 0, 'a'}, + {"symbol-prefix", 1, 0, 's'}, {"debug", 0, 0, 'd'}, {"warnings", 0, 0, 'w'}, {"quiet", 0, 0, 'q'}, @@ -776,14 +775,14 @@ int main(int argc, char **argv) {0, 0, 0, 0} }; - while ((o = getopt_long(argc, argv, "a:dwqVDr:T:ph", + while ((o = getopt_long(argc, argv, "s:dwqVDr:T:ph", &long_opts[0], NULL)) != EOF) #else /* __GNU_LIBRARY__ */ - while ((o = getopt(argc, argv, "a:dwqVDr:T:ph")) != EOF) + while ((o = getopt(argc, argv, "s:dwqVDr:T:ph")) != EOF) #endif /* __GNU_LIBRARY__ */ switch (o) { - case 'a': - arch = optarg; + case 's': + mod_prefix = optarg; break; case 'd': flag_debug++; @@ -826,9 +825,6 @@ int main(int argc, char **argv) genksyms_usage(); return 1; } - if ((strcmp(arch, "h8300") == 0) || (strcmp(arch, "blackfin") == 0) || - (strcmp(arch, "metag") == 0)) - mod_prefix = "_"; { extern int yydebug; extern int yy_flex_debug; -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-14 10:49 ` James Hogan @ 2013-03-15 4:37 ` Rusty Russell 0 siblings, 0 replies; 15+ messages in thread From: Rusty Russell @ 2013-03-15 4:37 UTC (permalink / raw) To: James Hogan Cc: Sam Ravnborg, Stephen Rothwell, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next, Paul Gortmaker, linux-mtd James Hogan <james.hogan@imgtec.com> writes: > On 14/03/13 04:00, Rusty Russell wrote: >> From: Rusty Russell <rusty@rustcorp.com.au> >> Subject: CONFIG_SYMBOL_PREFIX: cleanup. ... > Reviewed-by: James Hogan <james.hogan@imgtec.com> > Tested-by: James Hogan <james.hogan@imgtec.com> (metag) Thanks. > The only other special case of symbol prefixing I'm aware of is in > scripts/genksyms/genksyms.c. It makes the decision at runtime based > on the --arch=$ARCH argument, and is the only use of the arch argument: > >> if ((strcmp(arch, "h8300") == 0) || (strcmp(arch, "blackfin") == 0) || >> (strcmp(arch, "metag") == 0)) >> mod_prefix = "_"; > > Does the patch below look reasonable in addition to your patch? > (Note: I'm not sure if genksyms is only used internally or whether > it's API should be kept stable?). > > Thanks > James > > Subject: [PATCH] genksyms: pass symbol-prefix instead of arch Agreed, I've applied this. Thanks, Rusty. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC -next] linux/linkage.h: fix symbol prefix handling 2013-03-12 4:48 ` Rusty Russell 2013-03-12 12:33 ` Stephen Rothwell @ 2013-03-12 12:36 ` James Hogan 1 sibling, 0 replies; 15+ messages in thread From: James Hogan @ 2013-03-12 12:36 UTC (permalink / raw) To: Rusty Russell Cc: Stephen Rothwell, Al Viro, Michal Marek, Andrew Morton, Guenter Roeck, Jean Delvare, linux-kbuild, linux-kernel, Mike Frysinger, uclinux-dist-devel, linux-next Hi Rusty, On 12/03/13 04:48, Rusty Russell wrote: > v2: Rename CONFIG_SYMBOL_PREFIX_UNDERSCORE to CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX, > which is defined in arch/Kconfig and selected by the 3 archs which need it. Sorry I didn't get a chance to try your patch yesterday. > Subject: CONFIG_SYMBOL_PREFIX: cleanup. > > We have CONFIG_SYMBOL_PREFIX, which three archs define to the string > "_". But Al Viro broke this in "consolidate cond_syscall and > SYSCALL_ALIAS declarations", and he's not the first to do so. > > Using CONFIG_SYMBOL_PREFIX is awkward, since we usually just want to > prefix it so something. So various places define helpers which are > defined to nothing if CONFIG_SYMBOL_PREFIX isn't set: > > 1) include/asm-generic/unistd.h defines __SYMBOL_PREFIX. > 2) include/asm-generic/vmlinux.lds.h defines VMLINUX_SYMBOL(sym) > 3) include/linux/export.h defines MODULE_SYMBOL_PREFIX. > 4) include/linux/kernel.h defines SYMBOL_PREFIX (which differs from #7) > 5) kernel/modsign_certificate.S defines ASM_SYMBOL(sym) > 6) scripts/modpost.c defines MODULE_SYMBOL_PREFIX > 7) scripts/Makefile.lib defines SYMBOL_PREFIX on the commandline if > CONFIG_SYMBOL_PREFIX is set, so that we have a non-string version > for pasting. > > (arch/h8300/include/asm/linkage.h defines SYMBOL_NAME(), too). > > Let's solve this properly: > 1) No more generic prefix, just CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX. > 2) Make linux/export.h usable from asm. > 3) Define VMLINUX_SYMBOL, VMLINUX_SYMBOL_NAME and VMLINUX_SYMBOL_PREFIX_STR. You don't seem to define VMLINUX_SYMBOL_NAME > 4) Make everyone use them. > > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> After a few modifications detailed below (with a fixup patch at the end for convenience) it seems to work no worse than before for metag (module symbol versioning was apparently already broken, I need to look into that). Reviewed-by: James Hogan <james.hogan@imgtec.com> Tested-by: James Hogan <james.hogan@imgtec.com> (on metag) Cheers James > > diff --git a/Makefile b/Makefile > index a05ea42..9dc948d 100644 > --- a/Makefile > +++ b/Makefile > @@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN $(wildcard $(rm-files)) > # Run depmod only if we have System.map and depmod is executable > quiet_cmd_depmod = DEPMOD $(KERNELRELEASE) > cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \ > - $(KERNELRELEASE) "$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))" > + $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_SYMBOL_PREFIX_UNDERSCORE))" should this be CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX now? > diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig > index ae8551e..ba043e3 100644 > --- a/arch/h8300/Kconfig > +++ b/arch/h8300/Kconfig > @@ -12,10 +12,10 @@ config H8300 > select MODULES_USE_ELF_RELA > select OLD_SIGSUSPEND3 > select OLD_SIGACTION > + select HAVE_UNDERSCORE_SYMBOL_PREFIX > > -config SYMBOL_PREFIX > - string > - default "_" > +config SYMBOL_PREFIX_UNDERSCORE > + def_bool y Not needed anymore I think > diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig > index afc8973..88272ce 100644 > --- a/arch/metag/Kconfig > +++ b/arch/metag/Kconfig > @@ -1,6 +1,5 @@ > -config SYMBOL_PREFIX > - string > - default "_" > +config SYMBOL_PREFIX_UNDERSCORE > + def_bool y Same again. > diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h > index 4077b5d..80122d4 100644 > --- a/include/asm-generic/unistd.h > +++ b/include/asm-generic/unistd.h > @@ -1,4 +1,5 @@ > #include <uapi/asm-generic/unistd.h> > +#include <linux/export.h> > > /* > * These are required system calls, we should > @@ -17,12 +18,7 @@ > * but it doesn't work on all toolchains, so we just do it by hand > */ > #ifndef cond_syscall > -#ifdef CONFIG_SYMBOL_PREFIX > -#define __SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX > -#else > -#define __SYMBOL_PREFIX > -#endif > -#define cond_syscall(x) asm(".weak\t" __SYMBOL_PREFIX #x "\n\t" \ > - ".set\t" __SYMBOL_PREFIX #x "," \ > - __SYMBOL_PREFIX "sys_ni_syscall") > +#define cond_syscall(x) asm(".weak\t" SYMBOL_NAME_STR(x) "\n\t" \ > + ".set\t" SYMBOL_NAME_STR(x) "," \ > + SYMBOL_NAME_STR(sys_ni_syscall)) s/SYMBOL_NAME_STR/VMLINUX_SYMBOL_STR/ > diff --git a/include/linux/export.h b/include/linux/export.h > index 696c0f4..0080e93 100644 > --- a/include/linux/export.h > +++ b/include/linux/export.h > @@ -7,15 +7,27 @@ > * > * If you feel the need to add #include <linux/foo.h> to this file > * then you are doing something wrong and should go away silently. > + * > + * If you think the above arrogance just encourages more people to add > + * random crap to this file, you're not alone. :-) > */ > > /* Some toolchains use a `_' prefix for all user symbols. */ > -#ifdef CONFIG_SYMBOL_PREFIX > -#define MODULE_SYMBOL_PREFIX CONFIG_SYMBOL_PREFIX > +#ifdef CONFIG_SYMBOL_PREFIX_UNDERSCORE CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index 3d569d6..d767681 100644 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -74,9 +74,8 @@ kallsyms() > info KSYM ${2} > local kallsymopt; > > - if [ -n "${CONFIG_SYMBOL_PREFIX}" ]; then > - kallsymopt="${kallsymopt} \ > - --symbol-prefix=${CONFIG_SYMBOL_PREFIX}" > + if [ -n "${CONFIG_SYMBOL_PREFIX_UNDERSCORE}" ]; then CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX diff --git a/Makefile b/Makefile index 9dc948d..0b09ba5 100644 --- a/Makefile +++ b/Makefile @@ -1398,7 +1398,7 @@ quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN $(wildcard $(rm-files)) # Run depmod only if we have System.map and depmod is executable quiet_cmd_depmod = DEPMOD $(KERNELRELEASE) cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \ - $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_SYMBOL_PREFIX_UNDERSCORE))" + $(KERNELRELEASE) "$(patsubst y,_,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX))" # Create temporary dir for module support files # clean it up only when building all modules diff --git a/arch/h8300/Kconfig b/arch/h8300/Kconfig index ba043e3..2333cf6 100644 --- a/arch/h8300/Kconfig +++ b/arch/h8300/Kconfig @@ -14,9 +14,6 @@ config H8300 select OLD_SIGACTION select HAVE_UNDERSCORE_SYMBOL_PREFIX -config SYMBOL_PREFIX_UNDERSCORE - def_bool y - config MMU bool default n diff --git a/arch/metag/Kconfig b/arch/metag/Kconfig index 88272ce..2099617 100644 --- a/arch/metag/Kconfig +++ b/arch/metag/Kconfig @@ -1,6 +1,3 @@ -config SYMBOL_PREFIX_UNDERSCORE - def_bool y - config METAG def_bool y select EMBEDDED diff --git a/include/asm-generic/unistd.h b/include/asm-generic/unistd.h index 80122d4..15c0598 100644 --- a/include/asm-generic/unistd.h +++ b/include/asm-generic/unistd.h @@ -18,7 +18,7 @@ * but it doesn't work on all toolchains, so we just do it by hand */ #ifndef cond_syscall -#define cond_syscall(x) asm(".weak\t" SYMBOL_NAME_STR(x) "\n\t" \ - ".set\t" SYMBOL_NAME_STR(x) "," \ - SYMBOL_NAME_STR(sys_ni_syscall)) +#define cond_syscall(x) asm(".weak\t" VMLINUX_SYMBOL_STR(x) "\n\t" \ + ".set\t" VMLINUX_SYMBOL_STR(x) "," \ + VMLINUX_SYMBOL_STR(sys_ni_syscall)) #endif diff --git a/include/linux/export.h b/include/linux/export.h index 0080e93..fc83b2a 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -13,7 +13,7 @@ */ /* Some toolchains use a `_' prefix for all user symbols. */ -#ifdef CONFIG_SYMBOL_PREFIX_UNDERSCORE +#ifdef CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX #define __VMLINUX_SYMBOL(x) _##x #define __VMLINUX_SYMBOL_STR(x) "_" #x #define VMLINUX_SYMBOL_PREFIX_STR "_" diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index d767681..0149949 100644 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -74,7 +74,7 @@ kallsyms() info KSYM ${2} local kallsymopt; - if [ -n "${CONFIG_SYMBOL_PREFIX_UNDERSCORE}" ]; then + if [ -n "${CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX}" ]; then kallsymopt="${kallsymopt} --symbol-prefix=_" fi ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-03-15 5:04 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-03-07 11:44 [RFC -next] linux/linkage.h: fix symbol prefix handling James Hogan 2013-03-08 0:03 ` Rusty Russell 2013-03-08 9:15 ` James Hogan 2013-03-11 6:35 ` Rusty Russell 2013-03-11 12:07 ` Stephen Rothwell 2013-03-12 4:48 ` Rusty Russell 2013-03-12 12:33 ` Stephen Rothwell 2013-03-13 0:00 ` Rusty Russell 2013-03-13 6:31 ` Sam Ravnborg 2013-03-13 9:21 ` James Hogan 2013-03-13 18:15 ` Sam Ravnborg 2013-03-14 4:00 ` Rusty Russell 2013-03-14 10:49 ` James Hogan 2013-03-15 4:37 ` Rusty Russell 2013-03-12 12:36 ` James Hogan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).