disas: sparc: Fix integer overflow in compare_opcodes() for loop

[PATCH] disas: sparc: Fix integer overflow in compare_opcodes() for loop

Posted by Christian Barry 2 days, 22 hours ago

From: Christian Barry <christian.barry@usp.br>

Replaced left-shift of a literal 1 by i inside of two for loops in compare_opcodes() with a call to BIT(i). This makes it so 1 is interpreted as an unsigned long int, preventing overflows.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2618

Signed-off-by: Christian Barry <christian.barry@usp.br>
Co-developed-by: Eduardo Augusto Cavalcanti <eduardoaugustoabc@ime.usp.br>
Signed-off-by: Eduardo Augusto Cavalcanti <eduardoaugustoabc@ime.usp.br>
---
 disas/sparc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/disas/sparc.c b/disas/sparc.c
index 5689533ce1..6fc057e3a2 100644
--- a/disas/sparc.c
+++ b/disas/sparc.c
@@ -27,6 +27,7 @@
    see <http://www.gnu.org/licenses/>.  */
 
 #include "qemu/osdep.h"
+#include "qemu/bitops.h"
 #include "disas/dis-asm.h"
 
 /* The SPARC opcode table (and other related data) is defined in
@@ -2515,7 +2516,7 @@ compare_opcodes (const void * a, const void * b)
      another, it is important to order the opcodes in the right order.  */
   for (i = 0; i < 32; ++i)
     {
-      unsigned long int x = 1 << i;
+      unsigned long int x = BIT(i);
       int x0 = (match0 & x) != 0;
       int x1 = (match1 & x) != 0;
 
@@ -2525,7 +2526,7 @@ compare_opcodes (const void * a, const void * b)
 
   for (i = 0; i < 32; ++i)
     {
-      unsigned long int x = 1 << i;
+      unsigned long int x = BIT(i);
       int x0 = (lose0 & x) != 0;
       int x1 = (lose1 & x) != 0;
 
-- 
2.43.0

Re: [PATCH] disas: sparc: Fix integer overflow in compare_opcodes() for loop

Posted by Peter Maydell 2 days, 7 hours ago

On Wed, 27 May 2026 at 20:56, Christian Barry <christian.barry@usp.br> wrote:
>
> From: Christian Barry <christian.barry@usp.br>
>
> Replaced left-shift of a literal 1 by i inside of two for loops in compare_opcodes() with a call to BIT(i). This makes it so 1 is interpreted as an unsigned long int, preventing overflows.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2618
>
> Signed-off-by: Christian Barry <christian.barry@usp.br>
> Co-developed-by: Eduardo Augusto Cavalcanti <eduardoaugustoabc@ime.usp.br>
> Signed-off-by: Eduardo Augusto Cavalcanti <eduardoaugustoabc@ime.usp.br>
> ---
>  disas/sparc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/disas/sparc.c b/disas/sparc.c
> index 5689533ce1..6fc057e3a2 100644
> --- a/disas/sparc.c
> +++ b/disas/sparc.c
> @@ -27,6 +27,7 @@
>     see <http://www.gnu.org/licenses/>.  */
>
>  #include "qemu/osdep.h"
> +#include "qemu/bitops.h"
>  #include "disas/dis-asm.h"
>
>  /* The SPARC opcode table (and other related data) is defined in
> @@ -2515,7 +2516,7 @@ compare_opcodes (const void * a, const void * b)
>       another, it is important to order the opcodes in the right order.  */
>    for (i = 0; i < 32; ++i)

The loop boundary is 32, so i is at most 1.

>      {
> -      unsigned long int x = 1 << i;
> +      unsigned long int x = BIT(i);

The loop boundary is 32, so i is at most 31. QEMU uses the
-fwrapv compiler option, so "1 << 31" is well defined,
even though it shifts into the sign bit. It's worth noting
in the commit message that we are not actually fixing any
bug here, we are merely placating somebody's static analyzer.

>        int x0 = (match0 & x) != 0;
>        int x1 = (match1 & x) != 0;
>
> @@ -2525,7 +2526,7 @@ compare_opcodes (const void * a, const void * b)
>
>    for (i = 0; i < 32; ++i)
>      {
> -      unsigned long int x = 1 << i;
> +      unsigned long int x = BIT(i);
>        int x0 = (lose0 & x) != 0;
>        int x1 = (lose1 & x) != 0;

If we're going to bother to change this not-a-bug, it would
be  better to do it the way that upstream binutils did it,
as noted in the gitlab issue: using "1ul".

-- PMM

Re: [PATCH] disas: sparc: Fix integer overflow in compare_opcodes() for loop

Posted by Richard Henderson 2 days, 1 hour ago

On 5/28/26 02:17, Peter Maydell wrote:
>> @@ -2525,7 +2526,7 @@ compare_opcodes (const void * a, const void * b)
>>
>>     for (i = 0; i < 32; ++i)
>>       {
>> -      unsigned long int x = 1 << i;
>> +      unsigned long int x = BIT(i);
>>         int x0 = (lose0 & x) != 0;
>>         int x1 = (lose1 & x) != 0;
> 
> If we're going to bother to change this not-a-bug, it would
> be  better to do it the way that upstream binutils did it,
> as noted in the gitlab issue: using "1ul".

I would go further and use uint32_t not unsigned long at all.
If we're going to change anything.

r~

Re: [PATCH] disas: sparc: Fix integer overflow in compare_opcodes() for loop

Posted by Laurent Vivier 2 days, 9 hours ago

Le 27/05/2026 à 19:57, Christian Barry a écrit :
> From: Christian Barry <christian.barry@usp.br>
> 
> Replaced left-shift of a literal 1 by i inside of two for loops in compare_opcodes() with a call to BIT(i). This makes it so 1 is interpreted as an unsigned long int, preventing overflows.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2618
> 
> Signed-off-by: Christian Barry <christian.barry@usp.br>
> Co-developed-by: Eduardo Augusto Cavalcanti <eduardoaugustoabc@ime.usp.br>
> Signed-off-by: Eduardo Augusto Cavalcanti <eduardoaugustoabc@ime.usp.br>

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

> ---
>   disas/sparc.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/disas/sparc.c b/disas/sparc.c
> index 5689533ce1..6fc057e3a2 100644
> --- a/disas/sparc.c
> +++ b/disas/sparc.c
> @@ -27,6 +27,7 @@
>      see <http://www.gnu.org/licenses/>.  */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/bitops.h"
>   #include "disas/dis-asm.h"
>   
>   /* The SPARC opcode table (and other related data) is defined in
> @@ -2515,7 +2516,7 @@ compare_opcodes (const void * a, const void * b)
>        another, it is important to order the opcodes in the right order.  */
>     for (i = 0; i < 32; ++i)
>       {
> -      unsigned long int x = 1 << i;
> +      unsigned long int x = BIT(i);
>         int x0 = (match0 & x) != 0;
>         int x1 = (match1 & x) != 0;
>   
> @@ -2525,7 +2526,7 @@ compare_opcodes (const void * a, const void * b)
>   
>     for (i = 0; i < 32; ++i)
>       {
> -      unsigned long int x = 1 << i;
> +      unsigned long int x = BIT(i);
>         int x0 = (lose0 & x) != 0;
>         int x1 = (lose1 & x) != 0;
>