[RFC PATCH 1/8] qemu/int128: avoid undefined behavior in int128_lshift

matheus.ferst@eldorado.org.br posted 8 patches 3 years, 10 months ago
Maintainers: Aurelien Jarno <aurelien@aurel32.net>, Peter Maydell <peter.maydell@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>, "Cédric Le Goater" <clg@kaod.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>
[RFC PATCH 1/8] qemu/int128: avoid undefined behavior in int128_lshift
Posted by matheus.ferst@eldorado.org.br 3 years, 10 months ago
From: Matheus Ferst <matheus.ferst@eldorado.org.br>

Avoid the left shift of negative values in int128_lshift by casting
a/a.hi to unsigned.

Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 include/qemu/int128.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index 2c4064256c..2a19558ac6 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -85,7 +85,7 @@ static inline Int128 int128_rshift(Int128 a, int n)
 
 static inline Int128 int128_lshift(Int128 a, int n)
 {
-    return a << n;
+    return (__uint128_t)a << n;
 }
 
 static inline Int128 int128_add(Int128 a, Int128 b)
@@ -305,7 +305,7 @@ static inline Int128 int128_lshift(Int128 a, int n)
     if (n >= 64) {
         return int128_make128(0, l);
     } else if (n > 0) {
-        return int128_make128(l, (a.hi << n) | (a.lo >> (64 - n)));
+        return int128_make128(l, ((uint64_t)a.hi << n) | (a.lo >> (64 - n)));
     }
     return a;
 }
-- 
2.25.1
Re: [RFC PATCH 1/8] qemu/int128: avoid undefined behavior in int128_lshift
Posted by Richard Henderson 3 years, 10 months ago
On 3/30/22 11:59, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
> 
> Avoid the left shift of negative values in int128_lshift by casting
> a/a.hi to unsigned.
> 
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>

Eh, maybe.  We do this all over qemu, and I think any undefinedness you're thinking of in 
the base C standard is removed by the -fwrapv with which all files are compiled.


r~

> ---
>   include/qemu/int128.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/int128.h b/include/qemu/int128.h
> index 2c4064256c..2a19558ac6 100644
> --- a/include/qemu/int128.h
> +++ b/include/qemu/int128.h
> @@ -85,7 +85,7 @@ static inline Int128 int128_rshift(Int128 a, int n)
>   
>   static inline Int128 int128_lshift(Int128 a, int n)
>   {
> -    return a << n;
> +    return (__uint128_t)a << n;
>   }
>   
>   static inline Int128 int128_add(Int128 a, Int128 b)
> @@ -305,7 +305,7 @@ static inline Int128 int128_lshift(Int128 a, int n)
>       if (n >= 64) {
>           return int128_make128(0, l);
>       } else if (n > 0) {
> -        return int128_make128(l, (a.hi << n) | (a.lo >> (64 - n)));
> +        return int128_make128(l, ((uint64_t)a.hi << n) | (a.lo >> (64 - n)));
>       }
>       return a;
>   }