* [PATCH] exynos4210_gic: Suppress gcc9 format-truncation warnings
@ 2019-10-04 2:55 David Gibson
2019-10-04 9:39 ` Philippe Mathieu-Daudé
2019-10-14 12:51 ` Peter Maydell
0 siblings, 2 replies; 6+ messages in thread
From: David Gibson @ 2019-10-04 2:55 UTC (permalink / raw)
To: i.mitsyanko, peter.maydell; +Cc: qemu-arm, qemu-devel, David Gibson
exynos4210_gic_realize() prints the number of cpus into some temporary
buffers, but it only allows 3 bytes space for it. That's plenty - I'm
pretty sure that existing machines will only ever set this value to 2
(EXYNOS4210_NCPUS). But the compiler can't really be expected to figure
that out.
Some[*] gcc9 versions therefore emit -Wformat-truncation warnings. Fix
that by allowing more space in the temporary buffers - these are on stack
very briefly before being essentially strdup()ed inside the memory region
code, so there's not much cost to doing so.
[*] The bizarre thing here, is that I've long gotten these warnings
compiling in a 32-bit x86 container as host - Fedora 30 with
gcc-9.2.1-1.fc30.i686 - but it compiles just fine on my normal x86_64 host
- Fedora 30 with and gcc-9.2.1-1.fc30.x86_64.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/intc/exynos4210_gic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
index a1b699b6ba..2e5e47f9ec 100644
--- a/hw/intc/exynos4210_gic.c
+++ b/hw/intc/exynos4210_gic.c
@@ -290,8 +290,8 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
const char cpu_prefix[] = "exynos4210-gic-alias_cpu";
const char dist_prefix[] = "exynos4210-gic-alias_dist";
- char cpu_alias_name[sizeof(cpu_prefix) + 3];
- char dist_alias_name[sizeof(cpu_prefix) + 3];
+ char cpu_alias_name[sizeof(cpu_prefix) + 10];
+ char dist_alias_name[sizeof(cpu_prefix) + 10];
SysBusDevice *gicbusdev;
uint32_t i;
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] exynos4210_gic: Suppress gcc9 format-truncation warnings
2019-10-04 2:55 [PATCH] exynos4210_gic: Suppress gcc9 format-truncation warnings David Gibson
@ 2019-10-04 9:39 ` Philippe Mathieu-Daudé
2019-10-14 12:51 ` Peter Maydell
1 sibling, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-04 9:39 UTC (permalink / raw)
To: David Gibson, i.mitsyanko, peter.maydell, Eric Blake; +Cc: qemu-arm, qemu-devel
On 10/4/19 4:55 AM, David Gibson wrote:
> exynos4210_gic_realize() prints the number of cpus into some temporary
> buffers, but it only allows 3 bytes space for it. That's plenty - I'm
> pretty sure that existing machines will only ever set this value to 2
> (EXYNOS4210_NCPUS). But the compiler can't really be expected to figure
> that out.
>
> Some[*] gcc9 versions therefore emit -Wformat-truncation warnings. Fix
> that by allowing more space in the temporary buffers - these are on stack
> very briefly before being essentially strdup()ed inside the memory region
> code, so there's not much cost to doing so.
>
> [*] The bizarre thing here, is that I've long gotten these warnings
> compiling in a 32-bit x86 container as host - Fedora 30 with
> gcc-9.2.1-1.fc30.i686 - but it compiles just fine on my normal x86_64 host
> - Fedora 30 with and gcc-9.2.1-1.fc30.x86_64.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/intc/exynos4210_gic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
> index a1b699b6ba..2e5e47f9ec 100644
> --- a/hw/intc/exynos4210_gic.c
> +++ b/hw/intc/exynos4210_gic.c
> @@ -290,8 +290,8 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> const char cpu_prefix[] = "exynos4210-gic-alias_cpu";
> const char dist_prefix[] = "exynos4210-gic-alias_dist";
> - char cpu_alias_name[sizeof(cpu_prefix) + 3];
> - char dist_alias_name[sizeof(cpu_prefix) + 3];
> + char cpu_alias_name[sizeof(cpu_prefix) + 10];
> + char dist_alias_name[sizeof(cpu_prefix) + 10];
Hmm magic again... So GCC provides a new warning with no helpful
definitions about how to clean this :(
We already have:
#define UUID_FMT_LEN 36
What about adding/using UINT32_FMT_LEN?
> SysBusDevice *gicbusdev;
> uint32_t i;
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] exynos4210_gic: Suppress gcc9 format-truncation warnings
2019-10-04 2:55 [PATCH] exynos4210_gic: Suppress gcc9 format-truncation warnings David Gibson
2019-10-04 9:39 ` Philippe Mathieu-Daudé
@ 2019-10-14 12:51 ` Peter Maydell
2019-11-20 5:27 ` David Gibson
1 sibling, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2019-10-14 12:51 UTC (permalink / raw)
To: David Gibson; +Cc: Igor Mitsyanko, qemu-arm, QEMU Developers
On Fri, 4 Oct 2019 at 04:10, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> exynos4210_gic_realize() prints the number of cpus into some temporary
> buffers, but it only allows 3 bytes space for it. That's plenty - I'm
> pretty sure that existing machines will only ever set this value to 2
> (EXYNOS4210_NCPUS). But the compiler can't really be expected to figure
> that out.
>
> Some[*] gcc9 versions therefore emit -Wformat-truncation warnings. Fix
> that by allowing more space in the temporary buffers - these are on stack
> very briefly before being essentially strdup()ed inside the memory region
> code, so there's not much cost to doing so.
>
> [*] The bizarre thing here, is that I've long gotten these warnings
> compiling in a 32-bit x86 container as host - Fedora 30 with
> gcc-9.2.1-1.fc30.i686 - but it compiles just fine on my normal x86_64 host
> - Fedora 30 with and gcc-9.2.1-1.fc30.x86_64.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/intc/exynos4210_gic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
> index a1b699b6ba..2e5e47f9ec 100644
> --- a/hw/intc/exynos4210_gic.c
> +++ b/hw/intc/exynos4210_gic.c
> @@ -290,8 +290,8 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> const char cpu_prefix[] = "exynos4210-gic-alias_cpu";
> const char dist_prefix[] = "exynos4210-gic-alias_dist";
> - char cpu_alias_name[sizeof(cpu_prefix) + 3];
> - char dist_alias_name[sizeof(cpu_prefix) + 3];
> + char cpu_alias_name[sizeof(cpu_prefix) + 10];
> + char dist_alias_name[sizeof(cpu_prefix) + 10];
> SysBusDevice *gicbusdev;
> uint32_t i;
If we assert() that num_cpu is always <= EXYNOS4210_NCPUS
is that sufficient to clue gcc in that the buffer can't overflow?
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] exynos4210_gic: Suppress gcc9 format-truncation warnings
2019-10-14 12:51 ` Peter Maydell
@ 2019-11-20 5:27 ` David Gibson
2019-11-20 10:31 ` Peter Maydell
0 siblings, 1 reply; 6+ messages in thread
From: David Gibson @ 2019-11-20 5:27 UTC (permalink / raw)
To: Peter Maydell; +Cc: Igor Mitsyanko, qemu-arm, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 2611 bytes --]
On Mon, Oct 14, 2019 at 01:51:39PM +0100, Peter Maydell wrote:
> On Fri, 4 Oct 2019 at 04:10, David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > exynos4210_gic_realize() prints the number of cpus into some temporary
> > buffers, but it only allows 3 bytes space for it. That's plenty - I'm
> > pretty sure that existing machines will only ever set this value to 2
> > (EXYNOS4210_NCPUS). But the compiler can't really be expected to figure
> > that out.
> >
> > Some[*] gcc9 versions therefore emit -Wformat-truncation warnings. Fix
> > that by allowing more space in the temporary buffers - these are on stack
> > very briefly before being essentially strdup()ed inside the memory region
> > code, so there's not much cost to doing so.
> >
> > [*] The bizarre thing here, is that I've long gotten these warnings
> > compiling in a 32-bit x86 container as host - Fedora 30 with
> > gcc-9.2.1-1.fc30.i686 - but it compiles just fine on my normal x86_64 host
> > - Fedora 30 with and gcc-9.2.1-1.fc30.x86_64.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > hw/intc/exynos4210_gic.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
> > index a1b699b6ba..2e5e47f9ec 100644
> > --- a/hw/intc/exynos4210_gic.c
> > +++ b/hw/intc/exynos4210_gic.c
> > @@ -290,8 +290,8 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
> > SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> > const char cpu_prefix[] = "exynos4210-gic-alias_cpu";
> > const char dist_prefix[] = "exynos4210-gic-alias_dist";
> > - char cpu_alias_name[sizeof(cpu_prefix) + 3];
> > - char dist_alias_name[sizeof(cpu_prefix) + 3];
> > + char cpu_alias_name[sizeof(cpu_prefix) + 10];
> > + char dist_alias_name[sizeof(cpu_prefix) + 10];
> > SysBusDevice *gicbusdev;
> > uint32_t i;
>
> If we assert() that num_cpu is always <= EXYNOS4210_NCPUS
> is that sufficient to clue gcc in that the buffer can't overflow?
Interestingly, assert(s->num_cpu <= EXYNOS$210_NCPUS) is *not*
sufficient, but assert(i <= EXYNOS4210_NCPUS) within the loop *is*
enough. I've updated my patch accordingly.
This isn't 4.2 material, obviously. Should I just sit on it until 5.0
opens, or does one of you have someplace to stage the patch in the
meanwhile?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] exynos4210_gic: Suppress gcc9 format-truncation warnings
2019-11-20 5:27 ` David Gibson
@ 2019-11-20 10:31 ` Peter Maydell
2019-11-21 1:39 ` David Gibson
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2019-11-20 10:31 UTC (permalink / raw)
To: David Gibson; +Cc: Igor Mitsyanko, qemu-arm, QEMU Developers
On Wed, 20 Nov 2019 at 05:27, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Mon, Oct 14, 2019 at 01:51:39PM +0100, Peter Maydell wrote:
> > If we assert() that num_cpu is always <= EXYNOS4210_NCPUS
> > is that sufficient to clue gcc in that the buffer can't overflow?
>
> Interestingly, assert(s->num_cpu <= EXYNOS$210_NCPUS) is *not*
> sufficient, but assert(i <= EXYNOS4210_NCPUS) within the loop *is*
> enough. I've updated my patch accordingly.
>
> This isn't 4.2 material, obviously. Should I just sit on it until 5.0
> opens, or does one of you have someplace to stage the patch in the
> meanwhile?
Easy fixes for compiler warnings aren't inherently out of scope
for 4.2. I'm also collecting stuff for 5.0 anyway so I suggest you
just send the patch.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] exynos4210_gic: Suppress gcc9 format-truncation warnings
2019-11-20 10:31 ` Peter Maydell
@ 2019-11-21 1:39 ` David Gibson
0 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2019-11-21 1:39 UTC (permalink / raw)
To: Peter Maydell; +Cc: Igor Mitsyanko, qemu-arm, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]
On Wed, Nov 20, 2019 at 10:31:48AM +0000, Peter Maydell wrote:
> On Wed, 20 Nov 2019 at 05:27, David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > On Mon, Oct 14, 2019 at 01:51:39PM +0100, Peter Maydell wrote:
> > > If we assert() that num_cpu is always <= EXYNOS4210_NCPUS
> > > is that sufficient to clue gcc in that the buffer can't overflow?
> >
> > Interestingly, assert(s->num_cpu <= EXYNOS$210_NCPUS) is *not*
> > sufficient, but assert(i <= EXYNOS4210_NCPUS) within the loop *is*
> > enough. I've updated my patch accordingly.
> >
> > This isn't 4.2 material, obviously. Should I just sit on it until 5.0
> > opens, or does one of you have someplace to stage the patch in the
> > meanwhile?
>
> Easy fixes for compiler warnings aren't inherently out of scope
> for 4.2. I'm also collecting stuff for 5.0 anyway so I suggest you
> just send the patch.
Ok, done.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-21 1:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04 2:55 [PATCH] exynos4210_gic: Suppress gcc9 format-truncation warnings David Gibson
2019-10-04 9:39 ` Philippe Mathieu-Daudé
2019-10-14 12:51 ` Peter Maydell
2019-11-20 5:27 ` David Gibson
2019-11-20 10:31 ` Peter Maydell
2019-11-21 1:39 ` David Gibson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).