On Fri, Jun 8, 2018 at 3:04 AM Sedat Dilek <sedat.dilek@xxxxxxxxx> wrote: > > On Fri, Jun 8, 2018 at 9:59 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > > On Thu, Jun 7, 2018 at 10:49 PM, Nick Desaulniers > > <ndesaulniers@xxxxxxxxxx> wrote: > >> Functions marked extern inline do not emit an externally visible > >> function when the gnu89 C standard is used. Some KBUILD Makefiles > >> overwrite KBUILD_CFLAGS. This is an issue for GCC 5.1+ users as without > >> an explicit C standard specified, the default is gnu11. Since c99, the > >> semantics of extern inline have changed such that an externally visible > >> function is always emitted. This can lead to multiple definition errors > >> of extern inline functions at link time of compilation units whose build > >> files have removed an explicit C standard compiler flag for users of GCC > >> 5.1+ or Clang. > >> > >> Signed-off-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > >> Suggested-by: H. Peter Anvin <hpa@xxxxxxxxx> > >> Suggested-by: Joe Perches <joe@xxxxxxxxxxx> > > > > I suspect this will break Geert's gcc-4.1.2, which I think doesn't have that > > attribute yet (4.1.3 or higher have it according to the documentation. > > > > It wouldn't be hard to work around that if we want to keep that version > > working, or we could decide that it's time to officially stop supporting > > that version, but we should probably decide on one or the other. Heh, so earlier we decided against compiler flags (-std=gnu89 or -fgnu89-inline) in preference to function attributes. The function attribute is preferable as some of the Makefiles [accidentally?] overwrite KBUILD_CFLAGS, which is problematic for gcc 5.1 users as the implicit c standard used was changed to gnu11 from gnu89. What's nice is that to support gcc 4.1 users, we simply don't need to add any attribute, as their implicit c standard is gnu89 which has the semantics for extern inline that we want. I have a simple change to this patch that can support users of various gcc versions, see below: > Good point. > What is the minimum requirement of GCC version currently? > AFAICS x86/asm-goto support requires GCC >= 4.5? Yes, but that's only for x86, IIUC. It seems the kernel may have different minimum required versions of GCC based on arch then? That may be ok, but I'm not sure that's easy to keep track of without having it explicitly stated somewhere like the docs perhaps? > Just FYI... > ...saw the last days in upstream commits that kbuild/kconfig for > 4.18-rc1 offers possibilities to check for cc-version dependencies. Those will be helpful. If we want to pursue compiler flags, which get set some Makefiles, then yes. But I think a simpler change to my patch would be as below. It seems gcc did not get __has_attribute [0] until 5.1, but will define __GNUC_GNU_INLINE__ if supported. [1] Unfortunately, Clang does not define __GNUC_GNU_INLINE__ [2]. So a proper feature test might look like: ``` #ifndef __has_attribute #define __has_attribute(x) 0 #endif #if defined(__GNUC_GNU_INLINE__) || __has_attribute(gnu_inline) #define __gnu_inline __attribute__(gnu_inline) #endif #define inline inline __attribute__((always_inline, unused)) notrace __gnu_inline ``` Thoughts on this approach? I can send a v5 tomorrow if there's no major issues. Feedback appreciated, as always. [0] https://clang.llvm.org/docs/LanguageExtensions.html#has-attribute [1] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes [2] https://bugs.llvm.org/show_bug.cgi?id=37784 -- Thanks, ~Nick Desaulniers -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
