[Qemu-devel] [PATCH v1 0/5] Fix some GCC 9 build warnings

[Qemu-devel] [PATCH v1 0/5] Fix some GCC 9 build warnings

[Alistair Francis]

Alistair Francis (5):
  util/qemu-sockets: Fix GCC 9 build warnings
  hw/usb/hcd-xhci: Fix GCC 9 build warning
  hw/usb/dev-mtp: Fix GCC 9 build warning
  linux-user/uname: Fix GCC 9 build warnings
  linux-user/elfload: Fix GCC 9 build warnings

 hw/usb/dev-mtp.c     | 13 +++++++++++++
 hw/usb/hcd-xhci.h    |  2 +-
 linux-user/elfload.c |  4 ++--
 linux-user/uname.c   |  8 ++++++++
 util/qemu-sockets.c  |  4 ++--
 5 files changed, 26 insertions(+), 5 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v1 0/5] Fix some GCC 9 build warnings

[Alistair Francis]

Alistair Francis (5):
  util/qemu-sockets: Fix GCC 9 build warnings
  hw/usb/hcd-xhci: Fix GCC 9 build warning
  hw/usb/dev-mtp: Fix GCC 9 build warning
  linux-user/uname: Fix GCC 9 build warnings
  linux-user/elfload: Fix GCC 9 build warnings

 hw/usb/dev-mtp.c     | 13 +++++++++++++
 hw/usb/hcd-xhci.h    |  2 +-
 linux-user/elfload.c |  4 ++--
 linux-user/uname.c   |  8 ++++++++
 util/qemu-sockets.c  |  4 ++--
 5 files changed, 26 insertions(+), 5 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v1 1/5] util/qemu-sockets: Fix GCC 9 build warnings

[Alistair Francis]

Fix this warning when building with GCC9 on Fedora 30:
In function ‘strncpy’,
    inlined from ‘unix_connect_saddr.isra.0’ at util/qemu-sockets.c:925:5:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘strncpy’,
    inlined from ‘unix_listen_saddr.isra.0’ at util/qemu-sockets.c:880:5:

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 util/qemu-sockets.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 9705051690..4322652428 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -877,7 +877,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
 
     memset(&un, 0, sizeof(un));
     un.sun_family = AF_UNIX;
-    strncpy(un.sun_path, path, sizeof(un.sun_path));
+    strncpy(un.sun_path, path, sizeof(un.sun_path) - 1);
 
     if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
         error_setg_errno(errp, errno, "Failed to bind socket to %s", path);
@@ -922,7 +922,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
 
     memset(&un, 0, sizeof(un));
     un.sun_family = AF_UNIX;
-    strncpy(un.sun_path, saddr->path, sizeof(un.sun_path));
+    strncpy(un.sun_path, saddr->path, sizeof(un.sun_path) - 1);
 
     /* connect to peer */
     do {
-- 
2.21.0

[Qemu-devel] [PATCH v1 1/5] util/qemu-sockets: Fix GCC 9 build warnings

[Alistair Francis]

Fix this warning when building with GCC9 on Fedora 30:
In function ‘strncpy’,
    inlined from ‘unix_connect_saddr.isra.0’ at util/qemu-sockets.c:925:5:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘strncpy’,
    inlined from ‘unix_listen_saddr.isra.0’ at util/qemu-sockets.c:880:5:

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 util/qemu-sockets.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 9705051690..4322652428 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -877,7 +877,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
 
     memset(&un, 0, sizeof(un));
     un.sun_family = AF_UNIX;
-    strncpy(un.sun_path, path, sizeof(un.sun_path));
+    strncpy(un.sun_path, path, sizeof(un.sun_path) - 1);
 
     if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
         error_setg_errno(errp, errno, "Failed to bind socket to %s", path);
@@ -922,7 +922,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
 
     memset(&un, 0, sizeof(un));
     un.sun_family = AF_UNIX;
-    strncpy(un.sun_path, saddr->path, sizeof(un.sun_path));
+    strncpy(un.sun_path, saddr->path, sizeof(un.sun_path) - 1);
 
     /* connect to peer */
     do {
-- 
2.21.0

[Qemu-devel] [PATCH v1 2/5] hw/usb/hcd-xhci: Fix GCC 9 build warning

[Alistair Francis]

Fix this build warning with GCC 9 on Fedora 30:
hw/usb/hcd-xhci.c:3339:66: error: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Werror=format-truncation=]
 3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
      |                                                                  ^~
hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
 3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
      |                                                      ^~~~~~~~~~~~~~~
In file included from /usr/include/stdio.h:867,
                 from /home/alistair/qemu/include/qemu/osdep.h:99,
                 from hw/usb/hcd-xhci.c:21:
/usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output between 13 and 22 bytes into a destination of size 16
   67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |        __bos (__s), __fmt, __va_arg_pack ());
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/usb/hcd-xhci.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index 240caa4e51..9e4988abb6 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -133,7 +133,7 @@ typedef struct XHCIPort {
     uint32_t portnr;
     USBPort  *uport;
     uint32_t speedmask;
-    char name[16];
+    char name[24];
     MemoryRegion mem;
 } XHCIPort;
 
-- 
2.21.0

[Qemu-devel] [PATCH v1 2/5] hw/usb/hcd-xhci: Fix GCC 9 build warning

[Alistair Francis]

Fix this build warning with GCC 9 on Fedora 30:
hw/usb/hcd-xhci.c:3339:66: error: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Werror=format-truncation=]
 3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
      |                                                                  ^~
hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
 3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
      |                                                      ^~~~~~~~~~~~~~~
In file included from /usr/include/stdio.h:867,
                 from /home/alistair/qemu/include/qemu/osdep.h:99,
                 from hw/usb/hcd-xhci.c:21:
/usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output between 13 and 22 bytes into a destination of size 16
   67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |        __bos (__s), __fmt, __va_arg_pack ());
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/usb/hcd-xhci.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index 240caa4e51..9e4988abb6 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -133,7 +133,7 @@ typedef struct XHCIPort {
     uint32_t portnr;
     USBPort  *uport;
     uint32_t speedmask;
-    char name[16];
+    char name[24];
     MemoryRegion mem;
 } XHCIPort;
 
-- 
2.21.0

[Qemu-devel] [PATCH v1 3/5] hw/usb/dev-mtp: Fix GCC 9 build warning

[Alistair Francis]

Fix this warning with GCC 9 on Fedora 30:
hw/usb/dev-mtp.c:1715:36: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
 1715 |                             dataset->filename);
      |                             ~~~~~~~^~~~~~~~~~

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/usb/dev-mtp.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 99548b012d..6de85d99e6 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1711,9 +1711,22 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
     assert(!s->write_pending);
     assert(p != NULL);
 
+/*
+ * We are about to access a packed struct. We are confident that the pointer
+ * address won't be unalligned, so we ignore GCC warnings.
+ */
+#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
+#endif
+
     filename = utf16_to_str(MIN(dataset->length, filename_chars),
                             dataset->filename);
 
+#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
+#pragma GCC diagnostic pop
+#endif
+
     if (strchr(filename, '/')) {
         usb_mtp_queue_result(s, RES_PARAMETER_NOT_SUPPORTED, d->trans,
                              0, 0, 0, 0);
-- 
2.21.0

[Qemu-devel] [PATCH v1 3/5] hw/usb/dev-mtp: Fix GCC 9 build warning

[Alistair Francis]

Fix this warning with GCC 9 on Fedora 30:
hw/usb/dev-mtp.c:1715:36: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
 1715 |                             dataset->filename);
      |                             ~~~~~~~^~~~~~~~~~

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/usb/dev-mtp.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 99548b012d..6de85d99e6 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1711,9 +1711,22 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
     assert(!s->write_pending);
     assert(p != NULL);
 
+/*
+ * We are about to access a packed struct. We are confident that the pointer
+ * address won't be unalligned, so we ignore GCC warnings.
+ */
+#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
+#endif
+
     filename = utf16_to_str(MIN(dataset->length, filename_chars),
                             dataset->filename);
 
+#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
+#pragma GCC diagnostic pop
+#endif
+
     if (strchr(filename, '/')) {
         usb_mtp_queue_result(s, RES_PARAMETER_NOT_SUPPORTED, d->trans,
                              0, 0, 0, 0);
-- 
2.21.0

[Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Alistair Francis]

Fix this warning when building with GCC9 on Fedora 30:
In function ‘strncpy’,
    inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 linux-user/uname.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/linux-user/uname.c b/linux-user/uname.c
index 313b79dbad..293b2238f2 100644
--- a/linux-user/uname.c
+++ b/linux-user/uname.c
@@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
    * struct linux kernel uses).
    */
 
+#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstringop-truncation"
+#endif
+
   memset(buf, 0, sizeof(*buf));
   COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
   COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);
@@ -101,6 +106,9 @@ int sys_uname(struct new_utsname *buf)
 #endif
   return (0);
 
