On Tue, Jul 30, 2019 at 4:00 PM Denis Efremov <efremov@xxxxxxxxx> wrote: > > On 30.07.2019 01:26, Stephen Rothwell wrote: > > Hi Denis, > > > > On Mon, 29 Jul 2019 17:18:01 +0300 Denis Efremov <efremov@xxxxxxxxx> wrote: > >> > >> This patch adds a check to warn about static EXPORT_SYMBOL* functions > >> during the modpost. In most of the cases, a static symbol marked for > >> exporting is an odd combination that should be fixed either by deleting > >> the exporting mark or by removing the static attribute and adding the > >> appropriate declaration to headers. > > > > OK, this is now in linux-next and I am getting what look like false > > positives :-( > > > > My powerpc builds produce these: > > > > WARNING: "ahci_em_messages" [vmlinux] is the static EXPORT_SYMBOL_GPL > > WARNING: "ftrace_set_clr_event" [vmlinux] is the static EXPORT_SYMBOL_GPL > > WARNING: "empty_zero_page" [vmlinux] is the static EXPORT_SYMBOL > > WARNING: "jiffies" [vmlinux] is the static EXPORT_SYMBOL > > > > empty_zero_page (at least) is not static. It is defined in assembler ... > > This could be fixed either by adding the type, for example: > --- a/arch/x86/kernel/head_64.S > +++ b/arch/x86/kernel/head_64.S > @@ -478,6 +478,7 @@ EXPORT_SYMBOL(phys_base) > > __PAGE_ALIGNED_BSS > NEXT_PAGE(empty_zero_page) > +.type empty_zero_page, STT_OBJECT > .skip PAGE_SIZE > EXPORT_SYMBOL(empty_zero_page) This would require us to fix-up all assembly files, wouldn't it? > Or by updating the check in the patch: > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -1988,7 +1988,9 @@ static void read_symbols(const char *modname) > unsigned char bind = ELF_ST_BIND(sym->st_info); > unsigned char type = ELF_ST_TYPE(sym->st_info); > > - if (type == STT_OBJECT || type == STT_FUNC) { > + if (type == STT_OBJECT || > + type == STT_FUNC || > + type == STT_NOTYPE) { > > Do I need to resend the whole patch or create new "patch-on-patch"? I prefer this, but why do you need to check type? Doesn't this work? for (sym = info.symtab_start; sym < info.symtab_stop; sym++) { unsigned char bind = ELF_ST_BIND(sym->st_info); struct symbol *s = find_symbol(remove_dot(info.strtab + sym->st_name)); if (s && (bind == STB_GLOBAL || bind == STB_WEAK)) s->is_static = 0; } -- Best Regards Masahiro Yamada