[Qemu-devel] [PATCH 0/2] lsi53c895a: avoid integer undefined behaviours

[Qemu-devel] [PATCH 0/2] lsi53c895a: avoid integer undefined behaviours

[Peter Maydell]

More patches to fix clang sanitizer warnings; in this case we
can fix them and clean up the code a bit by replacing hand-coded
operations with functions from the bitops header.

Peter Maydell (2):
  hw/scsi/lsi53c895a: Use sextract32 for sign-extension
  hw/scsi/lsi53c895a: Use deposit32 rather than handcoded shift/mask

 hw/scsi/lsi53c895a.c |   19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCH 1/2] hw/scsi/lsi53c895a: Use sextract32 for sign-extension

[Peter Maydell]

Use sextract32() for doing sign-extension rather than rolling
our own implementation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/scsi/lsi53c895a.c |   16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 611f2aa..333c62f 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -998,12 +998,6 @@ bad:
     s->msg_action = 0;
 }
 
-/* Sign extend a 24-bit value.  */
-static inline int32_t sxt24(int32_t n)
-{
-    return (n << 8) >> 8;
-}
-
 #define LSI_BUF_SIZE 4096
 static void lsi_memcpy(LSIState *s, uint32_t dest, uint32_t src, int count)
 {
@@ -1083,7 +1077,7 @@ again:
             /* Table indirect addressing.  */
 
             /* 32-bit Table indirect */
-            offset = sxt24(addr);
+            offset = sextract32(addr, 0, 24);
             pci_dma_read(pci_dev, s->dsa + offset, buf, 8);
             /* byte count is stored in bits 0:23 only */
             s->dbc = cpu_to_le32(buf[0]) & 0xffffff;
@@ -1183,13 +1177,13 @@ again:
             uint32_t id;
 
             if (insn & (1 << 25)) {
-                id = read_dword(s, s->dsa + sxt24(insn));
+                id = read_dword(s, s->dsa + sextract32(insn, 0, 24));
             } else {
                 id = insn;
             }
             id = (id >> 16) & 0xf;
             if (insn & (1 << 26)) {
-                addr = s->dsp + sxt24(addr);
+                addr = s->dsp + sextract32(addr, 0, 24);
             }
             s->dnad = addr;
             switch (opcode) {
@@ -1385,7 +1379,7 @@ again:
             if (cond == jmp) {
                 if (insn & (1 << 23)) {
                     /* Relative address.  */
-                    addr = s->dsp + sxt24(addr);
+                    addr = s->dsp + sextract32(addr, 0, 24);
                 }
                 switch ((insn >> 27) & 7) {
                 case 0: /* Jump */
@@ -1438,7 +1432,7 @@ again:
             int i;
 
             if (insn & (1 << 28)) {
-                addr = s->dsa + sxt24(addr);
+                addr = s->dsa + sextract32(addr, 0, 24);
             }
             n = (insn & 7);
             reg = (insn >> 16) & 0xff;
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/2] hw/scsi/lsi53c895a: Use deposit32 rather than handcoded shift/mask

[Peter Maydell]

Use deposit32() rather than handcoded shifts/masks to update the
scratch registers. This is cleaner and incidentally avoids a clang
sanitizer complaint ("runtime error: left shift of 255 by 24 places
cannot be represented in type 'int'").

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/scsi/lsi53c895a.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 333c62f..9c64e7c 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -1870,8 +1870,7 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
             int shift;
             n = (offset - 0x58) >> 2;
             shift = (offset & 3) * 8;
-            s->scratch[n] &= ~(0xff << shift);
-            s->scratch[n] |= (val & 0xff) << shift;
+            s->scratch[n] = deposit32(s->scratch[n], shift, 8, val);
         } else {
             BADF("Unhandled writeb 0x%x = 0x%x\n", offset, val);
         }
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 0/2] lsi53c895a: avoid integer undefined behaviours

[Paolo Bonzini]

Il 23/08/2013 18:16, Peter Maydell ha scritto:
> More patches to fix clang sanitizer warnings; in this case we
> can fix them and clean up the code a bit by replacing hand-coded
> operations with functions from the bitops header.
> 
> Peter Maydell (2):
>   hw/scsi/lsi53c895a: Use sextract32 for sign-extension
>   hw/scsi/lsi53c895a: Use deposit32 rather than handcoded shift/mask
> 
>  hw/scsi/lsi53c895a.c |   19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 

Applied both to scsi-next, thanks.

Paolo