+#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
+#pragma GCC diagnostic pop
+#endif
 #undef COPY_UTSNAME_FIELD
 }
 
-- 
2.21.0

[Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Alistair Francis]

Fix this warning when building with GCC9 on Fedora 30:
In function ‘strncpy’,
    inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 linux-user/uname.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/linux-user/uname.c b/linux-user/uname.c
index 313b79dbad..293b2238f2 100644
--- a/linux-user/uname.c
+++ b/linux-user/uname.c
@@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
    * struct linux kernel uses).
    */
 
+#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstringop-truncation"
+#endif
+
   memset(buf, 0, sizeof(*buf));
   COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
   COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);
@@ -101,6 +106,9 @@ int sys_uname(struct new_utsname *buf)
 #endif
   return (0);
 
+#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
+#pragma GCC diagnostic pop
+#endif
 #undef COPY_UTSNAME_FIELD
 }
 
-- 
2.21.0

[Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Alistair Francis]

Fix this warning when building with GCC9 on Fedora 30:
In function ‘strncpy’,
    inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
    inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
    inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 linux-user/elfload.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index c1a26021f8..cbb7fc10fa 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3206,7 +3206,7 @@ static int fill_psinfo(struct target_elf_prpsinfo *psinfo, const TaskState *ts)
      * this field is not NUL-terminated.
      */
     (void) strncpy(psinfo->pr_fname, base_filename,
-                   sizeof(psinfo->pr_fname));
+                   sizeof(psinfo->pr_fname) - 1);
 
     g_free(base_filename);
     bswap_psinfo(psinfo);
@@ -3389,7 +3389,7 @@ static int fill_note_info(struct elf_note_info *info,
               sizeof (*info->prstatus), info->prstatus);
     fill_psinfo(info->psinfo, ts);
     fill_note(&info->notes[1], "CORE", NT_PRPSINFO,
-              sizeof (*info->psinfo), info->psinfo);
+              sizeof(*info->psinfo) - 1, info->psinfo);
     fill_auxv_note(&info->notes[2], ts);
     info->numnote = 3;
 
-- 
2.21.0

[Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Alistair Francis]

Fix this warning when building with GCC9 on Fedora 30:
In function ‘strncpy’,
    inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
    inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
    inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
/usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 linux-user/elfload.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index c1a26021f8..cbb7fc10fa 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3206,7 +3206,7 @@ static int fill_psinfo(struct target_elf_prpsinfo *psinfo, const TaskState *ts)
      * this field is not NUL-terminated.
      */
     (void) strncpy(psinfo->pr_fname, base_filename,
-                   sizeof(psinfo->pr_fname));
+                   sizeof(psinfo->pr_fname) - 1);
 
     g_free(base_filename);
     bswap_psinfo(psinfo);
@@ -3389,7 +3389,7 @@ static int fill_note_info(struct elf_note_info *info,
               sizeof (*info->prstatus), info->prstatus);
     fill_psinfo(info->psinfo, ts);
     fill_note(&info->notes[1], "CORE", NT_PRPSINFO,
-              sizeof (*info->psinfo), info->psinfo);
+              sizeof(*info->psinfo) - 1, info->psinfo);
     fill_auxv_note(&info->notes[2], ts);
     info->numnote = 3;
 
-- 
2.21.0

Re: [Qemu-devel] [PATCH v1 1/5] util/qemu-sockets: Fix GCC 9 build warnings

[Laurent Vivier]

Le 30/04/2019 à 22:08, Alistair Francis a écrit :
> Fix this warning when building with GCC9 on Fedora 30:
> In function ‘strncpy’,
>     inlined from ‘unix_connect_saddr.isra.0’ at util/qemu-sockets.c:925:5:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In function ‘strncpy’,
>     inlined from ‘unix_listen_saddr.isra.0’ at util/qemu-sockets.c:880:5:
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  util/qemu-sockets.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 9705051690..4322652428 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -877,7 +877,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
>  
>      memset(&un, 0, sizeof(un));
>      un.sun_family = AF_UNIX;
> -    strncpy(un.sun_path, path, sizeof(un.sun_path));
> +    strncpy(un.sun_path, path, sizeof(un.sun_path) - 1);
>  
>      if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
>          error_setg_errno(errp, errno, "Failed to bind socket to %s", path);
> @@ -922,7 +922,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
>  
>      memset(&un, 0, sizeof(un));
>      un.sun_family = AF_UNIX;
> -    strncpy(un.sun_path, saddr->path, sizeof(un.sun_path));
> +    strncpy(un.sun_path, saddr->path, sizeof(un.sun_path) - 1);
>  
>      /* connect to peer */
>      do {
> 

Your change reverts partially:

commit ad9579aaa16d5b385922d49edac2c96c79bcfb62
Author: Daniel P. Berrange <berrange@redhat.com>
Date:   Thu May 25 16:53:00 2017 +0100

    sockets: improve error reporting if UNIX socket path is too long

    The 'struct sockaddr_un' only allows 108 bytes for the socket
    path.

    If the user supplies a path, QEMU uses snprintf() to silently
    truncate it when too long. This is undesirable because the user
    will then be unable to connect to the path they asked for.

    If the user doesn't supply a path, QEMU builds one based on
    TMPDIR, but if that leads to an overlong path, it mistakenly
    uses error_setg_errno() with a stale errno value, because
    snprintf() does not set errno on truncation.

    In solving this the code needed some refactoring to ensure we
    don't pass 'un.sun_path' directly to any APIs which expect
    NUL-terminated strings, because the path is not required to
    be terminated.

    Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
    Message-Id: <20170525155300.22743-1-berrange@redhat.com>
    Reviewed-by: Eric Blake <eblake@redhat.com>
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 1/5] util/qemu-sockets: Fix GCC 9 build warnings

[Laurent Vivier]

Le 30/04/2019 à 22:08, Alistair Francis a écrit :
> Fix this warning when building with GCC9 on Fedora 30:
> In function ‘strncpy’,
>     inlined from ‘unix_connect_saddr.isra.0’ at util/qemu-sockets.c:925:5:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In function ‘strncpy’,
>     inlined from ‘unix_listen_saddr.isra.0’ at util/qemu-sockets.c:880:5:
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  util/qemu-sockets.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 9705051690..4322652428 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -877,7 +877,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
>  
>      memset(&un, 0, sizeof(un));
>      un.sun_family = AF_UNIX;
> -    strncpy(un.sun_path, path, sizeof(un.sun_path));
> +    strncpy(un.sun_path, path, sizeof(un.sun_path) - 1);
>  
>      if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) {
>          error_setg_errno(errp, errno, "Failed to bind socket to %s", path);
> @@ -922,7 +922,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
>  
>      memset(&un, 0, sizeof(un));
>      un.sun_family = AF_UNIX;
> -    strncpy(un.sun_path, saddr->path, sizeof(un.sun_path));
> +    strncpy(un.sun_path, saddr->path, sizeof(un.sun_path) - 1);
>  
>      /* connect to peer */
>      do {
> 

Your change reverts partially:

commit ad9579aaa16d5b385922d49edac2c96c79bcfb62
Author: Daniel P. Berrange <berrange@redhat.com>
Date:   Thu May 25 16:53:00 2017 +0100

    sockets: improve error reporting if UNIX socket path is too long

    The 'struct sockaddr_un' only allows 108 bytes for the socket
    path.

    If the user supplies a path, QEMU uses snprintf() to silently
    truncate it when too long. This is undesirable because the user
    will then be unable to connect to the path they asked for.

    If the user doesn't supply a path, QEMU builds one based on
    TMPDIR, but if that leads to an overlong path, it mistakenly
    uses error_setg_errno() with a stale errno value, because
    snprintf() does not set errno on truncation.

    In solving this the code needed some refactoring to ensure we
    don't pass 'un.sun_path' directly to any APIs which expect
    NUL-terminated strings, because the path is not required to
    be terminated.

    Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
    Message-Id: <20170525155300.22743-1-berrange@redhat.com>
    Reviewed-by: Eric Blake <eblake@redhat.com>
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 2/5] hw/usb/hcd-xhci: Fix GCC 9 build warning

[Laurent Vivier]

Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> Fix this build warning with GCC 9 on Fedora 30:
> hw/usb/hcd-xhci.c:3339:66: error: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Werror=format-truncation=]
>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>       |                                                                  ^~
> hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);

"i" cannot be greater than 15.

perhaps an "assert(i <= MAX(MAXPORTS_2, MAXPORTS_3))" can fix the warning ?

Thanks,
Laurent

