[PATCH 0/5] Compilation fixes for 7.0

[PATCH 0/5] Compilation fixes for 7.0

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

UST tracing fails to build on Fedora 36, with lttng-ust-2.13.1-2.fc36.x86_64.

There are also various GCC 12.0.1 warnings worth fixing.

Marc-André Lureau (5):
  trace: fix compilation with lttng-ust >= 2.13
  arm/digic: fix format-truncation warning
  arm/allwinner-a10: fix format-overflow warning
  intc/exynos4210_gic: fix format-overflow warning
  s390x/tcg: fix format-truncation warning

 meson.build                              | 4 ++--
 hw/arm/allwinner-a10.c                   | 4 +---
 hw/arm/digic.c                           | 5 +----
 hw/intc/exynos4210_gic.c                 | 9 +++------
 target/s390x/tcg/translate.c             | 4 ++--
 scripts/tracetool/format/ust_events_h.py | 4 ++--
 6 files changed, 11 insertions(+), 19 deletions(-)

-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH 1/5] trace: fix compilation with lttng-ust >= 2.13

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

On Fedora 36, with lttng-ust 2.13.1, compilation fails with:

In file included from trace/trace-ust-all.h:49085,
                 from trace/trace-ust-all.c:13:
/usr/include/lttng/tracepoint-event.h:67:10: error: #include expects "FILENAME" or <FILENAME>
   67 | #include LTTNG_UST_TRACEPOINT_INCLUDE
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

