[PATCH] tests/tcg/s390x: Test overflow conditions

[PATCH] tests/tcg/s390x: Test overflow conditions

[Gautam Agrawal]

Add a test to check for overflow conditions in s390x.
This patch is based on the following patches :
* https://git.qemu.org/?p=qemu.git;a=commitdiff;h=5a2e67a691501
* https://git.qemu.org/?p=qemu.git;a=commitdiff;h=fc6e0d0f2db51
 
Signed-off-by: Gautam Agrawal <gautamnagrawal@gmail.com>
---
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/overflow.c      | 58 +++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)
 create mode 100644 tests/tcg/s390x/overflow.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 3124172736..7f86de85b9 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -16,6 +16,7 @@ TESTS+=shift
 TESTS+=trap
 TESTS+=signals-s390x
 TESTS+=branch-relative-long
+TESTS+=overflow
 
 VECTOR_TESTS=vxeh2_vs
 VECTOR_TESTS+=vxeh2_vcvt
diff --git a/tests/tcg/s390x/overflow.c b/tests/tcg/s390x/overflow.c
new file mode 100644
index 0000000000..ea8a410b1a
--- /dev/null
+++ b/tests/tcg/s390x/overflow.c
@@ -0,0 +1,58 @@
+#include <stdio.h>
+
+int overflow_add_32(int x, int y)
+{
+    int sum;
+    return __builtin_add_overflow(x, y, &sum);
+}
+
+int overflow_add_64(long long x, long long y)
+{
+    long sum;
+    return __builtin_add_overflow(x, y, &sum);
+}
+
+int overflow_sub_32(int x, int y)
+{
+    int sum;
+    return __builtin_sub_overflow(x, y, &sum);
+}
+
+int overflow_sub_64(long long x, long long y)
+{
+    long sum;
+    return __builtin_sub_overflow(x, y, &sum);
+}
+
+int a1_add = -2147483648;
+int b1_add = -2147483648;
+long long a2_add = -9223372036854775808ULL;
+long long b2_add = -9223372036854775808ULL;
+
+int a1_sub;
+int b1_sub = -2147483648;
+long long a2_sub = 0L;
+long long b2_sub = -9223372036854775808ULL;
+
+int main()
+{
+    int ret = 0;
+
+    if (!overflow_add_32(a1_add, b1_add)) {
+        fprintf(stderr, "data overflow while adding 32 bits\n");
+        ret = 1;
+    }
+    if (!overflow_add_64(a2_add, b2_add)) {
+        fprintf(stderr, "data overflow while adding 64 bits\n");
+        ret = 1;
+    }
+    if (!overflow_sub_32(a1_sub, b1_sub)) {
+        fprintf(stderr, "data overflow while subtracting 32 bits\n");
+        ret = 1;
+    }
+    if (!overflow_sub_64(a2_sub, b2_sub)) {
+        fprintf(stderr, "data overflow while subtracting 64 bits\n");
+        ret = 1;
+    }
+    return ret;
+}
-- 
2.34.1

Re: [PATCH] tests/tcg/s390x: Test overflow conditions

[Alex Bennée]

Gautam Agrawal <gautamnagrawal@gmail.com> writes:

> Add a test to check for overflow conditions in s390x.
> This patch is based on the following patches :
> * https://git.qemu.org/?p=qemu.git;a=commitdiff;h=5a2e67a691501
> * https://git.qemu.org/?p=qemu.git;a=commitdiff;h=fc6e0d0f2db51
>  
> Signed-off-by: Gautam Agrawal <gautamnagrawal@gmail.com>

Acked-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

Re: [PATCH] tests/tcg/s390x: Test overflow conditions

[David Hildenbrand]