>       |                                                      ^~~~~~~~~~~~~~~
> In file included from /usr/include/stdio.h:867,
>                  from /home/alistair/qemu/include/qemu/osdep.h:99,
>                  from hw/usb/hcd-xhci.c:21:
> /usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output between 13 and 22 bytes into a destination of size 16
>    67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    68 |        __bos (__s), __fmt, __va_arg_pack ());
>       |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/usb/hcd-xhci.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
> index 240caa4e51..9e4988abb6 100644
> --- a/hw/usb/hcd-xhci.h
> +++ b/hw/usb/hcd-xhci.h
> @@ -133,7 +133,7 @@ typedef struct XHCIPort {
>      uint32_t portnr;
>      USBPort  *uport;
>      uint32_t speedmask;
> -    char name[16];
> +    char name[24];
>      MemoryRegion mem;
>  } XHCIPort;
>  
> 

Re: [Qemu-devel] [PATCH v1 2/5] hw/usb/hcd-xhci: Fix GCC 9 build warning

[Laurent Vivier]

Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> Fix this build warning with GCC 9 on Fedora 30:
> hw/usb/hcd-xhci.c:3339:66: error: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Werror=format-truncation=]
>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>       |                                                                  ^~
> hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);

"i" cannot be greater than 15.

perhaps an "assert(i <= MAX(MAXPORTS_2, MAXPORTS_3))" can fix the warning ?

Thanks,
Laurent

>       |                                                      ^~~~~~~~~~~~~~~
> In file included from /usr/include/stdio.h:867,
>                  from /home/alistair/qemu/include/qemu/osdep.h:99,
>                  from hw/usb/hcd-xhci.c:21:
> /usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output between 13 and 22 bytes into a destination of size 16
>    67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    68 |        __bos (__s), __fmt, __va_arg_pack ());
>       |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/usb/hcd-xhci.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
> index 240caa4e51..9e4988abb6 100644
> --- a/hw/usb/hcd-xhci.h
> +++ b/hw/usb/hcd-xhci.h
> @@ -133,7 +133,7 @@ typedef struct XHCIPort {
>      uint32_t portnr;
>      USBPort  *uport;
>      uint32_t speedmask;
> -    char name[16];
> +    char name[24];
>      MemoryRegion mem;
>  } XHCIPort;
>  
> 

Re: [Qemu-devel] [PATCH v1 1/5] util/qemu-sockets: Fix GCC 9 build warnings

[Eric Blake]

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

On 4/30/19 3:08 PM, Alistair Francis wrote:
> Fix this warning when building with GCC9 on Fedora 30:
> In function ‘strncpy’,
>     inlined from ‘unix_connect_saddr.isra.0’ at util/qemu-sockets.c:925:5:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In function ‘strncpy’,
>     inlined from ‘unix_listen_saddr.isra.0’ at util/qemu-sockets.c:880:5:
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  util/qemu-sockets.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 9705051690..4322652428 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -877,7 +877,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
>  
>      memset(&un, 0, sizeof(un));
>      un.sun_family = AF_UNIX;
> -    strncpy(un.sun_path, path, sizeof(un.sun_path));
> +    strncpy(un.sun_path, path, sizeof(un.sun_path) - 1);

NACK. Linux allows you to use the full width of un.sun_path (a NUL
terminator is required if you copy less than that, but not if you use
the full width). Rather, we may need to mark path as a potential
nonstring to silence the warning, or use memcpy instead of strncpy, or
some other workaround.  (Sadly, this is one of those odd places where
strncpy is actually the right function to use, but there are so many
other places where strncpy is used incorrectly that it has turned into a
battle to use it here)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v1 1/5] util/qemu-sockets: Fix GCC 9 build warnings

[Eric Blake]

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

On 4/30/19 3:08 PM, Alistair Francis wrote:
> Fix this warning when building with GCC9 on Fedora 30:
> In function ‘strncpy’,
>     inlined from ‘unix_connect_saddr.isra.0’ at util/qemu-sockets.c:925:5:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In function ‘strncpy’,
>     inlined from ‘unix_listen_saddr.isra.0’ at util/qemu-sockets.c:880:5:
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  util/qemu-sockets.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 9705051690..4322652428 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -877,7 +877,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
>  
>      memset(&un, 0, sizeof(un));
>      un.sun_family = AF_UNIX;
> -    strncpy(un.sun_path, path, sizeof(un.sun_path));
> +    strncpy(un.sun_path, path, sizeof(un.sun_path) - 1);

NACK. Linux allows you to use the full width of un.sun_path (a NUL
terminator is required if you copy less than that, but not if you use
the full width). Rather, we may need to mark path as a potential
nonstring to silence the warning, or use memcpy instead of strncpy, or
some other workaround.  (Sadly, this is one of those odd places where
strncpy is actually the right function to use, but there are so many
other places where strncpy is used incorrectly that it has turned into a
battle to use it here)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v1 3/5] hw/usb/dev-mtp: Fix GCC 9 build warning

[Laurent Vivier]

Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> Fix this warning with GCC 9 on Fedora 30:
> hw/usb/dev-mtp.c:1715:36: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
>  1715 |                             dataset->filename);
>       |                             ~~~~~~~^~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/usb/dev-mtp.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 99548b012d..6de85d99e6 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1711,9 +1711,22 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
>      assert(!s->write_pending);
>      assert(p != NULL);
>  
> +/*
> + * We are about to access a packed struct. We are confident that the pointer
> + * address won't be unalligned, so we ignore GCC warnings.
> + */
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
> +#endif
> +
>      filename = utf16_to_str(MIN(dataset->length, filename_chars),
>                              dataset->filename);
>  
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic pop
> +#endif
> +
>      if (strchr(filename, '/')) {
>          usb_mtp_queue_result(s, RES_PARAMETER_NOT_SUPPORTED, d->trans,
>                               0, 0, 0, 0);
> 

You should move and use PRAGMA_DISABLE_PACKED_WARNING and
PRAGMA_REENABLE_PACKED_WARNING from linux-user/qemu.h.

It has laready been very well tested :)

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 3/5] hw/usb/dev-mtp: Fix GCC 9 build warning

[Laurent Vivier]

Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> Fix this warning with GCC 9 on Fedora 30:
> hw/usb/dev-mtp.c:1715:36: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
>  1715 |                             dataset->filename);
>       |                             ~~~~~~~^~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/usb/dev-mtp.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 99548b012d..6de85d99e6 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1711,9 +1711,22 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
>      assert(!s->write_pending);
>      assert(p != NULL);
>  
> +/*
> + * We are about to access a packed struct. We are confident that the pointer
> + * address won't be unalligned, so we ignore GCC warnings.
> + */
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
> +#endif
> +
>      filename = utf16_to_str(MIN(dataset->length, filename_chars),
>                              dataset->filename);
>  
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic pop
> +#endif
> +
>      if (strchr(filename, '/')) {
>          usb_mtp_queue_result(s, RES_PARAMETER_NOT_SUPPORTED, d->trans,
>                               0, 0, 0, 0);
> 

You should move and use PRAGMA_DISABLE_PACKED_WARNING and
PRAGMA_REENABLE_PACKED_WARNING from linux-user/qemu.h.

It has laready been very well tested :)

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Laurent Vivier]

Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> Fix this warning when building with GCC9 on Fedora 30:
> In function ‘strncpy’,
>     inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  linux-user/uname.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/linux-user/uname.c b/linux-user/uname.c
> index 313b79dbad..293b2238f2 100644
> --- a/linux-user/uname.c
> +++ b/linux-user/uname.c
> @@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
>     * struct linux kernel uses).
>     */
>  
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
> +#endif
> +
>    memset(buf, 0, sizeof(*buf));
>    COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
>    COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);
> @@ -101,6 +106,9 @@ int sys_uname(struct new_utsname *buf)
>  #endif
>    return (0);
>  
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic pop
> +#endif
>  #undef COPY_UTSNAME_FIELD
>  }
>  
> 

You should use PRAGMA_DISABLE_PACKED_WARNING and
PRAGMA_REENABLE_PACKED_WARNING from linux-user/qemu.h.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Laurent Vivier]

Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> Fix this warning when building with GCC9 on Fedora 30:
> In function ‘strncpy’,
>     inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  linux-user/uname.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/linux-user/uname.c b/linux-user/uname.c
> index 313b79dbad..293b2238f2 100644
> --- a/linux-user/uname.c
> +++ b/linux-user/uname.c
> @@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
>     * struct linux kernel uses).
>     */
>  
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
> +#endif
> +
>    memset(buf, 0, sizeof(*buf));
>    COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
>    COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);
> @@ -101,6 +106,9 @@ int sys_uname(struct new_utsname *buf)
>  #endif
>    return (0);
>  
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic pop
> +#endif
>  #undef COPY_UTSNAME_FIELD
>  }
>  
> 

You should use PRAGMA_DISABLE_PACKED_WARNING and
PRAGMA_REENABLE_PACKED_WARNING from linux-user/qemu.h.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Laurent Vivier]

Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> Fix this warning when building with GCC9 on Fedora 30:
> In function ‘strncpy’,
>     inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
>     inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
>     inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  linux-user/elfload.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index c1a26021f8..cbb7fc10fa 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3206,7 +3206,7 @@ static int fill_psinfo(struct target_elf_prpsinfo *psinfo, const TaskState *ts)

       /*
        * Using strncpy here is fine: at max-length,
>       * this field is not NUL-terminated.
>       */>      (void) strncpy(psinfo->pr_fname, base_filename,
> -                   sizeof(psinfo->pr_fname));
> +                   sizeof(psinfo->pr_fname) - 1);

