* [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow
@ 2014-03-21 1:25 Olivier Danet
2014-03-21 7:07 ` Mark Cave-Ayland
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Olivier Danet @ 2014-03-21 1:25 UTC (permalink / raw)
To: qemu-devel, Richard Henderson, Mark Cave-Ayland, Blue Swirl
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 <odanet@caramail.com>
---
target-sparc/helper.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index f3c7fbf..ae7740b 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -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;
overflow = 1;
+ } else {
+ x0 = x0 / x1;
+ if ((int32_t) x0 != x0) {
+ x0 = x0 < 0 ? INT32_MIN : INT32_MAX;
+ overflow = 1;
+ }
}
if (cc) {
--
1.8.1.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow
2014-03-21 1:25 [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow Olivier Danet
@ 2014-03-21 7:07 ` Mark Cave-Ayland
2014-03-21 15:23 ` Richard Henderson
2014-03-21 15:23 ` Richard Henderson
2014-03-24 21:07 ` Mark Cave-Ayland
2 siblings, 1 reply; 9+ messages in thread
From: Mark Cave-Ayland @ 2014-03-21 7:07 UTC (permalink / raw)
To: Olivier Danet; +Cc: Blue Swirl, qemu-devel, Richard Henderson
On 21/03/14 01:25, Olivier Danet wrote:
> 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<odanet@caramail.com>
> ---
> target-sparc/helper.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/target-sparc/helper.c b/target-sparc/helper.c
> index f3c7fbf..ae7740b 100644
> --- a/target-sparc/helper.c
> +++ b/target-sparc/helper.c
> @@ -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;
> overflow = 1;
> + } else {
> + x0 = x0 / x1;
> + if ((int32_t) x0 != x0) {
> + x0 = x0< 0 ? INT32_MIN : INT32_MAX;
> + overflow = 1;
> + }
> }
>
> if (cc) {
Hi Olivier,
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.
Having said that, I will definitely give it a test over the next couple
of days when I get a moment.
ATB,
Mark.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow
2014-03-21 1:25 [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow Olivier Danet
2014-03-21 7:07 ` Mark Cave-Ayland
@ 2014-03-21 15:23 ` Richard Henderson
2014-03-24 21:07 ` Mark Cave-Ayland
2 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2014-03-21 15:23 UTC (permalink / raw)
To: Olivier Danet, qemu-devel, Mark Cave-Ayland, Blue Swirl
On 03/20/2014 06:25 PM, Olivier Danet wrote:
> 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 <odanet@caramail.com>
> ---
> target-sparc/helper.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow
2014-03-21 7:07 ` Mark Cave-Ayland
@ 2014-03-21 15:23 ` Richard Henderson
0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2014-03-21 15:23 UTC (permalink / raw)
To: Mark Cave-Ayland, Olivier Danet; +Cc: Blue Swirl, qemu-devel
On 03/21/2014 12:07 AM, Mark Cave-Ayland wrote:
>
> 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.
I think one patch is fine.
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow
2014-03-21 1:25 [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow Olivier Danet
2014-03-21 7:07 ` Mark Cave-Ayland
2014-03-21 15:23 ` Richard Henderson
@ 2014-03-24 21:07 ` Mark Cave-Ayland
2014-03-24 21:36 ` Peter Maydell
2 siblings, 1 reply; 9+ messages in thread
From: Mark Cave-Ayland @ 2014-03-24 21:07 UTC (permalink / raw)
To: Olivier Danet; +Cc: Blue Swirl, Peter Maydell, qemu-devel, Richard Henderson
On 21/03/14 01:25, Olivier Danet wrote:
> 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<odanet@caramail.com>
> ---
> target-sparc/helper.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/target-sparc/helper.c b/target-sparc/helper.c
> index f3c7fbf..ae7740b 100644
> --- a/target-sparc/helper.c
> +++ b/target-sparc/helper.c
> @@ -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;
> overflow = 1;
> + } else {
> + x0 = x0 / x1;
> + if ((int32_t) x0 != x0) {
> + x0 = x0< 0 ? INT32_MIN : INT32_MAX;
> + overflow = 1;
> + }
> }
>
> if (cc) {
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 <mark.cave-ayland@ilande.co.uk>
Peter - given that this prevents a guest from crashing the QEMU host, is
it a candidate for 2.0?
ATB,
Mark.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow
2014-03-24 21:07 ` Mark Cave-Ayland
@ 2014-03-24 21:36 ` Peter Maydell
2014-03-24 22:43 ` Andreas Färber
0 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2014-03-24 21:36 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: Blue Swirl, Richard Henderson, Olivier Danet, qemu-devel
On 24 March 2014 21:07, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote:
> 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 <mark.cave-ayland@ilande.co.uk>
>
> 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.
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow
2014-03-24 21:36 ` Peter Maydell
@ 2014-03-24 22:43 ` Andreas Färber
2014-03-26 15:03 ` Mark Cave-Ayland
0 siblings, 1 reply; 9+ messages in thread
From: Andreas Färber @ 2014-03-24 22:43 UTC (permalink / raw)
To: Mark Cave-Ayland
Cc: Blue Swirl, Peter Maydell, qemu-devel, Olivier Danet, Richard Henderson
Am 24.03.2014 22:36, schrieb Peter Maydell:
> On 24 March 2014 21:07, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote:
>> 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 <mark.cave-ayland@ilande.co.uk>
>>
>> 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. :)
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow
2014-03-24 22:43 ` Andreas Färber
@ 2014-03-26 15:03 ` Mark Cave-Ayland
2014-03-26 15:20 ` Andreas Färber
0 siblings, 1 reply; 9+ messages in thread
From: Mark Cave-Ayland @ 2014-03-26 15:03 UTC (permalink / raw)
To: Andreas Färber
Cc: Blue Swirl, Peter Maydell, qemu-devel, Olivier Danet, Richard Henderson
On 24/03/14 22:43, Andreas Färber wrote:
> Am 24.03.2014 22:36, schrieb Peter Maydell:
>> On 24 March 2014 21:07, Mark Cave-Ayland<mark.cave-ayland@ilande.co.uk> wrote:
>>> 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<mark.cave-ayland@ilande.co.uk>
>>>
>>> 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. :)
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".
ATB,
Mark.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow
2014-03-26 15:03 ` Mark Cave-Ayland
@ 2014-03-26 15:20 ` Andreas Färber
0 siblings, 0 replies; 9+ messages in thread
From: Andreas Färber @ 2014-03-26 15:20 UTC (permalink / raw)
To: Mark Cave-Ayland
Cc: Blue Swirl, Peter Maydell, qemu-devel, Olivier Danet, Richard Henderson
Hi Mark,
Am 26.03.2014 16:03, schrieb Mark Cave-Ayland:
> On 24/03/14 22:43, Andreas Färber wrote:
>> Am 24.03.2014 22:36, schrieb Peter Maydell:
>>>> 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. :)
>
> 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".
Both, and the space after the topic, i.e. s/sparc :/target-sparc:/.
Cheers,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-03-26 15:21 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-21 1:25 [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow Olivier Danet
2014-03-21 7:07 ` Mark Cave-Ayland
2014-03-21 15:23 ` Richard Henderson
2014-03-21 15:23 ` Richard Henderson
2014-03-24 21:07 ` Mark Cave-Ayland
2014-03-24 21:36 ` Peter Maydell
2014-03-24 22:43 ` Andreas Färber
2014-03-26 15:03 ` Mark Cave-Ayland
2014-03-26 15:20 ` Andreas Färber
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.