On 27.05.22 12:11, Gautam Agrawal wrote:
> Add a test to check for overflow conditions in s390x.
> This patch is based on the following patches :
> * https://git.qemu.org/?p=qemu.git;a=commitdiff;h=5a2e67a691501
> * https://git.qemu.org/?p=qemu.git;a=commitdiff;h=fc6e0d0f2db51
>  
> Signed-off-by: Gautam Agrawal <gautamnagrawal@gmail.com>
> ---
>  tests/tcg/s390x/Makefile.target |  1 +
>  tests/tcg/s390x/overflow.c      | 58 +++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
>  create mode 100644 tests/tcg/s390x/overflow.c
> 
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 3124172736..7f86de85b9 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -16,6 +16,7 @@ TESTS+=shift
>  TESTS+=trap
>  TESTS+=signals-s390x
>  TESTS+=branch-relative-long
> +TESTS+=overflow
>  
>  VECTOR_TESTS=vxeh2_vs
>  VECTOR_TESTS+=vxeh2_vcvt
> diff --git a/tests/tcg/s390x/overflow.c b/tests/tcg/s390x/overflow.c
> new file mode 100644
> index 0000000000..ea8a410b1a
> --- /dev/null
> +++ b/tests/tcg/s390x/overflow.c
> @@ -0,0 +1,58 @@
> +#include <stdio.h>
> +
> +int overflow_add_32(int x, int y)
> +{
> +    int sum;
> +    return __builtin_add_overflow(x, y, &sum);
> +}
> +
> +int overflow_add_64(long long x, long long y)
> +{
> +    long sum;

Just wondering, why "long long" in input and "long" in output?

> +    return __builtin_add_overflow(x, y, &sum);
> +}
> +
> +int overflow_sub_32(int x, int y)
> +{
> +    int sum;
> +    return __builtin_sub_overflow(x, y, &sum);
> +}
> +
> +int overflow_sub_64(long long x, long long y)
> +{
> +    long sum;
> +    return __builtin_sub_overflow(x, y, &sum);

nit: I'd call all local variables "ret" or "res".


Apart from that LGTM.

-- 
Thanks,

David / dhildenb

Re: [PATCH] tests/tcg/s390x: Test overflow conditions

[Thomas Huth]

  Hi!

On 30/05/2022 11.50, David Hildenbrand wrote:
> On 27.05.22 12:11, Gautam Agrawal wrote:
>> Add a test to check for overflow conditions in s390x.
>> This patch is based on the following patches :
>> * https://git.qemu.org/?p=qemu.git;a=commitdiff;h=5a2e67a691501
>> * https://git.qemu.org/?p=qemu.git;a=commitdiff;h=fc6e0d0f2db51
>>   
>> Signed-off-by: Gautam Agrawal <gautamnagrawal@gmail.com>
>> ---
>>   tests/tcg/s390x/Makefile.target |  1 +
>>   tests/tcg/s390x/overflow.c      | 58 +++++++++++++++++++++++++++++++++
>>   2 files changed, 59 insertions(+)
>>   create mode 100644 tests/tcg/s390x/overflow.c
>>
>> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
>> index 3124172736..7f86de85b9 100644
>> --- a/tests/tcg/s390x/Makefile.target
>> +++ b/tests/tcg/s390x/Makefile.target
>> @@ -16,6 +16,7 @@ TESTS+=shift
>>   TESTS+=trap
>>   TESTS+=signals-s390x
>>   TESTS+=branch-relative-long
>> +TESTS+=overflow
>>   
>>   VECTOR_TESTS=vxeh2_vs
>>   VECTOR_TESTS+=vxeh2_vcvt
>> diff --git a/tests/tcg/s390x/overflow.c b/tests/tcg/s390x/overflow.c
>> new file mode 100644
>> index 0000000000..ea8a410b1a
>> --- /dev/null
>> +++ b/tests/tcg/s390x/overflow.c
>> @@ -0,0 +1,58 @@
>> +#include <stdio.h>
>> +
>> +int overflow_add_32(int x, int y)
>> +{
>> +    int sum;
>> +    return __builtin_add_overflow(x, y, &sum);
>> +}
>> +
>> +int overflow_add_64(long long x, long long y)
>> +{
>> +    long sum;
> 
> Just wondering, why "long long" in input and "long" in output?

It's been like this in the original test program that has been supplied in 
https://gitlab.com/qemu-project/qemu/-/issues/616 and .../618 - but I agree 
it likely makes more sense to use the same type everywhere (i.e. switch sum 
from long to long long).

>> +    return __builtin_add_overflow(x, y, &sum);
>> +}
>> +
>> +int overflow_sub_32(int x, int y)
>> +{
>> +    int sum;
>> +    return __builtin_sub_overflow(x, y, &sum);
>> +}
>> +
>> +int overflow_sub_64(long long x, long long y)
>> +{
>> +    long sum;
>> +    return __builtin_sub_overflow(x, y, &sum);
> 
> nit: I'd call all local variables "ret" or "res".

Well, "sum" is not the return value here, so "ret" could be confusing, too. 
"res" or "diff" might be a good choice here, though. Gautam, what do you think?

  Thomas

Re: [PATCH] tests/tcg/s390x: Test overflow conditions

[Gautam Agrawal]

Hi,
On Mon, 30 May 2022 at 16:05, Thomas Huth <thuth@redhat.com> wrote:
>
>   Hi!
>
> On 30/05/2022 11.50, David Hildenbrand wrote:
> > On 27.05.22 12:11, Gautam Agrawal wrote:
> >> Add a test to check for overflow conditions in s390x.
> >> This patch is based on the following patches :
> >> * https://git.qemu.org/?p=qemu.git;a=commitdiff;h=5a2e67a691501
> >> * https://git.qemu.org/?p=qemu.git;a=commitdiff;h=fc6e0d0f2db51
> >>
> >> Signed-off-by: Gautam Agrawal <gautamnagrawal@gmail.com>
> >> ---
> >>   tests/tcg/s390x/Makefile.target |  1 +
> >>   tests/tcg/s390x/overflow.c      | 58 +++++++++++++++++++++++++++++++++
> >>   2 files changed, 59 insertions(+)
> >>   create mode 100644 tests/tcg/s390x/overflow.c
> >>
> >> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> >> index 3124172736..7f86de85b9 100644
> >> --- a/tests/tcg/s390x/Makefile.target
> >> +++ b/tests/tcg/s390x/Makefile.target
> >> @@ -16,6 +16,7 @@ TESTS+=shift
> >>   TESTS+=trap
> >>   TESTS+=signals-s390x
> >>   TESTS+=branch-relative-long
> >> +TESTS+=overflow
> >>
> >>   VECTOR_TESTS=vxeh2_vs
> >>   VECTOR_TESTS+=vxeh2_vcvt
> >> diff --git a/tests/tcg/s390x/overflow.c b/tests/tcg/s390x/overflow.c
> >> new file mode 100644
> >> index 0000000000..ea8a410b1a
> >> --- /dev/null
> >> +++ b/tests/tcg/s390x/overflow.c
> >> @@ -0,0 +1,58 @@
> >> +#include <stdio.h>
> >> +
> >> +int overflow_add_32(int x, int y)
> >> +{
> >> +    int sum;
> >> +    return __builtin_add_overflow(x, y, &sum);
> >> +}
> >> +
> >> +int overflow_add_64(long long x, long long y)
> >> +{
> >> +    long sum;
> >
> > Just wondering, why "long long" in input and "long" in output?

> It's been like this in the original test program that has been supplied in
> https://gitlab.com/qemu-project/qemu/-/issues/616 and .../618 - but I agree
> it likely makes more sense to use the same type everywhere (i.e. switch sum
> from long to long long).

I will correct the type in next patch.

>
> >> +    return __builtin_add_overflow(x, y, &sum);
> >> +}
> >> +
> >> +int overflow_sub_32(int x, int y)
> >> +{
> >> +    int sum;
> >> +    return __builtin_sub_overflow(x, y, &sum);
> >> +}
> >> +
> >> +int overflow_sub_64(long long x, long long y)
> >> +{
> >> +    long sum;
> >> +    return __builtin_sub_overflow(x, y, &sum);
> >
> > nit: I'd call all local variables "ret" or "res".
>
> Well, "sum" is not the return value here, so "ret" could be confusing, too.
> "res" or "diff" might be a good choice here, though. Gautam, what do you think?

I agree "res" sounds better.

Regards,
Gautam Agrawal