[Qemu-devel] [PATCH] configure: Make epoll_create1 test work around SPARC glibc bug

[Qemu-devel] [PATCH] configure: Make epoll_create1 test work around SPARC glibc bug

[Peter Maydell]

Work around a SPARC glibc bug which caused the epoll_create1 configure
test to wrongly claim that the function was present. Some versions of
SPARC glibc provided the function in the library but didn't declare
it in the include file; the result is that gcc warns about an implicit
declaration but a link succeeds. So we build the configure test with
-Werror to avoid the test passing but then a -Werror qemu build
failing.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index da2da04..d9a8fdc 100755
--- a/configure
+++ b/configure
@@ -2229,7 +2229,12 @@ int main(void)
     return 0;
 }
 EOF
-if compile_prog "$ARCH_CFLAGS" "" ; then
+# We need to build this with -Werror to handle a bug in some
+# SPARC glibc where the function is defined but not declared
+# in the header file. Without -Werror gcc warns about an implicit
+# declaration of the function but the link succeeds; however
+# qemu itself will fail to build if it is compiled -Werror.
+if compile_prog "$ARCH_CFLAGS -Werror" "" ; then
   epoll_create1=yes
 fi
 
-- 
1.7.1

Re: [Qemu-devel] [PATCH] configure: Make epoll_create1 test work around SPARC glibc bug

[Blue Swirl]

On Tue, Apr 19, 2011 at 11:57 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> Work around a SPARC glibc bug which caused the epoll_create1 configure
> test to wrongly claim that the function was present. Some versions of
> SPARC glibc provided the function in the library but didn't declare
> it in the include file; the result is that gcc warns about an implicit
> declaration but a link succeeds. So we build the configure test with
> -Werror to avoid the test passing but then a -Werror qemu build
> failing.

But then epoll would not be used. How about checking for the buggy
glibc and doing something similar to how Solaris madvise() problem is
handled in osdep.c? Both this and madvise() hack could be configure
time checks, though.

Re: [Qemu-devel] [PATCH] configure: Make epoll_create1 test work around SPARC glibc bug

[Peter Maydell]

On 19 April 2011 20:37, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Tue, Apr 19, 2011 at 11:57 AM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> Work around a SPARC glibc bug which caused the epoll_create1 configure
>> test to wrongly claim that the function was present. Some versions of
>> SPARC glibc provided the function in the library but didn't declare
>> it in the include file; the result is that gcc warns about an implicit
>> declaration but a link succeeds. So we build the configure test with
>> -Werror to avoid the test passing but then a -Werror qemu build
>> failing.
>
> But then epoll would not be used.

I think that's fine -- on a system which isn't advertising epoll
in its include files we shouldn't be trying to use it. It might
be buggy, or not the same function at all, for instance.

Anybody who actually cares about epoll can upgrade their libc :-)

-- PMM

Re: [Qemu-devel] [PATCH] configure: Make epoll_create1 test work around SPARC glibc bug

[Blue Swirl]

On Tue, Apr 19, 2011 at 10:48 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 19 April 2011 20:37, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Tue, Apr 19, 2011 at 11:57 AM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> Work around a SPARC glibc bug which caused the epoll_create1 configure
>>> test to wrongly claim that the function was present. Some versions of
>>> SPARC glibc provided the function in the library but didn't declare
>>> it in the include file; the result is that gcc warns about an implicit
>>> declaration but a link succeeds. So we build the configure test with
>>> -Werror to avoid the test passing but then a -Werror qemu build
>>> failing.
>>
>> But then epoll would not be used.
>
> I think that's fine -- on a system which isn't advertising epoll
> in its include files we shouldn't be trying to use it. It might
> be buggy, or not the same function at all, for instance.
>
> Anybody who actually cares about epoll can upgrade their libc :-)

Maybe epoll is not so interesting as madvise.

But the check is not very specific, there could be some unrelated
warning with the headers. How about checking in the compiled file for
for example EPOLLIN, that should give a clear build failure if the
header is missing?

Re: [Qemu-devel] [PATCH] configure: Make epoll_create1 test work around SPARC glibc bug

[Peter Maydell]

On 19 April 2011 20:59, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Tue, Apr 19, 2011 at 10:48 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 19 April 2011 20:37, Blue Swirl <blauwirbel@gmail.com> wrote:
>>> But then epoll would not be used.
>>
>> I think that's fine -- on a system which isn't advertising epoll
>> in its include files we shouldn't be trying to use it. It might
>> be buggy, or not the same function at all, for instance.
>>
>> Anybody who actually cares about epoll can upgrade their libc :-)
>
> Maybe epoll is not so interesting as madvise.

Indeed, it isn't; the only reason we check for it in configure is
so we know whether we can pass through the relevant syscalls for
linux-user mode. it's a pretty recent innovation so any guest app
using it should have a fallback path if it gets ENOSYS.
Incidentally, note that this configure check is for epoll_create1(),
not epoll_create(). [Some systems have only the former, so it has
a separate configure check.]

madvise() is used by qemu itself, so we care more there.

> But the check is not very specific, there could be some unrelated
> warning with the headers.

The check isn't supposed to be very specific; the idea is that it
does basically what the actual qemu source code does, ie just
use the function in a program that's compiled with -Werror. If there's
an unrelated problem with the header that produces a compile
warning then we also want that to cause the test to fail, because
that too will cause qemu compilation to fail later.

I think that the ideal for configure tests is that they test
for exactly the set of functionality used by the program itself;
that's what I'm trying to do here.