Read the comment above :)

>  
>      g_free(base_filename);
>      bswap_psinfo(psinfo);
> @@ -3389,7 +3389,7 @@ static int fill_note_info(struct elf_note_info *info,
>                sizeof (*info->prstatus), info->prstatus);
>      fill_psinfo(info->psinfo, ts);
>      fill_note(&info->notes[1], "CORE", NT_PRPSINFO,
> -              sizeof (*info->psinfo), info->psinfo);
> +              sizeof(*info->psinfo) - 1, info->psinfo);

Why?

>      fill_auxv_note(&info->notes[2], ts);
>      info->numnote = 3;
>  
> 

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Laurent Vivier]

Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> Fix this warning when building with GCC9 on Fedora 30:
> In function ‘strncpy’,
>     inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
>     inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
>     inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  linux-user/elfload.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index c1a26021f8..cbb7fc10fa 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3206,7 +3206,7 @@ static int fill_psinfo(struct target_elf_prpsinfo *psinfo, const TaskState *ts)

       /*
        * Using strncpy here is fine: at max-length,
>       * this field is not NUL-terminated.
>       */>      (void) strncpy(psinfo->pr_fname, base_filename,
> -                   sizeof(psinfo->pr_fname));
> +                   sizeof(psinfo->pr_fname) - 1);

Read the comment above :)

>  
>      g_free(base_filename);
>      bswap_psinfo(psinfo);
> @@ -3389,7 +3389,7 @@ static int fill_note_info(struct elf_note_info *info,
>                sizeof (*info->prstatus), info->prstatus);
>      fill_psinfo(info->psinfo, ts);
>      fill_note(&info->notes[1], "CORE", NT_PRPSINFO,
> -              sizeof (*info->psinfo), info->psinfo);
> +              sizeof(*info->psinfo) - 1, info->psinfo);

Why?

>      fill_auxv_note(&info->notes[2], ts);
>      info->numnote = 3;
>  
> 

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 3/5] hw/usb/dev-mtp: Fix GCC 9 build warning

[Eric Blake]

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

On 4/30/19 3:09 PM, Alistair Francis wrote:
> Fix this warning with GCC 9 on Fedora 30:
> hw/usb/dev-mtp.c:1715:36: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
>  1715 |                             dataset->filename);
>       |                             ~~~~~~~^~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/usb/dev-mtp.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 99548b012d..6de85d99e6 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1711,9 +1711,22 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
>      assert(!s->write_pending);
>      assert(p != NULL);
>  
> +/*
> + * We are about to access a packed struct. We are confident that the pointer
> + * address won't be unalligned, so we ignore GCC warnings.

unaligned

Aren't there other potential proposed patches floating around for this
issue that do not involve messing with pragmas?

> + */
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
> +#endif
> +
>      filename = utf16_to_str(MIN(dataset->length, filename_chars),
>                              dataset->filename);
>  
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic pop
> +#endif
> +
>      if (strchr(filename, '/')) {
>          usb_mtp_queue_result(s, RES_PARAMETER_NOT_SUPPORTED, d->trans,
>                               0, 0, 0, 0);
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v1 3/5] hw/usb/dev-mtp: Fix GCC 9 build warning

[Eric Blake]

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

On 4/30/19 3:09 PM, Alistair Francis wrote:
> Fix this warning with GCC 9 on Fedora 30:
> hw/usb/dev-mtp.c:1715:36: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
>  1715 |                             dataset->filename);
>       |                             ~~~~~~~^~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/usb/dev-mtp.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 99548b012d..6de85d99e6 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -1711,9 +1711,22 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
>      assert(!s->write_pending);
>      assert(p != NULL);
>  
> +/*
> + * We are about to access a packed struct. We are confident that the pointer
> + * address won't be unalligned, so we ignore GCC warnings.

unaligned

Aren't there other potential proposed patches floating around for this
issue that do not involve messing with pragmas?

> + */
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
> +#endif
> +
>      filename = utf16_to_str(MIN(dataset->length, filename_chars),
>                              dataset->filename);
>  
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic pop
> +#endif
> +
>      if (strchr(filename, '/')) {
>          usb_mtp_queue_result(s, RES_PARAMETER_NOT_SUPPORTED, d->trans,
>                               0, 0, 0, 0);
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Eric Blake]

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

On 4/30/19 3:09 PM, Alistair Francis wrote:
> Fix this warning when building with GCC9 on Fedora 30:
> In function ‘strncpy’,
>     inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  linux-user/uname.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/linux-user/uname.c b/linux-user/uname.c
> index 313b79dbad..293b2238f2 100644
> --- a/linux-user/uname.c
> +++ b/linux-user/uname.c
> @@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
>     * struct linux kernel uses).
>     */
>  
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
> +#endif

Why do we need the pragma?

> +
>    memset(buf, 0, sizeof(*buf));

We are prezeroing the entire field, at which point...

>    COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
>    COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);

...using strncpy() for a shorter string is wasteful (we're writing the
tail end twice), and for a long string is warning-prone.  Why not
rewrite COPY_UTSNAME_FIELD() to use memcpy() for the MIN(strlen(src),
__NEW_UTS_LEN) and drop the write of the trailing NUL, since it will
already be NUL from your memset()?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Eric Blake]

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

On 4/30/19 3:09 PM, Alistair Francis wrote:
> Fix this warning when building with GCC9 on Fedora 30:
> In function ‘strncpy’,
>     inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  linux-user/uname.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/linux-user/uname.c b/linux-user/uname.c
> index 313b79dbad..293b2238f2 100644
> --- a/linux-user/uname.c
> +++ b/linux-user/uname.c
> @@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
>     * struct linux kernel uses).
>     */
>  
> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
> +#endif

Why do we need the pragma?

> +
>    memset(buf, 0, sizeof(*buf));

We are prezeroing the entire field, at which point...

>    COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
>    COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);

...using strncpy() for a shorter string is wasteful (we're writing the
tail end twice), and for a long string is warning-prone.  Why not
rewrite COPY_UTSNAME_FIELD() to use memcpy() for the MIN(strlen(src),
__NEW_UTS_LEN) and drop the write of the trailing NUL, since it will
already be NUL from your memset()?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v1 3/5] hw/usb/dev-mtp: Fix GCC 9 build warning

[Laurent Vivier]

Le 30/04/2019 à 22:28, Laurent Vivier a écrit :
> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
>> Fix this warning with GCC 9 on Fedora 30:
>> hw/usb/dev-mtp.c:1715:36: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
>>  1715 |                             dataset->filename);
>>       |                             ~~~~~~~^~~~~~~~~~
>>
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  hw/usb/dev-mtp.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
>> index 99548b012d..6de85d99e6 100644
>> --- a/hw/usb/dev-mtp.c
>> +++ b/hw/usb/dev-mtp.c
>> @@ -1711,9 +1711,22 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
>>      assert(!s->write_pending);
>>      assert(p != NULL);
>>  
>> +/*
>> + * We are about to access a packed struct. We are confident that the pointer
>> + * address won't be unalligned, so we ignore GCC warnings.
>> + */
>> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
>> +#endif
>> +
>>      filename = utf16_to_str(MIN(dataset->length, filename_chars),
>>                              dataset->filename);
>>  
>> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
>> +#pragma GCC diagnostic pop
>> +#endif
>> +
>>      if (strchr(filename, '/')) {
>>          usb_mtp_queue_result(s, RES_PARAMETER_NOT_SUPPORTED, d->trans,
>>                               0, 0, 0, 0);
>>
> 
> You should move and use PRAGMA_DISABLE_PACKED_WARNING and
> PRAGMA_REENABLE_PACKED_WARNING from linux-user/qemu.h.
> 
> It has laready been very well tested :)


Oh, they are for clang and a false positive, so forget my comment (and
the following one)

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 3/5] hw/usb/dev-mtp: Fix GCC 9 build warning

[Laurent Vivier]