In lttng-ust commit 41858e2b6e8 ("Fix: don't do macro expansion in
tracepoint file name") from 2012, starting from lttng-ust 2.1, the API
was changed to expect TRACEPOINT_INCLUDE to be defined as a string.

In lttng-ust commit d2966b4b0b2 ("Remove TRACEPOINT_INCLUDE_FILE
macro"), in 2021, the compatibility macro was removed.

Use the "new" API from 2012, and bump the version requirement to 2.1 to
fix compilation with >= 2.13.

According to repology, all distributions we support have >= 2.1 (centos
8 has oldest with 2.8.1 afaict)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 meson.build                              | 4 ++--
 scripts/tracetool/format/ust_events_h.py | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index aef724ad3c0e..13c8b01b2aec 100644
--- a/meson.build
+++ b/meson.build
@@ -455,8 +455,8 @@ if 'CONFIG_GIO' in config_host
 endif
 lttng = not_found
 if 'ust' in get_option('trace_backends')
-  lttng = dependency('lttng-ust', required: true, method: 'pkg-config',
-                     kwargs: static_kwargs)
+  lttng = dependency('lttng-ust', required: true, version: '>= 2.1',
+                     method: 'pkg-config', kwargs: static_kwargs)
 endif
 pixman = not_found
 if have_system or have_tools
diff --git a/scripts/tracetool/format/ust_events_h.py b/scripts/tracetool/format/ust_events_h.py
index 6ce559f6cc6e..b99fe6896baf 100644
--- a/scripts/tracetool/format/ust_events_h.py
+++ b/scripts/tracetool/format/ust_events_h.py
@@ -29,8 +29,8 @@ def generate(events, backend, group):
         '#undef TRACEPOINT_PROVIDER',
         '#define TRACEPOINT_PROVIDER qemu',
         '',
-        '#undef TRACEPOINT_INCLUDE_FILE',
-        '#define TRACEPOINT_INCLUDE_FILE ./%s' % include,
+        '#undef TRACEPOINT_INCLUDE',
+        '#define TRACEPOINT_INCLUDE "./%s"' % include,
         '',
         '#if !defined (TRACE_%s_GENERATED_UST_H) || \\'  % group.upper(),
         '     defined(TRACEPOINT_HEADER_MULTI_READ)',
-- 
2.35.1.273.ge6ebfd0e8cbb

Re: [PATCH 1/5] trace: fix compilation with lttng-ust >= 2.13

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

On Mon, Mar 28, 2022 at 12:47:13PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> On Fedora 36, with lttng-ust 2.13.1, compilation fails with:
> 
> In file included from trace/trace-ust-all.h:49085,
>                  from trace/trace-ust-all.c:13:
> /usr/include/lttng/tracepoint-event.h:67:10: error: #include expects "FILENAME" or <FILENAME>
>    67 | #include LTTNG_UST_TRACEPOINT_INCLUDE
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> In lttng-ust commit 41858e2b6e8 ("Fix: don't do macro expansion in
> tracepoint file name") from 2012, starting from lttng-ust 2.1, the API
> was changed to expect TRACEPOINT_INCLUDE to be defined as a string.
> 
> In lttng-ust commit d2966b4b0b2 ("Remove TRACEPOINT_INCLUDE_FILE
> macro"), in 2021, the compatibility macro was removed.
> 
> Use the "new" API from 2012, and bump the version requirement to 2.1 to
> fix compilation with >= 2.13.
> 
> According to repology, all distributions we support have >= 2.1 (centos
> 8 has oldest with 2.8.1 afaict)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  meson.build                              | 4 ++--
>  scripts/tracetool/format/ust_events_h.py | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Thank you!

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/5] trace: fix compilation with lttng-ust >= 2.13

[Marc-André Lureau]

Hi

On Wed, Mar 30, 2022 at 4:06 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Mon, Mar 28, 2022 at 12:47:13PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > On Fedora 36, with lttng-ust 2.13.1, compilation fails with:
> >
> > In file included from trace/trace-ust-all.h:49085,
> >                  from trace/trace-ust-all.c:13:
> > /usr/include/lttng/tracepoint-event.h:67:10: error: #include expects "FILENAME" or <FILENAME>
> >    67 | #include LTTNG_UST_TRACEPOINT_INCLUDE
> >       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > In lttng-ust commit 41858e2b6e8 ("Fix: don't do macro expansion in
> > tracepoint file name") from 2012, starting from lttng-ust 2.1, the API
> > was changed to expect TRACEPOINT_INCLUDE to be defined as a string.
> >
> > In lttng-ust commit d2966b4b0b2 ("Remove TRACEPOINT_INCLUDE_FILE
> > macro"), in 2021, the compatibility macro was removed.
> >
> > Use the "new" API from 2012, and bump the version requirement to 2.1 to
> > fix compilation with >= 2.13.
> >
> > According to repology, all distributions we support have >= 2.1 (centos
> > 8 has oldest with 2.8.1 afaict)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  meson.build                              | 4 ++--
> >  scripts/tracetool/format/ust_events_h.py | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
>
> Thank you!
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Given that there is a dependency bump, would you rather wait for 7.1
or do you think it should be included in 7.0? I think the latter
should be safe.

thanks

Re: [PATCH 1/5] trace: fix compilation with lttng-ust >= 2.13

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1979 bytes --]

On Thu, Mar 31, 2022 at 07:45:48PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Mar 30, 2022 at 4:06 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Mon, Mar 28, 2022 at 12:47:13PM +0400, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > On Fedora 36, with lttng-ust 2.13.1, compilation fails with:
> > >
> > > In file included from trace/trace-ust-all.h:49085,
> > >                  from trace/trace-ust-all.c:13:
> > > /usr/include/lttng/tracepoint-event.h:67:10: error: #include expects "FILENAME" or <FILENAME>
> > >    67 | #include LTTNG_UST_TRACEPOINT_INCLUDE
> > >       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > In lttng-ust commit 41858e2b6e8 ("Fix: don't do macro expansion in
> > > tracepoint file name") from 2012, starting from lttng-ust 2.1, the API
> > > was changed to expect TRACEPOINT_INCLUDE to be defined as a string.
> > >
> > > In lttng-ust commit d2966b4b0b2 ("Remove TRACEPOINT_INCLUDE_FILE
> > > macro"), in 2021, the compatibility macro was removed.
> > >
> > > Use the "new" API from 2012, and bump the version requirement to 2.1 to
> > > fix compilation with >= 2.13.
> > >
> > > According to repology, all distributions we support have >= 2.1 (centos
> > > 8 has oldest with 2.8.1 afaict)
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  meson.build                              | 4 ++--
> > >  scripts/tracetool/format/ust_events_h.py | 4 ++--
> > >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > Thank you!
> >
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Given that there is a dependency bump, would you rather wait for 7.1
> or do you think it should be included in 7.0? I think the latter
> should be safe.

Yes, since you've investigated the minimum version available across
supported distros I think we can apply this patch.

Thanks,
Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 2/5] arm/digic: fix format-truncation warning

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

../hw/arm/digic.c: In function ‘digic_init’:
../hw/arm/digic.c:45:54: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 5 [-Werror=format-truncation=]
   45 |         snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
      |                                                      ^~

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/arm/digic.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index 614232165cdc..6df554797734 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -39,10 +39,7 @@ static void digic_init(Object *obj)
     object_initialize_child(obj, "cpu", &s->cpu, ARM_CPU_TYPE_NAME("arm946"));
 
     for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
-#define DIGIC_TIMER_NAME_MLEN    11
-        char name[DIGIC_TIMER_NAME_MLEN];
-
-        snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
+        g_autofree char *name = g_strdup_printf("timer[%d]", i);
         object_initialize_child(obj, name, &s->timer[i], TYPE_DIGIC_TIMER);
     }
 
-- 
2.35.1.273.ge6ebfd0e8cbb

Re: [PATCH 2/5] arm/digic: fix format-truncation warning

[Peter Maydell]

On Mon, 28 Mar 2022 at 09:47, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> ../hw/arm/digic.c: In function ‘digic_init’:
> ../hw/arm/digic.c:45:54: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 5 [-Werror=format-truncation=]
>    45 |         snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
>       |                                                      ^~
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/arm/digic.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/hw/arm/digic.c b/hw/arm/digic.c
> index 614232165cdc..6df554797734 100644
> --- a/hw/arm/digic.c
> +++ b/hw/arm/digic.c
> @@ -39,10 +39,7 @@ static void digic_init(Object *obj)
>      object_initialize_child(obj, "cpu", &s->cpu, ARM_CPU_TYPE_NAME("arm946"));
>
>      for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> -#define DIGIC_TIMER_NAME_MLEN    11
> -        char name[DIGIC_TIMER_NAME_MLEN];
> -
> -        snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
> +        g_autofree char *name = g_strdup_printf("timer[%d]", i);
>          object_initialize_child(obj, name, &s->timer[i], TYPE_DIGIC_TIMER);
>      }

This is a clear compiler false positive, given that DIGIC4_NB_TIMERS is 3.
But this is a nicer way to write the code than the static array anyway.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 2/5] arm/digic: fix format-truncation warning

[Thomas Huth]

On 28/03/2022 10.47, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> ../hw/arm/digic.c: In function ‘digic_init’:
> ../hw/arm/digic.c:45:54: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 5 [-Werror=format-truncation=]
>     45 |         snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
>        |                                                      ^~
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   hw/arm/digic.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/hw/arm/digic.c b/hw/arm/digic.c
> index 614232165cdc..6df554797734 100644
> --- a/hw/arm/digic.c
> +++ b/hw/arm/digic.c
> @@ -39,10 +39,7 @@ static void digic_init(Object *obj)
>       object_initialize_child(obj, "cpu", &s->cpu, ARM_CPU_TYPE_NAME("arm946"));
>   
>       for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> -#define DIGIC_TIMER_NAME_MLEN    11
> -        char name[DIGIC_TIMER_NAME_MLEN];
> -
> -        snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
> +        g_autofree char *name = g_strdup_printf("timer[%d]", i);
>           object_initialize_child(obj, name, &s->timer[i], TYPE_DIGIC_TIMER);
>       }
>   

DIGIC4_NB_TIMERS is 3 ... so the compiler should also be able to see that 
the string fits fine into the name[] array here. That really sounds like 
your GCC 12.0 is buggy. Please open a compiler bug ticket.

  Thomas

[PATCH 3/5] arm/allwinner-a10: fix format-overflow warning

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

../hw/arm/allwinner-a10.c: In function ‘aw_a10_realize’:
../hw/arm/allwinner-a10.c:135:35: error: ‘%d’ directive writing between 1 and 11 bytes into a region of size 8 [-Werror=format-overflow=]
  135 |             sprintf(bus, "usb-bus.%d", i);
      |                                   ^~

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/arm/allwinner-a10.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index 05e84728cb34..79082289ea5b 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -130,9 +130,7 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
         int i;
 
         for (i = 0; i < AW_A10_NUM_USB; i++) {
-            char bus[16];
-
-            sprintf(bus, "usb-bus.%d", i);
+            g_autofree char *bus = g_strdup_printf("usb-bus.%d", i);
 
             object_property_set_bool(OBJECT(&s->ehci[i]), "companion-enable",
                                      true, &error_fatal);
-- 
2.35.1.273.ge6ebfd0e8cbb

Re: [PATCH 3/5] arm/allwinner-a10: fix format-overflow warning

[Peter Maydell]

On Mon, 28 Mar 2022 at 09:48, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> ../hw/arm/allwinner-a10.c: In function ‘aw_a10_realize’:
> ../hw/arm/allwinner-a10.c:135:35: error: ‘%d’ directive writing between 1 and 11 bytes into a region of size 8 [-Werror=format-overflow=]
>   135 |             sprintf(bus, "usb-bus.%d", i);
>       |                                   ^~
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/arm/allwinner-a10.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
> index 05e84728cb34..79082289ea5b 100644
> --- a/hw/arm/allwinner-a10.c
> +++ b/hw/arm/allwinner-a10.c
> @@ -130,9 +130,7 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
>          int i;
>
>          for (i = 0; i < AW_A10_NUM_USB; i++) {
> -            char bus[16];
> -
> -            sprintf(bus, "usb-bus.%d", i);
> +            g_autofree char *bus = g_strdup_printf("usb-bus.%d", i);
>
>              object_property_set_bool(OBJECT(&s->ehci[i]), "companion-enable",
>                                       true, &error_fatal);

Another false positive. Maybe we should feed this back to the
compiler devs for them to improve their warning generation?

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH 4/5] intc/exynos4210_gic: fix format-overflow warning

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/intc/exynos4210_gic.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
index bc73d1f11524..5b6f4330a5f3 100644
--- a/hw/intc/exynos4210_gic.c
+++ b/hw/intc/exynos4210_gic.c
@@ -289,10 +289,6 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
     Object *obj = OBJECT(dev);
     Exynos4210GicState *s = EXYNOS4210_GIC(obj);
     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];
     SysBusDevice *gicbusdev;
     uint32_t n = s->num_cpu;
     uint32_t i;
@@ -322,8 +318,10 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
      */
     assert(n <= EXYNOS4210_NCPUS);
     for (i = 0; i < n; i++) {
+        g_autofree char *cpu_alias_name = g_strdup_printf("exynos4210-gic-alias_cpu%x", i);
+        g_autofree char *dist_alias_name = g_strdup_printf("exynos4210-gic-alias_dist%x", i);
+
         /* Map CPU interface per SMP Core */
-        sprintf(cpu_alias_name, "%s%x", cpu_prefix, i);
         memory_region_init_alias(&s->cpu_alias[i], obj,
                                  cpu_alias_name,
                                  sysbus_mmio_get_region(gicbusdev, 1),
@@ -333,7 +331,6 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
                 EXYNOS4210_EXT_GIC_CPU_GET_OFFSET(i), &s->cpu_alias[i]);
 
         /* Map Distributor per SMP Core */
-        sprintf(dist_alias_name, "%s%x", dist_prefix, i);
         memory_region_init_alias(&s->dist_alias[i], obj,
                                  dist_alias_name,
                                  sysbus_mmio_get_region(gicbusdev, 0),
-- 
2.35.1.273.ge6ebfd0e8cbb

Re: [PATCH 4/5] intc/exynos4210_gic: fix format-overflow warning

[Peter Maydell]

On Mon, 28 Mar 2022 at 09:48, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/intc/exynos4210_gic.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
> index bc73d1f11524..5b6f4330a5f3 100644
> --- a/hw/intc/exynos4210_gic.c
> +++ b/hw/intc/exynos4210_gic.c
> @@ -289,10 +289,6 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
>      Object *obj = OBJECT(dev);
>      Exynos4210GicState *s = EXYNOS4210_GIC(obj);
>      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];
>      SysBusDevice *gicbusdev;
>      uint32_t n = s->num_cpu;
>      uint32_t i;
> @@ -322,8 +318,10 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
>       */
>      assert(n <= EXYNOS4210_NCPUS);
>      for (i = 0; i < n; i++) {
> +        g_autofree char *cpu_alias_name = g_strdup_printf("exynos4210-gic-alias_cpu%x", i);
> +        g_autofree char *dist_alias_name = g_strdup_printf("exynos4210-gic-alias_dist%x", i);
> +
>          /* Map CPU interface per SMP Core */
> -        sprintf(cpu_alias_name, "%s%x", cpu_prefix, i);
>          memory_region_init_alias(&s->cpu_alias[i], obj,
>                                   cpu_alias_name,
>                                   sysbus_mmio_get_region(gicbusdev, 1),
> @@ -333,7 +331,6 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
>                  EXYNOS4210_EXT_GIC_CPU_GET_OFFSET(i), &s->cpu_alias[i]);
>
>          /* Map Distributor per SMP Core */
> -        sprintf(dist_alias_name, "%s%x", dist_prefix, i);
>          memory_region_init_alias(&s->dist_alias[i], obj,
>                                   dist_alias_name,
>                                   sysbus_mmio_get_region(gicbusdev, 0),
> --

More false positives, but here the new code is massively better than the old.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 4/5] intc/exynos4210_gic: fix format-overflow warning

[Philippe Mathieu-Daudé]

On 28/3/22 11:15, Peter Maydell wrote:
> On Mon, 28 Mar 2022 at 09:48, <marcandre.lureau@redhat.com> wrote:
>>
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>   hw/intc/exynos4210_gic.c | 9 +++------
>>   1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
>> index bc73d1f11524..5b6f4330a5f3 100644
>> --- a/hw/intc/exynos4210_gic.c
>> +++ b/hw/intc/exynos4210_gic.c
>> @@ -289,10 +289,6 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
>>       Object *obj = OBJECT(dev);
>>       Exynos4210GicState *s = EXYNOS4210_GIC(obj);
>>       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];
>>       SysBusDevice *gicbusdev;
>>       uint32_t n = s->num_cpu;
>>       uint32_t i;
>> @@ -322,8 +318,10 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
>>        */
>>       assert(n <= EXYNOS4210_NCPUS);
>>       for (i = 0; i < n; i++) {
>> +        g_autofree char *cpu_alias_name = g_strdup_printf("exynos4210-gic-alias_cpu%x", i);
>> +        g_autofree char *dist_alias_name = g_strdup_printf("exynos4210-gic-alias_dist%x", i);

Dubious '%x' format... Maybe change to '%u'?

>>           /* Map CPU interface per SMP Core */
>> -        sprintf(cpu_alias_name, "%s%x", cpu_prefix, i);
>>           memory_region_init_alias(&s->cpu_alias[i], obj,
>>                                    cpu_alias_name,
>>                                    sysbus_mmio_get_region(gicbusdev, 1),
>> @@ -333,7 +331,6 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
>>                   EXYNOS4210_EXT_GIC_CPU_GET_OFFSET(i), &s->cpu_alias[i]);
>>
>>           /* Map Distributor per SMP Core */
>> -        sprintf(dist_alias_name, "%s%x", dist_prefix, i);
>>           memory_region_init_alias(&s->dist_alias[i], obj,
>>                                    dist_alias_name,
>>                                    sysbus_mmio_get_region(gicbusdev, 0),
>> --
> 
> More false positives, but here the new code is massively better than the old.
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> thanks
> -- PMM
> 

Re: [PATCH 4/5] intc/exynos4210_gic: fix format-overflow warning

[Peter Maydell]

On Tue, 29 Mar 2022 at 18:08, Philippe Mathieu-Daudé
<philippe.mathieu.daude@gmail.com> wrote:
>
> On 28/3/22 11:15, Peter Maydell wrote:
> > On Mon, 28 Mar 2022 at 09:48, <marcandre.lureau@redhat.com> wrote:
> >>
> >> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> ---
> >>   hw/intc/exynos4210_gic.c | 9 +++------
> >>   1 file changed, 3 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
> >> index bc73d1f11524..5b6f4330a5f3 100644
> >> --- a/hw/intc/exynos4210_gic.c
> >> +++ b/hw/intc/exynos4210_gic.c
> >> @@ -289,10 +289,6 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
> >>       Object *obj = OBJECT(dev);
> >>       Exynos4210GicState *s = EXYNOS4210_GIC(obj);
> >>       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];
> >>       SysBusDevice *gicbusdev;
> >>       uint32_t n = s->num_cpu;
> >>       uint32_t i;
> >> @@ -322,8 +318,10 @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp)
> >>        */
> >>       assert(n <= EXYNOS4210_NCPUS);
> >>       for (i = 0; i < n; i++) {
> >> +        g_autofree char *cpu_alias_name = g_strdup_printf("exynos4210-gic-alias_cpu%x", i);
> >> +        g_autofree char *dist_alias_name = g_strdup_printf("exynos4210-gic-alias_dist%x", i);
>
> Dubious '%x' format... Maybe change to '%u'?

Makes no difference given that EXYNOS4210_NCPUS is 2, but I guess so.

-- PMM

[PATCH 5/5] s390x/tcg: fix format-truncation warning

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

../target/s390x/tcg/translate.c: In function ‘s390x_translate_init’:
../target/s390x/tcg/translate.c:224:64: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 3 [-Werror=format-truncation=]
  224 |         snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
      |                                                                ^~

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 target/s390x/tcg/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 5acfc0ff9b4e..a082342a0424 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -199,7 +199,7 @@ static TCGv_i64 regs[16];
 
 void s390x_translate_init(void)
 {
-    int i;
+    size_t i;
 
     psw_addr = tcg_global_mem_new_i64(cpu_env,
                                       offsetof(CPUS390XState, psw.addr),
@@ -221,7 +221,7 @@ void s390x_translate_init(void)
                                    "cc_vr");
 
     for (i = 0; i < 16; i++) {
-        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
+        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%zu", i);
         regs[i] = tcg_global_mem_new(cpu_env,
                                      offsetof(CPUS390XState, regs[i]),
                                      cpu_reg_names[i]);
-- 
2.35.1.273.ge6ebfd0e8cbb

Re: [PATCH 5/5] s390x/tcg: fix format-truncation warning

[Thomas Huth]

On 28/03/2022 10.47, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> ../target/s390x/tcg/translate.c: In function ‘s390x_translate_init’:
> ../target/s390x/tcg/translate.c:224:64: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 3 [-Werror=format-truncation=]
>    224 |         snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
>        |                                                                ^~
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   target/s390x/tcg/translate.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 5acfc0ff9b4e..a082342a0424 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -199,7 +199,7 @@ static TCGv_i64 regs[16];
>   
>   void s390x_translate_init(void)
>   {
> -    int i;
> +    size_t i;
>   
>       psw_addr = tcg_global_mem_new_i64(cpu_env,
>                                         offsetof(CPUS390XState, psw.addr),
> @@ -221,7 +221,7 @@ void s390x_translate_init(void)
>                                      "cc_vr");
>   
>       for (i = 0; i < 16; i++) {
> -        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
> +        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%zu", i);

The compiler is *really* too stupid to see that i is in the range between 0 
and 16 here??? ... that rather sounds like a compiler bug to me than 
something that we should work-around in the QEMU source code. Considering 
that you're using a x.0 release of GCC, please file a bug against GCC 
instead. If they refuse to fix it for GCC 12.1 or later, we can revisit this 
patch, but currently, I really think this should be fixed in GCC instead.

  Thomas

Re: [PATCH 5/5] s390x/tcg: fix format-truncation warning

[Marc-André Lureau]

Hi

On Mon, Mar 28, 2022 at 12:59 PM Thomas Huth <thuth@redhat.com> wrote:
>
> On 28/03/2022 10.47, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > ../target/s390x/tcg/translate.c: In function ‘s390x_translate_init’:
> > ../target/s390x/tcg/translate.c:224:64: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 3 [-Werror=format-truncation=]
> >    224 |         snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
> >        |                                                                ^~
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   target/s390x/tcg/translate.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> > index 5acfc0ff9b4e..a082342a0424 100644
> > --- a/target/s390x/tcg/translate.c
> > +++ b/target/s390x/tcg/translate.c
> > @@ -199,7 +199,7 @@ static TCGv_i64 regs[16];
> >
> >   void s390x_translate_init(void)
> >   {
> > -    int i;
> > +    size_t i;
> >
> >       psw_addr = tcg_global_mem_new_i64(cpu_env,
> >                                         offsetof(CPUS390XState, psw.addr),
> > @@ -221,7 +221,7 @@ void s390x_translate_init(void)
> >                                      "cc_vr");
> >
> >       for (i = 0; i < 16; i++) {
> > -        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
> > +        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%zu", i);
>
> The compiler is *really* too stupid to see that i is in the range between 0
> and 16 here??? ... that rather sounds like a compiler bug to me than
> something that we should work-around in the QEMU source code. Considering
> that you're using a x.0 release of GCC, please file a bug against GCC
> instead. If they refuse to fix it for GCC 12.1 or later, we can revisit this
> patch, but currently, I really think this should be fixed in GCC instead.
>

I opened: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105080

Although I think the "workaround" is simple and would avoid users &
distributions having to add --disable-werror, and possibly miss other
issues.

Re: [PATCH 5/5] s390x/tcg: fix format-truncation warning

[Thomas Huth]

On 28/03/2022 12.22, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Mar 28, 2022 at 12:59 PM Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 28/03/2022 10.47, marcandre.lureau@redhat.com wrote:
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> ../target/s390x/tcg/translate.c: In function ‘s390x_translate_init’:
>>> ../target/s390x/tcg/translate.c:224:64: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 3 [-Werror=format-truncation=]
>>>     224 |         snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
>>>         |                                                                ^~
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>    target/s390x/tcg/translate.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
>>> index 5acfc0ff9b4e..a082342a0424 100644
>>> --- a/target/s390x/tcg/translate.c
>>> +++ b/target/s390x/tcg/translate.c
>>> @@ -199,7 +199,7 @@ static TCGv_i64 regs[16];
>>>
>>>    void s390x_translate_init(void)
>>>    {
>>> -    int i;
>>> +    size_t i;
>>>
>>>        psw_addr = tcg_global_mem_new_i64(cpu_env,
>>>                                          offsetof(CPUS390XState, psw.addr),
>>> @@ -221,7 +221,7 @@ void s390x_translate_init(void)
>>>                                       "cc_vr");
>>>
>>>        for (i = 0; i < 16; i++) {
>>> -        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
>>> +        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%zu", i);
>>
>> The compiler is *really* too stupid to see that i is in the range between 0
>> and 16 here??? ... that rather sounds like a compiler bug to me than
>> something that we should work-around in the QEMU source code. Considering
>> that you're using a x.0 release of GCC, please file a bug against GCC
>> instead. If they refuse to fix it for GCC 12.1 or later, we can revisit this
>> patch, but currently, I really think this should be fixed in GCC instead.
>>
> 
> I opened: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105080
> 
> Although I think the "workaround" is simple and would avoid users &
> distributions having to add --disable-werror, and possibly miss other
> issues.

But I really dislike adding such noise to the history (it spoils "git blame" 
for example) if it is not really necessary. In this case, it also rather 
helps to show distros that they are shipping a buggy compiler and they 
should fix their compiler instead before it hits their users. So I think it 
would be rather bad to fix this in QEMU now.

  Thomas

Re: [PATCH 5/5] s390x/tcg: fix format-truncation warning

[Thomas Huth]

On 28/03/2022 12.22, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Mar 28, 2022 at 12:59 PM Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 28/03/2022 10.47, marcandre.lureau@redhat.com wrote:
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> ../target/s390x/tcg/translate.c: In function ‘s390x_translate_init’:
>>> ../target/s390x/tcg/translate.c:224:64: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 3 [-Werror=format-truncation=]
>>>     224 |         snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
>>>         |                                                                ^~
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>    target/s390x/tcg/translate.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
>>> index 5acfc0ff9b4e..a082342a0424 100644
>>> --- a/target/s390x/tcg/translate.c
>>> +++ b/target/s390x/tcg/translate.c
>>> @@ -199,7 +199,7 @@ static TCGv_i64 regs[16];
>>>
>>>    void s390x_translate_init(void)
>>>    {
>>> -    int i;
>>> +    size_t i;
>>>
>>>        psw_addr = tcg_global_mem_new_i64(cpu_env,
>>>                                          offsetof(CPUS390XState, psw.addr),
>>> @@ -221,7 +221,7 @@ void s390x_translate_init(void)
>>>                                       "cc_vr");
>>>
>>>        for (i = 0; i < 16; i++) {
>>> -        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
>>> +        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%zu", i);
>>
>> The compiler is *really* too stupid to see that i is in the range between 0
>> and 16 here??? ... that rather sounds like a compiler bug to me than
>> something that we should work-around in the QEMU source code. Considering
>> that you're using a x.0 release of GCC, please file a bug against GCC
>> instead. If they refuse to fix it for GCC 12.1 or later, we can revisit this
>> patch, but currently, I really think this should be fixed in GCC instead.
>>
> 
> I opened: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105080

Thanks! Looks like it will be fixed in the final release of GCC 12, so I 
think we can definitely drop this QEMU patch here.

  Thomas

Re: [PATCH 5/5] s390x/tcg: fix format-truncation warning

[David Hildenbrand]

On 28.03.22 10:47, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> ../target/s390x/tcg/translate.c: In function ‘s390x_translate_init’:
> ../target/s390x/tcg/translate.c:224:64: error: ‘%d’ directive output may be truncated writing between 1 and 11 bytes into a region of size 3 [-Werror=format-truncation=]
>   224 |         snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
>       |                                                                ^~
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  target/s390x/tcg/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 5acfc0ff9b4e..a082342a0424 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -199,7 +199,7 @@ static TCGv_i64 regs[16];
>  
>  void s390x_translate_init(void)
>  {
> -    int i;
> +    size_t i;
>  
>      psw_addr = tcg_global_mem_new_i64(cpu_env,
>                                        offsetof(CPUS390XState, psw.addr),
> @@ -221,7 +221,7 @@ void s390x_translate_init(void)
>                                     "cc_vr");
>  
>      for (i = 0; i < 16; i++) {
> -        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%d", i);
> +        snprintf(cpu_reg_names[i], sizeof(cpu_reg_names[0]), "r%zu", i);
>          regs[i] = tcg_global_mem_new(cpu_env,
>                                       offsetof(CPUS390XState, regs[i]),
>                                       cpu_reg_names[i]);

Ehm, what?

How can "r0" ... "r15" ever consume more than 3 bytes + "\0"?

-- 
Thanks,

David / dhildenb