> How about checking in the compiled file for
> for example EPOLLIN, that should give a clear build failure if the
> header is missing?

If the header was missing then that would already be causing a
compilation failure. The issue with this particular version of
SPARC glibc is that the header is present but doesn't declare the
function.

Looking for EPOLLIN isn't a useful test for "is epoll_create1()
present and OK?" because:
(1) EPOLLIN will be defined on systems which only have epoll_create(),
so "is EPOLLIN present?" can be true when "is epoll_create1() OK?" is false
(2) we don't ever actually use EPOLLIN, so checking for it could
potentially cause us to not use epoll_create1() even if it was
present and usable

-- PMM

Re: [Qemu-devel] [PATCH] configure: Make epoll_create1 test work around SPARC glibc bug

[Blue Swirl]

On Tue, Apr 19, 2011 at 11:16 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 19 April 2011 20:59, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Tue, Apr 19, 2011 at 10:48 PM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> On 19 April 2011 20:37, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>> But then epoll would not be used.
>>>
>>> I think that's fine -- on a system which isn't advertising epoll
>>> in its include files we shouldn't be trying to use it. It might
>>> be buggy, or not the same function at all, for instance.
>>>
>>> Anybody who actually cares about epoll can upgrade their libc :-)
>>
>> Maybe epoll is not so interesting as madvise.
>
> Indeed, it isn't; the only reason we check for it in configure is
> so we know whether we can pass through the relevant syscalls for
> linux-user mode. it's a pretty recent innovation so any guest app
> using it should have a fallback path if it gets ENOSYS.
> Incidentally, note that this configure check is for epoll_create1(),
> not epoll_create(). [Some systems have only the former, so it has
> a separate configure check.]
>
> madvise() is used by qemu itself, so we care more there.
>
>> But the check is not very specific, there could be some unrelated
>> warning with the headers.
>
> The check isn't supposed to be very specific; the idea is that it
> does basically what the actual qemu source code does, ie just
> use the function in a program that's compiled with -Werror. If there's
> an unrelated problem with the header that produces a compile
> warning then we also want that to cause the test to fail, because
> that too will cause qemu compilation to fail later.
>
> I think that the ideal for configure tests is that they test
> for exactly the set of functionality used by the program itself;
> that's what I'm trying to do here.
>
>> How about checking in the compiled file for
>> for example EPOLLIN, that should give a clear build failure if the
>> header is missing?
>
> If the header was missing then that would already be causing a
> compilation failure. The issue with this particular version of
> SPARC glibc is that the header is present but doesn't declare the
> function.
>
> Looking for EPOLLIN isn't a useful test for "is epoll_create1()
> present and OK?" because:
> (1) EPOLLIN will be defined on systems which only have epoll_create(),
> so "is EPOLLIN present?" can be true when "is epoll_create1() OK?" is false
> (2) we don't ever actually use EPOLLIN, so checking for it could
> potentially cause us to not use epoll_create1() even if it was
> present and usable

Sorry, I just picked a define without much thought. A more specific
one would be flags parameter of epoll_create1(), like EPOLL_CLOEXEC or
EPOLL_NONBLOCK. We don't use them now since the target system call
argument is passed untranslated to host, but that is actually not
correct, since the bit definitions could be different. So checking for
one of those should be OK.

Re: [Qemu-devel] [PATCH] configure: Make epoll_create1 test work around SPARC glibc bug

[Peter Maydell]

On 19 April 2011 21:36, Blue Swirl <blauwirbel@gmail.com> wrote:
> Sorry, I just picked a define without much thought. A more specific
> one would be flags parameter of epoll_create1(), like EPOLL_CLOEXEC or
> EPOLL_NONBLOCK. We don't use them now since the target system call
> argument is passed untranslated to host, but that is actually not
> correct, since the bit definitions could be different. So checking for
> one of those should be OK.

Unfortunately the header file on the system in question defines
both EPOLL_CLOEXEC and EPOLL_NONBLOCK even though it doesn't
prototype epoll_create1(). So this idea won't work.
The bug we are effectively trying to work around is the one fixed
by this libc patch:
http://sourceware.org/ml/libc-alpha/2010-08/msg00128.html

The only problem with the header is that it doesn't declare the
function, so the only way to detect it is to do something that
will fail if the function isn't declared, like compiling -Werror.

-- PMM

Re: [Qemu-devel] [PATCH] configure: Make epoll_create1 test work around SPARC glibc bug

[Blue Swirl]

On Wed, Apr 20, 2011 at 9:53 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 19 April 2011 21:36, Blue Swirl <blauwirbel@gmail.com> wrote:
>> Sorry, I just picked a define without much thought. A more specific
>> one would be flags parameter of epoll_create1(), like EPOLL_CLOEXEC or
>> EPOLL_NONBLOCK. We don't use them now since the target system call
>> argument is passed untranslated to host, but that is actually not
>> correct, since the bit definitions could be different. So checking for
>> one of those should be OK.
>
> Unfortunately the header file on the system in question defines
> both EPOLL_CLOEXEC and EPOLL_NONBLOCK even though it doesn't
> prototype epoll_create1(). So this idea won't work.
> The bug we are effectively trying to work around is the one fixed
> by this libc patch:
> http://sourceware.org/ml/libc-alpha/2010-08/msg00128.html
>
> The only problem with the header is that it doesn't declare the
> function, so the only way to detect it is to do something that
> will fail if the function isn't declared, like compiling -Werror.

This also fails without -Werror:
#include <sys/epoll.h>

int main(void)
{
    epoll_create1;
    return 0;
}