Le 30/04/2019 à 22:28, Laurent Vivier a écrit :
> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
>> Fix this warning with GCC 9 on Fedora 30:
>> hw/usb/dev-mtp.c:1715:36: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
>>  1715 |                             dataset->filename);
>>       |                             ~~~~~~~^~~~~~~~~~
>>
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  hw/usb/dev-mtp.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
>> index 99548b012d..6de85d99e6 100644
>> --- a/hw/usb/dev-mtp.c
>> +++ b/hw/usb/dev-mtp.c
>> @@ -1711,9 +1711,22 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
>>      assert(!s->write_pending);
>>      assert(p != NULL);
>>  
>> +/*
>> + * We are about to access a packed struct. We are confident that the pointer
>> + * address won't be unalligned, so we ignore GCC warnings.
>> + */
>> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
>> +#endif
>> +
>>      filename = utf16_to_str(MIN(dataset->length, filename_chars),
>>                              dataset->filename);
>>  
>> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
>> +#pragma GCC diagnostic pop
>> +#endif
>> +
>>      if (strchr(filename, '/')) {
>>          usb_mtp_queue_result(s, RES_PARAMETER_NOT_SUPPORTED, d->trans,
>>                               0, 0, 0, 0);
>>
> 
> You should move and use PRAGMA_DISABLE_PACKED_WARNING and
> PRAGMA_REENABLE_PACKED_WARNING from linux-user/qemu.h.
> 
> It has laready been very well tested :)


Oh, they are for clang and a false positive, so forget my comment (and
the following one)

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 3/5] hw/usb/dev-mtp: Fix GCC 9 build warning

[Alistair Francis]

On Tue, Apr 30, 2019 at 1:37 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 4/30/19 3:09 PM, Alistair Francis wrote:
> > Fix this warning with GCC 9 on Fedora 30:
> > hw/usb/dev-mtp.c:1715:36: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
> >  1715 |                             dataset->filename);
> >       |                             ~~~~~~~^~~~~~~~~~
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  hw/usb/dev-mtp.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> > index 99548b012d..6de85d99e6 100644
> > --- a/hw/usb/dev-mtp.c
> > +++ b/hw/usb/dev-mtp.c
> > @@ -1711,9 +1711,22 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
> >      assert(!s->write_pending);
> >      assert(p != NULL);
> >
> > +/*
> > + * We are about to access a packed struct. We are confident that the pointer
> > + * address won't be unalligned, so we ignore GCC warnings.
>
> unaligned

Fixed.

>
> Aren't there other potential proposed patches floating around for this
> issue that do not involve messing with pragmas?

I'm not sure, I haven't seen anything. I'm more then open to a better
fix, this is the best option I could think of.

Alistair

>
> > + */
> > +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
> > +#endif
> > +
> >      filename = utf16_to_str(MIN(dataset->length, filename_chars),
> >                              dataset->filename);
> >
> > +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> > +#pragma GCC diagnostic pop
> > +#endif
> > +
> >      if (strchr(filename, '/')) {
> >          usb_mtp_queue_result(s, RES_PARAMETER_NOT_SUPPORTED, d->trans,
> >                               0, 0, 0, 0);
> >
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>

Re: [Qemu-devel] [PATCH v1 3/5] hw/usb/dev-mtp: Fix GCC 9 build warning

[Alistair Francis]

On Tue, Apr 30, 2019 at 1:37 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 4/30/19 3:09 PM, Alistair Francis wrote:
> > Fix this warning with GCC 9 on Fedora 30:
> > hw/usb/dev-mtp.c:1715:36: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
> >  1715 |                             dataset->filename);
> >       |                             ~~~~~~~^~~~~~~~~~
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  hw/usb/dev-mtp.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> > index 99548b012d..6de85d99e6 100644
> > --- a/hw/usb/dev-mtp.c
> > +++ b/hw/usb/dev-mtp.c
> > @@ -1711,9 +1711,22 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
> >      assert(!s->write_pending);
> >      assert(p != NULL);
> >
> > +/*
> > + * We are about to access a packed struct. We are confident that the pointer
> > + * address won't be unalligned, so we ignore GCC warnings.
>
> unaligned

Fixed.

>
> Aren't there other potential proposed patches floating around for this
> issue that do not involve messing with pragmas?

I'm not sure, I haven't seen anything. I'm more then open to a better
fix, this is the best option I could think of.

Alistair

>
> > + */
> > +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
> > +#endif
> > +
> >      filename = utf16_to_str(MIN(dataset->length, filename_chars),
> >                              dataset->filename);
> >
> > +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> > +#pragma GCC diagnostic pop
> > +#endif
> > +
> >      if (strchr(filename, '/')) {
> >          usb_mtp_queue_result(s, RES_PARAMETER_NOT_SUPPORTED, d->trans,
> >                               0, 0, 0, 0);
> >
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>

Re: [Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Laurent Vivier]

Le 30/04/2019 à 22:30, Laurent Vivier a écrit :
> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
>> Fix this warning when building with GCC9 on Fedora 30:
>> In function ‘strncpy’,
>>     inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  linux-user/uname.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/linux-user/uname.c b/linux-user/uname.c
>> index 313b79dbad..293b2238f2 100644
>> --- a/linux-user/uname.c
>> +++ b/linux-user/uname.c
>> @@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
>>     * struct linux kernel uses).
>>     */
>>  
>> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
>> +#endif
>> +
>>    memset(buf, 0, sizeof(*buf));
>>    COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
>>    COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);
>> @@ -101,6 +106,9 @@ int sys_uname(struct new_utsname *buf)
>>  #endif
>>    return (0);
>>  
>> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
>> +#pragma GCC diagnostic pop
>> +#endif
>>  #undef COPY_UTSNAME_FIELD
>>  }
>>  
>>
> 
> You should use PRAGMA_DISABLE_PACKED_WARNING and
> PRAGMA_REENABLE_PACKED_WARNING from linux-user/qemu.h.

Ok, I should read correctly, the pragma here is not related to them

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Laurent Vivier]

Le 30/04/2019 à 22:30, Laurent Vivier a écrit :
> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
>> Fix this warning when building with GCC9 on Fedora 30:
>> In function ‘strncpy’,
>>     inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  linux-user/uname.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/linux-user/uname.c b/linux-user/uname.c
>> index 313b79dbad..293b2238f2 100644
>> --- a/linux-user/uname.c
>> +++ b/linux-user/uname.c
>> @@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
>>     * struct linux kernel uses).
>>     */
>>  
>> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
>> +#endif
>> +
>>    memset(buf, 0, sizeof(*buf));
>>    COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
>>    COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);
>> @@ -101,6 +106,9 @@ int sys_uname(struct new_utsname *buf)
>>  #endif
>>    return (0);
>>  
>> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
>> +#pragma GCC diagnostic pop
>> +#endif
>>  #undef COPY_UTSNAME_FIELD
>>  }
>>  
>>
> 
> You should use PRAGMA_DISABLE_PACKED_WARNING and
> PRAGMA_REENABLE_PACKED_WARNING from linux-user/qemu.h.

Ok, I should read correctly, the pragma here is not related to them

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Laurent Vivier]

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

Le 30/04/2019 à 22:40, Eric Blake a écrit :
> On 4/30/19 3:09 PM, Alistair Francis wrote:
>> Fix this warning when building with GCC9 on Fedora 30:
>> In function ‘strncpy’,
>>     inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  linux-user/uname.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/linux-user/uname.c b/linux-user/uname.c
>> index 313b79dbad..293b2238f2 100644
>> --- a/linux-user/uname.c
>> +++ b/linux-user/uname.c
>> @@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
>>     * struct linux kernel uses).
>>     */
>>  
>> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
>> +#endif
> 
> Why do we need the pragma?
> 
>> +
>>    memset(buf, 0, sizeof(*buf));
> 
> We are prezeroing the entire field, at which point...
> 
>>    COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
>>    COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);
> 
> ...using strncpy() for a shorter string is wasteful (we're writing the
> tail end twice), and for a long string is warning-prone.  Why not
> rewrite COPY_UTSNAME_FIELD() to use memcpy() for the MIN(strlen(src),
> __NEW_UTS_LEN) and drop the write of the trailing NUL, since it will
> already be NUL from your memset()?
> 

We must modify this very carefully because I think there is some magic
in COPY_UTSNAME_FIELD() we could miss.

#define COPY_UTSNAME_FIELD(dest, src) \
  do { \
      /* __NEW_UTS_LEN doesn't include terminating null */ \
      (void) strncpy((dest), (src), __NEW_UTS_LEN); \
      (dest)[__NEW_UTS_LEN] = '\0'; \
  } while (0)

Thanks,
Laurent


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Laurent Vivier]

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

Le 30/04/2019 à 22:40, Eric Blake a écrit :
> On 4/30/19 3:09 PM, Alistair Francis wrote:
>> Fix this warning when building with GCC9 on Fedora 30:
>> In function ‘strncpy’,
>>     inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  linux-user/uname.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/linux-user/uname.c b/linux-user/uname.c
>> index 313b79dbad..293b2238f2 100644
>> --- a/linux-user/uname.c
>> +++ b/linux-user/uname.c
>> @@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
>>     * struct linux kernel uses).
>>     */
>>  
>> +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
>> +#endif
> 
> Why do we need the pragma?
> 
>> +
>>    memset(buf, 0, sizeof(*buf));
> 
> We are prezeroing the entire field, at which point...
> 
>>    COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
>>    COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);
> 
> ...using strncpy() for a shorter string is wasteful (we're writing the
> tail end twice), and for a long string is warning-prone.  Why not
> rewrite COPY_UTSNAME_FIELD() to use memcpy() for the MIN(strlen(src),
> __NEW_UTS_LEN) and drop the write of the trailing NUL, since it will
> already be NUL from your memset()?
> 

