| Message ID | 1447687010-4471-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
On 17 November 2015 at 09:45, Paolo Bonzini <pbonzini@redhat.com> wrote: > Again: let's stop this madness!!!!!!!!!!!!!!!!!!!!!! > > (Yes, so many exclamation marks). > > This is clearly computing -32768 * 2^N, not -(32768 * 2^N). The latter > is totally, utterly wrong, because 32768 is _not even expressible_ as a > 16-bit fixed point value, which OPL_MINOUT/OPL_MAXOUT obviously are. > > I'll shortly post a patch to disable this obnoxious warning. This is undefined behaviour by the language spec. If clang is warning about it they obviously don't want to guarantee that they aren't ever going to rely on this UB for optimisation. I agree that it's a silly thing to have be UB, but that's what C is. I don't think we should be silencing this warning (or the runtime ubsan one) and I do think we should be fixing our UBs. I agree that the rephrasing isn't great; if you have a preferred non-UB way to write it I'm open to suggestions. thanks -- PMM
diff --git a/hw/audio/fmopl.c b/hw/audio/fmopl.c index 81c0c1b..807b29c 100644 --- a/hw/audio/fmopl.c +++ b/hw/audio/fmopl.c @@ -72,7 +72,7 @@ static int opl_dbg_maxchip,opl_dbg_chip; /* final output shift , limit minimum and maximum */ #define OPL_OUTSB (TL_BITS+3-16) /* OPL output final shift 16bit */ #define OPL_MAXOUT (0x7fff<<OPL_OUTSB) -#define OPL_MINOUT (-0x8000<<OPL_OUTSB) +#define OPL_MINOUT (-(0x8000<<OPL_OUTSB)) /* -------------------- quality selection --------------------- */
Newer versions of clang warn: hw/audio/fmopl.c:1085:39: warning: shifting a negative signed value is undefined [-Wshift-negative-value] data = Limit( outd[0] , OPL_MAXOUT, OPL_MINOUT ); ^~~~~~~~~~ hw/audio/fmopl.c:75:28: note: expanded from macro 'OPL_MINOUT' #define OPL_MINOUT (-0x8000<<OPL_OUTSB) ~~~~~~~^ Rephrase the definition of OPL_MINOUT to avoid the undefined behaviour. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/audio/fmopl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.9.1