| Message ID | 20180417025328.25431-3-richard.henderson@linaro.org |
|---|---|
| State | Accepted |
| Commit | ce8d4082054519f2eaac39958edde502860a7fc6 |
| Headers | show |
| Series | softfloat fixes | expand |
On 17 April 2018 at 03:53, Richard Henderson <richard.henderson@linaro.org> wrote: > Without bounding the increment, we can overflow exp either here > in scalbn_decomposed or when adding the bias in round_canonical. > This can result in e.g. underflowing to 0 instead of overflowing > to infinity. > > The old softfloat code did bound the increment. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > fpu/softfloat.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index ba6e654050..a589f328c9 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -1883,6 +1883,12 @@ static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s) > return return_nan(a, s); > } > if (a.cls == float_class_normal) { > + /* The largest float type (even though not supported by FloatParts) > + * is float128, which has a 15 bit exponent. Bounding N to 16 bits > + * still allows rounding to infinity, without allowing overflow > + * within the int32_t that backs FloatParts.exp. > + */ > + n = MIN(MAX(n, -0x10000), 0x10000); > a.exp += n; > } > return a; > -- Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
Richard Henderson <richard.henderson@linaro.org> writes: > Without bounding the increment, we can overflow exp either here > in scalbn_decomposed or when adding the bias in round_canonical. > This can result in e.g. underflowing to 0 instead of overflowing > to infinity. > > The old softfloat code did bound the increment. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > fpu/softfloat.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index ba6e654050..a589f328c9 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -1883,6 +1883,12 @@ static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s) > return return_nan(a, s); > } > if (a.cls == float_class_normal) { > + /* The largest float type (even though not supported by FloatParts) > + * is float128, which has a 15 bit exponent. Bounding N to 16 bits > + * still allows rounding to infinity, without allowing overflow > + * within the int32_t that backs FloatParts.exp. > + */ > + n = MIN(MAX(n, -0x10000), 0x10000); > a.exp += n; > } > return a; Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> (risu FWIW although it obviously didn't catch this failure ;-) -- Alex Bennée
On 17 April 2018 at 14:51, Alex Bennée <alex.bennee@linaro.org> wrote: > > Richard Henderson <richard.henderson@linaro.org> writes: > >> Without bounding the increment, we can overflow exp either here >> in scalbn_decomposed or when adding the bias in round_canonical. >> This can result in e.g. underflowing to 0 instead of overflowing >> to infinity. >> >> The old softfloat code did bound the increment. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> fpu/softfloat.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/fpu/softfloat.c b/fpu/softfloat.c >> index ba6e654050..a589f328c9 100644 >> --- a/fpu/softfloat.c >> +++ b/fpu/softfloat.c >> @@ -1883,6 +1883,12 @@ static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s) >> return return_nan(a, s); >> } >> if (a.cls == float_class_normal) { >> + /* The largest float type (even though not supported by FloatParts) >> + * is float128, which has a 15 bit exponent. Bounding N to 16 bits >> + * still allows rounding to infinity, without allowing overflow >> + * within the int32_t that backs FloatParts.exp. >> + */ >> + n = MIN(MAX(n, -0x10000), 0x10000); >> a.exp += n; >> } >> return a; > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > Tested-by: Alex Bennée <alex.bennee@linaro.org> > > (risu FWIW although it obviously didn't catch this failure ;-) Thanks; applied this patch to master. -- PMM
diff --git a/fpu/softfloat.c b/fpu/softfloat.c index ba6e654050..a589f328c9 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -1883,6 +1883,12 @@ static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s) return return_nan(a, s); } if (a.cls == float_class_normal) { + /* The largest float type (even though not supported by FloatParts) + * is float128, which has a 15 bit exponent. Bounding N to 16 bits + * still allows rounding to infinity, without allowing overflow + * within the int32_t that backs FloatParts.exp. + */ + n = MIN(MAX(n, -0x10000), 0x10000); a.exp += n; } return a;
Without bounding the increment, we can overflow exp either here in scalbn_decomposed or when adding the bias in round_canonical. This can result in e.g. underflowing to 0 instead of overflowing to infinity. The old softfloat code did bound the increment. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- fpu/softfloat.c | 6 ++++++ 1 file changed, 6 insertions(+) -- 2.14.3