We must modify this very carefully because I think there is some magic
in COPY_UTSNAME_FIELD() we could miss.

#define COPY_UTSNAME_FIELD(dest, src) \
  do { \
      /* __NEW_UTS_LEN doesn't include terminating null */ \
      (void) strncpy((dest), (src), __NEW_UTS_LEN); \
      (dest)[__NEW_UTS_LEN] = '\0'; \
  } while (0)

Thanks,
Laurent


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH v1 2/5] hw/usb/hcd-xhci: Fix GCC 9 build warning

[Alistair Francis]

On Tue, Apr 30, 2019 at 1:24 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> > Fix this build warning with GCC 9 on Fedora 30:
> > hw/usb/hcd-xhci.c:3339:66: error: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Werror=format-truncation=]
> >  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
> >       |                                                                  ^~
> > hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
> >  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>
> "i" cannot be greater than 15.
>
> perhaps an "assert(i <= MAX(MAXPORTS_2, MAXPORTS_3))" can fix the warning ?

It does seem to stop the warnings, I'll change this patch to use a g_assert().

Alistair

>
> Thanks,
> Laurent
>
> >       |                                                      ^~~~~~~~~~~~~~~
> > In file included from /usr/include/stdio.h:867,
> >                  from /home/alistair/qemu/include/qemu/osdep.h:99,
> >                  from hw/usb/hcd-xhci.c:21:
> > /usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output between 13 and 22 bytes into a destination of size 16
> >    67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
> >       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    68 |        __bos (__s), __fmt, __va_arg_pack ());
> >       |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  hw/usb/hcd-xhci.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
> > index 240caa4e51..9e4988abb6 100644
> > --- a/hw/usb/hcd-xhci.h
> > +++ b/hw/usb/hcd-xhci.h
> > @@ -133,7 +133,7 @@ typedef struct XHCIPort {
> >      uint32_t portnr;
> >      USBPort  *uport;
> >      uint32_t speedmask;
> > -    char name[16];
> > +    char name[24];
> >      MemoryRegion mem;
> >  } XHCIPort;
> >
> >
>

Re: [Qemu-devel] [PATCH v1 2/5] hw/usb/hcd-xhci: Fix GCC 9 build warning

[Alistair Francis]

On Tue, Apr 30, 2019 at 1:24 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> > Fix this build warning with GCC 9 on Fedora 30:
> > hw/usb/hcd-xhci.c:3339:66: error: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Werror=format-truncation=]
> >  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
> >       |                                                                  ^~
> > hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
> >  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>
> "i" cannot be greater than 15.
>
> perhaps an "assert(i <= MAX(MAXPORTS_2, MAXPORTS_3))" can fix the warning ?

It does seem to stop the warnings, I'll change this patch to use a g_assert().

Alistair

>
> Thanks,
> Laurent
>
> >       |                                                      ^~~~~~~~~~~~~~~
> > In file included from /usr/include/stdio.h:867,
> >                  from /home/alistair/qemu/include/qemu/osdep.h:99,
> >                  from hw/usb/hcd-xhci.c:21:
> > /usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output between 13 and 22 bytes into a destination of size 16
> >    67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
> >       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    68 |        __bos (__s), __fmt, __va_arg_pack ());
> >       |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  hw/usb/hcd-xhci.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
> > index 240caa4e51..9e4988abb6 100644
> > --- a/hw/usb/hcd-xhci.h
> > +++ b/hw/usb/hcd-xhci.h
> > @@ -133,7 +133,7 @@ typedef struct XHCIPort {
> >      uint32_t portnr;
> >      USBPort  *uport;
> >      uint32_t speedmask;
> > -    char name[16];
> > +    char name[24];
> >      MemoryRegion mem;
> >  } XHCIPort;
> >
> >
>

Re: [Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Alistair Francis]

On Tue, Apr 30, 2019 at 1:36 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> > Fix this warning when building with GCC9 on Fedora 30:
> > In function ‘strncpy’,
> >     inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
> >     inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
> >     inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
> > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
> >   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
> >       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  linux-user/elfload.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> > index c1a26021f8..cbb7fc10fa 100644
> > --- a/linux-user/elfload.c
> > +++ b/linux-user/elfload.c
> > @@ -3206,7 +3206,7 @@ static int fill_psinfo(struct target_elf_prpsinfo *psinfo, const TaskState *ts)
>
>        /*
>         * Using strncpy here is fine: at max-length,
> >       * this field is not NUL-terminated.
> >       */>      (void) strncpy(psinfo->pr_fname, base_filename,
> > -                   sizeof(psinfo->pr_fname));
> > +                   sizeof(psinfo->pr_fname) - 1);
>
> Read the comment above :)

GCC can't read the comment though. The only other option I can think
of is using a pragma, which I avoided using unless I had to. Would you
prefer a pragma here? Or do you have a better solution?

>
> >
> >      g_free(base_filename);
> >      bswap_psinfo(psinfo);
> > @@ -3389,7 +3389,7 @@ static int fill_note_info(struct elf_note_info *info,
> >                sizeof (*info->prstatus), info->prstatus);
> >      fill_psinfo(info->psinfo, ts);
> >      fill_note(&info->notes[1], "CORE", NT_PRPSINFO,
> > -              sizeof (*info->psinfo), info->psinfo);
> > +              sizeof(*info->psinfo) - 1, info->psinfo);
>
> Why?

Same issue as above.

Alistair

>
> >      fill_auxv_note(&info->notes[2], ts);
> >      info->numnote = 3;
> >
> >
>
> Thanks,
> Laurent

Re: [Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Alistair Francis]

On Tue, Apr 30, 2019 at 1:36 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> > Fix this warning when building with GCC9 on Fedora 30:
> > In function ‘strncpy’,
> >     inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
> >     inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
> >     inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
> > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
> >   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
> >       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  linux-user/elfload.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> > index c1a26021f8..cbb7fc10fa 100644
> > --- a/linux-user/elfload.c
> > +++ b/linux-user/elfload.c
> > @@ -3206,7 +3206,7 @@ static int fill_psinfo(struct target_elf_prpsinfo *psinfo, const TaskState *ts)
>
>        /*
>         * Using strncpy here is fine: at max-length,
> >       * this field is not NUL-terminated.
> >       */>      (void) strncpy(psinfo->pr_fname, base_filename,
> > -                   sizeof(psinfo->pr_fname));
> > +                   sizeof(psinfo->pr_fname) - 1);
>
> Read the comment above :)

GCC can't read the comment though. The only other option I can think
of is using a pragma, which I avoided using unless I had to. Would you
prefer a pragma here? Or do you have a better solution?

>
> >
> >      g_free(base_filename);
> >      bswap_psinfo(psinfo);
> > @@ -3389,7 +3389,7 @@ static int fill_note_info(struct elf_note_info *info,
> >                sizeof (*info->prstatus), info->prstatus);
> >      fill_psinfo(info->psinfo, ts);
> >      fill_note(&info->notes[1], "CORE", NT_PRPSINFO,
> > -              sizeof (*info->psinfo), info->psinfo);
> > +              sizeof(*info->psinfo) - 1, info->psinfo);
>
> Why?

Same issue as above.

Alistair

>
> >      fill_auxv_note(&info->notes[2], ts);
> >      info->numnote = 3;
> >
> >
>
> Thanks,
> Laurent

Re: [Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Alistair Francis]

On Tue, Apr 30, 2019 at 1:40 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 4/30/19 3:09 PM, Alistair Francis wrote:
> > Fix this warning when building with GCC9 on Fedora 30:
> > In function ‘strncpy’,
> >     inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
> > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
> >   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
> >       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  linux-user/uname.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/linux-user/uname.c b/linux-user/uname.c
> > index 313b79dbad..293b2238f2 100644
> > --- a/linux-user/uname.c
> > +++ b/linux-user/uname.c
> > @@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
> >     * struct linux kernel uses).
> >     */
> >
> > +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wstringop-truncation"
> > +#endif
>
> Why do we need the pragma?
>
> > +
> >    memset(buf, 0, sizeof(*buf));
>
> We are prezeroing the entire field, at which point...
>
> >    COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
> >    COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);
>
> ...using strncpy() for a shorter string is wasteful (we're writing the
> tail end twice), and for a long string is warning-prone.  Why not
> rewrite COPY_UTSNAME_FIELD() to use memcpy() for the MIN(strlen(src),
> __NEW_UTS_LEN) and drop the write of the trailing NUL, since it will
> already be NUL from your memset()?

I'm happy to do that, I'll change it to memcpy().

Alistair

>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>

Re: [Qemu-devel] [PATCH v1 4/5] linux-user/uname: Fix GCC 9 build warnings

