[Qemu-devel] [PATCH] fix compilation/link with clang, target-i386/cpu.c

[Qemu-devel] [PATCH] fix compilation/link with clang, target-i386/cpu.c

[Andreas Tobler]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

Hello,

Paolo asked me to test and submit the below patch to fix compilation and
link with clang.

Paolo reduced the issue to a clang bug where dead code is not properly
eliminated before linktime. (the clang bug ID: 17882)

Thanks,
Andreas


Signed-off-by: Andreas Tobler <address@hidden>



[-- Attachment #2: target-i386.diff --]
[-- Type: text/plain, Size: 490 bytes --]

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 864c80e..6d3e5fd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2196,7 +2196,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         *ebx = 0;
         *ecx = 0;
         *edx = 0;
-        if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) || !kvm_enabled()) {
+        if (!kvm_enabled() || !(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
             break;
         }
         kvm_mask =

Re: [Qemu-devel] [PATCH] fix compilation/link with clang, target-i386/cpu.c

[Peter Maydell]

On 11 November 2013 20:10, Andreas Tobler <andreast@freebsd.org> wrote:
> Paolo asked me to test and submit the below patch to fix compilation and
> link with clang.
>
> Paolo reduced the issue to a clang bug where dead code is not properly
> eliminated before linktime. (the clang bug ID: 17882)

Thanks for the patch. However, it looks a bit odd to me. Can
you quote the error message clang produces, please?

I think I would agree with the commenter in the bug report you
reference (http://llvm.org/bugs/show_bug.cgi?id=17882)
that this is not a clang bug. We shouldn't be relying on the
compiler's dead code elimination to get rid of references to
functions that don't exist in certain configurations. This will
always be unreliable (especially if compiling without optimization).
Instead we should either be using ifdefs or stub functions (probably
the latter in this case).

If you put a stub implementation of kvm_arch_get_supported_cpuid()
into target-i386/kvm-stub.c does this fix the compilation issue?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] fix compilation/link with clang, target-i386/cpu.c

[Andreas Tobler]

On 11.11.13 22:02, Peter Maydell wrote:
> On 11 November 2013 20:10, Andreas Tobler <andreast@freebsd.org> wrote:
>> Paolo asked me to test and submit the below patch to fix compilation and
>> link with clang.
>>
>> Paolo reduced the issue to a clang bug where dead code is not properly
>> eliminated before linktime. (the clang bug ID: 17882)
> 
> Thanks for the patch. However, it looks a bit odd to me. Can
> you quote the error message clang produces, please?

[tcx58:build/qemu/objdir] andreast% gmake
  CC    x86_64-softmmu/target-i386/cpu.o
  LINK  x86_64-softmmu/qemu-system-x86_64
target-i386/cpu.o: In function `cpu_x86_cpuid':
/export/devel/net/src/qemu/qemu-master/target-i386/cpu.c:2203: undefined
reference to `kvm_arch_get_supported_cpuid'
/export/devel/net/src/qemu/qemu-master/target-i386/cpu.c:2204: undefined
reference to `kvm_arch_get_supported_cpuid'
/export/devel/net/src/qemu/qemu-master/target-i386/cpu.c:2223: undefined
reference to `kvm_arch_get_supported_cpuid'
cc: error: linker command failed with exit code 1 (use -v to see invocation)
gmake[1]: *** [qemu-system-x86_64] Error 1
gmake: *** [subdir-x86_64-softmmu] Error 2

> I think I would agree with the commenter in the bug report you
> reference (http://llvm.org/bugs/show_bug.cgi?id=17882)
> that this is not a clang bug. We shouldn't be relying on the
> compiler's dead code elimination to get rid of references to
> functions that don't exist in certain configurations. This will
> always be unreliable (especially if compiling without optimization).
> Instead we should either be using ifdefs or stub functions (probably
> the latter in this case).

I know it is a difficult business. And probably you're right, but from a
dump users point of view I do not agree.

I'm used to gcc, which is able to compile this, and I expect other
compilers to be able to do the same. The compiler should work for me not
vice versa :)

> If you put a stub implementation of kvm_arch_get_supported_cpuid()
> into target-i386/kvm-stub.c does this fix the compilation issue?

Will try and let you know.
Thanks,
Andreas

Re: [Qemu-devel] [PATCH] fix compilation/link with clang, target-i386/cpu.c

[Peter Maydell]

On 11 November 2013 21:12, Andreas Tobler <andreast@freebsd.org> wrote:
> On 11.11.13 22:02, Peter Maydell wrote:
>> I think I would agree with the commenter in the bug report you
>> reference (http://llvm.org/bugs/show_bug.cgi?id=17882)
>> that this is not a clang bug. We shouldn't be relying on the
>> compiler's dead code elimination to get rid of references to
>> functions that don't exist in certain configurations. This will
>> always be unreliable (especially if compiling without optimization).
>> Instead we should either be using ifdefs or stub functions (probably
>> the latter in this case).
>
> I know it is a difficult business. And probably you're right, but from a
> dump users point of view I do not agree.
>
> I'm used to gcc, which is able to compile this, and I expect other
> compilers to be able to do the same. The compiler should work for me not
> vice versa :)

Unfortunately C doesn't work that way. If you rely on behaviour
which isn't guaranteed by the compiler authors then you have
unreliable programs. I bet if you asked the gcc developers they'd
say they didn't guarantee this to work either.

>> If you put a stub implementation of kvm_arch_get_supported_cpuid()
>> into target-i386/kvm-stub.c does this fix the compilation issue?
>
> Will try and let you know.

I've actually reproduced this with my clang/macos system (it
was masked by another build failure for which there's a patch
on list that hasn't been applied yet). I'm working up a patch
that I'll send shortly.

thanks
-- PMM