[libvirt] [PATCH 0/6] Ensure clean compile with clang
From: "Daniel P. Berrange" <berrange@redhat.com> There are various problems building libvirt with clang, which mostly revolve around compiler warning handling. This series fixes all the problems I see with clang 3.2 on a Fedora 19 x86_64 host. Daniel P. Berrange (6): Ensure consistent enablement of gcc 'diagnostic' pragma Workaround issue with clang and inline functions with static vars Ignore cast alignment warnings in inotify code for Xen. Correctly detect warning flags with clang Only pass -export-dynamic to linker, not compiler Don't duplicate compiler warning flags when linking configure.ac | 4 ++-- daemon/Makefile.am | 1 - m4/virt-compile-warnings.m4 | 29 +++++++++++++++++++++++++---- src/Makefile.am | 11 +++-------- src/internal.h | 2 +- src/xen/xen_inotify.c | 3 +++ tests/vircgroupmock.c | 7 ++++++- tools/Makefile.am | 4 +--- 8 files changed, 41 insertions(+), 20 deletions(-) -- 1.8.2.1
From: "Daniel P. Berrange" <berrange@redhat.com> The virt-compile-warnings.m4 file would do an explicit check for whether the compile could use the 'diagnostic' pragma push/pop feature. The src/internal.h file would then only enable it for GCC >= 4.6 This breaks with clang which supports the pragma but does not claim GCC 4.6 compat. Export a variable from the m4 check to the header file so they are consistent. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- m4/virt-compile-warnings.m4 | 3 +++ src/internal.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index e054913..fbeb3eb 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -94,6 +94,9 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wmissing-prototypes" dontwarn="$dontwarn -Wmissing-declarations" dontwarn="$dontwarn -Wcast-align" + else + AC_DEFINE_UNQUOTED([WORKING_PRAGMA_PUSH], 1, + [Define to 1 if gcc supports pragma push/pop]) fi dnl Check whether strchr(s, char variable) causes a bogus compile diff --git a/src/internal.h b/src/internal.h index d819aa3..03c2493 100644 --- a/src/internal.h +++ b/src/internal.h @@ -215,7 +215,7 @@ # endif /* __GNUC__ */ -# if __GNUC_PREREQ (4, 6) +# if WORKING_PRAGMA_PUSH # define VIR_WARNINGS_NO_CAST_ALIGN \ _Pragma ("GCC diagnostic push") \ _Pragma ("GCC diagnostic ignored \"-Wcast-align\"") -- 1.8.2.1
[adding bug-gnulib] On 05/13/2013 06:17 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virt-compile-warnings.m4 file would do an explicit check for whether the compile could use the 'diagnostic' pragma push/pop feature. The src/internal.h file would then only enable it for GCC >= 4.6
This breaks with clang which supports the pragma but does not claim GCC 4.6 compat. Export a variable from the m4 check to the header file so they are consistent.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- m4/virt-compile-warnings.m4 | 3 +++ src/internal.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
ACK. Hmm, I wonder if gnulib should use something similar, for the places where it uses pragma gcc push.
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index e054913..fbeb3eb 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -94,6 +94,9 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wmissing-prototypes" dontwarn="$dontwarn -Wmissing-declarations" dontwarn="$dontwarn -Wcast-align" + else + AC_DEFINE_UNQUOTED([WORKING_PRAGMA_PUSH], 1, + [Define to 1 if gcc supports pragma push/pop]) fi
dnl Check whether strchr(s, char variable) causes a bogus compile diff --git a/src/internal.h b/src/internal.h index d819aa3..03c2493 100644 --- a/src/internal.h +++ b/src/internal.h @@ -215,7 +215,7 @@ # endif /* __GNUC__ */
-# if __GNUC_PREREQ (4, 6) +# if WORKING_PRAGMA_PUSH # define VIR_WARNINGS_NO_CAST_ALIGN \ _Pragma ("GCC diagnostic push") \ _Pragma ("GCC diagnostic ignored \"-Wcast-align\"")
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/13/13 12:16, Eric Blake wrote:
I wonder if gnulib should use something similar, for the places where it uses pragma gcc push.
I hope not. The only place gnulib does a push, is to turn off bogus diagnostics that I hope clang doesn't generate. If I'm wrong I suppose we can add an "|| __clang__", but let's hope I'm right.
From: "Daniel P. Berrange" <berrange@redhat.com> Clang does not like it when you pass a static variable to an inline function vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an inline function with external linkage [-Werror,-Wstatic-in-inline] Just make the var non-static to avoid this Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tests/vircgroupmock.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 17ea75f..f1a5700 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -37,7 +37,12 @@ static int (*realaccess)(const char *path, int mode); static int (*reallstat)(const char *path, struct stat *sb); static int (*real__lxstat)(int ver, const char *path, struct stat *sb); static int (*realmkdir)(const char *path, mode_t mode); -static char *fakesysfsdir; + +/* Don't make static, since it causes problems with clang + * when passed as an arg to asprintf() + * vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an inline function with external linkage [-Werror,-Wstatic-in-inline] + */ +char *fakesysfsdir; # define SYSFS_PREFIX "/not/really/sys/fs/cgroup/" -- 1.8.2.1
On 05/13/2013 06:17 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Clang does not like it when you pass a static variable to an inline function
vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an inline function with external linkage [-Werror,-Wstatic-in-inline]
Just make the var non-static to avoid this
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tests/vircgroupmock.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
ACK. I don't know if that is a case where gcc is silently allowing a non-conforming extension beyond C99, or whether it is a case of clang just being brain-dead; but either way, the workaround is painless enough that I don't mind the wider visibility. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> The inotify Xen code causes a cast alignment warning, but this is harmless since the kernel inotify interface will ensure sufficient alignment of the inotify structs in the buffer being read Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/xen/xen_inotify.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index b032bba..e13c572 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -278,7 +278,10 @@ reread: if (got < sizeof(struct inotify_event)) goto cleanup; /* bad */ + VIR_WARNINGS_NO_CAST_ALIGN e = (struct inotify_event *)tmp; + VIR_WARNINGS_RESET + tmp += sizeof(struct inotify_event); got -= sizeof(struct inotify_event); -- 1.8.2.1
On 05/13/2013 06:17 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The inotify Xen code causes a cast alignment warning, but this is harmless since the kernel inotify interface will ensure sufficient alignment of the inotify structs in the buffer being read
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/xen/xen_inotify.c | 3 +++ 1 file changed, 3 insertions(+)
ACK.
diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index b032bba..e13c572 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -278,7 +278,10 @@ reread: if (got < sizeof(struct inotify_event)) goto cleanup; /* bad */
+ VIR_WARNINGS_NO_CAST_ALIGN e = (struct inotify_event *)tmp; + VIR_WARNINGS_RESET + tmp += sizeof(struct inotify_event); got -= sizeof(struct inotify_event);
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Clang will happily claim to support any warning flags unless the -Werror and -Wunknown-warning-option flags are set. Thus we need to make sure these are set when testing for clags. We must also set the clang specific warning flags -Wno-unused-command-line-argument to avoid a warning from the ssp-buffer-size flag when linking .o files. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- m4/virt-compile-warnings.m4 | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index fbeb3eb..5803d48 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -60,6 +60,18 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # gcc 4.4.6 complains this is C++ only; gcc 4.7.0 implies this from -Wall dontwarn="$dontwarn -Wenum-compare" + # clang rather horribly ignores unknown warning flags by + # default. Thus to get gl_WARN_ADD to reliably detect + # flags, we need to set '-Werror -Wunknown-warning-option' + # in CFLAGS while probing support + WARN_CFLAGS= + orig_CFLAGS="$CFLAGS" + gl_WARN_ADD([-Wunknown-warning-option]) + if test -n "$WARN_CFLAGS" ; then + WARN_CFLAGS= + CFLAGS="-Werror -Wunknown-warning-option $CFLAGS" + fi + # gcc 4.2 treats attribute(format) as an implicit attribute(nonnull), # which triggers spurious warnings for our usage AC_CACHE_CHECK([whether gcc -Wformat allows NULL strings], @@ -185,6 +197,13 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl gl_WARN_ADD([-fstack-protector]) gl_WARN_ADD([-fstack-protector-all]) gl_WARN_ADD([--param=ssp-buffer-size=4]) + dnl Even though it supports it, clang complains about + dnl use of --param=ssp-buffer-size=4 unless used with + dnl the -c arg. It doesn't like it when used with args + dnl that just link together .o files. Unfortunately + dnl we can't avoid that with automake, so we must turn + dnl off the following clang specific warning + gl_WARN_ADD([-Wno-unused-command-line-argument]) ;; esac gl_WARN_ADD([-fexceptions]) @@ -222,4 +241,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_DEFINE_UNQUOTED([BROKEN_GCC_WLOGICALOP], 1, [Define to 1 if gcc -Wlogical-op reports false positives on strchr]) fi + + # Remove stuff we set for clang + CFLAGS="$orig_CFLAGS" ]) -- 1.8.2.1
[adding bug-gnulib] On 05/13/2013 06:17 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Clang will happily claim to support any warning flags unless the -Werror and -Wunknown-warning-option flags are set. Thus we need to make sure these are set when testing for clags.
We must also set the clang specific warning flags -Wno-unused-command-line-argument to avoid a warning from the ssp-buffer-size flag when linking .o files.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- m4/virt-compile-warnings.m4 | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
Some of this needs to be imported into upstream gnulib.
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index fbeb3eb..5803d48 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -60,6 +60,18 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # gcc 4.4.6 complains this is C++ only; gcc 4.7.0 implies this from -Wall dontwarn="$dontwarn -Wenum-compare"
+ # clang rather horribly ignores unknown warning flags by + # default. Thus to get gl_WARN_ADD to reliably detect + # flags, we need to set '-Werror -Wunknown-warning-option' + # in CFLAGS while probing support + WARN_CFLAGS= + orig_CFLAGS="$CFLAGS" + gl_WARN_ADD([-Wunknown-warning-option]) + if test -n "$WARN_CFLAGS" ; then + WARN_CFLAGS= + CFLAGS="-Werror -Wunknown-warning-option $CFLAGS" + fi + # gcc 4.2 treats attribute(format) as an implicit attribute(nonnull), # which triggers spurious warnings for our usage AC_CACHE_CHECK([whether gcc -Wformat allows NULL strings], @@ -185,6 +197,13 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl gl_WARN_ADD([-fstack-protector]) gl_WARN_ADD([-fstack-protector-all]) gl_WARN_ADD([--param=ssp-buffer-size=4]) + dnl Even though it supports it, clang complains about + dnl use of --param=ssp-buffer-size=4 unless used with + dnl the -c arg. It doesn't like it when used with args + dnl that just link together .o files. Unfortunately + dnl we can't avoid that with automake, so we must turn + dnl off the following clang specific warning + gl_WARN_ADD([-Wno-unused-command-line-argument]) ;; esac gl_WARN_ADD([-fexceptions]) @@ -222,4 +241,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ AC_DEFINE_UNQUOTED([BROKEN_GCC_WLOGICALOP], 1, [Define to 1 if gcc -Wlogical-op reports false positives on strchr]) fi + + # Remove stuff we set for clang + CFLAGS="$orig_CFLAGS" ])
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/13/2013 12:28 PM, Eric Blake wrote:
Some of this needs to be imported into upstream gnulib.
I gave it a try, by installing the following. I'll follow up on bug-gnulib with 7 other clang-related patches.
From 45fc031e607cb82dce777228d9334cdac16ca648 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Wed, 15 May 2013 00:07:15 -0700 Subject: [PATCH 1/8] warnings: port to clang
Problem reported by Daniel P. Berrange via Eric Blake in <http://lists.gnu.org/archive/html/bug-gnulib/2013-05/msg00055.html>. * m4/warnings.m4 (gl_UNKNOWN_WARNINGS_ARE_ERRORS): New macro. (gl_WARN_ADD): Use it. --- ChangeLog | 8 ++++++++ m4/warnings.m4 | 15 ++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 08f2fe0..b7aceaa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2013-05-15 Paul Eggert <eggert@cs.ucla.edu> + + warnings: port to clang + Problem reported by Daniel P. Berrange via Eric Blake in + <http://lists.gnu.org/archive/html/bug-gnulib/2013-05/msg00055.html>. + * m4/warnings.m4 (gl_UNKNOWN_WARNINGS_ARE_ERRORS): New macro. + (gl_WARN_ADD): Use it. + 2013-05-11 Jim Meyering <meyering@fb.com> quotearg: do not read beyond end of buffer diff --git a/m4/warnings.m4 b/m4/warnings.m4 index 4b2ac38..1848732 100644 --- a/m4/warnings.m4 +++ b/m4/warnings.m4 @@ -1,4 +1,4 @@ -# warnings.m4 serial 7 +# warnings.m4 serial 8 dnl Copyright (C) 2008-2013 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -27,7 +27,7 @@ AC_DEFUN([gl_COMPILER_OPTION_IF], AS_VAR_PUSHDEF([gl_Flags], [_AC_LANG_PREFIX[]FLAGS])dnl AC_CACHE_CHECK([whether _AC_LANG compiler handles $1], m4_defn([gl_Warn]), [ gl_save_compiler_FLAGS="$gl_Flags" - gl_AS_VAR_APPEND(m4_defn([gl_Flags]), [" $1"]) + gl_AS_VAR_APPEND(m4_defn([gl_Flags]), [" $gl_unknown_warnings_are_errors $1"]) AC_COMPILE_IFELSE([m4_default([$4], [AC_LANG_PROGRAM([])])], [AS_VAR_SET(gl_Warn, [yes])], [AS_VAR_SET(gl_Warn, [no])]) @@ -38,6 +38,14 @@ AS_VAR_POPDEF([gl_Flags])dnl AS_VAR_POPDEF([gl_Warn])dnl ]) +# gl_UNKNOWN_WARNINGS_ARE_ERRORS +# ------------------------------ +# Clang doesn't complain about unknown warning options unless one also +# specifies -Wunknown-warning-option -Werror. Detect this. +AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS], +[gl_COMPILER_OPTION_IF([-Werror -Wunknown-warning-option], + [gl_unknown_warnings_are_errors='-Wunknown-warning-option -Werror'], + [gl_unknown_warnings_are_errors=])]) # gl_WARN_ADD(OPTION, [VARIABLE = WARN_CFLAGS], # [PROGRAM = AC_LANG_PROGRAM()]) @@ -47,7 +55,8 @@ AS_VAR_POPDEF([gl_Warn])dnl # # If VARIABLE is a variable name, AC_SUBST it. AC_DEFUN([gl_WARN_ADD], -[gl_COMPILER_OPTION_IF([$1], +[AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS]) +gl_COMPILER_OPTION_IF([$1], [gl_AS_VAR_APPEND(m4_if([$2], [], [[WARN_CFLAGS]], [[$2]]), [" $1"])], [], [$3]) -- 1.7.11.7
From: "Daniel P. Berrange" <berrange@redhat.com> Clang does not like the -export-dynamic flag. The compiler does not need it in the first place, so we can avoid the problem by only setting it for the linker Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 4 ++-- src/Makefile.am | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 53f78de..9e31c39 100644 --- a/configure.ac +++ b/configure.ac @@ -2154,7 +2154,7 @@ if test "$with_driver_modules" = "yes" || test "$with_driver_modules" = "check"; fi if test "$with_driver_modules" = "yes" ; then - DRIVER_MODULE_CFLAGS="-export-dynamic" + DRIVER_MODULE_LDFLAGS="-export-dynamic" case $ac_cv_search_dlopen in no*) DRIVER_MODULE_LIBS= ;; *) DRIVER_MODULE_LIBS=$ac_cv_search_dlopen ;; @@ -2162,7 +2162,7 @@ if test "$with_driver_modules" = "yes" ; then AC_DEFINE_UNQUOTED([WITH_DRIVER_MODULES], 1, [whether to build drivers as modules]) fi AM_CONDITIONAL([WITH_DRIVER_MODULES], [test "$with_driver_modules" != "no"]) -AC_SUBST([DRIVER_MODULE_CFLAGS]) +AC_SUBST([DRIVER_MODULE_LDFLAGS]) AC_SUBST([DRIVER_MODULE_LIBS]) diff --git a/src/Makefile.am b/src/Makefile.am index 4312c3c..9b9f9f2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -14,13 +14,13 @@ INCLUDES = -I../gnulib/lib \ -DIN_LIBVIRT \ $(GETTEXT_CPPFLAGS) -AM_CFLAGS = $(DRIVER_MODULE_CFLAGS) \ - $(LIBXML_CFLAGS) \ +AM_CFLAGS = $(LIBXML_CFLAGS) \ $(WARN_CFLAGS) \ $(LOCK_CHECKING_CFLAGS) \ $(WIN32_EXTRA_CFLAGS) \ $(COVERAGE_CFLAGS) -AM_LDFLAGS = $(COVERAGE_LDFLAGS) +AM_LDFLAGS = $(DRIVER_MODULE_LDFLAGS) \ + $(COVERAGE_LDFLAGS) EXTRA_DIST = $(conf_DATA) util/keymaps.csv -- 1.8.2.1
On 05/13/2013 06:17 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Clang does not like the -export-dynamic flag. The compiler does not need it in the first place, so we can avoid the problem by only setting it for the linker
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 4 ++-- src/Makefile.am | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-)
ACK.
diff --git a/configure.ac b/configure.ac index 53f78de..9e31c39 100644 --- a/configure.ac +++ b/configure.ac @@ -2154,7 +2154,7 @@ if test "$with_driver_modules" = "yes" || test "$with_driver_modules" = "check"; fi
if test "$with_driver_modules" = "yes" ; then - DRIVER_MODULE_CFLAGS="-export-dynamic" + DRIVER_MODULE_LDFLAGS="-export-dynamic" case $ac_cv_search_dlopen in no*) DRIVER_MODULE_LIBS= ;; *) DRIVER_MODULE_LIBS=$ac_cv_search_dlopen ;; @@ -2162,7 +2162,7 @@ if test "$with_driver_modules" = "yes" ; then AC_DEFINE_UNQUOTED([WITH_DRIVER_MODULES], 1, [whether to build drivers as modules]) fi AM_CONDITIONAL([WITH_DRIVER_MODULES], [test "$with_driver_modules" != "no"]) -AC_SUBST([DRIVER_MODULE_CFLAGS]) +AC_SUBST([DRIVER_MODULE_LDFLAGS]) AC_SUBST([DRIVER_MODULE_LIBS])
diff --git a/src/Makefile.am b/src/Makefile.am index 4312c3c..9b9f9f2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -14,13 +14,13 @@ INCLUDES = -I../gnulib/lib \ -DIN_LIBVIRT \ $(GETTEXT_CPPFLAGS)
-AM_CFLAGS = $(DRIVER_MODULE_CFLAGS) \ - $(LIBXML_CFLAGS) \ +AM_CFLAGS = $(LIBXML_CFLAGS) \ $(WARN_CFLAGS) \ $(LOCK_CHECKING_CFLAGS) \ $(WIN32_EXTRA_CFLAGS) \ $(COVERAGE_CFLAGS) -AM_LDFLAGS = $(COVERAGE_LDFLAGS) +AM_LDFLAGS = $(DRIVER_MODULE_LDFLAGS) \ + $(COVERAGE_LDFLAGS)
EXTRA_DIST = $(conf_DATA) util/keymaps.csv
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Automake already passes all CFLAGS to the linker too, so it is not neccesary to set WARN_LDFLAGS in addition to the WARN_CFLAGS variable. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/Makefile.am | 1 - m4/virt-compile-warnings.m4 | 4 ---- src/Makefile.am | 5 ----- tools/Makefile.am | 4 +--- 4 files changed, 1 insertion(+), 13 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 64126e5..f48dc85 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -111,7 +111,6 @@ libvirtd_CFLAGS = \ -DQEMUD_PID_FILE="\"$(QEMUD_PID_FILE)\"" libvirtd_LDFLAGS = \ - $(WARN_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ $(COVERAGE_LDFLAGS) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 5803d48..5174472 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -224,10 +224,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ gl_WARN_ADD([-Werror]) fi - WARN_LDFLAGS=$WARN_CFLAGS - AC_SUBST([WARN_CFLAGS]) - AC_SUBST([WARN_LDFLAGS]) - dnl Needed to keep compile quiet on python 2.4 save_WARN_CFLAGS=$WARN_CFLAGS WARN_CFLAGS= diff --git a/src/Makefile.am b/src/Makefile.am index 9b9f9f2..6c626ac 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1979,7 +1979,6 @@ if WITH_LIBVIRTD libexec_PROGRAMS += libvirt_iohelper libvirt_iohelper_SOURCES = $(UTIL_IO_HELPER_SOURCES) libvirt_iohelper_LDFLAGS = \ - $(WARN_LDFLAGS) \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ @@ -2003,7 +2002,6 @@ libexec_PROGRAMS += libvirt_parthelper libvirt_parthelper_SOURCES = $(STORAGE_HELPER_DISK_SOURCES) libvirt_parthelper_LDFLAGS = \ - $(WARN_LDFLAGS) \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ @@ -2036,7 +2034,6 @@ libvirt_sanlock_helper_CFLAGS = \ $(PIE_CFLAGS) \ $(NULL) libvirt_sanlock_helper_LDFLAGS = \ - $(WARN_LDFLAGS) \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ @@ -2053,7 +2050,6 @@ libvirt_lxc_SOURCES = \ $(NODE_INFO_SOURCES) \ $(DATATYPES_SOURCES) libvirt_lxc_LDFLAGS = \ - $(WARN_LDFLAGS) \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ @@ -2098,7 +2094,6 @@ libexec_PROGRAMS += virt-aa-helper virt_aa_helper_SOURCES = $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES) virt_aa_helper_LDFLAGS = \ - $(WARN_LDFLAGS) \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ diff --git a/tools/Makefile.am b/tools/Makefile.am index 07c9f43..4136b84 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -98,7 +98,6 @@ virt_host_validate_SOURCES = \ $(NULL) virt_host_validate_LDFLAGS = \ - $(WARN_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ $(COVERAGE_LDFLAGS) \ @@ -131,10 +130,9 @@ virsh_SOURCES = \ virsh-volume.c virsh-volume.h \ $(NULL) -virsh_LDFLAGS = $(WARN_LDFLAGS) $(COVERAGE_LDFLAGS) +virsh_LDFLAGS = $(COVERAGE_LDFLAGS) virsh_LDADD = \ $(STATIC_BINARIES) \ - $(WARN_LDFLAGS) \ $(PIE_LDFLAGS) \ $(RELRO_LDFLAGS) \ ../src/libvirt.la \ -- 1.8.2.1
On 05/13/2013 06:17 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Automake already passes all CFLAGS to the linker too, so it is not neccesary to set WARN_LDFLAGS in addition to the
s/neccesary/necessary/
WARN_CFLAGS variable.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/Makefile.am | 1 - m4/virt-compile-warnings.m4 | 4 ---- src/Makefile.am | 5 ----- tools/Makefile.am | 4 +--- 4 files changed, 1 insertion(+), 13 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
There are various problems building libvirt with clang, which mostly revolve around compiler warning handling. This series fixes all the problems I see with clang 3.2 on a Fedora 19 x86_64 host.
Daniel P. Berrange (6): Ensure consistent enablement of gcc 'diagnostic' pragma Workaround issue with clang and inline functions with static vars Ignore cast alignment warnings in inotify code for Xen. Correctly detect warning flags with clang Only pass -export-dynamic to linker, not compiler Don't duplicate compiler warning flags when linking
configure.ac | 4 ++-- daemon/Makefile.am | 1 - m4/virt-compile-warnings.m4 | 29 +++++++++++++++++++++++++---- src/Makefile.am | 11 +++-------- src/internal.h | 2 +- src/xen/xen_inotify.c | 3 +++ tests/vircgroupmock.c | 7 ++++++- tools/Makefile.am | 4 +--- 8 files changed, 41 insertions(+), 20 deletions(-)
Tested this patchset on FreeBSD with clang 3.1. Had a warning like that when building examples: gmake[2]: Entering directory `/usr/home/novel/code/libvirt/examples/openauth' CC openauth-openauth.o CCLD openauth clang: warning: argument unused during compilation: '-g' [-Wunused-command-line-argument] gmake[2]: Leaving directory `/usr/home/novel/code/libvirt/examples/openauth' The problem was fixed by this change: diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 5174472..dc0e7d7 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -191,7 +191,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl -fstack-protector stuff passes gl_WARN_ADD with gcc dnl on Mingw32, but fails when actually used case $host in - *-*-linux*) + *-*-linux*|*-*-freebsd*) dnl Fedora only uses -fstack-protector, but doesn't seem to dnl be great overhead in adding -fstack-protector-all instead dnl gl_WARN_ADD([-fstack-protector]) I still have another warning in gnulib: CC regex.lo In file included from regex.c:70: ./regex_internal.c:1397:11: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare] if (idx < 0 || idx >= set->nelem) ~~~ ^ ~ 1 warning generated. But I'm not sure it affects the build with -Werror because I don't use it since it still fails at gcrypt.h deprecated stuff anyways (and I haven't test a fix for that with clang I posted in other thread). Roman Bogorodskiy
On Mon, May 13, 2013 at 01:17:21PM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
There are various problems building libvirt with clang, which mostly revolve around compiler warning handling. This series fixes all the problems I see with clang 3.2 on a Fedora 19 x86_64 host.
FYI, I measured the following compile times for the src/ directory GCC: 2 minutes 40 seconds CLang: 1 minute 50 seconds So Clang cuts approx 30% off the GCC compile time, which is quite nice. I might have to switch to running clang by default for libvirt :-) Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 13.05.2013 18:09, Daniel P. Berrange wrote:
On Mon, May 13, 2013 at 01:17:21PM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
There are various problems building libvirt with clang, which mostly revolve around compiler warning handling. This series fixes all the problems I see with clang 3.2 on a Fedora 19 x86_64 host.
FYI, I measured the following compile times for the src/ directory
GCC: 2 minutes 40 seconds CLang: 1 minute 50 seconds
So Clang cuts approx 30% off the GCC compile time, which is quite nice. I might have to switch to running clang by default for libvirt :-)
Daniel
FYI: ./autogen --system: 4m40.064s make -j5 all : 1m22.870s so I think compilation is not the major time consumer here :) Michal
On Mon, May 13, 2013 at 07:53:49PM +0200, Michal Privoznik wrote:
On 13.05.2013 18:09, Daniel P. Berrange wrote:
On Mon, May 13, 2013 at 01:17:21PM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
There are various problems building libvirt with clang, which mostly revolve around compiler warning handling. This series fixes all the problems I see with clang 3.2 on a Fedora 19 x86_64 host.
FYI, I measured the following compile times for the src/ directory
GCC: 2 minutes 40 seconds CLang: 1 minute 50 seconds
So Clang cuts approx 30% off the GCC compile time, which is quite nice. I might have to switch to running clang by default for libvirt :-)
Daniel
FYI:
./autogen --system: 4m40.064s make -j5 all : 1m22.870s
so I think compilation is not the major time consumer here :)
I run 'autogen.sh' perhaps 2-3 times a day. I run 'make' as many as 10-20 times an *hour*. So yes, compilation is the major time consumer. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Michal Privoznik -
Paul Eggert -
Roman Bogorodskiy