Re: [libvirt] [PATCH] Do not inline virNumaNodeIsAvailable

On Mon, Mar 09, 2015 at 02:03:16PM +0100, Martin Kletzander wrote: > On Mon, Mar 09, 2015 at 10:09:17AM +0000, Daniel P. Berrange wrote: > >On Mon, Mar 09, 2015 at 11:01:31AM +0100, Martin Kletzander wrote: > >> On Thu, Mar 05, 2015 at 11:09:41AM +0000, Daniel P. Berrange wrote: > >> >On Thu, Mar 05, 2015 at 12:05:52PM +0100, Ján Tomko wrote: > >> >>Explicitly request that virNumaNodeIsAvailable not be inlined. > >> >>This fixes the test suite when building with clang (3.5.1). > >> > > >> >Huh, so clang will inline functions, even if they are exported > >> >in the .so library ? Is there some clang compiler flag we can > >> >use to stop that ? I'd only expect it to inline stuff which > >> >was declared static, or whose impl body was in the header file > >> > > >> > >> If I understand it correctly, that means that clang is not > >> "compatible" with gcc. > >> > >> Excerpt from gcc online docs [1]: > >> > >> When a function is both inline and static, if all calls to the > >> function are integrated into the caller, and the function's address > >> is never used, then the function's own assembler code is never > >> referenced. > >> > >> Excerpt from gcc online docs [1]: > >> > >> By default, Clang builds C code in GNU C11 mode, so it uses standard > >> C99 semantics for the inline keyword. These semantics are different > >> from those in GNU C89 mode, which is the default mode in versions of > >> GCC prior to 5.0. > >> > >> However further reading of the second documentation and c89 semantics > >> it doesn't say anything about the fact that such function should be > >> inlined. > > > >But we haven't added the 'inline' keyword to this function at > >all - it is just a normal function marked for export in the > >.so file, so I'm puzzelled why it is getting inlined. > > > > Exactly, that's what I'm trying to find out as well. > > >> > >> Anyway, is this clang 3.6 specific? I don't have this problem when > >> compiling with 3.5. Nor does this show with gcc -std=gnu11. I'm > >> getting 3.6 to check whether that's the difference. > >> > > After updating clang and llvm from 3.5 to 3.6, I still don't get this > error. And I have only 4 (fake) nodes available, so it _is_ rewriting > that function. I'm getting the error with 3.5.1, as I said in the commit message. These are the failing qemuxml2argvtest cases: 60) QEMU XML-2-ARGV hugepages-pages ... libvirt: error : internal error: NUMA node 1 is unavailable 63) QEMU XML-2-ARGV hugepages-shared ... libvirt: error : internal error: NUMA node 1 is unavailable 324) QEMU XML-2-ARGV numatune-memnode ... libvirt: error : internal error: NUMA node 1 is unavailable 326) QEMU XML-2-ARGV numatune-memnode-no-memory ... libvirt: error : internal error: NUMA node 3 is unavailable 329) QEMU XML-2-ARGV numatune-auto-prefer ... libvirt: error : internal error: NUMA node 1 is unavailable So with 4 fake nodes, the tests could still pass even if the function is not mocked. Try changing the nodeset in #326 to 4 if it fails. > > >> [1] https://gcc.gnu.org/onlinedocs/gcc/Inline.html > >> [2] http://clang.llvm.org/compatibility.html > >> > >> >>--- > >> >>This only leaves the mysterious check-protocol failure. > >> > >> That's not that mysterious, it's just that we check the order and > >> clang sorts enums before structs, but gcc doesn't. Also clang adds > >> "public:" to structs, so it probably treats it as a C++ or C# structs > >> or something? > >> > > By the way if I compile with clang with -std=gnu11 or -std=gnu99, the > "public:" stuff is gone :) > It is mysterious, because it doesn't fail consistently. It was working for me after I tried it again after 'git clean -fxd', today it failed again (though I don't remember if I ran autogen again).