On Fri, May 25, 2018 at 3:38 PM Nick Desaulniers <ndesaulniers@xxxxxxxxxx> wrote: > > On Fri, May 25, 2018 at 2:06 PM H. Peter Anvin <hpa@xxxxxxxxx> wrote: > > On 05/25/18 13:36, Nick Desaulniers wrote: > > > On Fri, May 25, 2018 at 10:56 AM <hpa@xxxxxxxxx> wrote: > > >> You need the extern inline in the .h file and the out-of-line .S file > > > I don't see how you can specify to the linker from assembly source that > > > this function should be treated as `extern inline`? > > "extern inline" is a C directive. In the header file you should provide > > the inlinable implementation (which is already there.) It means that "if > > you don't want to inline this there is an external implementation > > available." > > oh you put `extern inline` on the definition, not the declaration?! What?! > > http://m68hc11.serveftp.org/inline-1.php > > in fact documents that trick. Wow, I've never seen that before. Seems > like there's not too many instances in the kernel. > > This seems to inline native_save_fl() properly into the call sites, but the > boot code now fails to link due to multiple definitions when trying link > the objects from arch/x86/boot/compressed/ into vmlinux. > > When disassembling the the arch/x86/boot/compressed/ objects, I can see > they're pulling in the `extern inline` c versions, but still outlining > them! (gcc and clang). > > That seems to contradict the statement from the above link: > "In no case is the function compiled on its own, not even if you refer to > its address explicitly" > > This is kind of funny; first we were wondering why native_save_fl was > getting outlined as a static inline function, which Alistair tracked down > to the incorrect use as a function pointer. Now I'm playing why is > native_save_fl getting outlined as an extern inline function. > > If I move the .S file to be in arch/x86/kernel/boot/compressed, the boot > code now links but then paravirt.o fails to link. Referring to the .S from > both Makefiles results in multiple definitions that the linker doesn't > like. (and the boot code still containing outlines). > > I don't understand how `extern inline` signals to the linker that "this > version should be taken if you find no others". From what I can tell with > `readelf -a` and `objdump -d`, a function marked `extern inline` vs not > look similar. > > Then again, arch/x86/boot/compressed/Makefile completely resets > KBUILD_CFLAGS and KBUILD_AFLAGS, which may be problematic (if there's some > kind of command line flag that affects `extern inline` linkage). Aha! This (wiping away CFLAGS) was it! The rules for `extern inline` for gnu89 and c99 are exactly the opposite! [0][1] c99: "a function defined extern inline will always, emit an externally visible function." but then gnu89: "gnu89 semantics of inline and extern inline are essentially the exact opposite of those in C99" This is elaborated more concisely in [2]. If I add `-std=gnu89` to the KBUILD_CFLAGS for compilation units that are missing that flag but include irqflags.h, I stop getting multiple definition linkage errors and link the expected kernel image! This is something to seriously consider for whoever might ever want to change the kernel's C standard: that you'll end up flipping the semantics of `extern inline`. Sounds like `-fgnu89-inline` compiler flag or `gnu_inline` attribute could be used to correct such a situation. + KBUILD maintainers and mailing list. Completely overwriting the KBUILD_CFLAGS from sub directory Makefiles (using the := operator) has been making me feel uneasy, since we very carefully try to set the necessary flags for Clang in the top level Makefile. In this case, the C standard is different for at least 2 subdirectories, which has implications for at least the above case with the linkage of `extern inline` functions. Will do further testing and cut a patch set tomorrow. [0] https://en.wikipedia.org/wiki/Inline_function#C99 [1] https://en.wikipedia.org/wiki/Inline_function#gnu89 [2] http://blahg.josefsipek.net/?p=529 -- Thanks, ~Nick Desaulniers -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html