Date: Sat, 23 Jul 2016 11:40:20 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> Cc: freebsd-bugs@freebsd.org Subject: Re: [Bug 211304] 11.0 -r303168 buildkernel via devel/amd64-gcc fails for: dev/cxgbe/common/t4_hw.c warning: overflow in implicit constant conversion; more Message-ID: <20160723113358.C1884@besplex.bde.org> In-Reply-To: <bug-211304-8-ocCVhQkllT@https.bugs.freebsd.org/bugzilla/> References: <bug-211304-8@https.bugs.freebsd.org/bugzilla/> <bug-211304-8-ocCVhQkllT@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 23 Jul 2016 a bug that doesn't want replies wrote: > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=211304 > > --- Comment #3 from Mark Millard <markmi@dsl-only.net> --- > (In reply to Mark Millard from comment #0) > > The code in question for the overflow in an implicit constant conversion is > for: > > c.err_to_clearinit = cpu_to_be32( > V_FW_HELLO_CMD_MASTERDIS(master == MASTER_CANT) | > V_FW_HELLO_CMD_MASTERFORCE(master == MASTER_MUST) | > V_FW_HELLO_CMD_MBMASTER(master == MASTER_MUST ? > mbox : M_FW_HELLO_CMD_MBMASTER) | > V_FW_HELLO_CMD_MBASYNCNOT(evt_mbox) | > V_FW_HELLO_CMD_STAGE(FW_HELLO_CMD_STAGE_OS) | > F_FW_HELLO_CMD_CLEARINIT); > > V_FW_HELLO_CMD_MASTERDIS uses S_FW_HELLO_CMD_MASTERDIS: > > # grep V_FW_HELLO_CMD_MASTERDIS > /usr/src/sys/modules/cxgbe/if_cxgbe/../../../dev/cxgbe/*/* > /usr/src/sys/modules/cxgbe/if_cxgbe/../../../dev/cxgbe/common/t4_hw.c: > V_FW_HELLO_CMD_MASTERDIS(master == MASTER_CANT) | > /usr/src/sys/modules/cxgbe/if_cxgbe/../../../dev/cxgbe/firmware/t4fw_interface.h:#define > V_FW_HELLO_CMD_MASTERDIS(x) ((x) << S_FW_HELLO_CMD_MASTERDIS) > /usr/src/sys/modules/cxgbe/if_cxgbe/../../../dev/cxgbe/firmware/t4fw_interface.h:#define > F_FW_HELLO_CMD_MASTERDIS V_FW_HELLO_CMD_MASTERDIS(1U) > > S_FW_HELLO_CMD_MASTERDIS is 29: > > # grep S_FW_HELLO_CMD_MASTERDIS > /usr/src/sys/modules/cxgbe/if_cxgbe/../../../dev/cxgbe/*/* > /usr/src/sys/modules/cxgbe/if_cxgbe/../../../dev/cxgbe/firmware/t4fw_interface.h:#define > S_FW_HELLO_CMD_MASTERDIS 29 > /usr/src/sys/modules/cxgbe/if_cxgbe/../../../dev/cxgbe/firmware/t4fw_interface.h:#define > V_FW_HELLO_CMD_MASTERDIS(x) ((x) << S_FW_HELLO_CMD_MASTERDIS) > /usr/src/sys/modules/cxgbe/if_cxgbe/../../../dev/cxgbe/firmware/t4fw_interface.h: > (((x) >> S_FW_HELLO_CMD_MASTERDIS) & M_FW_HELLO_CMD_MASTERDIS) > > S_FW_HELLO_CMD_MASTERDIS being 29 means that V_FW_HELLO_CMD_MASTERDIS(x)'s > > ((x) << S_FW_HELLO_CMD_MASTERDIS) > > is far more than 16 bits wide. (I'll not list the other example bits.) > > For: > > ./x86/endian.h:75:53: note: in definition of macro '__bswap16' > __bswap16_gen((__uint16_t)(x)) : __bswap16_var(x))) > ^ > > the __bswap16_var is (from x86/endian.h ): > > static __inline __uint16_t > __bswap16_var(__uint16_t _x) > { > > return (__bswap16_gen(_x)); > } > > and so there is an implicit truncation to 16 bits before __bswap16_gen(_x) and > its & 0xffff masking is involved. And that is what the compiler is complaining > about: the implicit status of the truncation. > > The truncated value appears to be fine for the code's purpose. Try this old patch. The extra casts might fix this. The removal of masks is a style fix that perhaps depends on the casts to work. IIRC, without the casts, bswap16(x) and bswap32(x) can return extra bits in some macro cases, so they are incompatible with the KPI. In the inline function cases, function semantics of course prevent the extra bits from being returned, but the code might be a bit sloppy in depending on this. X Index: endian.h X =================================================================== X --- endian.h (revision 302972) X +++ endian.h (working copy) X @@ -65,20 +65,20 @@ X X #define __bswap16_gen(x) (__uint16_t)((x) << 8 | (x) >> 8) X #define __bswap32_gen(x) \ X - (((__uint32_t)__bswap16((x) & 0xffff) << 16) | __bswap16((x) >> 16)) X + (((__uint32_t)__bswap16(x) << 16) | __bswap16((x) >> 16)) X #define __bswap64_gen(x) \ X - (((__uint64_t)__bswap32((x) & 0xffffffff) << 32) | __bswap32((x) >> 32)) X + (((__uint64_t)__bswap32(x) << 32) | __bswap32((x) >> 32)) X X #ifdef __GNUCLIKE_BUILTIN_CONSTANT_P X #define __bswap16(x) \ X ((__uint16_t)(__builtin_constant_p(x) ? \ X - __bswap16_gen((__uint16_t)(x)) : __bswap16_var(x))) X + __bswap16_gen((__uint16_t)(x)) : __bswap16_var((__uint16_t)(x)))) X #define __bswap32(x) \ X (__builtin_constant_p(x) ? \ X - __bswap32_gen((__uint32_t)(x)) : __bswap32_var(x)) X + __bswap32_gen((__uint32_t)(x)) : __bswap32_var((__uint32_t)(x))) X #define __bswap64(x) \ X (__builtin_constant_p(x) ? \ X - __bswap64_gen((__uint64_t)(x)) : __bswap64_var(x)) X + __bswap64_gen((__uint64_t)(x)) : __bswap64_var((__uint64_t)(x))) X #else X /* XXX these are broken for use in static initializers. */ X #define __bswap16(x) __bswap16_var(x) Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160723113358.C1884>