* [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit
@ 2014-03-10 19:10 Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 01/12] target-i386: " Peter Maydell
` (12 more replies)
0 siblings, 13 replies; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
This is a set of patches which silence clang -fsanitize=undefined
warnings about shifting left into the sign bit of a signed value.
Typically this is the result of "1 << 31" and similar constructs;
the fix is to add a "U" suffix to the 1 so that we do unsigned
arithmetic rather than signed arithmetic.
Since these patches are very minor changes to a fairly
wide ranging set of files, it seems easiest to send these
through the -trivial queue. Happy to split the series up
if people disagree.
Mostly I think these warnings are not particularly exciting
(even the adversarial optimizations preferred by modern
compilers will probably not break "1 << 31") but there are
a lot of them, the fix is pretty trivial, and getting rid of
them allows us to see the interesting sanitizer warnings more
clearly.
My method here has been just to look at the warnings produced
during a 'make check' run; no doubt actually running a guest
for various platforms would identify more of these.
thanks
-- PMM
Peter Maydell (12):
target-i386: Avoid shifting left into sign bit
hw/intc/apic.c: Use uint32_t for mask word in foreach_apic
hw/pci/pci_host.c: Avoid shifting left into sign bit
hw/i386/acpi_build.c: Avoid shifting left into sign bit
target-mips: Avoid shifting left into sign bit
hw/usb/hcd-ohci.c: Avoid shifting left into sign bit
hw/intc/openpic: Avoid shifting left into sign bit
hw/ppc: Avoid shifting left into sign bit
tests/libqos/pci-pc: Avoid shifting left into sign bit
hw/intc/slavio_intctl: Avoid shifting left into sign bit
hw/intc/xilinx_intc: Avoid shifting left into sign bit
hw/pci-host/apb.c: Avoid shifting left into sign bit
hw/i386/acpi-build.c | 2 +-
hw/intc/apic.c | 5 +++--
hw/intc/openpic.c | 4 ++--
hw/intc/slavio_intctl.c | 2 +-
hw/intc/xilinx_intc.c | 3 ++-
hw/pci-host/apb.c | 2 +-
hw/pci/pci_host.c | 3 ++-
hw/ppc/ppc.c | 2 +-
hw/ppc/ppc440_bamboo.c | 4 ++--
hw/ppc/ppc4xx_devs.c | 2 +-
hw/ppc/ppc_booke.c | 4 ++--
hw/ppc/virtex_ml507.c | 4 ++--
hw/usb/hcd-ohci.c | 6 +++---
target-i386/cpu.h | 8 ++++----
target-mips/cpu.h | 2 +-
target-mips/helper.c | 8 ++++----
target-mips/op_helper.c | 2 +-
target-mips/translate_init.c | 22 +++++++++++-----------
tests/libqos/pci-pc.c | 12 ++++++------
19 files changed, 50 insertions(+), 47 deletions(-)
--
1.9.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 01/12] target-i386: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 21:57 ` Michael S. Tsirkin
2014-03-10 19:10 ` [Qemu-devel] [PATCH 02/12] hw/intc/apic.c: Use uint32_t for mask word in foreach_apic Peter Maydell
` (11 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Add 'U' suffixes where necessary to avoid (1 << 31) which
shifts left into the sign bit, which is undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-i386/cpu.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 0014acc..064f987 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -202,7 +202,7 @@
#define CR0_NE_MASK (1 << 5)
#define CR0_WP_MASK (1 << 16)
#define CR0_AM_MASK (1 << 18)
-#define CR0_PG_MASK (1 << 31)
+#define CR0_PG_MASK (1U << 31)
#define CR4_VME_MASK (1 << 0)
#define CR4_PVI_MASK (1 << 1)
@@ -436,7 +436,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_HT (1 << 28)
#define CPUID_TM (1 << 29)
#define CPUID_IA64 (1 << 30)
-#define CPUID_PBE (1 << 31)
+#define CPUID_PBE (1U << 31)
#define CPUID_EXT_SSE3 (1 << 0)
#define CPUID_EXT_PCLMULQDQ (1 << 1)
@@ -467,7 +467,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_EXT_AVX (1 << 28)
#define CPUID_EXT_F16C (1 << 29)
#define CPUID_EXT_RDRAND (1 << 30)
-#define CPUID_EXT_HYPERVISOR (1 << 31)
+#define CPUID_EXT_HYPERVISOR (1U << 31)
#define CPUID_EXT2_FPU (1 << 0)
#define CPUID_EXT2_VME (1 << 1)
@@ -496,7 +496,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_EXT2_RDTSCP (1 << 27)
#define CPUID_EXT2_LM (1 << 29)
#define CPUID_EXT2_3DNOWEXT (1 << 30)
-#define CPUID_EXT2_3DNOW (1 << 31)
+#define CPUID_EXT2_3DNOW (1U << 31)
/* CPUID[8000_0001].EDX bits that are aliase of CPUID[1].EDX bits on AMD CPUs */
#define CPUID_EXT2_AMD_ALIASES (CPUID_EXT2_FPU | CPUID_EXT2_VME | \
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 02/12] hw/intc/apic.c: Use uint32_t for mask word in foreach_apic
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 01/12] target-i386: " Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 19:39 ` Stefan Weil
2014-03-10 21:56 ` Michael S. Tsirkin
2014-03-10 19:10 ` [Qemu-devel] [PATCH 03/12] hw/pci/pci_host.c: Avoid shifting left into sign bit Peter Maydell
` (10 subsequent siblings)
12 siblings, 2 replies; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Use unsigned arithmetic for operations on the mask word
in the foreach_apic() macro, to avoid relying on undefined
behaviour when shifting into the sign bit.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/intc/apic.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 361ae90..e137882 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -201,12 +201,13 @@ static void apic_external_nmi(APICCommonState *s)
#define foreach_apic(apic, deliver_bitmask, code) \
{\
- int __i, __j, __mask;\
+ int __i, __j;\
+ uint32_t __mask;\
for(__i = 0; __i < MAX_APIC_WORDS; __i++) {\
__mask = deliver_bitmask[__i];\
if (__mask) {\
for(__j = 0; __j < 32; __j++) {\
- if (__mask & (1 << __j)) {\
+ if (__mask & (1U << __j)) {\
apic = local_apics[__i * 32 + __j];\
if (apic) {\
code;\
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 03/12] hw/pci/pci_host.c: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 01/12] target-i386: " Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 02/12] hw/intc/apic.c: Use uint32_t for mask word in foreach_apic Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 20:03 ` Stefan Weil
2014-03-10 21:58 ` Michael S. Tsirkin
2014-03-10 19:10 ` [Qemu-devel] [PATCH 04/12] hw/i386/acpi_build.c: " Peter Maydell
` (9 subsequent siblings)
12 siblings, 2 replies; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Add U suffix to avoid undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/pci/pci_host.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 77c7d1f..2c17916 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -142,8 +142,9 @@ static uint64_t pci_host_data_read(void *opaque,
{
PCIHostState *s = opaque;
uint32_t val;
- if (!(s->config_reg & (1 << 31)))
+ if (!(s->config_reg & (1u << 31))) {
return 0xffffffff;
+ }
val = pci_data_read(s->bus, s->config_reg | (addr & 3), len);
PCI_DPRINTF("read addr " TARGET_FMT_plx " len %d val %x\n",
addr, len, val);
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 04/12] hw/i386/acpi_build.c: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
` (2 preceding siblings ...)
2014-03-10 19:10 ` [Qemu-devel] [PATCH 03/12] hw/pci/pci_host.c: Avoid shifting left into sign bit Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 21:56 ` Michael S. Tsirkin
2014-03-10 19:10 ` [Qemu-devel] [PATCH 05/12] target-mips: " Peter Maydell
` (8 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Add U suffix to avoid undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/i386/acpi-build.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b1a7ebb..46d4e60 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -813,7 +813,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
build_append_byte(notify, 0x7B); /* AndOp */
build_append_byte(notify, 0x68); /* Arg0Op */
- build_append_int(notify, 0x1 << i);
+ build_append_int(notify, 0x1U << i);
build_append_byte(notify, 0x00); /* NullName */
build_append_byte(notify, 0x86); /* NotifyOp */
build_append_nameseg(notify, "S%.02X_", PCI_DEVFN(i, 0));
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 05/12] target-mips: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
` (3 preceding siblings ...)
2014-03-10 19:10 ` [Qemu-devel] [PATCH 04/12] hw/i386/acpi_build.c: " Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 06/12] hw/usb/hcd-ohci.c: " Peter Maydell
` (7 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Add U suffix to various places where we shift a 1 left by 31,
to avoid undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-mips/cpu.h | 2 +-
target-mips/helper.c | 8 ++++----
target-mips/op_helper.c | 2 +-
target-mips/translate_init.c | 22 +++++++++++-----------
4 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 60c8061..25e97be 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -803,7 +803,7 @@ static inline void compute_hflags(CPUMIPSState *env)
and disable the MIPS IV extensions to the MIPS III ISA.
Some other MIPS IV CPUs ignore the bit, so the check here
would be too restrictive for them. */
- if (env->CP0_Status & (1 << CP0St_CU3)) {
+ if (env->CP0_Status & (1U << CP0St_CU3)) {
env->hflags |= MIPS_HFLAG_COP1X;
}
}
diff --git a/target-mips/helper.c b/target-mips/helper.c
index 33e0e88..68eb9f6 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -452,7 +452,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
env->hflags &= ~(MIPS_HFLAG_KSU);
/* EJTAG probe trap enable is not implemented... */
if (!(env->CP0_Status & (1 << CP0St_EXL)))
- env->CP0_Cause &= ~(1 << CP0Ca_BD);
+ env->CP0_Cause &= ~(1U << CP0Ca_BD);
env->active_tc.PC = (int32_t)0xBFC00480;
set_hflags_for_handler(env);
break;
@@ -472,7 +472,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
env->hflags |= MIPS_HFLAG_64 | MIPS_HFLAG_CP0;
env->hflags &= ~(MIPS_HFLAG_KSU);
if (!(env->CP0_Status & (1 << CP0St_EXL)))
- env->CP0_Cause &= ~(1 << CP0Ca_BD);
+ env->CP0_Cause &= ~(1U << CP0Ca_BD);
env->active_tc.PC = (int32_t)0xBFC00000;
set_hflags_for_handler(env);
break;
@@ -610,9 +610,9 @@ void mips_cpu_do_interrupt(CPUState *cs)
if (!(env->CP0_Status & (1 << CP0St_EXL))) {
env->CP0_EPC = exception_resume_pc(env);
if (env->hflags & MIPS_HFLAG_BMASK) {
- env->CP0_Cause |= (1 << CP0Ca_BD);
+ env->CP0_Cause |= (1U << CP0Ca_BD);
} else {
- env->CP0_Cause &= ~(1 << CP0Ca_BD);
+ env->CP0_Cause &= ~(1U << CP0Ca_BD);
}
env->CP0_Status |= (1 << CP0St_EXL);
env->hflags |= MIPS_HFLAG_64 | MIPS_HFLAG_CP0;
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 2ef6633..256c3c9 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -646,7 +646,7 @@ static void sync_c0_tcstatus(CPUMIPSState *cpu, int tc,
{
uint32_t status;
uint32_t tcu, tmx, tasid, tksu;
- uint32_t mask = ((1 << CP0St_CU3)
+ uint32_t mask = ((1U << CP0St_CU3)
| (1 << CP0St_CU2)
| (1 << CP0St_CU1)
| (1 << CP0St_CU0)
diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index 29d39e2..c765825 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -22,20 +22,20 @@
/* Have config1, uncached coherency */
#define MIPS_CONFIG0 \
- ((1 << CP0C0_M) | (0x2 << CP0C0_K0))
+ ((1U << CP0C0_M) | (0x2 << CP0C0_K0))
/* Have config2, no coprocessor2 attached, no MDMX support attached,
no performance counters, watch registers present,
no code compression, EJTAG present, no FPU */
#define MIPS_CONFIG1 \
-((1 << CP0C1_M) | \
+((1U << CP0C1_M) | \
(0 << CP0C1_C2) | (0 << CP0C1_MD) | (0 << CP0C1_PC) | \
(1 << CP0C1_WR) | (0 << CP0C1_CA) | (1 << CP0C1_EP) | \
(0 << CP0C1_FP))
/* Have config3, no tertiary/secondary caches implemented */
#define MIPS_CONFIG2 \
-((1 << CP0C2_M))
+((1U << CP0C2_M))
/* No config4, no DSP ASE, no large physaddr (PABITS),
no external interrupt controller, no vectored interrupts,
@@ -301,16 +301,16 @@ static const mips_def_t mips_defs[] =
(1 << FCR0_D) | (1 << FCR0_S) | (0x95 << FCR0_PRID),
.CP0_SRSCtl = (0xf << CP0SRSCtl_HSS),
.CP0_SRSConf0_rw_bitmask = 0x3fffffff,
- .CP0_SRSConf0 = (1 << CP0SRSC0_M) | (0x3fe << CP0SRSC0_SRS3) |
+ .CP0_SRSConf0 = (1U << CP0SRSC0_M) | (0x3fe << CP0SRSC0_SRS3) |
(0x3fe << CP0SRSC0_SRS2) | (0x3fe << CP0SRSC0_SRS1),
.CP0_SRSConf1_rw_bitmask = 0x3fffffff,
- .CP0_SRSConf1 = (1 << CP0SRSC1_M) | (0x3fe << CP0SRSC1_SRS6) |
+ .CP0_SRSConf1 = (1U << CP0SRSC1_M) | (0x3fe << CP0SRSC1_SRS6) |
(0x3fe << CP0SRSC1_SRS5) | (0x3fe << CP0SRSC1_SRS4),
.CP0_SRSConf2_rw_bitmask = 0x3fffffff,
- .CP0_SRSConf2 = (1 << CP0SRSC2_M) | (0x3fe << CP0SRSC2_SRS9) |
+ .CP0_SRSConf2 = (1U << CP0SRSC2_M) | (0x3fe << CP0SRSC2_SRS9) |
(0x3fe << CP0SRSC2_SRS8) | (0x3fe << CP0SRSC2_SRS7),
.CP0_SRSConf3_rw_bitmask = 0x3fffffff,
- .CP0_SRSConf3 = (1 << CP0SRSC3_M) | (0x3fe << CP0SRSC3_SRS12) |
+ .CP0_SRSConf3 = (1U << CP0SRSC3_M) | (0x3fe << CP0SRSC3_SRS12) |
(0x3fe << CP0SRSC3_SRS11) | (0x3fe << CP0SRSC3_SRS10),
.CP0_SRSConf4_rw_bitmask = 0x3fffffff,
.CP0_SRSConf4 = (0x3fe << CP0SRSC4_SRS15) |
@@ -355,8 +355,8 @@ static const mips_def_t mips_defs[] =
(0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA) |
(1 << CP0C1_CA),
.CP0_Config2 = MIPS_CONFIG2,
- .CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_M),
- .CP0_Config4 = MIPS_CONFIG4 | (1 << CP0C4_M),
+ .CP0_Config3 = MIPS_CONFIG3 | (1U << CP0C3_M),
+ .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M),
.CP0_Config4_rw_bitmask = 0,
.CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_UFR),
.CP0_Config5_rw_bitmask = (0 << CP0C5_M) | (1 << CP0C5_K) |
@@ -668,7 +668,7 @@ static void mvp_init (CPUMIPSState *env, const mips_def_t *def)
programmable cache partitioning implemented, number of allocatable
and sharable TLB entries, MVP has allocatable TCs, 2 VPEs
implemented, 5 TCs implemented. */
- env->mvp->CP0_MVPConf0 = (1 << CP0MVPC0_M) | (1 << CP0MVPC0_TLBS) |
+ env->mvp->CP0_MVPConf0 = (1U << CP0MVPC0_M) | (1 << CP0MVPC0_TLBS) |
(0 << CP0MVPC0_GS) | (1 << CP0MVPC0_PCP) |
// TODO: actually do 2 VPEs.
// (1 << CP0MVPC0_TCA) | (0x1 << CP0MVPC0_PVPE) |
@@ -682,7 +682,7 @@ static void mvp_init (CPUMIPSState *env, const mips_def_t *def)
/* Allocatable CP1 have media extensions, allocatable CP1 have FP support,
no UDI implemented, no CP2 implemented, 1 CP1 implemented. */
- env->mvp->CP0_MVPConf1 = (1 << CP0MVPC1_CIM) | (1 << CP0MVPC1_CIF) |
+ env->mvp->CP0_MVPConf1 = (1U << CP0MVPC1_CIM) | (1 << CP0MVPC1_CIF) |
(0x0 << CP0MVPC1_PCX) | (0x0 << CP0MVPC1_PCP2) |
(0x1 << CP0MVPC1_PCP1);
}
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 06/12] hw/usb/hcd-ohci.c: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
` (4 preceding siblings ...)
2014-03-10 19:10 ` [Qemu-devel] [PATCH 05/12] target-mips: " Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 07/12] hw/intc/openpic: " Peter Maydell
` (6 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Add U suffix to avoid undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/usb/hcd-ohci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 3d35058b..9dcfb99 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -242,7 +242,7 @@ struct ohci_iso_td {
#define OHCI_INTR_FNO (1<<5) /* Frame number overflow */
#define OHCI_INTR_RHSC (1<<6) /* Root hub status change */
#define OHCI_INTR_OC (1<<30) /* Ownership change */
-#define OHCI_INTR_MIE (1<<31) /* Master Interrupt Enable */
+#define OHCI_INTR_MIE (1U<<31) /* Master Interrupt Enable */
#define OHCI_HCCA_SIZE 0x100
#define OHCI_HCCA_MASK 0xffffff00
@@ -253,7 +253,7 @@ struct ohci_iso_td {
#define OHCI_FMI_FSMPS 0xffff0000
#define OHCI_FMI_FIT 0x80000000
-#define OHCI_FR_RT (1<<31)
+#define OHCI_FR_RT (1U<<31)
#define OHCI_LS_THRESH 0x628
@@ -270,7 +270,7 @@ struct ohci_iso_td {
#define OHCI_RHS_DRWE (1<<15)
#define OHCI_RHS_LPSC (1<<16)
#define OHCI_RHS_OCIC (1<<17)
-#define OHCI_RHS_CRWE (1<<31)
+#define OHCI_RHS_CRWE (1U<<31)
#define OHCI_PORT_CCS (1<<0)
#define OHCI_PORT_PES (1<<1)
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 07/12] hw/intc/openpic: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
` (5 preceding siblings ...)
2014-03-10 19:10 ` [Qemu-devel] [PATCH 06/12] hw/usb/hcd-ohci.c: " Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 08/12] hw/ppc: " Peter Maydell
` (5 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Add U suffix to avoid undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/intc/openpic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
index 7df72f4..cfd7f06 100644
--- a/hw/intc/openpic.c
+++ b/hw/intc/openpic.c
@@ -123,7 +123,7 @@ static FslMpicInfo fsl_mpic_42 = {
#define TCCR_TOG 0x80000000 /* toggles when decrement to zero */
#define IDR_EP_SHIFT 31
-#define IDR_EP_MASK (1 << IDR_EP_SHIFT)
+#define IDR_EP_MASK (1U << IDR_EP_SHIFT)
#define IDR_CI0_SHIFT 30
#define IDR_CI1_SHIFT 29
#define IDR_P1_SHIFT 1
@@ -220,7 +220,7 @@ typedef struct IRQSource {
} IRQSource;
#define IVPR_MASK_SHIFT 31
-#define IVPR_MASK_MASK (1 << IVPR_MASK_SHIFT)
+#define IVPR_MASK_MASK (1U << IVPR_MASK_SHIFT)
#define IVPR_ACTIVITY_SHIFT 30
#define IVPR_ACTIVITY_MASK (1 << IVPR_ACTIVITY_SHIFT)
#define IVPR_MODE_SHIFT 29
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 08/12] hw/ppc: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
` (6 preceding siblings ...)
2014-03-10 19:10 ` [Qemu-devel] [PATCH 07/12] hw/intc/openpic: " Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 09/12] tests/libqos/pci-pc: " Peter Maydell
` (4 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Add U suffix to various places where we were doing "1 << 31",
which is undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/ppc/ppc.c | 2 +-
hw/ppc/ppc440_bamboo.c | 4 ++--
hw/ppc/ppc4xx_devs.c | 2 +-
hw/ppc/ppc_booke.c | 4 ++--
hw/ppc/virtex_ml507.c | 4 ++--
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 0e82719..9c2a132 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -1002,7 +1002,7 @@ static void cpu_4xx_wdt_cb (void *opaque)
case 0x1:
timer_mod(ppc40x_timer->wdt_timer, next);
ppc40x_timer->wdt_next = next;
- env->spr[SPR_40x_TSR] |= 1 << 31;
+ env->spr[SPR_40x_TSR] |= 1U << 31;
break;
case 0x2:
timer_mod(ppc40x_timer->wdt_timer, next);
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index ec15bab..2ddc2ed 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -128,7 +128,7 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
tlb->attr = 0;
tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
- tlb->size = 1 << 31; /* up to 0x80000000 */
+ tlb->size = 1U << 31; /* up to 0x80000000 */
tlb->EPN = va & TARGET_PAGE_MASK;
tlb->RPN = pa & TARGET_PAGE_MASK;
tlb->PID = 0;
@@ -136,7 +136,7 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
tlb = &env->tlb.tlbe[1];
tlb->attr = 0;
tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
- tlb->size = 1 << 31; /* up to 0xffffffff */
+ tlb->size = 1U << 31; /* up to 0xffffffff */
tlb->EPN = 0x80000000 & TARGET_PAGE_MASK;
tlb->RPN = 0x80000000 & TARGET_PAGE_MASK;
tlb->PID = 0;
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index 9160ee7..8a43111 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -161,7 +161,7 @@ static void ppcuic_set_irq (void *opaque, int irq_num, int level)
uint32_t mask, sr;
uic = opaque;
- mask = 1 << (31-irq_num);
+ mask = 1U << (31-irq_num);
LOG_UIC("%s: irq %d level %d uicsr %08" PRIx32
" mask %08" PRIx32 " => %08" PRIx32 " %08" PRIx32 "\n",
__func__, irq_num, level,
diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c
index d839960..773c4ea 100644
--- a/hw/ppc/ppc_booke.c
+++ b/hw/ppc/ppc_booke.c
@@ -34,7 +34,7 @@
/* Timer Control Register */
#define TCR_WP_SHIFT 30 /* Watchdog Timer Period */
-#define TCR_WP_MASK (0x3 << TCR_WP_SHIFT)
+#define TCR_WP_MASK (0x3U << TCR_WP_SHIFT)
#define TCR_WRC_SHIFT 28 /* Watchdog Timer Reset Control */
#define TCR_WRC_MASK (0x3 << TCR_WRC_SHIFT)
#define TCR_WIE (1 << 27) /* Watchdog Timer Interrupt Enable */
@@ -58,7 +58,7 @@
#define TSR_WRS_SHIFT 28 /* Watchdog Timer Reset Status */
#define TSR_WRS_MASK (0x3 << TSR_WRS_SHIFT)
#define TSR_WIS (1 << 30) /* Watchdog Timer Interrupt Status */
-#define TSR_ENW (1 << 31) /* Enable Next Watchdog Timer */
+#define TSR_ENW (1U << 31) /* Enable Next Watchdog Timer */
typedef struct booke_timer_t booke_timer_t;
struct booke_timer_t {
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index ce8ea91..3e3569d 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -71,7 +71,7 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
tlb->attr = 0;
tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
- tlb->size = 1 << 31; /* up to 0x80000000 */
+ tlb->size = 1U << 31; /* up to 0x80000000 */
tlb->EPN = va & TARGET_PAGE_MASK;
tlb->RPN = pa & TARGET_PAGE_MASK;
tlb->PID = 0;
@@ -79,7 +79,7 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
tlb = &env->tlb.tlbe[1];
tlb->attr = 0;
tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
- tlb->size = 1 << 31; /* up to 0xffffffff */
+ tlb->size = 1U << 31; /* up to 0xffffffff */
tlb->EPN = 0x80000000 & TARGET_PAGE_MASK;
tlb->RPN = 0x80000000 & TARGET_PAGE_MASK;
tlb->PID = 0;
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 09/12] tests/libqos/pci-pc: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
` (7 preceding siblings ...)
2014-03-10 19:10 ` [Qemu-devel] [PATCH 08/12] hw/ppc: " Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 10/12] hw/intc/slavio_intctl: " Peter Maydell
` (3 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Add U suffix when doing "1 << 31" to avoid undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
tests/libqos/pci-pc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
index 3bde8ab..bf741a4 100644
--- a/tests/libqos/pci-pc.c
+++ b/tests/libqos/pci-pc.c
@@ -110,37 +110,37 @@ static void qpci_pc_io_writel(QPCIBus *bus, void *addr, uint32_t value)
static uint8_t qpci_pc_config_readb(QPCIBus *bus, int devfn, uint8_t offset)
{
- outl(0xcf8, (1 << 31) | (devfn << 8) | offset);
+ outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
return inb(0xcfc);
}
static uint16_t qpci_pc_config_readw(QPCIBus *bus, int devfn, uint8_t offset)
{
- outl(0xcf8, (1 << 31) | (devfn << 8) | offset);
+ outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
return inw(0xcfc);
}
static uint32_t qpci_pc_config_readl(QPCIBus *bus, int devfn, uint8_t offset)
{
- outl(0xcf8, (1 << 31) | (devfn << 8) | offset);
+ outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
return inl(0xcfc);
}
static void qpci_pc_config_writeb(QPCIBus *bus, int devfn, uint8_t offset, uint8_t value)
{
- outl(0xcf8, (1 << 31) | (devfn << 8) | offset);
+ outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
outb(0xcfc, value);
}
static void qpci_pc_config_writew(QPCIBus *bus, int devfn, uint8_t offset, uint16_t value)
{
- outl(0xcf8, (1 << 31) | (devfn << 8) | offset);
+ outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
outw(0xcfc, value);
}
static void qpci_pc_config_writel(QPCIBus *bus, int devfn, uint8_t offset, uint32_t value)
{
- outl(0xcf8, (1 << 31) | (devfn << 8) | offset);
+ outl(0xcf8, (1U << 31) | (devfn << 8) | offset);
outl(0xcfc, value);
}
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 10/12] hw/intc/slavio_intctl: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
` (8 preceding siblings ...)
2014-03-10 19:10 ` [Qemu-devel] [PATCH 09/12] tests/libqos/pci-pc: " Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 11/12] hw/intc/xilinx_intc: " Peter Maydell
` (2 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Add 'U' suffix to avoid undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/intc/slavio_intctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/intc/slavio_intctl.c b/hw/intc/slavio_intctl.c
index 41a1672..b10fb66 100644
--- a/hw/intc/slavio_intctl.c
+++ b/hw/intc/slavio_intctl.c
@@ -272,7 +272,7 @@ static void slavio_check_interrupts(SLAVIO_INTCTLState *s, int set_irqs)
CPU_IRQ_TIMER_IN;
if (i == s->target_cpu) {
for (j = 0; j < 32; j++) {
- if ((s->intregm_pending & (1 << j)) && intbit_to_level[j]) {
+ if ((s->intregm_pending & (1U << j)) && intbit_to_level[j]) {
s->slaves[i].intreg_pending |= 1 << intbit_to_level[j];
}
}
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 11/12] hw/intc/xilinx_intc: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
` (9 preceding siblings ...)
2014-03-10 19:10 ` [Qemu-devel] [PATCH 10/12] hw/intc/slavio_intctl: " Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 12/12] hw/pci-host/apb.c: " Peter Maydell
2014-03-10 22:00 ` [Qemu-devel] [PATCH 00/12] " Michael S. Tsirkin
12 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Avoid undefined behaviour shifting left into the sign bit.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/intc/xilinx_intc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index 4a10398..1b228ff 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -71,8 +71,9 @@ static void update_irq(struct xlx_pic *p)
/* Update the vector register. */
for (i = 0; i < 32; i++) {
- if (p->regs[R_IPR] & (1 << i))
+ if (p->regs[R_IPR] & (1U << i)) {
break;
+ }
}
if (i == 32)
i = ~0;
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 12/12] hw/pci-host/apb.c: Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
` (10 preceding siblings ...)
2014-03-10 19:10 ` [Qemu-devel] [PATCH 11/12] hw/intc/xilinx_intc: " Peter Maydell
@ 2014-03-10 19:10 ` Peter Maydell
2014-03-10 21:59 ` Michael S. Tsirkin
2014-03-10 22:00 ` [Qemu-devel] [PATCH 00/12] " Michael S. Tsirkin
12 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2014-03-10 19:10 UTC (permalink / raw
To: qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Add U suffix to avoid undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/pci-host/apb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 1b399dd..a6869b8 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -58,7 +58,7 @@ do { printf("APB: " fmt , ## __VA_ARGS__); } while (0)
#define PBM_PCI_IMR_MASK 0x7fffffff
#define PBM_PCI_IMR_ENABLED 0x80000000
-#define POR (1 << 31)
+#define POR (1U << 31)
#define SOFT_POR (1 << 30)
#define SOFT_XIR (1 << 29)
#define BTN_POR (1 << 28)
--
1.9.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 02/12] hw/intc/apic.c: Use uint32_t for mask word in foreach_apic
2014-03-10 19:10 ` [Qemu-devel] [PATCH 02/12] hw/intc/apic.c: Use uint32_t for mask word in foreach_apic Peter Maydell
@ 2014-03-10 19:39 ` Stefan Weil
2014-03-10 21:56 ` Michael S. Tsirkin
1 sibling, 0 replies; 23+ messages in thread
From: Stefan Weil @ 2014-03-10 19:39 UTC (permalink / raw
To: Peter Maydell, qemu-devel; +Cc: qemu-trivial, qemu-ppc, patches
Am 10.03.2014 20:10, schrieb Peter Maydell:
> Use unsigned arithmetic for operations on the mask word
> in the foreach_apic() macro, to avoid relying on undefined
> behaviour when shifting into the sign bit.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/intc/apic.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index 361ae90..e137882 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -201,12 +201,13 @@ static void apic_external_nmi(APICCommonState *s)
>
> #define foreach_apic(apic, deliver_bitmask, code) \
> {\
> - int __i, __j, __mask;\
> + int __i, __j;\
> + uint32_t __mask;\
> for(__i = 0; __i < MAX_APIC_WORDS; __i++) {\
> __mask = deliver_bitmask[__i];\
> if (__mask) {\
> for(__j = 0; __j < 32; __j++) {\
> - if (__mask & (1 << __j)) {\
> + if (__mask & (1U << __j)) {\
> apic = local_apics[__i * 32 + __j];\
> if (apic) {\
> code;\
>
The declaration of __mask could be moved inside the for block and be
combined with the assignment, but that's not strictly necessary.
Reviewed-by: Stefan Weil <sw@weilnetz.de>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 03/12] hw/pci/pci_host.c: Avoid shifting left into sign bit
2014-03-10 19:10 ` [Qemu-devel] [PATCH 03/12] hw/pci/pci_host.c: Avoid shifting left into sign bit Peter Maydell
@ 2014-03-10 20:03 ` Stefan Weil
2014-03-10 21:46 ` Michael S. Tsirkin
2014-03-10 21:58 ` Michael S. Tsirkin
1 sibling, 1 reply; 23+ messages in thread
From: Stefan Weil @ 2014-03-10 20:03 UTC (permalink / raw
To: Peter Maydell, Michael S. Tsirkin
Cc: qemu-trivial, qemu-ppc, qemu-devel, patches
Am 10.03.2014 20:10, schrieb Peter Maydell:
> Add U suffix to avoid undefined behaviour.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/pci/pci_host.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index 77c7d1f..2c17916 100644
> --- a/hw/pci/pci_host.c
> +++ b/hw/pci/pci_host.c
> @@ -142,8 +142,9 @@ static uint64_t pci_host_data_read(void *opaque,
> {
> PCIHostState *s = opaque;
> uint32_t val;
> - if (!(s->config_reg & (1 << 31)))
> + if (!(s->config_reg & (1u << 31))) {
> return 0xffffffff;
I suggest fixing that 0xffffffff, too. Do we expect here a 32 bit value
(-1) which is expanded to a 64 bit value? Then 0xffffffffffffffffULL
would be correct. Otherwise 0xffffffffU is clearer.
Michael, are 8 byte reads possible here? If yes, the current code is wrong.
> + }
> val = pci_data_read(s->bus, s->config_reg | (addr & 3), len);
> PCI_DPRINTF("read addr " TARGET_FMT_plx " len %d val %x\n",
> addr, len, val);
>
What about using the BIT macro from qemu/bitops.h? I think that BIT(31)
looks better than (1u << 31).
Stefan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 03/12] hw/pci/pci_host.c: Avoid shifting left into sign bit
2014-03-10 20:03 ` Stefan Weil
@ 2014-03-10 21:46 ` Michael S. Tsirkin
0 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2014-03-10 21:46 UTC (permalink / raw
To: Stefan Weil; +Cc: qemu-trivial, Peter Maydell, qemu-ppc, qemu-devel, patches
On Mon, Mar 10, 2014 at 09:03:08PM +0100, Stefan Weil wrote:
> Am 10.03.2014 20:10, schrieb Peter Maydell:
> > Add U suffix to avoid undefined behaviour.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > hw/pci/pci_host.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> > index 77c7d1f..2c17916 100644
> > --- a/hw/pci/pci_host.c
> > +++ b/hw/pci/pci_host.c
> > @@ -142,8 +142,9 @@ static uint64_t pci_host_data_read(void *opaque,
> > {
> > PCIHostState *s = opaque;
> > uint32_t val;
> > - if (!(s->config_reg & (1 << 31)))
> > + if (!(s->config_reg & (1u << 31))) {
> > return 0xffffffff;
>
> I suggest fixing that 0xffffffff, too. Do we expect here a 32 bit value
> (-1) which is expanded to a 64 bit value? Then 0xffffffffffffffffULL
> would be correct. Otherwise 0xffffffffU is clearer.
Hmm why is this clearer? The spec says:
The type of an integer constant is the first of the corresponding list
in which its value can be represented.
Basically 0x<anything> is always a positive value and when cast to
uint64_t is not sign extended.
> Michael, are 8 byte reads possible here? If yes, the current code is wrong.
AFAIK they aren't possible on any platform we support.
So since we discard high bits 0 seems cleaner.
> > + }
> > val = pci_data_read(s->bus, s->config_reg | (addr & 3), len);
> > PCI_DPRINTF("read addr " TARGET_FMT_plx " len %d val %x\n",
> > addr, len, val);
> >
>
> What about using the BIT macro from qemu/bitops.h? I think that BIT(31)
> looks better than (1u << 31).
>
> Stefan
I slightly prefer 1u << 31 personally, standard C as opposed to qemu macros.
--
MST
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 04/12] hw/i386/acpi_build.c: Avoid shifting left into sign bit
2014-03-10 19:10 ` [Qemu-devel] [PATCH 04/12] hw/i386/acpi_build.c: " Peter Maydell
@ 2014-03-10 21:56 ` Michael S. Tsirkin
0 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2014-03-10 21:56 UTC (permalink / raw
To: Peter Maydell; +Cc: qemu-trivial, qemu-ppc, qemu-devel, patches
On Mon, Mar 10, 2014 at 07:10:40PM +0000, Peter Maydell wrote:
> Add U suffix to avoid undefined behaviour.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/i386/acpi-build.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b1a7ebb..46d4e60 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -813,7 +813,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
>
> build_append_byte(notify, 0x7B); /* AndOp */
> build_append_byte(notify, 0x68); /* Arg0Op */
> - build_append_int(notify, 0x1 << i);
> + build_append_int(notify, 0x1U << i);
> build_append_byte(notify, 0x00); /* NullName */
> build_append_byte(notify, 0x86); /* NotifyOp */
> build_append_nameseg(notify, "S%.02X_", PCI_DEVFN(i, 0));
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 02/12] hw/intc/apic.c: Use uint32_t for mask word in foreach_apic
2014-03-10 19:10 ` [Qemu-devel] [PATCH 02/12] hw/intc/apic.c: Use uint32_t for mask word in foreach_apic Peter Maydell
2014-03-10 19:39 ` Stefan Weil
@ 2014-03-10 21:56 ` Michael S. Tsirkin
1 sibling, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2014-03-10 21:56 UTC (permalink / raw
To: Peter Maydell; +Cc: qemu-trivial, qemu-ppc, qemu-devel, patches
On Mon, Mar 10, 2014 at 07:10:38PM +0000, Peter Maydell wrote:
> Use unsigned arithmetic for operations on the mask word
> in the foreach_apic() macro, to avoid relying on undefined
> behaviour when shifting into the sign bit.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/intc/apic.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index 361ae90..e137882 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -201,12 +201,13 @@ static void apic_external_nmi(APICCommonState *s)
>
> #define foreach_apic(apic, deliver_bitmask, code) \
> {\
> - int __i, __j, __mask;\
> + int __i, __j;\
> + uint32_t __mask;\
> for(__i = 0; __i < MAX_APIC_WORDS; __i++) {\
> __mask = deliver_bitmask[__i];\
> if (__mask) {\
> for(__j = 0; __j < 32; __j++) {\
> - if (__mask & (1 << __j)) {\
> + if (__mask & (1U << __j)) {\
> apic = local_apics[__i * 32 + __j];\
> if (apic) {\
> code;\
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 01/12] target-i386: Avoid shifting left into sign bit
2014-03-10 19:10 ` [Qemu-devel] [PATCH 01/12] target-i386: " Peter Maydell
@ 2014-03-10 21:57 ` Michael S. Tsirkin
0 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2014-03-10 21:57 UTC (permalink / raw
To: Peter Maydell; +Cc: qemu-trivial, qemu-ppc, qemu-devel, patches
On Mon, Mar 10, 2014 at 07:10:37PM +0000, Peter Maydell wrote:
> Add 'U' suffixes where necessary to avoid (1 << 31) which
> shifts left into the sign bit, which is undefined behaviour.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
While not required for correctness,
I think it would be cleaner to change them all to 1U, for consistency.
> ---
> target-i386/cpu.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 0014acc..064f987 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -202,7 +202,7 @@
> #define CR0_NE_MASK (1 << 5)
> #define CR0_WP_MASK (1 << 16)
> #define CR0_AM_MASK (1 << 18)
> -#define CR0_PG_MASK (1 << 31)
> +#define CR0_PG_MASK (1U << 31)
>
> #define CR4_VME_MASK (1 << 0)
> #define CR4_PVI_MASK (1 << 1)
> @@ -436,7 +436,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_HT (1 << 28)
> #define CPUID_TM (1 << 29)
> #define CPUID_IA64 (1 << 30)
> -#define CPUID_PBE (1 << 31)
> +#define CPUID_PBE (1U << 31)
>
> #define CPUID_EXT_SSE3 (1 << 0)
> #define CPUID_EXT_PCLMULQDQ (1 << 1)
> @@ -467,7 +467,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_EXT_AVX (1 << 28)
> #define CPUID_EXT_F16C (1 << 29)
> #define CPUID_EXT_RDRAND (1 << 30)
> -#define CPUID_EXT_HYPERVISOR (1 << 31)
> +#define CPUID_EXT_HYPERVISOR (1U << 31)
>
> #define CPUID_EXT2_FPU (1 << 0)
> #define CPUID_EXT2_VME (1 << 1)
> @@ -496,7 +496,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
> #define CPUID_EXT2_RDTSCP (1 << 27)
> #define CPUID_EXT2_LM (1 << 29)
> #define CPUID_EXT2_3DNOWEXT (1 << 30)
> -#define CPUID_EXT2_3DNOW (1 << 31)
> +#define CPUID_EXT2_3DNOW (1U << 31)
>
> /* CPUID[8000_0001].EDX bits that are aliase of CPUID[1].EDX bits on AMD CPUs */
> #define CPUID_EXT2_AMD_ALIASES (CPUID_EXT2_FPU | CPUID_EXT2_VME | \
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 03/12] hw/pci/pci_host.c: Avoid shifting left into sign bit
2014-03-10 19:10 ` [Qemu-devel] [PATCH 03/12] hw/pci/pci_host.c: Avoid shifting left into sign bit Peter Maydell
2014-03-10 20:03 ` Stefan Weil
@ 2014-03-10 21:58 ` Michael S. Tsirkin
1 sibling, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2014-03-10 21:58 UTC (permalink / raw
To: Peter Maydell; +Cc: qemu-trivial, qemu-ppc, qemu-devel, patches
On Mon, Mar 10, 2014 at 07:10:39PM +0000, Peter Maydell wrote:
> Add U suffix to avoid undefined behaviour.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/pci/pci_host.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index 77c7d1f..2c17916 100644
> --- a/hw/pci/pci_host.c
> +++ b/hw/pci/pci_host.c
> @@ -142,8 +142,9 @@ static uint64_t pci_host_data_read(void *opaque,
> {
> PCIHostState *s = opaque;
> uint32_t val;
> - if (!(s->config_reg & (1 << 31)))
> + if (!(s->config_reg & (1u << 31))) {
> return 0xffffffff;
> + }
> val = pci_data_read(s->bus, s->config_reg | (addr & 3), len);
> PCI_DPRINTF("read addr " TARGET_FMT_plx " len %d val %x\n",
> addr, len, val);
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 12/12] hw/pci-host/apb.c: Avoid shifting left into sign bit
2014-03-10 19:10 ` [Qemu-devel] [PATCH 12/12] hw/pci-host/apb.c: " Peter Maydell
@ 2014-03-10 21:59 ` Michael S. Tsirkin
2014-03-14 16:45 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
0 siblings, 1 reply; 23+ messages in thread
From: Michael S. Tsirkin @ 2014-03-10 21:59 UTC (permalink / raw
To: Peter Maydell; +Cc: qemu-trivial, qemu-ppc, qemu-devel, patches
On Mon, Mar 10, 2014 at 07:10:48PM +0000, Peter Maydell wrote:
> Add U suffix to avoid undefined behaviour.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
While not required for correctness, it would be cleaner
to change all constants around this line to 1U <<, for consistency.
> ---
> hw/pci-host/apb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> index 1b399dd..a6869b8 100644
> --- a/hw/pci-host/apb.c
> +++ b/hw/pci-host/apb.c
> @@ -58,7 +58,7 @@ do { printf("APB: " fmt , ## __VA_ARGS__); } while (0)
> #define PBM_PCI_IMR_MASK 0x7fffffff
> #define PBM_PCI_IMR_ENABLED 0x80000000
>
> -#define POR (1 << 31)
> +#define POR (1U << 31)
> #define SOFT_POR (1 << 30)
> #define SOFT_XIR (1 << 29)
> #define BTN_POR (1 << 28)
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
` (11 preceding siblings ...)
2014-03-10 19:10 ` [Qemu-devel] [PATCH 12/12] hw/pci-host/apb.c: " Peter Maydell
@ 2014-03-10 22:00 ` Michael S. Tsirkin
12 siblings, 0 replies; 23+ messages in thread
From: Michael S. Tsirkin @ 2014-03-10 22:00 UTC (permalink / raw
To: Peter Maydell; +Cc: qemu-trivial, qemu-ppc, qemu-devel, patches
On Mon, Mar 10, 2014 at 07:10:36PM +0000, Peter Maydell wrote:
> This is a set of patches which silence clang -fsanitize=undefined
> warnings about shifting left into the sign bit of a signed value.
> Typically this is the result of "1 << 31" and similar constructs;
> the fix is to add a "U" suffix to the 1 so that we do unsigned
> arithmetic rather than signed arithmetic.
>
> Since these patches are very minor changes to a fairly
> wide ranging set of files, it seems easiest to send these
> through the -trivial queue. Happy to split the series up
> if people disagree.
I'm fine with this for pci and pc bits.
> Mostly I think these warnings are not particularly exciting
> (even the adversarial optimizations preferred by modern
> compilers will probably not break "1 << 31") but there are
> a lot of them, the fix is pretty trivial, and getting rid of
> them allows us to see the interesting sanitizer warnings more
> clearly.
>
> My method here has been just to look at the warnings produced
> during a 'make check' run; no doubt actually running a guest
> for various platforms would identify more of these.
>
> thanks
> -- PMM
>
> Peter Maydell (12):
> target-i386: Avoid shifting left into sign bit
> hw/intc/apic.c: Use uint32_t for mask word in foreach_apic
> hw/pci/pci_host.c: Avoid shifting left into sign bit
> hw/i386/acpi_build.c: Avoid shifting left into sign bit
> target-mips: Avoid shifting left into sign bit
> hw/usb/hcd-ohci.c: Avoid shifting left into sign bit
> hw/intc/openpic: Avoid shifting left into sign bit
> hw/ppc: Avoid shifting left into sign bit
> tests/libqos/pci-pc: Avoid shifting left into sign bit
> hw/intc/slavio_intctl: Avoid shifting left into sign bit
> hw/intc/xilinx_intc: Avoid shifting left into sign bit
> hw/pci-host/apb.c: Avoid shifting left into sign bit
>
> hw/i386/acpi-build.c | 2 +-
> hw/intc/apic.c | 5 +++--
> hw/intc/openpic.c | 4 ++--
> hw/intc/slavio_intctl.c | 2 +-
> hw/intc/xilinx_intc.c | 3 ++-
> hw/pci-host/apb.c | 2 +-
> hw/pci/pci_host.c | 3 ++-
> hw/ppc/ppc.c | 2 +-
> hw/ppc/ppc440_bamboo.c | 4 ++--
> hw/ppc/ppc4xx_devs.c | 2 +-
> hw/ppc/ppc_booke.c | 4 ++--
> hw/ppc/virtex_ml507.c | 4 ++--
> hw/usb/hcd-ohci.c | 6 +++---
> target-i386/cpu.h | 8 ++++----
> target-mips/cpu.h | 2 +-
> target-mips/helper.c | 8 ++++----
> target-mips/op_helper.c | 2 +-
> target-mips/translate_init.c | 22 +++++++++++-----------
> tests/libqos/pci-pc.c | 12 ++++++------
> 19 files changed, 50 insertions(+), 47 deletions(-)
>
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH 12/12] hw/pci-host/apb.c: Avoid shifting left into sign bit
2014-03-10 21:59 ` Michael S. Tsirkin
@ 2014-03-14 16:45 ` Michael Tokarev
0 siblings, 0 replies; 23+ messages in thread
From: Michael Tokarev @ 2014-03-14 16:45 UTC (permalink / raw
To: Michael S. Tsirkin
Cc: qemu-trivial, Peter Maydell, qemu-ppc, qemu-devel, patches
11.03.2014 01:59, Michael S. Tsirkin wrote:
> On Mon, Mar 10, 2014 at 07:10:48PM +0000, Peter Maydell wrote:
>> Add U suffix to avoid undefined behaviour.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> While not required for correctness, it would be cleaner
> to change all constants around this line to 1U <<, for consistency.
I agree, this is what I thought as well when looking at the result.
I can fix when applying if you like.
Thanks,
/mjt
>> ---
>> hw/pci-host/apb.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
>> index 1b399dd..a6869b8 100644
>> --- a/hw/pci-host/apb.c
>> +++ b/hw/pci-host/apb.c
>> @@ -58,7 +58,7 @@ do { printf("APB: " fmt , ## __VA_ARGS__); } while (0)
>> #define PBM_PCI_IMR_MASK 0x7fffffff
>> #define PBM_PCI_IMR_ENABLED 0x80000000
>>
>> -#define POR (1 << 31)
>> +#define POR (1U << 31)
>> #define SOFT_POR (1 << 30)
>> #define SOFT_XIR (1 << 29)
>> #define BTN_POR (1 << 28)
>> 1.9.0
>>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2014-03-14 16:45 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-10 19:10 [Qemu-devel] [PATCH 00/12] Avoid shifting left into sign bit Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 01/12] target-i386: " Peter Maydell
2014-03-10 21:57 ` Michael S. Tsirkin
2014-03-10 19:10 ` [Qemu-devel] [PATCH 02/12] hw/intc/apic.c: Use uint32_t for mask word in foreach_apic Peter Maydell
2014-03-10 19:39 ` Stefan Weil
2014-03-10 21:56 ` Michael S. Tsirkin
2014-03-10 19:10 ` [Qemu-devel] [PATCH 03/12] hw/pci/pci_host.c: Avoid shifting left into sign bit Peter Maydell
2014-03-10 20:03 ` Stefan Weil
2014-03-10 21:46 ` Michael S. Tsirkin
2014-03-10 21:58 ` Michael S. Tsirkin
2014-03-10 19:10 ` [Qemu-devel] [PATCH 04/12] hw/i386/acpi_build.c: " Peter Maydell
2014-03-10 21:56 ` Michael S. Tsirkin
2014-03-10 19:10 ` [Qemu-devel] [PATCH 05/12] target-mips: " Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 06/12] hw/usb/hcd-ohci.c: " Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 07/12] hw/intc/openpic: " Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 08/12] hw/ppc: " Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 09/12] tests/libqos/pci-pc: " Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 10/12] hw/intc/slavio_intctl: " Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 11/12] hw/intc/xilinx_intc: " Peter Maydell
2014-03-10 19:10 ` [Qemu-devel] [PATCH 12/12] hw/pci-host/apb.c: " Peter Maydell
2014-03-10 21:59 ` Michael S. Tsirkin
2014-03-14 16:45 ` [Qemu-devel] [Qemu-trivial] " Michael Tokarev
2014-03-10 22:00 ` [Qemu-devel] [PATCH 00/12] " Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.