| Message ID | CAPgLHd9hwKRQMr1dnzJon-nVkkgKxcL5M2jEY8AF6Sv8OZ+aOg@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Fri, 2013-11-15 at 14:46 +0800, Wei Yongjun wrote: > After the following kernel commit, sparse warning lot of > EXPORT_SYMBOL()'d symbol non-static like this: > > CHECK net/sctp/socket.c > net/sctp/socket.c:4292:1: warning: > symbol '__ksymtab_sctp_do_peeloff' was not declared. Should it be static? > Author: Andi Kleen <ak@linux.intel.com> > Date: Wed Oct 23 10:57:58 2013 +1030 > > asmlinkage, module: Make ksymtab and kcrctab symbols and __this_module __visible > > Make the ksymtab symbols for EXPORT_SYMBOL visible. > This prevents the LTO compiler from adding a .NUMBER prefix, > which avoids various problems in later export processing. > Any idea? Well, sparse is clearly "right", for all it cares it might very well be static, but it seems this is necessary for something in the kernel and we clearly can't forward-declare it in a header file. Perhaps we can add some annotation to say "__attribute__((yes_I_know_but_really_dont_want_this_to_be_static))" to suppress this warning? This is getting annoying to me as well :-) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Well, sparse is clearly "right", for all it cares it might very well be > static, but it seems this is necessary for something in the kernel and > we clearly can't forward-declare it in a header file. Perhaps we can add > some annotation to say > "__attribute__((yes_I_know_but_really_dont_want_this_to_be_static))" to > suppress this warning? This is getting annoying to me as well :-) We could do something like typeof(foo); in the macro. Not sure if that would make sparse happy. Also this is really working around a problem upto gcc 4.8. that was fixed in gcc 4.9 (adding numerical postfixes to all symbols) If it's ok to let LTO only support 4.9+ the patches could be reverted. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2013-11-24 at 12:28 -0800, Andi Kleen wrote: > > Well, sparse is clearly "right", for all it cares it might very well be > > static, but it seems this is necessary for something in the kernel and > > we clearly can't forward-declare it in a header file. Perhaps we can add > > some annotation to say > > "__attribute__((yes_I_know_but_really_dont_want_this_to_be_static))" to > > suppress this warning? This is getting annoying to me as well :-) > > We could do something like > > typeof(foo); > > in the macro. Not sure if that would make sparse happy. I wouldn't think so, after all that's just using the symbol, not declaring it, and using it clearly happens all the time. I only see an annotation as a solution, which is ugly but this crops up everywhere and makes sparse output pretty much unreadable. > Also this is really working around a problem upto gcc 4.8. that was fixed > in gcc 4.9 (adding numerical postfixes to all symbols) If it's ok to > let LTO only support 4.9+ the patches could be reverted. That I can't comment on. :) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Nov 24, 2013 at 12:28:51PM -0800, Andi Kleen wrote: > > Well, sparse is clearly "right", for all it cares it might very well be > > static, but it seems this is necessary for something in the kernel and > > we clearly can't forward-declare it in a header file. Perhaps we can add > > some annotation to say > > "__attribute__((yes_I_know_but_really_dont_want_this_to_be_static))" to > > suppress this warning? This is getting annoying to me as well :-) > > We could do something like > > typeof(foo); > > in the macro. Not sure if that would make sparse happy. If you're just looking to mollify sparse, the easiest way is to put a prototype of the symbol right before the symbol itself. > Also this is really working around a problem upto gcc 4.8. that was fixed > in gcc 4.9 (adding numerical postfixes to all symbols) If it's ok to let LTO only support 4.9+ the patches could be reverted. That seems like the best solution, assuming the later scripts can't be fixed to cope with the numeric suffixes from 4.8. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2013-11-24 at 12:45 -0800, Josh Triplett wrote: > On Sun, Nov 24, 2013 at 12:28:51PM -0800, Andi Kleen wrote: > > > Well, sparse is clearly "right", for all it cares it might very well be > > > static, but it seems this is necessary for something in the kernel and > > > we clearly can't forward-declare it in a header file. Perhaps we can add > > > some annotation to say > > > "__attribute__((yes_I_know_but_really_dont_want_this_to_be_static))" to > > > suppress this warning? This is getting annoying to me as well :-) > > > > We could do something like > > > > typeof(foo); > > > > in the macro. Not sure if that would make sparse happy. > > If you're just looking to mollify sparse, I am, pretty much, since now I get a ton of output and have to either grep it or dig through it... :) > the easiest way is to > put a prototype of the symbol right before the symbol itself. Oh, right, good point, thanks Josh. Andi, do you want to do that? Otherwise I can post a patch. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/export.h b/include/linux/export.h index 412cd50..3f2793d 100644 --- a/include/linux/export.h +++ b/include/linux/export.h @@ -43,7 +43,7 @@ extern struct module __this_module; /* Mark the CRC weak since genksyms apparently decides not to * generate a checksums for some symbols */ #define __CRC_SYMBOL(sym, sec) \ - extern void *__crc_##sym __attribute__((weak)); \ + extern __visible void *__crc_##sym __attribute__((weak)); \ static const unsigned long __kcrctab_##sym \ __used \ __attribute__((section("___kcrctab" sec "+" #sym), unused)) \ @@ -59,7 +59,7 @@ extern struct module __this_module; static const char __kstrtab_##sym[] \ __attribute__((section("__ksymtab_strings"), aligned(1))) \ = VMLINUX_SYMBOL_STR(sym); \ - static const struct kernel_symbol __ksymtab_##sym \ + __visible const struct kernel_symbol __ksymtab_##sym \ __used \ __attribute__((section("___ksymtab" sec "+" #sym), unused)) \ = { (unsigned long)&sym, __kstrtab_##sym }
After the following kernel commit, sparse warning lot of EXPORT_SYMBOL()'d symbol non-static like this: CHECK net/sctp/socket.c net/sctp/socket.c:4292:1: warning: symbol '__ksymtab_sctp_do_peeloff' was not declared. Should it be static? commit e0f244c63fc9d192dfd399cc2677bbdca61994b1 Author: Andi Kleen <ak@linux.intel.com> Date: Wed Oct 23 10:57:58 2013 +1030 asmlinkage, module: Make ksymtab and kcrctab symbols and __this_module __visible Make the ksymtab symbols for EXPORT_SYMBOL visible. This prevents the LTO compiler from adding a .NUMBER prefix, which avoids various problems in later export processing. Cc: rusty@rustcorp.com.au Signed-off-by: Andi Kleen <ak@linux.intel.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Cc: rusty@rustcorp.com.au Signed-off-by: Andi Kleen <ak@linux.intel.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Any idea? Regards, Yongjun Wei -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html