On Wed, 27 May 2026 at 20:56, Christian Barry <[email protected]> wrote:
>
> From: Christian Barry <[email protected]>
>
> 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 <[email protected]>
> Co-developed-by: Eduardo Augusto Cavalcanti <[email protected]>
> Signed-off-by: Eduardo Augusto Cavalcanti <[email protected]>
> ---
> 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
