[libc][printf] Fix out-of-range shift in float320 printf #144542
Conversation
If you enable `LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_FLOAT320` and use a
`%f` style printf format directive to print a nonzero number too small
to show up in the output digits, e.g. `printf("%.2f", 0.001)`, then
the output would be intermittently incorrect, because
`DyadicFloat::as_mantissa_type_rounded` would try to shift the 320-bit
mantissa right by more than 320 bits, invoking the 'undefined
behavior' clause commented in the `shift()` function in `big_int.h`.
There were already tests in the libc test suite exercising this case,
e.g. the subnormal tests in `LlvmLibcSPrintfTest.FloatDecimalConv` use
`%f` at the default precision of 6 decimal places on tiny numbers such
as 2^-1027. But because the behavior is undefined, they don't visibly
fail all the time, and in all previous test runs we'd tried with
USE_FLOAT320, they had got lucky.
The fix is simply to detect an out-of-range right shift before doing
it, and instead just set the output value to zero.
|
@llvm/pr-subscribers-libc Author: Simon Tatham (statham-arm) ChangesIf you enable There were already tests in the libc test suite exercising this case, e.g. the subnormal tests in The fix is simply to detect an out-of-range right shift before doing it, and instead just set the output value to zero. Full diff: https://github.com/llvm/llvm-project/pull/144542.diff 1 Files Affected:
diff --git a/libc/src/__support/FPUtil/dyadic_float.h b/libc/src/__support/FPUtil/dyadic_float.h
index 6c3e1520e5aff..4c77d3c541cdf 100644
--- a/libc/src/__support/FPUtil/dyadic_float.h
+++ b/libc/src/__support/FPUtil/dyadic_float.h
@@ -465,7 +465,10 @@ template <size_t Bits> struct DyadicFloat {
// exponents coming in to this function _shouldn't_ be that large). The
// result should always end up as a positive size_t.
size_t shift = -static_cast<size_t>(exponent);
- new_mant >>= shift;
+ if (shift >= Bits)
+ new_mant = 0;
+ else
+ new_mant >>= shift;
round_dir = rounding_direction(mantissa, shift, sign);
if (round_dir > 0)
++new_mant;
|
If you enable `LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_FLOAT320` and use a
`%f` style printf format directive to print a nonzero number too small
to show up in the output digits, e.g. `printf("%.2f", 0.001)`, then the
output would be intermittently incorrect, because
`DyadicFloat::as_mantissa_type_rounded` would try to shift the 320-bit
mantissa right by more than 320 bits, invoking the 'undefined behavior'
clause commented in the `shift()` function in `big_int.h`.
There were already tests in the libc test suite exercising this case,
e.g. the subnormal tests in `LlvmLibcSPrintfTest.FloatDecimalConv` use
`%f` at the default precision of 6 decimal places on tiny numbers such
as 2^-1027. But because the behavior is undefined, they don't visibly
fail all the time, and in all previous test runs we'd tried with
USE_FLOAT320, they had got lucky.
The fix is simply to detect an out-of-range right shift before doing it,
and instead just set the output value to zero.
If you enable
LIBC_CONF_PRINTF_FLOAT_TO_STR_USE_FLOAT320and use a%fstyle printf format directive to print a nonzero number too small to show up in the output digits, e.g.printf("%.2f", 0.001), then the output would be intermittently incorrect, becauseDyadicFloat::as_mantissa_type_roundedwould try to shift the 320-bit mantissa right by more than 320 bits, invoking the 'undefined behavior' clause commented in theshift()function inbig_int.h.There were already tests in the libc test suite exercising this case, e.g. the subnormal tests in
LlvmLibcSPrintfTest.FloatDecimalConvuse%fat the default precision of 6 decimal places on tiny numbers such as 2^-1027. But because the behavior is undefined, they don't visibly fail all the time, and in all previous test runs we'd tried with USE_FLOAT320, they had got lucky.The fix is simply to detect an out-of-range right shift before doing it, and instead just set the output value to zero.