[Alistair Francis]

On Tue, Apr 30, 2019 at 1:40 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 4/30/19 3:09 PM, Alistair Francis wrote:
> > Fix this warning when building with GCC9 on Fedora 30:
> > In function ‘strncpy’,
> >     inlined from ‘sys_uname’ at /home/alistair/qemu/linux-user/uname.c:94:3:
> > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ output may be truncated copying 64 bytes from a string of length 64 [-Werror=stringop-truncation]
> >   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
> >       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  linux-user/uname.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/linux-user/uname.c b/linux-user/uname.c
> > index 313b79dbad..293b2238f2 100644
> > --- a/linux-user/uname.c
> > +++ b/linux-user/uname.c
> > @@ -90,6 +90,11 @@ int sys_uname(struct new_utsname *buf)
> >     * struct linux kernel uses).
> >     */
> >
> > +#if defined(CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE) && QEMU_GNUC_PREREQ(9, 0)
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wstringop-truncation"
> > +#endif
>
> Why do we need the pragma?
>
> > +
> >    memset(buf, 0, sizeof(*buf));
>
> We are prezeroing the entire field, at which point...
>
> >    COPY_UTSNAME_FIELD(buf->sysname, uts_buf.sysname);
> >    COPY_UTSNAME_FIELD(buf->nodename, uts_buf.nodename);
>
> ...using strncpy() for a shorter string is wasteful (we're writing the
> tail end twice), and for a long string is warning-prone.  Why not
> rewrite COPY_UTSNAME_FIELD() to use memcpy() for the MIN(strlen(src),
> __NEW_UTS_LEN) and drop the write of the trailing NUL, since it will
> already be NUL from your memset()?

I'm happy to do that, I'll change it to memcpy().

Alistair

>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>

Re: [Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Laurent Vivier]

Le 30/04/2019 à 23:01, Alistair Francis a écrit :
> On Tue, Apr 30, 2019 at 1:36 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
>>> Fix this warning when building with GCC9 on Fedora 30:
>>> In function ‘strncpy’,
>>>     inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
>>>     inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
>>>     inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
>>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
>>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>>> ---
>>>  linux-user/elfload.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>>> index c1a26021f8..cbb7fc10fa 100644
>>> --- a/linux-user/elfload.c
>>> +++ b/linux-user/elfload.c
>>> @@ -3206,7 +3206,7 @@ static int fill_psinfo(struct target_elf_prpsinfo *psinfo, const TaskState *ts)
>>
>>        /*
>>         * Using strncpy here is fine: at max-length,
>>>       * this field is not NUL-terminated.
>>>       */>      (void) strncpy(psinfo->pr_fname, base_filename,
>>> -                   sizeof(psinfo->pr_fname));
>>> +                   sizeof(psinfo->pr_fname) - 1);
>>
>> Read the comment above :)
> 
> GCC can't read the comment though. The only other option I can think
> of is using a pragma, which I avoided using unless I had to. Would you
> prefer a pragma here? Or do you have a better solution?
> 

perhaps:

memcpy(psinfo->pr_fname, base_filename, MIN(strlen(base_filename) + 1, sizeof(psinfo->pr_fname));

?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Laurent Vivier]

Le 30/04/2019 à 23:01, Alistair Francis a écrit :
> On Tue, Apr 30, 2019 at 1:36 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
>>> Fix this warning when building with GCC9 on Fedora 30:
>>> In function ‘strncpy’,
>>>     inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
>>>     inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
>>>     inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
>>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
>>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>>> ---
>>>  linux-user/elfload.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>>> index c1a26021f8..cbb7fc10fa 100644
>>> --- a/linux-user/elfload.c
>>> +++ b/linux-user/elfload.c
>>> @@ -3206,7 +3206,7 @@ static int fill_psinfo(struct target_elf_prpsinfo *psinfo, const TaskState *ts)
>>
>>        /*
>>         * Using strncpy here is fine: at max-length,
>>>       * this field is not NUL-terminated.
>>>       */>      (void) strncpy(psinfo->pr_fname, base_filename,
>>> -                   sizeof(psinfo->pr_fname));
>>> +                   sizeof(psinfo->pr_fname) - 1);
>>
>> Read the comment above :)
> 
> GCC can't read the comment though. The only other option I can think
> of is using a pragma, which I avoided using unless I had to. Would you
> prefer a pragma here? Or do you have a better solution?
> 

perhaps:

memcpy(psinfo->pr_fname, base_filename, MIN(strlen(base_filename) + 1, sizeof(psinfo->pr_fname));

?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Eric Blake]

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

On 4/30/19 4:01 PM, Alistair Francis wrote:
> On Tue, Apr 30, 2019 at 1:36 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
>>> Fix this warning when building with GCC9 on Fedora 30:
>>> In function ‘strncpy’,
>>>     inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
>>>     inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
>>>     inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
>>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
>>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>

>>        /*
>>         * Using strncpy here is fine: at max-length,
>>>       * this field is not NUL-terminated.
>>>       */>      (void) strncpy(psinfo->pr_fname, base_filename,
>>> -                   sizeof(psinfo->pr_fname));
>>> +                   sizeof(psinfo->pr_fname) - 1);
>>
>> Read the comment above :)
> 
> GCC can't read the comment though. The only other option I can think
> of is using a pragma, which I avoided using unless I had to. Would you
> prefer a pragma here? Or do you have a better solution?

psinfo is struct target_elf_prpsinfo, which we declare.  Why not just
use the QEMU_NONSTRING attribute in the declaration, to tell the
compiler our exact intents (untested, but something like this):

diff --git i/linux-user/elfload.c w/linux-user/elfload.c
index c1a26021f8d..6ebb2eeb957 100644
--- i/linux-user/elfload.c
+++ w/linux-user/elfload.c
@@ -2872,7 +2872,7 @@ struct target_elf_prpsinfo {
     target_gid_t pr_gid;
     target_pid_t pr_pid, pr_ppid, pr_pgrp, pr_sid;
     /* Lots missing */
-    char    pr_fname[16];           /* filename of executable */
+    char    pr_fname[16] QEMU_NONSTRING;           /* filename of
executable */
     char    pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
 };




-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Eric Blake]

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

On 4/30/19 4:01 PM, Alistair Francis wrote:
> On Tue, Apr 30, 2019 at 1:36 PM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
>>> Fix this warning when building with GCC9 on Fedora 30:
>>> In function ‘strncpy’,
>>>     inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
>>>     inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
>>>     inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
>>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
>>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>

>>        /*
>>         * Using strncpy here is fine: at max-length,
>>>       * this field is not NUL-terminated.
>>>       */>      (void) strncpy(psinfo->pr_fname, base_filename,
>>> -                   sizeof(psinfo->pr_fname));
>>> +                   sizeof(psinfo->pr_fname) - 1);
>>
>> Read the comment above :)
> 
> GCC can't read the comment though. The only other option I can think
> of is using a pragma, which I avoided using unless I had to. Would you
> prefer a pragma here? Or do you have a better solution?

psinfo is struct target_elf_prpsinfo, which we declare.  Why not just
use the QEMU_NONSTRING attribute in the declaration, to tell the
compiler our exact intents (untested, but something like this):

diff --git i/linux-user/elfload.c w/linux-user/elfload.c
index c1a26021f8d..6ebb2eeb957 100644
--- i/linux-user/elfload.c
+++ w/linux-user/elfload.c
@@ -2872,7 +2872,7 @@ struct target_elf_prpsinfo {
     target_gid_t pr_gid;
     target_pid_t pr_pid, pr_ppid, pr_pgrp, pr_sid;
     /* Lots missing */
-    char    pr_fname[16];           /* filename of executable */
+    char    pr_fname[16] QEMU_NONSTRING;           /* filename of
executable */
     char    pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
 };




-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v1 1/5] util/qemu-sockets: Fix GCC 9 build warnings

[Eric Blake]

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

On 4/30/19 3:25 PM, Eric Blake wrote:
> On 4/30/19 3:08 PM, Alistair Francis wrote:
>> Fix this warning when building with GCC9 on Fedora 30:
>> In function ‘strncpy’,
>>     inlined from ‘unix_connect_saddr.isra.0’ at util/qemu-sockets.c:925:5:
>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-truncation]
>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> In function ‘strncpy’,
>>     inlined from ‘unix_listen_saddr.isra.0’ at util/qemu-sockets.c:880:5:
>>

>> -    strncpy(un.sun_path, path, sizeof(un.sun_path));
>> +    strncpy(un.sun_path, path, sizeof(un.sun_path) - 1);
> 
> NACK. Linux allows you to use the full width of un.sun_path (a NUL
> terminator is required if you copy less than that, but not if you use
> the full width). Rather, we may need to mark path as a potential
> nonstring to silence the warning, or use memcpy instead of strncpy, or
> some other workaround.  (Sadly, this is one of those odd places where
> strncpy is actually the right function to use, but there are so many
> other places where strncpy is used incorrectly that it has turned into a
> battle to use it here)

