On Fri, 27 Mar 2020 at 09:49, Alex Bennée <alex.ben...@linaro.org> wrote: > > The undefined behaviour checker pointed out that a shift of 64 would > lead to undefined behaviour. > > Signed-off-by: Alex Bennée <alex.ben...@linaro.org> > --- > fpu/softfloat.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index 301ce3b537b..444d35920dd 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -3834,9 +3834,14 @@ void normalizeFloatx80Subnormal(uint64_t aSig, int32_t > *zExpPtr, > { > int8_t shiftCount; > > - shiftCount = clz64(aSig); > - *zSigPtr = aSig<<shiftCount; > - *zExpPtr = 1 - shiftCount; > + if (aSig) { > + shiftCount = clz64(aSig); > + *zSigPtr = aSig << shiftCount; > + *zExpPtr = 1 - shiftCount; > + } else { > + *zSigPtr = 0; > + *zExpPtr = 1 - 64; > + } > }
Ah yes, I saw this one in Coverity: CID 1421991. RTH marked the Coverity issue as a false positive with the rationale "We assume an out-of-range shift count is merely IMPLEMENTATION DEFINED and not UNDEFINED (in the Arm ARM sense), and so cannot turn a 0 value into a non-zero value." but I think I disagree with that. We can assume that for the TCG IR where we get to define shift semantics because we're doing the codegen, but we can't assume it in C code, because it's not included in the set of extended guarantees provided by -fwrapv as far as I know. That said, is it valid for this function to be called with a zero aSig value ? I think all these normalizeFloat*Subnormal() functions assume non-zero sig input, and the only callsite where it's not clearly obvious that this is obvious that the sig input is non-zero is the call to normalizeFloatx80Subnormal() from addFloatx80Sigs(). So perhaps we just need to check and fix that callsite ?? thanks -- PMM