diff mbox

[v2] configure: sanity check the glib library that pkg-config finds

Message ID 1447414307-5295-1-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé Nov. 13, 2015, 11:31 a.m. UTC
Developers on 64-bit machines will often try to perform a
32-bit build of QEMU by running

  ./configure --extra-cflags="-m32"

Unfortunately if PKG_CONFIG_LIBDIR is not set to point to
the location of the 32-bit pkg-config files, then configure
will silently pick up the 64-bit pkg-config files and still
succeed.

This causes a problem for glib because it means QEMU will
be pulling in /usr/lib64/glib-2.0/include/glibconfig.h
instead of /usr/lib/glib-2.0/include/glibconfig.h

This causes problems because the 'gsize' type (defined as
'unsigned long') will no longer be fully compatible with
the 'size_t' type (defined as 'unsigned int'). Although
both are the same size, the compiler refuses to allow
casts from 'unsigned long *' to 'unsigned int *' as they
are different pointer types. This results in non-obvious
compiler errors when building QEMU eg

qga/commands-posix.c: In function ‘qmp_guest_set_user_password’:
qga/commands-posix.c:1912:55: error: passing argument 2 of ‘g_base64_decode’ from incompatible pointer type [-Werror=incompatible-pointer-types]
     rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
                                                            ^
In file included from /usr/include/glib-2.0/glib.h:35:0,
                 from qga/commands-posix.c:14:
/usr/include/glib-2.0/glib/gbase64.h:52:9: note: expected ‘gsize * {aka long unsigned int *}’ but argument is of type ‘size_t * {aka unsigned int *}’
 guchar *g_base64_decode         (const gchar  *text,
         ^
cc1: all warnings being treated as errors

To detect this problem, add a check to configure that
verifies that GLIB_SIZEOF_SIZE_T matches sizeof(size_t).
If this fails print a warning suggesting that the dev
probably needs to set PKG_CONFIG_LIBDIR.

On Fedora x86_64 it passes with any of:

 # ./configure
 # PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ./configure --extra-cflags="-m32"
 # PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig ./configure --extra-cflags="-m64"

And fails with a mis-match

 # PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig ./configure --extra-cflags="-m32"
 # PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ./configure --extra-cflags="-m64"

ERROR: sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T.
       You probably need to set PKG_CONFIG_LIBDIR
       to point to the right pkg-config files for your
       build target

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
Changed in v2:

 - Add 'return 0' to silence compiler warning on some
   platforms about returning without a value

 configure | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Dr. David Alan Gilbert Nov. 13, 2015, 12:54 p.m. UTC | #1
* Daniel P. Berrange (berrange@redhat.com) wrote:
> Developers on 64-bit machines will often try to perform a
> 32-bit build of QEMU by running
> 
>   ./configure --extra-cflags="-m32"
> 
> Unfortunately if PKG_CONFIG_LIBDIR is not set to point to
> the location of the 32-bit pkg-config files, then configure
> will silently pick up the 64-bit pkg-config files and still
> succeed.

Having fallen into this trap, thanks; seems to do the trick on RHEL7.x:


# /root/try/../qemu/configure --prefix=/root/try  --enable-trace-backends=stderr --extra-cflags="-m32" --extra-ldflags="-m32"

ERROR: sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T.
       You probably need to set PKG_CONFIG_LIBDIR
       to point to the right pkg-config files for your
       build target

# export PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig
# /root/try/../qemu/configure --prefix=/root/try  --enable-trace-backends=stderr --extra-cflags="-m32" --extra-ldflags="-m32"
Install prefix    /root/try
BIOS directory    /root/try/share/qemu
....

and the gsize def ( https://developer.gnome.org/glib/stable/glib-Basic-Types.html#gsize )
does say it should always match size_t size.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Dave

> 
> This causes a problem for glib because it means QEMU will
> be pulling in /usr/lib64/glib-2.0/include/glibconfig.h
> instead of /usr/lib/glib-2.0/include/glibconfig.h
> 
> This causes problems because the 'gsize' type (defined as
> 'unsigned long') will no longer be fully compatible with
> the 'size_t' type (defined as 'unsigned int'). Although
> both are the same size, the compiler refuses to allow
> casts from 'unsigned long *' to 'unsigned int *' as they
> are different pointer types. This results in non-obvious
> compiler errors when building QEMU eg
> 
> qga/commands-posix.c: In function ‘qmp_guest_set_user_password’:
> qga/commands-posix.c:1912:55: error: passing argument 2 of ‘g_base64_decode’ from incompatible pointer type [-Werror=incompatible-pointer-types]
>      rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
>                                                             ^
> In file included from /usr/include/glib-2.0/glib.h:35:0,
>                  from qga/commands-posix.c:14:
> /usr/include/glib-2.0/glib/gbase64.h:52:9: note: expected ‘gsize * {aka long unsigned int *}’ but argument is of type ‘size_t * {aka unsigned int *}’
>  guchar *g_base64_decode         (const gchar  *text,
>          ^
> cc1: all warnings being treated as errors
> 
> To detect this problem, add a check to configure that
> verifies that GLIB_SIZEOF_SIZE_T matches sizeof(size_t).
> If this fails print a warning suggesting that the dev
> probably needs to set PKG_CONFIG_LIBDIR.
> 
> On Fedora x86_64 it passes with any of:
> 
>  # ./configure
>  # PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ./configure --extra-cflags="-m32"
>  # PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig ./configure --extra-cflags="-m64"
> 
> And fails with a mis-match
> 
>  # PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig ./configure --extra-cflags="-m32"
>  # PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ./configure --extra-cflags="-m64"
> 
> ERROR: sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T.
>        You probably need to set PKG_CONFIG_LIBDIR
>        to point to the right pkg-config files for your
>        build target
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> Changed in v2:
> 
>  - Add 'return 0' to silence compiler warning on some
>    platforms about returning without a value
> 
>  configure | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/configure b/configure
> index f75df4b..c53334f 100755
> --- a/configure
> +++ b/configure
> @@ -2939,6 +2939,27 @@ for i in $glib_modules; do
>      fi
>  done
>  
> +# Sanity check that the current size_t matches the
> +# size that glib thinks it should be. This catches
> +# problems on multi-arch where people try to build
> +# 32-bit QEMU while pointing at 64-bit glib headers
> +cat > $TMPC <<EOF
> +#include <glib.h>
> +#include <unistd.h>
> +
> +int main(void) {
> +   G_STATIC_ASSERT(sizeof(size_t) == GLIB_SIZEOF_SIZE_T);
> +   return 0;
> +}
> +EOF
> +
> +if ! compile_prog "-Werror $CFLAGS" "$LIBS" ; then
> +    error_exit "sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T."\
> +               "You probably need to set PKG_CONFIG_LIBDIR"\
> +	       "to point to the right pkg-config files for your"\
> +	       "build target"
> +fi
> +
>  # g_test_trap_subprocess added in 2.38. Used by some tests.
>  glib_subprocess=yes
>  if ! $pkg_config --atleast-version=2.38 glib-2.0; then
> -- 
> 2.5.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Gerd Hoffmann Nov. 13, 2015, 1:01 p.m. UTC | #2
On Fr, 2015-11-13 at 11:31 +0000, Daniel P. Berrange wrote:
> Developers on 64-bit machines will often try to perform a
> 32-bit build of QEMU by running
> 
>   ./configure --extra-cflags="-m32"
> 
> Unfortunately if PKG_CONFIG_LIBDIR is not set to point to
> the location of the 32-bit pkg-config files, then configure
> will silently pick up the 64-bit pkg-config files and still
> succeed.

While being at it:  I think it would be a good idea to record
PKG_CONFIG_LIBDIR in config.status, otherwise it'll be lost the next
time configure is touched and make figures it needs to re-run configure.
Peter Maydell Nov. 13, 2015, 1:01 p.m. UTC | #3
On 13 November 2015 at 11:31, Daniel P. Berrange <berrange@redhat.com> wrote:
> Developers on 64-bit machines will often try to perform a
> 32-bit build of QEMU by running
>
>   ./configure --extra-cflags="-m32"
>
> Unfortunately if PKG_CONFIG_LIBDIR is not set to point to
> the location of the 32-bit pkg-config files, then configure
> will silently pick up the 64-bit pkg-config files and still
> succeed.

Will this also handle all the other -config programs that
seem to have sprouted up? Looking in configure we also call
 sdl-config
 sdl2-config
 libgcrypt-config
 aalib-config
 curl-config

thanks
-- PMM
Daniel P. Berrangé Nov. 13, 2015, 2:16 p.m. UTC | #4
On Fri, Nov 13, 2015 at 01:01:52PM +0000, Peter Maydell wrote:
> On 13 November 2015 at 11:31, Daniel P. Berrange <berrange@redhat.com> wrote:
> > Developers on 64-bit machines will often try to perform a
> > 32-bit build of QEMU by running
> >
> >   ./configure --extra-cflags="-m32"
> >
> > Unfortunately if PKG_CONFIG_LIBDIR is not set to point to
> > the location of the 32-bit pkg-config files, then configure
> > will silently pick up the 64-bit pkg-config files and still
> > succeed.
> 
> Will this also handle all the other -config programs that
> seem to have sprouted up? Looking in configure we also call
>  sdl-config
>  sdl2-config
>  libgcrypt-config
>  aalib-config
>  curl-config

For custom non-pkg-config programs there's not any good general
purpose solution for them to look for 32-bit versions while on
64-bit hosts, at least not when 32-bit and 64-bit are both
installed into the same /usr prefix. If the 32-bit stuff is
installed in a completely separate root prefix, then you can
sometimes update $PATH to ensure the right -config binary is
found. This is a good reason why libs are encouraged to switch
to using pkg-config instead of home-grown alternatives

Regards,
Daniel
Daniel P. Berrangé Nov. 17, 2015, 5:25 p.m. UTC | #5
On Fri, Nov 13, 2015 at 02:01:12PM +0100, Gerd Hoffmann wrote:
> On Fr, 2015-11-13 at 11:31 +0000, Daniel P. Berrange wrote:
> > Developers on 64-bit machines will often try to perform a
> > 32-bit build of QEMU by running
> > 
> >   ./configure --extra-cflags="-m32"
> > 
> > Unfortunately if PKG_CONFIG_LIBDIR is not set to point to
> > the location of the 32-bit pkg-config files, then configure
> > will silently pick up the 64-bit pkg-config files and still
> > succeed.
> 
> While being at it:  I think it would be a good idea to record
> PKG_CONFIG_LIBDIR in config.status, otherwise it'll be lost the next
> time configure is touched and make figures it needs to re-run configure.

In the spirit of doing one logical change per patch, I'm sending a
separate patch to cover this suggestion.

Regards,
Daniel
Peter Maydell Jan. 25, 2016, 2:42 p.m. UTC | #6
On 13 November 2015 at 11:31, Daniel P. Berrange <berrange@redhat.com> wrote:
> Developers on 64-bit machines will often try to perform a
> 32-bit build of QEMU by running
>
>   ./configure --extra-cflags="-m32"
>
> Unfortunately if PKG_CONFIG_LIBDIR is not set to point to
> the location of the 32-bit pkg-config files, then configure
> will silently pick up the 64-bit pkg-config files and still
> succeed.
>
> This causes a problem for glib because it means QEMU will
> be pulling in /usr/lib64/glib-2.0/include/glibconfig.h
> instead of /usr/lib/glib-2.0/include/glibconfig.h
>
> This causes problems because the 'gsize' type (defined as
> 'unsigned long') will no longer be fully compatible with
> the 'size_t' type (defined as 'unsigned int'). Although
> both are the same size, the compiler refuses to allow
> casts from 'unsigned long *' to 'unsigned int *' as they
> are different pointer types. This results in non-obvious
> compiler errors when building QEMU eg
>
> qga/commands-posix.c: In function ‘qmp_guest_set_user_password’:
> qga/commands-posix.c:1912:55: error: passing argument 2 of ‘g_base64_decode’ from incompatible pointer type [-Werror=incompatible-pointer-types]
>      rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
>                                                             ^
> In file included from /usr/include/glib-2.0/glib.h:35:0,
>                  from qga/commands-posix.c:14:
> /usr/include/glib-2.0/glib/gbase64.h:52:9: note: expected ‘gsize * {aka long unsigned int *}’ but argument is of type ‘size_t * {aka unsigned int *}’
>  guchar *g_base64_decode         (const gchar  *text,
>          ^
> cc1: all warnings being treated as errors
>
> To detect this problem, add a check to configure that
> verifies that GLIB_SIZEOF_SIZE_T matches sizeof(size_t).
> If this fails print a warning suggesting that the dev
> probably needs to set PKG_CONFIG_LIBDIR.
>
> On Fedora x86_64 it passes with any of:
>
>  # ./configure
>  # PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ./configure --extra-cflags="-m32"
>  # PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig ./configure --extra-cflags="-m64"
>
> And fails with a mis-match
>
>  # PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig ./configure --extra-cflags="-m32"
>  # PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ./configure --extra-cflags="-m64"
>
> ERROR: sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T.
>        You probably need to set PKG_CONFIG_LIBDIR
>        to point to the right pkg-config files for your
>        build target
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

I just noticed this never got applied to master -- did it just fall through
the cracks, or is there an issue with it?

thanks
-- PMM
Daniel P. Berrangé Jan. 25, 2016, 2:47 p.m. UTC | #7
On Mon, Jan 25, 2016 at 02:42:10PM +0000, Peter Maydell wrote:
> On 13 November 2015 at 11:31, Daniel P. Berrange <berrange@redhat.com> wrote:
> > Developers on 64-bit machines will often try to perform a
> > 32-bit build of QEMU by running
> >
> >   ./configure --extra-cflags="-m32"
> >
> > Unfortunately if PKG_CONFIG_LIBDIR is not set to point to
> > the location of the 32-bit pkg-config files, then configure
> > will silently pick up the 64-bit pkg-config files and still
> > succeed.
> >
> > This causes a problem for glib because it means QEMU will
> > be pulling in /usr/lib64/glib-2.0/include/glibconfig.h
> > instead of /usr/lib/glib-2.0/include/glibconfig.h
> >
> > This causes problems because the 'gsize' type (defined as
> > 'unsigned long') will no longer be fully compatible with
> > the 'size_t' type (defined as 'unsigned int'). Although
> > both are the same size, the compiler refuses to allow
> > casts from 'unsigned long *' to 'unsigned int *' as they
> > are different pointer types. This results in non-obvious
> > compiler errors when building QEMU eg
> >
> > qga/commands-posix.c: In function ‘qmp_guest_set_user_password’:
> > qga/commands-posix.c:1912:55: error: passing argument 2 of ‘g_base64_decode’ from incompatible pointer type [-Werror=incompatible-pointer-types]
> >      rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
> >                                                             ^
> > In file included from /usr/include/glib-2.0/glib.h:35:0,
> >                  from qga/commands-posix.c:14:
> > /usr/include/glib-2.0/glib/gbase64.h:52:9: note: expected ‘gsize * {aka long unsigned int *}’ but argument is of type ‘size_t * {aka unsigned int *}’
> >  guchar *g_base64_decode         (const gchar  *text,
> >          ^
> > cc1: all warnings being treated as errors
> >
> > To detect this problem, add a check to configure that
> > verifies that GLIB_SIZEOF_SIZE_T matches sizeof(size_t).
> > If this fails print a warning suggesting that the dev
> > probably needs to set PKG_CONFIG_LIBDIR.
> >
> > On Fedora x86_64 it passes with any of:
> >
> >  # ./configure
> >  # PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ./configure --extra-cflags="-m32"
> >  # PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig ./configure --extra-cflags="-m64"
> >
> > And fails with a mis-match
> >
> >  # PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig ./configure --extra-cflags="-m32"
> >  # PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ./configure --extra-cflags="-m64"
> >
> > ERROR: sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T.
> >        You probably need to set PKG_CONFIG_LIBDIR
> >        to point to the right pkg-config files for your
> >        build target
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> 
> I just noticed this never got applied to master -- did it just fall through
> the cracks, or is there an issue with it?

AFAIK, no one raised a blocking issue with it.

Regards,
Daniel
Peter Maydell Jan. 25, 2016, 2:58 p.m. UTC | #8
On 25 January 2016 at 14:47, Daniel P. Berrange <berrange@redhat.com> wrote:
> AFAIK, no one raised a blocking issue with it.

I've just found one :-)  which is that it fails on my OSX build system:

config-temp/qemu-conf.c:5:4: error: unused typedef '_GStaticAssert_5'
[-Werror,-Wunused-local-typedef]
   G_STATIC_ASSERT(sizeof(size_t) == GLIB_SIZEOF_SIZE_T);
   ^
/sw/include/glib-2.0/glib/gmacros.h:143:96: note: expanded from macro
'G_STATIC_ASSERT'
#define G_STATIC_ASSERT(expr) typedef struct { char
Compile_Time_Assertion[(expr) ? 1 : -1]; } G_PASTE (_GStaticAssert_,
__LINE__)

                        ^
/sw/include/glib-2.0/glib/gmacros.h:142:47: note: expanded from macro 'G_PASTE'
#define G_PASTE(identifier1,identifier2)      G_PASTE_ARGS
(identifier1, identifier2)
                                              ^
/sw/include/glib-2.0/glib/gmacros.h:141:47: note: expanded from macro
'G_PASTE_ARGS'
#define G_PASTE_ARGS(identifier1,identifier2) identifier1 ## identifier2
                                              ^
<scratch space>:170:1: note: expanded from here
_GStaticAssert_5
^
1 error generated.

The annotation of gmacros.h to mark the typedef as deliberately unused
didn't happen until after the version of glib I have. G_STATIC_ASSERT
is probably best avoided in QEMU code (we have QEMU_BUILD_BUG_ON
anyway).

thanks
-- PMM
Daniel P. Berrangé Jan. 25, 2016, 3 p.m. UTC | #9
On Mon, Jan 25, 2016 at 02:58:22PM +0000, Peter Maydell wrote:
> On 25 January 2016 at 14:47, Daniel P. Berrange <berrange@redhat.com> wrote:
> > AFAIK, no one raised a blocking issue with it.
> 
> I've just found one :-)  which is that it fails on my OSX build system:
> 
> config-temp/qemu-conf.c:5:4: error: unused typedef '_GStaticAssert_5'
> [-Werror,-Wunused-local-typedef]
>    G_STATIC_ASSERT(sizeof(size_t) == GLIB_SIZEOF_SIZE_T);
>    ^
> /sw/include/glib-2.0/glib/gmacros.h:143:96: note: expanded from macro
> 'G_STATIC_ASSERT'
> #define G_STATIC_ASSERT(expr) typedef struct { char
> Compile_Time_Assertion[(expr) ? 1 : -1]; } G_PASTE (_GStaticAssert_,
> __LINE__)
> 
>                         ^
> /sw/include/glib-2.0/glib/gmacros.h:142:47: note: expanded from macro 'G_PASTE'
> #define G_PASTE(identifier1,identifier2)      G_PASTE_ARGS
> (identifier1, identifier2)
>                                               ^
> /sw/include/glib-2.0/glib/gmacros.h:141:47: note: expanded from macro
> 'G_PASTE_ARGS'
> #define G_PASTE_ARGS(identifier1,identifier2) identifier1 ## identifier2
>                                               ^
> <scratch space>:170:1: note: expanded from here
> _GStaticAssert_5
> ^
> 1 error generated.
> 
> The annotation of gmacros.h to mark the typedef as deliberately unused
> didn't happen until after the version of glib I have. G_STATIC_ASSERT
> is probably best avoided in QEMU code (we have QEMU_BUILD_BUG_ON
> anyway).

I'm travelling for a few days, but will investigate and re-post an update
once i return.

Regards,
Daniel
diff mbox

Patch

diff --git a/configure b/configure
index f75df4b..c53334f 100755
--- a/configure
+++ b/configure
@@ -2939,6 +2939,27 @@  for i in $glib_modules; do
     fi
 done
 
+# Sanity check that the current size_t matches the
+# size that glib thinks it should be. This catches
+# problems on multi-arch where people try to build
+# 32-bit QEMU while pointing at 64-bit glib headers
+cat > $TMPC <<EOF
+#include <glib.h>
+#include <unistd.h>
+
+int main(void) {
+   G_STATIC_ASSERT(sizeof(size_t) == GLIB_SIZEOF_SIZE_T);
+   return 0;
+}
+EOF
+
+if ! compile_prog "-Werror $CFLAGS" "$LIBS" ; then
+    error_exit "sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T."\
+               "You probably need to set PKG_CONFIG_LIBDIR"\
+	       "to point to the right pkg-config files for your"\
+	       "build target"
+fi
+
 # g_test_trap_subprocess added in 2.38. Used by some tests.
 glib_subprocess=yes
 if ! $pkg_config --atleast-version=2.38 glib-2.0; then