sparc : 32bits integer division overflow

The signed integer division -0x8000_0000_0000_0000 / -1 must be handled separately to avoid an overflow on the QEMU host.

Negative overflow must be a negative number for correct sign extension in Sparc64 mode. Use<stdint.h> constants.

Signed-off-by Olivier Danet<[email protected]>

---
  target-sparc/helper.c | 17 ++++++++++-------
  1 file changed, 10 insertions(+), 7 deletions(-)

target-sparc/helper.c +4 -7

index f3c7fbf..ae7740b 100644
@@ -85,8 +85,8 @@ static target_ulong helper_udiv_common(CPUSPARCState *env, target_ulong a,
      }
 
      x0 = x0 / x1;
-    if (x0>  0xffffffff) {
-        x0 = 0xffffffff;
+    if (x0>  UINT32_MAX) {
+        x0 = UINT32_MAX;
          overflow = 1;
      }
 

@@ -122,12 +122,15 @@ static target_ulong helper_sdiv_common(CPUSPARCState *env, target_ulong a,
      if (x1 == 0) {
          cpu_restore_state(CPU(cpu), GETPC());
          helper_raise_exception(env, TT_DIV_ZERO);
-    }
-
-    x0 = x0 / x1;
-    if ((int32_t) x0 != x0) {
-        x0 = x0<  0 ? 0x80000000 : 0x7fffffff;
+    } else if (x1 == -1&&  x0 == INT64_MIN) {
+        x0 = INT32_MAX;

The signed integer division -0x8000_0000_0000_0000 / -1 must be handled separately to avoid an overflow on the QEMU host.

Negative overflow must be a negative number for correct sign extension in Sparc64 mode. Use <stdint.h> constants.

Signed-off-by Olivier Danet <[email protected]>

---
 target-sparc/helper.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

This basic patch looks good to me. My only comment is that I suspect for bisection purposes it may be better to split this into 2 patches - one to perform the conversion of all existing constants to INT*_MAX and INT*_MIN, and then a second to add your change to prevent the crash. I'll let Richard have the final say though.

The signed integer division -0x8000_0000_0000_0000 / -1 must be handled separately to avoid an overflow on the QEMU host.

Negative overflow must be a negative number for correct sign extension in Sparc64 mode. Use<stdint.h> constants.

Signed-off-by Olivier Danet<[email protected]>

---
  target-sparc/helper.c | 17 ++++++++++-------
  1 file changed, 10 insertions(+), 7 deletions(-)

target-sparc/helper.c +4 -7

index f3c7fbf..ae7740b 100644
@@ -85,8 +85,8 @@ static target_ulong helper_udiv_common(CPUSPARCState *env, target_ulong a,
      }
 
      x0 = x0 / x1;
-    if (x0>  0xffffffff) {
-        x0 = 0xffffffff;
+    if (x0>  UINT32_MAX) {
+        x0 = UINT32_MAX;
          overflow = 1;
      }
 

@@ -122,12 +122,15 @@ static target_ulong helper_sdiv_common(CPUSPARCState *env, target_ulong a,
      if (x1 == 0) {
          cpu_restore_state(CPU(cpu), GETPC());
          helper_raise_exception(env, TT_DIV_ZERO);
-    }
-
-    x0 = x0 / x1;
-    if ((int32_t) x0 != x0) {
-        x0 = x0<  0 ? 0x80000000 : 0x7fffffff;
+    } else if (x1 == -1&&  x0 == INT64_MIN) {
+        x0 = INT32_MAX;

This patch fixes the original bug report, and doesn't appear to have any ill-effects on my SPARC32/SPARC64 image collection boot tests so:

Tested-by Mark Cave-Ayland <[email protected]>

Peter - given that this prevents a guest from crashing the QEMU host, is it a candidate for 2.0?

Quoted 7 lines

This patch fixes the original bug report, and doesn't appear to have any ill-effects on my SPARC32/SPARC64 image collection boot tests so:

Tested-by Mark Cave-Ayland <[email protected]>

Peter - given that this prevents a guest from crashing the QEMU host, is it a candidate for 2.0?

Yes, I think so -- it's small and self-contained, it fixes a crash bug, and it's been reviewed. If you want to put it in a pull request I'll apply it.

Am 24.03.2014 22:36, schrieb Peter Maydell:

Quoted 6 lines · Mark Cave-Ayland

Quoted 5 lines

This patch fixes the original bug report, and doesn't appear to have any ill-effects on my SPARC32/SPARC64 image collection boot tests so:

Tested-by Mark Cave-Ayland<[email protected]>

Quoted 6 lines

Quoted 2 lines

Peter - given that this prevents a guest from crashing the QEMU host, is it a candidate for 2.0?

Yes, I think so -- it's small and self-contained, it fixes a crash bug, and it's been reviewed. If you want to put it in a pull request I'll apply it.

Mark, please fix up the subject then. :)

Quoted 9 lines

Am 24.03.2014 22:36, schrieb Peter Maydell:

Quoted 7 lines

Quoted 3 lines

Peter - given that this prevents a guest from crashing the QEMU host, is it
a candidate for 2.0?

Yes, I think so -- it's small and self-contained, it fixes a crash bug, and it's been reviewed. If you want to put it in a pull request I'll apply it.

Quoted 1 line

Mark, please fix up the subject then. :)

Hi Andreas,

Do you mean s/sparc/target-sparc/ ? My main observation was that I could also prefix the part after the colon with the word "fix".