| Message ID | 20200608071409.17024-1-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/openrisc/openrisc_sim: Add assertion to silent GCC warning | expand |
On 08/06/2020 09.14, Philippe Mathieu-Daudé wrote: > When compiling with GCC 10 (Fedora 32) using CFLAGS=-O2 we get: > > CC or1k-softmmu/hw/openrisc/openrisc_sim.o > hw/openrisc/openrisc_sim.c: In function ‘openrisc_sim_init’: > hw/openrisc/openrisc_sim.c:87:42: error: ‘cpu_irqs[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > 87 | sysbus_connect_irq(s, i, cpu_irqs[i][irq_pin]); > | ~~~~~~~~^~~ > > While humans can tell smp_cpus will always be in the [1, 2] range, > (openrisc_sim_machine_init sets mc->max_cpus = 2), the compiler > can't. > > Add an assertion to give the compiler a hint there's no use of > uninitialized data. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1874073 > Reported-by: Martin Liška <mliska@suse.cz> > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/openrisc/openrisc_sim.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c > index d08ce61811..02f5259e5e 100644 > --- a/hw/openrisc/openrisc_sim.c > +++ b/hw/openrisc/openrisc_sim.c > @@ -134,6 +134,7 @@ static void openrisc_sim_init(MachineState *machine) > int n; > unsigned int smp_cpus = machine->smp.cpus; > > + assert(smp_cpus >= 1 && smp_cpus <= 2); > for (n = 0; n < smp_cpus; n++) { > cpu = OPENRISC_CPU(cpu_create(machine->cpu_type)); > if (cpu == NULL) { > Reviewed-by: Thomas Huth <thuth@redhat.com>
On 6/8/20 2:14 AM, Philippe Mathieu-Daudé wrote: > When compiling with GCC 10 (Fedora 32) using CFLAGS=-O2 we get: In the subject: s/silent/silence/ > > CC or1k-softmmu/hw/openrisc/openrisc_sim.o > hw/openrisc/openrisc_sim.c: In function ‘openrisc_sim_init’: > hw/openrisc/openrisc_sim.c:87:42: error: ‘cpu_irqs[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > 87 | sysbus_connect_irq(s, i, cpu_irqs[i][irq_pin]); > | ~~~~~~~~^~~ > > While humans can tell smp_cpus will always be in the [1, 2] range, > (openrisc_sim_machine_init sets mc->max_cpus = 2), the compiler > can't. > > Add an assertion to give the compiler a hint there's no use of > uninitialized data. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1874073 > Reported-by: Martin Liška <mliska@suse.cz> > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/openrisc/openrisc_sim.c | 1 + > 1 file changed, 1 insertion(+) Tested-by: Eric Blake <eblake@redhat.com> With the typo fixed, Reviewed-by: Eric Blake <eblake@redhat.com>
On Mon, Jun 08, 2020 at 10:33:24AM -0500, Eric Blake wrote: > On 6/8/20 2:14 AM, Philippe Mathieu-Daudé wrote: > > When compiling with GCC 10 (Fedora 32) using CFLAGS=-O2 we get: > > In the subject: s/silent/silence/ > > > > > CC or1k-softmmu/hw/openrisc/openrisc_sim.o > > hw/openrisc/openrisc_sim.c: In function ‘openrisc_sim_init’: > > hw/openrisc/openrisc_sim.c:87:42: error: ‘cpu_irqs[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > > 87 | sysbus_connect_irq(s, i, cpu_irqs[i][irq_pin]); > > | ~~~~~~~~^~~ > > > > While humans can tell smp_cpus will always be in the [1, 2] range, > > (openrisc_sim_machine_init sets mc->max_cpus = 2), the compiler > > can't. > > > > Add an assertion to give the compiler a hint there's no use of > > uninitialized data. > > > > Buglink: https://bugs.launchpad.net/qemu/+bug/1874073 > > Reported-by: Martin Liška <mliska@suse.cz> > > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > --- > > hw/openrisc/openrisc_sim.c | 1 + > > 1 file changed, 1 insertion(+) > > Tested-by: Eric Blake <eblake@redhat.com> > > With the typo fixed, > Reviewed-by: Eric Blake <eblake@redhat.com> Acked-by: Stafford Horne <shorne@gmail.com> I see there are now two patches for this, I kind of like this assert fix. Shall I queue it for OpenRISC pulling? Or can someone else pick this up? -Stafford
On Wed, Jun 10, 2020 at 05:42:27AM +0900, Stafford Horne wrote: > On Mon, Jun 08, 2020 at 10:33:24AM -0500, Eric Blake wrote: > > On 6/8/20 2:14 AM, Philippe Mathieu-Daudé wrote: > > > When compiling with GCC 10 (Fedora 32) using CFLAGS=-O2 we get: > > > > In the subject: s/silent/silence/ > > > > > > > > CC or1k-softmmu/hw/openrisc/openrisc_sim.o > > > hw/openrisc/openrisc_sim.c: In function ‘openrisc_sim_init’: > > > hw/openrisc/openrisc_sim.c:87:42: error: ‘cpu_irqs[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > > > 87 | sysbus_connect_irq(s, i, cpu_irqs[i][irq_pin]); > > > | ~~~~~~~~^~~ > > > > > > While humans can tell smp_cpus will always be in the [1, 2] range, > > > (openrisc_sim_machine_init sets mc->max_cpus = 2), the compiler > > > can't. > > > > > > Add an assertion to give the compiler a hint there's no use of > > > uninitialized data. > > > > > > Buglink: https://bugs.launchpad.net/qemu/+bug/1874073 > > > Reported-by: Martin Liška <mliska@suse.cz> > > > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > --- > > > hw/openrisc/openrisc_sim.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > Tested-by: Eric Blake <eblake@redhat.com> > > > > With the typo fixed, > > Reviewed-by: Eric Blake <eblake@redhat.com> > > Acked-by: Stafford Horne <shorne@gmail.com> > > I see there are now two patches for this, I kind of like this assert fix. > > Shall I queue it for OpenRISC pulling? Or can someone else pick this up? Hello, Sorry, I see v2 of this patch has already been picked up. -Stafford
On 6/9/20 3:42 PM, Stafford Horne wrote: >>> While humans can tell smp_cpus will always be in the [1, 2] range, >>> (openrisc_sim_machine_init sets mc->max_cpus = 2), the compiler >>> can't. >>> >>> Add an assertion to give the compiler a hint there's no use of >>> uninitialized data. >>> > Acked-by: Stafford Horne <shorne@gmail.com> > > I see there are now two patches for this, I kind of like this assert fix. > > Shall I queue it for OpenRISC pulling? Or can someone else pick this up? Looks like Laurent already volunteered to queue it through the trivial patches tree.
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c index d08ce61811..02f5259e5e 100644 --- a/hw/openrisc/openrisc_sim.c +++ b/hw/openrisc/openrisc_sim.c @@ -134,6 +134,7 @@ static void openrisc_sim_init(MachineState *machine) int n; unsigned int smp_cpus = machine->smp.cpus; + assert(smp_cpus >= 1 && smp_cpus <= 2); for (n = 0; n < smp_cpus; n++) { cpu = OPENRISC_CPU(cpu_create(machine->cpu_type)); if (cpu == NULL) {
When compiling with GCC 10 (Fedora 32) using CFLAGS=-O2 we get: CC or1k-softmmu/hw/openrisc/openrisc_sim.o hw/openrisc/openrisc_sim.c: In function ‘openrisc_sim_init’: hw/openrisc/openrisc_sim.c:87:42: error: ‘cpu_irqs[0]’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 87 | sysbus_connect_irq(s, i, cpu_irqs[i][irq_pin]); | ~~~~~~~~^~~ While humans can tell smp_cpus will always be in the [1, 2] range, (openrisc_sim_machine_init sets mc->max_cpus = 2), the compiler can't. Add an assertion to give the compiler a hint there's no use of uninitialized data. Buglink: https://bugs.launchpad.net/qemu/+bug/1874073 Reported-by: Martin Liška <mliska@suse.cz> Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/openrisc/openrisc_sim.c | 1 + 1 file changed, 1 insertion(+)