We don't have control over un (that's from the libc system headers), but
does adding the QEMU_NONSTRING attribute to our declaration of path
serve to silence the warning?

In short, I think most of this series should look at the use of the
QEMU_NONSTRING macro, as that macro goes hand-in-hand with strncpy() for
informing the compiler exactly when we know that we are copying
something that has fixed length and may or may not be NUL-terminated.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Alistair Francis]

On Tue, Apr 30, 2019 at 2:10 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 4/30/19 4:01 PM, Alistair Francis wrote:
> > On Tue, Apr 30, 2019 at 1:36 PM Laurent Vivier <laurent@vivier.eu> wrote:
> >>
> >> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> >>> Fix this warning when building with GCC9 on Fedora 30:
> >>> In function ‘strncpy’,
> >>>     inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
> >>>     inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
> >>>     inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
> >>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
> >>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
> >>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>
>
> >>        /*
> >>         * Using strncpy here is fine: at max-length,
> >>>       * this field is not NUL-terminated.
> >>>       */>      (void) strncpy(psinfo->pr_fname, base_filename,
> >>> -                   sizeof(psinfo->pr_fname));
> >>> +                   sizeof(psinfo->pr_fname) - 1);
> >>
> >> Read the comment above :)
> >
> > GCC can't read the comment though. The only other option I can think
> > of is using a pragma, which I avoided using unless I had to. Would you
> > prefer a pragma here? Or do you have a better solution?
>
> psinfo is struct target_elf_prpsinfo, which we declare.  Why not just
> use the QEMU_NONSTRING attribute in the declaration, to tell the
> compiler our exact intents (untested, but something like this):
>
> diff --git i/linux-user/elfload.c w/linux-user/elfload.c
> index c1a26021f8d..6ebb2eeb957 100644
> --- i/linux-user/elfload.c
> +++ w/linux-user/elfload.c
> @@ -2872,7 +2872,7 @@ struct target_elf_prpsinfo {
>      target_gid_t pr_gid;
>      target_pid_t pr_pid, pr_ppid, pr_pgrp, pr_sid;
>      /* Lots missing */
> -    char    pr_fname[16];           /* filename of executable */
> +    char    pr_fname[16] QEMU_NONSTRING;           /* filename of
> executable */
>      char    pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
>  };

I didn't know about that property, that fixes it.

Alistair

>
>
>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>

Re: [Qemu-devel] [PATCH v1 5/5] linux-user/elfload: Fix GCC 9 build warnings

[Alistair Francis]

On Tue, Apr 30, 2019 at 2:10 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 4/30/19 4:01 PM, Alistair Francis wrote:
> > On Tue, Apr 30, 2019 at 1:36 PM Laurent Vivier <laurent@vivier.eu> wrote:
> >>
> >> Le 30/04/2019 à 22:09, Alistair Francis a écrit :
> >>> Fix this warning when building with GCC9 on Fedora 30:
> >>> In function ‘strncpy’,
> >>>     inlined from ‘fill_psinfo’ at /home/alistair/qemu/linux-user/elfload.c:3208:12,
> >>>     inlined from ‘fill_note_info’ at /home/alistair/qemu/linux-user/elfload.c:3390:5,
> >>>     inlined from ‘elf_core_dump’ at /home/alistair/qemu/linux-user/elfload.c:3539:9:
> >>> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Werror=stringop-truncation]
> >>>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
> >>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>
>
> >>        /*
> >>         * Using strncpy here is fine: at max-length,
> >>>       * this field is not NUL-terminated.
> >>>       */>      (void) strncpy(psinfo->pr_fname, base_filename,
> >>> -                   sizeof(psinfo->pr_fname));
> >>> +                   sizeof(psinfo->pr_fname) - 1);
> >>
> >> Read the comment above :)
> >
> > GCC can't read the comment though. The only other option I can think
> > of is using a pragma, which I avoided using unless I had to. Would you
> > prefer a pragma here? Or do you have a better solution?
>
> psinfo is struct target_elf_prpsinfo, which we declare.  Why not just
> use the QEMU_NONSTRING attribute in the declaration, to tell the
> compiler our exact intents (untested, but something like this):
>
> diff --git i/linux-user/elfload.c w/linux-user/elfload.c
> index c1a26021f8d..6ebb2eeb957 100644
> --- i/linux-user/elfload.c
> +++ w/linux-user/elfload.c
> @@ -2872,7 +2872,7 @@ struct target_elf_prpsinfo {
>      target_gid_t pr_gid;
>      target_pid_t pr_pid, pr_ppid, pr_pgrp, pr_sid;
>      /* Lots missing */
> -    char    pr_fname[16];           /* filename of executable */
> +    char    pr_fname[16] QEMU_NONSTRING;           /* filename of
> executable */
>      char    pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
>  };

I didn't know about that property, that fixes it.

Alistair

>
>
>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>

Re: [Qemu-devel] [PATCH v1 1/5] util/qemu-sockets: Fix GCC 9 build warnings

[Alistair Francis]

On Tue, Apr 30, 2019 at 2:16 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 4/30/19 3:25 PM, Eric Blake wrote:
> > On 4/30/19 3:08 PM, Alistair Francis wrote:
> >> Fix this warning when building with GCC9 on Fedora 30:
> >> In function ‘strncpy’,
> >>     inlined from ‘unix_connect_saddr.isra.0’ at util/qemu-sockets.c:925:5:
> >> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-truncation]
> >>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
> >>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> In function ‘strncpy’,
> >>     inlined from ‘unix_listen_saddr.isra.0’ at util/qemu-sockets.c:880:5:
> >>
>
> >> -    strncpy(un.sun_path, path, sizeof(un.sun_path));
> >> +    strncpy(un.sun_path, path, sizeof(un.sun_path) - 1);
> >
> > NACK. Linux allows you to use the full width of un.sun_path (a NUL
> > terminator is required if you copy less than that, but not if you use
> > the full width). Rather, we may need to mark path as a potential
> > nonstring to silence the warning, or use memcpy instead of strncpy, or
> > some other workaround.  (Sadly, this is one of those odd places where
> > strncpy is actually the right function to use, but there are so many
> > other places where strncpy is used incorrectly that it has turned into a
> > battle to use it here)
>
> We don't have control over un (that's from the libc system headers), but
> does adding the QEMU_NONSTRING attribute to our declaration of path
> serve to silence the warning?

I don't think that would fix it, I'll double check though.

>
> In short, I think most of this series should look at the use of the
> QEMU_NONSTRING macro, as that macro goes hand-in-hand with strncpy() for
> informing the compiler exactly when we know that we are copying
> something that has fixed length and may or may not be NUL-terminated.

I have changed the series to use memcpy() and QEMU_NONSTRING. I'll
send a v2 out soon with all your comments addressed.

Alistair

>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>

Re: [Qemu-devel] [PATCH v1 1/5] util/qemu-sockets: Fix GCC 9 build warnings

[Alistair Francis]

On Tue, Apr 30, 2019 at 2:16 PM Eric Blake <eblake@redhat.com> wrote:
>
> On 4/30/19 3:25 PM, Eric Blake wrote:
> > On 4/30/19 3:08 PM, Alistair Francis wrote:
> >> Fix this warning when building with GCC9 on Fedora 30:
> >> In function ‘strncpy’,
> >>     inlined from ‘unix_connect_saddr.isra.0’ at util/qemu-sockets.c:925:5:
> >> /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 108 equals destination size [-Werror=stringop-truncation]
> >>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
> >>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> In function ‘strncpy’,
> >>     inlined from ‘unix_listen_saddr.isra.0’ at util/qemu-sockets.c:880:5:
> >>
>
> >> -    strncpy(un.sun_path, path, sizeof(un.sun_path));
> >> +    strncpy(un.sun_path, path, sizeof(un.sun_path) - 1);
> >
> > NACK. Linux allows you to use the full width of un.sun_path (a NUL
> > terminator is required if you copy less than that, but not if you use
> > the full width). Rather, we may need to mark path as a potential
> > nonstring to silence the warning, or use memcpy instead of strncpy, or
> > some other workaround.  (Sadly, this is one of those odd places where
> > strncpy is actually the right function to use, but there are so many
> > other places where strncpy is used incorrectly that it has turned into a
> > battle to use it here)
>
> We don't have control over un (that's from the libc system headers), but
> does adding the QEMU_NONSTRING attribute to our declaration of path
> serve to silence the warning?

I don't think that would fix it, I'll double check though.

>
> In short, I think most of this series should look at the use of the
> QEMU_NONSTRING macro, as that macro goes hand-in-hand with strncpy() for
> informing the compiler exactly when we know that we are copying
> something that has fixed length and may or may not be NUL-terminated.

I have changed the series to use memcpy() and QEMU_NONSTRING. I'll
send a v2 out soon with all your comments addressed.

Alistair

>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>