The implementation of the fxtract instruction treats all nonzero
operands as normal numbers, so yielding incorrect results for invalid
formats, infinities, NaNs and subnormal and pseudo-denormal operands.
Implement appropriate handling of all those cases.
Signed-off-by: Joseph Myers <joseph@codesourcery.com>
---
target/i386/fpu_helper.c | 25 +++++-
tests/tcg/i386/test-i386-fxtract.c | 120 +++++++++++++++++++++++++++++
2 files changed, 144 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/i386/test-i386-fxtract.c
diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
index 792a128a6d..71a696a863 100644
--- a/target/i386/fpu_helper.c
+++ b/target/i386/fpu_helper.c
@@ -767,10 +767,33 @@ void helper_fxtract(CPUX86State *env)
&env->fp_status);
fpush(env);
ST0 = temp.d;
+ } else if (floatx80_invalid_encoding(ST0)) {
+ float_raise(float_flag_invalid, &env->fp_status);
+ ST0 = floatx80_default_nan(&env->fp_status);
+ fpush(env);
+ ST0 = ST1;
+ } else if (floatx80_is_any_nan(ST0)) {
+ if (floatx80_is_signaling_nan(ST0, &env->fp_status)) {
+ float_raise(float_flag_invalid, &env->fp_status);
+ ST0 = floatx80_silence_nan(ST0, &env->fp_status);
+ }
+ fpush(env);
+ ST0 = ST1;
+ } else if (floatx80_is_infinity(ST0)) {
+ fpush(env);
+ ST0 = ST1;
+ ST1 = floatx80_infinity;
} else {
int expdif;
- expdif = EXPD(temp) - EXPBIAS;
+ if (EXPD(temp) == 0) {
+ int shift = clz64(temp.l.lower);
+ temp.l.lower <<= shift;
+ expdif = 1 - EXPBIAS - shift;
+ float_raise(float_flag_input_denormal, &env->fp_status);
+ } else {
+ expdif = EXPD(temp) - EXPBIAS;
+ }
/* DP exponent bias */
ST0 = int32_to_floatx80(expdif, &env->fp_status);
fpush(env);
diff --git a/tests/tcg/i386/test-i386-fxtract.c b/tests/tcg/i386/test-i386-fxtract.c
new file mode 100644
index 0000000000..64fd93d333
--- /dev/null
+++ b/tests/tcg/i386/test-i386-fxtract.c
@@ -0,0 +1,120 @@
+/* Test fxtract instruction. */
+
+#include <stdint.h>
+#include <stdio.h>
+
+union u {
+ struct { uint64_t sig; uint16_t sign_exp; } s;
+ long double ld;
+};
+
+volatile union u ld_pseudo_m16382 = { .s = { UINT64_C(1) << 63, 0 } };
+volatile union u ld_invalid_1 = { .s = { 1, 1234 } };
+volatile union u ld_invalid_2 = { .s = { 0, 1234 } };
+volatile union u ld_invalid_3 = { .s = { 0, 0x7fff } };
+volatile union u ld_invalid_4 = { .s = { (UINT64_C(1) << 63) - 1, 0x7fff } };
+
+volatile long double ld_sig, ld_exp;
+
+int isnan_ld(long double x)
+{
+ union u tmp = { .ld = x };
+ return ((tmp.s.sign_exp & 0x7fff) == 0x7fff &&
+ (tmp.s.sig >> 63) != 0 &&
+ (tmp.s.sig << 1) != 0);
+}
+
+int issignaling_ld(long double x)
+{
+ union u tmp = { .ld = x };
+ return isnan_ld(x) && (tmp.s.sig & UINT64_C(0x4000000000000000)) == 0;
+}
+
+int main(void)
+{
+ int ret = 0;
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) : "0" (2.5L));
+ if (ld_sig != 1.25L || ld_exp != 1.0L) {
+ printf("FAIL: fxtract 2.5\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) : "0" (0.0L));
+ if (ld_sig != 0.0L || __builtin_copysignl(1.0L, ld_sig) != 1.0L ||
+ ld_exp != -__builtin_infl()) {
+ printf("FAIL: fxtract 0.0\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) : "0" (-0.0L));
+ if (ld_sig != -0.0L || __builtin_copysignl(1.0L, ld_sig) != -1.0L ||
+ ld_exp != -__builtin_infl()) {
+ printf("FAIL: fxtract -0.0\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) :
+ "0" (__builtin_infl()));
+ if (ld_sig != __builtin_infl() || ld_exp != __builtin_infl()) {
+ printf("FAIL: fxtract inf\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) :
+ "0" (-__builtin_infl()));
+ if (ld_sig != -__builtin_infl() || ld_exp != __builtin_infl()) {
+ printf("FAIL: fxtract -inf\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) :
+ "0" (__builtin_nanl("")));
+ if (!isnan_ld(ld_sig) || issignaling_ld(ld_sig) ||
+ !isnan_ld(ld_exp) || issignaling_ld(ld_exp)) {
+ printf("FAIL: fxtract qnan\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) :
+ "0" (__builtin_nansl("")));
+ if (!isnan_ld(ld_sig) || issignaling_ld(ld_sig) ||
+ !isnan_ld(ld_exp) || issignaling_ld(ld_exp)) {
+ printf("FAIL: fxtract snan\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) :
+ "0" (0x1p-16445L));
+ if (ld_sig != 1.0L || ld_exp != -16445.0L) {
+ printf("FAIL: fxtract subnormal\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) :
+ "0" (ld_pseudo_m16382.ld));
+ if (ld_sig != 1.0L || ld_exp != -16382.0L) {
+ printf("FAIL: fxtract pseudo\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) :
+ "0" (ld_invalid_1.ld));
+ if (!isnan_ld(ld_sig) || issignaling_ld(ld_sig) ||
+ !isnan_ld(ld_exp) || issignaling_ld(ld_exp)) {
+ printf("FAIL: fxtract invalid 1\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) :
+ "0" (ld_invalid_2.ld));
+ if (!isnan_ld(ld_sig) || issignaling_ld(ld_sig) ||
+ !isnan_ld(ld_exp) || issignaling_ld(ld_exp)) {
+ printf("FAIL: fxtract invalid 2\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) :
+ "0" (ld_invalid_3.ld));
+ if (!isnan_ld(ld_sig) || issignaling_ld(ld_sig) ||
+ !isnan_ld(ld_exp) || issignaling_ld(ld_exp)) {
+ printf("FAIL: fxtract invalid 3\n");
+ ret = 1;
+ }
+ __asm__ volatile ("fxtract" : "=t" (ld_sig), "=u" (ld_exp) :
+ "0" (ld_invalid_4.ld));
+ if (!isnan_ld(ld_sig) || issignaling_ld(ld_sig) ||
+ !isnan_ld(ld_exp) || issignaling_ld(ld_exp)) {
+ printf("FAIL: fxtract invalid 4\n");
+ ret = 1;
+ }
+ return ret;
+}
--
2.17.1
--
Joseph S. Myers
joseph@codesourcery.com
Joseph Myers <joseph@codesourcery.com> writes: > The implementation of the fxtract instruction treats all nonzero > operands as normal numbers, so yielding incorrect results for invalid > formats, infinities, NaNs and subnormal and pseudo-denormal operands. > Implement appropriate handling of all those cases. > > Signed-off-by: Joseph Myers <joseph@codesourcery.com> > --- > target/i386/fpu_helper.c | 25 +++++- > tests/tcg/i386/test-i386-fxtract.c | 120 +++++++++++++++++++++++++++++ > 2 files changed, 144 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/i386/test-i386-fxtract.c > For test tests: Acked-by: Alex Bennée <alex.bennee@linaro.org> -- Alex Bennée
On Thu, May 07, 2020 at 12:43:30AM +0000, Joseph Myers wrote:
> The implementation of the fxtract instruction treats all nonzero
> operands as normal numbers, so yielding incorrect results for invalid
> formats, infinities, NaNs and subnormal and pseudo-denormal operands.
> Implement appropriate handling of all those cases.
>
> Signed-off-by: Joseph Myers <joseph@codesourcery.com>
> ---
> target/i386/fpu_helper.c | 25 +++++-
> tests/tcg/i386/test-i386-fxtract.c | 120 +++++++++++++++++++++++++++++
> 2 files changed, 144 insertions(+), 1 deletion(-)
> create mode 100644 tests/tcg/i386/test-i386-fxtract.c
>
> diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
> index 792a128a6d..71a696a863 100644
> --- a/target/i386/fpu_helper.c
> +++ b/target/i386/fpu_helper.c
> @@ -767,10 +767,33 @@ void helper_fxtract(CPUX86State *env)
> &env->fp_status);
> fpush(env);
> ST0 = temp.d;
> + } else if (floatx80_invalid_encoding(ST0)) {
> + float_raise(float_flag_invalid, &env->fp_status);
> + ST0 = floatx80_default_nan(&env->fp_status);
> + fpush(env);
> + ST0 = ST1;
> + } else if (floatx80_is_any_nan(ST0)) {
> + if (floatx80_is_signaling_nan(ST0, &env->fp_status)) {
> + float_raise(float_flag_invalid, &env->fp_status);
> + ST0 = floatx80_silence_nan(ST0, &env->fp_status);
> + }
> + fpush(env);
> + ST0 = ST1;
> + } else if (floatx80_is_infinity(ST0)) {
> + fpush(env);
> + ST0 = ST1;
> + ST1 = floatx80_infinity;
> } else {
> int expdif;
>
> - expdif = EXPD(temp) - EXPBIAS;
> + if (EXPD(temp) == 0) {
> + int shift = clz64(temp.l.lower);
> + temp.l.lower <<= shift;
Coverity reports the following. It looks like a false positive
because floatx80_is_zero() would be true if both EXPD(temp) and
temp.l.lower were zero, but maybe I'm missing something.
________________________________________________________________________________________________________
*** CID 1429970: Integer handling issues (BAD_SHIFT)
/target/i386/fpu_helper.c: 922 in helper_fxtract()
916 ST1 = floatx80_infinity;
917 } else {
918 int expdif;
919
920 if (EXPD(temp) == 0) {
921 int shift = clz64(temp.l.lower);
>>> CID 1429970: Integer handling issues (BAD_SHIFT)
>>> In expression "temp.l.lower <<= shift", left shifting by more than 63 bits has undefined behavior. The shift amount, "shift", is 64.
922 temp.l.lower <<= shift;
923 expdif = 1 - EXPBIAS - shift;
924 float_raise(float_flag_input_denormal, &env->fp_status);
925 } else {
926 expdif = EXPD(temp) - EXPBIAS;
927 }
> + expdif = 1 - EXPBIAS - shift;
> + float_raise(float_flag_input_denormal, &env->fp_status);
> + } else {
> + expdif = EXPD(temp) - EXPBIAS;
> + }
> /* DP exponent bias */
> ST0 = int32_to_floatx80(expdif, &env->fp_status);
> fpush(env);
--
Eduardo
On Tue, 23 Jun 2020, Eduardo Habkost wrote:
> > + if (EXPD(temp) == 0) {
> > + int shift = clz64(temp.l.lower);
> > + temp.l.lower <<= shift;
>
> Coverity reports the following. It looks like a false positive
> because floatx80_is_zero() would be true if both EXPD(temp) and
> temp.l.lower were zero, but maybe I'm missing something.
Yes, that looks like a false positive to me.
--
Joseph S. Myers
joseph@codesourcery.com
On Tue, 23 Jun 2020 at 23:01, Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Tue, 23 Jun 2020, Eduardo Habkost wrote:
>
> > > + if (EXPD(temp) == 0) {
> > > + int shift = clz64(temp.l.lower);
> > > + temp.l.lower <<= shift;
> >
> > Coverity reports the following. It looks like a false positive
> > because floatx80_is_zero() would be true if both EXPD(temp) and
> > temp.l.lower were zero, but maybe I'm missing something.
>
> Yes, that looks like a false positive to me.
Thanks; I've marked it as a fp in the coverity UI.
-- PMM
© 2016 - 2026 Red Hat, Inc.