[PATCH] target/alpha: Don't corrupt error_code with unknown softfloat flags

[PATCH] target/alpha: Don't corrupt error_code with unknown softfloat flags

[Peter Maydell]

In do_cvttq() we set env->error_code with what is supposed to be a
set of FPCR exception bit values.  However, if the set of float
exception flags we get back from softfloat for the conversion
includes a flag which is not one of the three we expect here
(invalid_cvti, invalid, inexact) then we will fall through the
if-ladder and set env->error_code to the unconverted softfloat
exception_flag value.  This will then cause us to take a spurious
exception.

This is harmless now, but when we add new floating point exception
flags to softfloat it will cause problems.  Add an else clause to the
if-ladder to make it ignore any float exception flags it doesn't care
about.

Specifically, without this fix, 'make check-tcg' will fail for Alpha
when the commit adding float_flag_input_denormal_used lands.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I guess I forgot to run a 'make check-tcg' for all targets until
I was getting around to queuing the FEAT_AFP series :-/

 target/alpha/fpu_helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/alpha/fpu_helper.c b/target/alpha/fpu_helper.c
index 63d9e9ce39c..f810a9b6a47 100644
--- a/target/alpha/fpu_helper.c
+++ b/target/alpha/fpu_helper.c
@@ -476,6 +476,8 @@ static uint64_t do_cvttq(CPUAlphaState *env, uint64_t a, int roundmode)
             exc = FPCR_INV;
         } else if (exc & float_flag_inexact) {
             exc = FPCR_INE;
+        } else {
+            exc = 0;
         }
     }
     env->error_code = exc;
-- 
2.34.1

Re: [PATCH] target/alpha: Don't corrupt error_code with unknown softfloat flags

[Philippe Mathieu-Daudé]

On 11/2/25 14:06, Peter Maydell wrote:
> In do_cvttq() we set env->error_code with what is supposed to be a
> set of FPCR exception bit values.  However, if the set of float
> exception flags we get back from softfloat for the conversion
> includes a flag which is not one of the three we expect here
> (invalid_cvti, invalid, inexact) then we will fall through the
> if-ladder and set env->error_code to the unconverted softfloat
> exception_flag value.  This will then cause us to take a spurious
> exception.
> 
> This is harmless now, but when we add new floating point exception
> flags to softfloat it will cause problems.  Add an else clause to the
> if-ladder to make it ignore any float exception flags it doesn't care
> about.
> 
> Specifically, without this fix, 'make check-tcg' will fail for Alpha
> when the commit adding float_flag_input_denormal_used lands.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I guess I forgot to run a 'make check-tcg' for all targets until
> I was getting around to queuing the FEAT_AFP series :-/
> 
>   target/alpha/fpu_helper.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/alpha/fpu_helper.c b/target/alpha/fpu_helper.c
> index 63d9e9ce39c..f810a9b6a47 100644
> --- a/target/alpha/fpu_helper.c
> +++ b/target/alpha/fpu_helper.c
> @@ -476,6 +476,8 @@ static uint64_t do_cvttq(CPUAlphaState *env, uint64_t a, int roundmode)
>               exc = FPCR_INV;
>           } else if (exc & float_flag_inexact) {
>               exc = FPCR_INE;
> +        } else {
> +            exc = 0;
>           }
>       }
>       env->error_code = exc;

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

But (pre-existing) it would be clearer to not use the same variable
for 2 distinct things. As a preliminary cleanup:

-- >8 --
diff --git a/target/alpha/fpu_helper.c b/target/alpha/fpu_helper.c
index 63d9e9ce39c..d1a17b71423 100644
--- a/target/alpha/fpu_helper.c
+++ b/target/alpha/fpu_helper.c
@@ -455,26 +455,27 @@ static uint64_t do_cvttq(CPUAlphaState *env, 
uint64_t a, int roundmode)
  {
      float64 fa;
      int64_t ret;
-    uint32_t exc;
+    uint32_t exc = 0;
+    int flags;

      fa = t_to_float64(a);
      ret = float64_to_int64_modulo(fa, roundmode, &FP_STATUS);

-    exc = get_float_exception_flags(&FP_STATUS);
-    if (unlikely(exc)) {
+    flags = get_float_exception_flags(&FP_STATUS);
+    if (unlikely(flags)) {
          set_float_exception_flags(0, &FP_STATUS);

          /* We need to massage the resulting exceptions. */
-        if (exc & float_flag_invalid_cvti) {
+        if (flags & float_flag_invalid_cvti) {
              /* Overflow, either normal or infinity. */
              if (float64_is_infinity(fa)) {
                  exc = FPCR_INV;
              } else {
                  exc = FPCR_IOV | FPCR_INE;
              }
-        } else if (exc & float_flag_invalid) {
+        } else if (flags & float_flag_invalid) {
              exc = FPCR_INV;
-        } else if (exc & float_flag_inexact) {
+        } else if (flags & float_flag_inexact) {
              exc = FPCR_INE;
          }
      }
---

Re: [PATCH] target/alpha: Don't corrupt error_code with unknown softfloat flags

[Richard Henderson]

On 2/11/25 05:06, Peter Maydell wrote:
> In do_cvttq() we set env->error_code with what is supposed to be a
> set of FPCR exception bit values.  However, if the set of float
> exception flags we get back from softfloat for the conversion
> includes a flag which is not one of the three we expect here
> (invalid_cvti, invalid, inexact) then we will fall through the
> if-ladder and set env->error_code to the unconverted softfloat
> exception_flag value.  This will then cause us to take a spurious
> exception.
> 
> This is harmless now, but when we add new floating point exception
> flags to softfloat it will cause problems.  Add an else clause to the
> if-ladder to make it ignore any float exception flags it doesn't care
> about.
> 
> Specifically, without this fix, 'make check-tcg' will fail for Alpha
> when the commit adding float_flag_input_denormal_used lands.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I guess I forgot to run a 'make check-tcg' for all targets until
> I was getting around to queuing the FEAT_AFP series :-/
> 
>   target/alpha/fpu_helper.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/target/alpha/fpu_helper.c b/target/alpha/fpu_helper.c
> index 63d9e9ce39c..f810a9b6a47 100644
> --- a/target/alpha/fpu_helper.c
> +++ b/target/alpha/fpu_helper.c
> @@ -476,6 +476,8 @@ static uint64_t do_cvttq(CPUAlphaState *env, uint64_t a, int roundmode)
>               exc = FPCR_INV;
>           } else if (exc & float_flag_inexact) {
>               exc = FPCR_INE;
> +        } else {
> +            exc = 0;
>           }
>       }
>       env->error_code = exc;

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

And I think there's a case for
Fixes: aa3bad5b59e7 ("target/alpha: Use float64_to_int64_modulo for CVTTQ")


r~