[libvirt] [PATCH 0/2] More clang-inspired fixes
Hopefully, the split of library/non-library code will let us tighten the frame limit in the future. Ján Tomko (2): util: ignore -Wcast-align in virNetlinkDumpCommand Split out -Wframe-larger-than warning from WARN_CLFAGS daemon/Makefile.am | 3 +++ m4/virt-compile-warnings.m4 | 4 ++-- src/Makefile.am | 1 + src/util/virnetlink.c | 3 ++- tests/Makefile.am | 2 ++ tools/Makefile.am | 6 ++++++ 6 files changed, 16 insertions(+), 3 deletions(-) -- 2.10.2
Similar to commit b202c39 ignore the warning that breaks the build with clang: util/virnetlink.c:365:52: error: cast from 'char *' to 'struct nlmsghdr *' increases required alignment from 1 to 4 [-Werror,-Wcast-align] for (msg = resp; NLMSG_OK(msg, len); msg = NLMSG_NEXT(msg, len)) { ^~~~~~~~~~~~~~~~~~~~ /usr/include/linux/netlink.h:87:7: note: expanded from macro 'NLMSG_NEXT' (struct nlmsghdr*)(((char*)(nlh)) + NLMSG_ALIGN((nlh)->nlmsg_len))) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ --- src/util/virnetlink.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 9bc1f0f..92ecf77 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -361,8 +361,9 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, while (!end) { len = nl_recv(nlhandle, &nladdr, (unsigned char **)&resp, NULL); - + VIR_WARNINGS_NO_CAST_ALIGN for (msg = resp; NLMSG_OK(msg, len); msg = NLMSG_NEXT(msg, len)) { + VIR_WARNINGS_RESET if (msg->nlmsg_type == NLMSG_DONE) end = true; -- 2.10.2
On Tue, 2017-04-04 at 12:57 +0200, Ján Tomko wrote:
Similar to commit b202c39 ignore the warning that breaks the build with clang: util/virnetlink.c:365:52: error: cast from 'char *' to 'struct nlmsghdr *' increases required alignment from 1 to 4 [-Werror,-Wcast-align] for (msg = resp; NLMSG_OK(msg, len); msg = NLMSG_NEXT(msg, len)) { ^~~~~~~~~~~~~~~~~~~~ /usr/include/linux/netlink.h:87:7: note: expanded from macro 'NLMSG_NEXT' (struct nlmsghdr*)(((char*)(nlh)) + NLMSG_ALIGN((nlh)->nlmsg_len))) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ACK -- Andrea Bolognani / Red Hat / Virtualization
Introduce STRICT_FRAME_LIMIT_CFLAGS that will be used for the library code and RELAXED_FRAME_LIMIT_CFLAGS for daemon code and the test code. Raising the limit for tests allows building them with clang with optimizations disabled. --- daemon/Makefile.am | 3 +++ m4/virt-compile-warnings.m4 | 4 ++-- src/Makefile.am | 1 + tests/Makefile.am | 2 ++ tools/Makefile.am | 6 ++++++ 5 files changed, 14 insertions(+), 2 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 60c7368..5deab1e 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -124,6 +124,7 @@ libvirtd_conf_la_CFLAGS = \ $(LIBXML_CFLAGS) \ $(XDR_CFLAGS) \ $(WARN_CFLAGS) $(PIE_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(NULL) libvirtd_conf_la_LDFLAGS = \ @@ -143,6 +144,7 @@ libvirtd_admin_la_CFLAGS = \ $(XDR_CFLAGS) \ $(PIE_CFLAGS) \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(LIBXML_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(NULL) @@ -177,6 +179,7 @@ libvirtd_CFLAGS = \ $(LIBXML_CFLAGS) $(GNUTLS_CFLAGS) $(SASL_CFLAGS) \ $(XDR_CFLAGS) $(DBUS_CFLAGS) $(LIBNL_CFLAGS) \ $(WARN_CFLAGS) $(PIE_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(COVERAGE_CFLAGS) \ -DQEMUD_PID_FILE="\"$(QEMUD_PID_FILE)\"" diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 17fdf9d..768a5c8 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -169,8 +169,8 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # This should be < 256 really. Currently we're down to 4096, # but using 1024 bytes sized buffers (mostly for virStrerror) # stops us from going down further - wantwarn="$wantwarn -Wframe-larger-than=4096" - dnl wantwarn="$wantwarn -Wframe-larger-than=256" + gl_WARN_ADD(["-Wframe-larger-than=4096"], [STRICT_FRAME_LIMIT_CFLAGS]) + gl_WARN_ADD(["-Wframe-larger-than=25600"], [RELAXED_FRAME_LIMIT_CFLAGS]) # Extra special flags dnl -fstack-protector stuff passes gl_WARN_ADD with gcc diff --git a/src/Makefile.am b/src/Makefile.am index 75e4344..f8e5017 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -39,6 +39,7 @@ INCLUDES = -I../gnulib/lib \ AM_CFLAGS = $(LIBXML_CFLAGS) \ $(WARN_CFLAGS) \ + $(STRICT_FRAME_LIMIT_CFLAGS) \ $(LOCK_CHECKING_CFLAGS) \ $(WIN32_EXTRA_CFLAGS) \ $(COVERAGE_CFLAGS) diff --git a/tests/Makefile.am b/tests/Makefile.am index a6f189b..35f02d5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -46,6 +46,7 @@ AM_CFLAGS = \ $(APPARMOR_CFLAGS) \ $(YAJL_CFLAGS) \ $(COVERAGE_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(WARN_CFLAGS) AM_LDFLAGS = \ @@ -925,6 +926,7 @@ commandhelper_SOURCES = \ commandhelper.c commandhelper_LDADD = \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(NO_INDIRECT_LDFLAGS) \ $(PROBES_O) \ ../src/libvirt_util.la \ diff --git a/tools/Makefile.am b/tools/Makefile.am index 162d8e5..76e97eb 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -182,6 +182,7 @@ virt_host_validate_LDADD = \ virt_host_validate_CFLAGS = \ $(LIBXML_CFLAGS) \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(NULL) @@ -208,6 +209,7 @@ virt_login_shell_CFLAGS = \ -DLIBVIRT_SETUID_RPC_CLIENT \ $(LIBXML_CFLAGS) \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) @@ -239,6 +241,7 @@ virsh_LDADD = \ libvirt_shell.la virsh_CFLAGS = \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(LIBXML_CFLAGS) @@ -260,6 +263,7 @@ virt_admin_LDADD = \ $(NULL) virt_admin_CFLAGS = \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(LIBXML_CFLAGS) \ @@ -499,6 +503,7 @@ nss_libnss_libvirt_impl_la_CFLAGS = \ -DLIBVIRT_NSS \ $(AM_CFLAGS) \ $(WARN_CFLAGS) \ + $(STRICT_FRAME_LIMIT_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(LIBXML_CFLAGS) @@ -529,6 +534,7 @@ nss_libnss_libvirt_guest_impl_la_CFLAGS = \ -DLIBVIRT_NSS_GUEST \ $(AM_CFLAGS) \ $(WARN_CFLAGS) \ + $(STRICT_FRAME_LIMIT_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(LIBXML_CFLAGS) -- 2.10.2
On Tue, Apr 04, 2017 at 12:57:59PM +0200, Ján Tomko wrote:
Introduce STRICT_FRAME_LIMIT_CFLAGS that will be used for the library code and RELAXED_FRAME_LIMIT_CFLAGS for daemon code and the test code.
Raising the limit for tests allows building them with clang with optimizations disabled. --- daemon/Makefile.am | 3 +++ m4/virt-compile-warnings.m4 | 4 ++-- src/Makefile.am | 1 + tests/Makefile.am | 2 ++ tools/Makefile.am | 6 ++++++ 5 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 60c7368..5deab1e 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -124,6 +124,7 @@ libvirtd_conf_la_CFLAGS = \ $(LIBXML_CFLAGS) \ $(XDR_CFLAGS) \ $(WARN_CFLAGS) $(PIE_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(NULL) libvirtd_conf_la_LDFLAGS = \ @@ -143,6 +144,7 @@ libvirtd_admin_la_CFLAGS = \ $(XDR_CFLAGS) \ $(PIE_CFLAGS) \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(LIBXML_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(NULL) @@ -177,6 +179,7 @@ libvirtd_CFLAGS = \ $(LIBXML_CFLAGS) $(GNUTLS_CFLAGS) $(SASL_CFLAGS) \ $(XDR_CFLAGS) $(DBUS_CFLAGS) $(LIBNL_CFLAGS) \ $(WARN_CFLAGS) $(PIE_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(COVERAGE_CFLAGS) \ -DQEMUD_PID_FILE="\"$(QEMUD_PID_FILE)\""
Daemons should be using the strict limit.
diff --git a/tools/Makefile.am b/tools/Makefile.am index 162d8e5..76e97eb 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -182,6 +182,7 @@ virt_host_validate_LDADD = \ virt_host_validate_CFLAGS = \ $(LIBXML_CFLAGS) \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(NULL) @@ -208,6 +209,7 @@ virt_login_shell_CFLAGS = \ -DLIBVIRT_SETUID_RPC_CLIENT \ $(LIBXML_CFLAGS) \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS)
@@ -239,6 +241,7 @@ virsh_LDADD = \ libvirt_shell.la virsh_CFLAGS = \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(LIBXML_CFLAGS) @@ -260,6 +263,7 @@ virt_admin_LDADD = \ $(NULL) virt_admin_CFLAGS = \ $(WARN_CFLAGS) \ + $(RELAXED_FRAME_LIMIT_CFLAGS) \ $(PIE_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(LIBXML_CFLAGS) \
Tools should use the strict limit too. Only non-production code should have relaxed limit - ie tests. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
On Tue, 2017-04-04 at 12:57 +0200, Ján Tomko wrote:
Introduce STRICT_FRAME_LIMIT_CFLAGS that will be used for the library code and RELAXED_FRAME_LIMIT_CFLAGS for daemon code and the test code.
Raising the limit for tests allows building them with clang with optimizations disabled.
I'd like to see this being handled once per Makefile.am so that we don't end up with some programs where the check is skipped. -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani -
Daniel P. Berrange -
Ján Tomko