[PATCH 0/7] Fix warnings found by gcc 8

[PATCH 0/7] Fix warnings found by gcc 8

[Marek Marczykowski-Górecki]

A few patches enabling build with gcc 8.

Marek Marczykowski-Górecki (7):
  tools/libxc: fix strncpy size
  tools/misc: fix hypothetical buffer overflow in xen-lowmemd
  tools/blktap2: fix hypothetical buffer overflow
  tools/blktap2: fix possible '\0' truncation
  tools/xenpmd: fix possible '\0' truncation
  tools/gdbsx: fix -Wstringop-truncation warning
  tools/kdd: mute spurious gcc warning

 tools/blktap2/drivers/block-qcow.c      |  3 ++-
 tools/blktap2/drivers/tapdisk-control.c |  5 +++--
 tools/blktap2/drivers/tapdisk-vbd.c     |  3 ++-
 tools/blktap2/vhd/lib/vhd-util-read.c   |  2 +-
 tools/debugger/gdbsx/gx/gx_main.c       |  2 +-
 tools/debugger/kdd/kdd.c                |  3 +++
 tools/libxc/xc_pm.c                     |  2 +-
 tools/misc/xen-lowmemd.c                |  2 +-
 tools/xenpmd/xenpmd.c                   | 12 ++++++++----
 9 files changed, 22 insertions(+), 12 deletions(-)

base-commit: eabb83121226d5a6a5a68da3a913ac0b5bb1e0cf
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/7] tools/libxc: fix strncpy size

[Marek Marczykowski-Górecki]

gcc-8 warns about possible truncation of trailing '\0'.
Final character is overridden by '\0' anyway, so don't bother to copy
it.

This fixes compile failure:

    xc_pm.c: In function 'xc_set_cpufreq_gov':
    xc_pm.c:308:5: error: 'strncpy' specified bound 16 equals destination size [-Werror=stringop-truncation]
         strncpy(scaling_governor, govname, CPUFREQ_NAME_LEN);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    cc1: all warnings being treated as errors

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/libxc/xc_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxc/xc_pm.c b/tools/libxc/xc_pm.c
index 67e2418..6f8d548 100644
--- a/tools/libxc/xc_pm.c
+++ b/tools/libxc/xc_pm.c
@@ -305,7 +305,7 @@ int xc_set_cpufreq_gov(xc_interface *xch, int cpuid, char *govname)
     sysctl.cmd = XEN_SYSCTL_pm_op;
     sysctl.u.pm_op.cmd = SET_CPUFREQ_GOV;
     sysctl.u.pm_op.cpuid = cpuid;
-    strncpy(scaling_governor, govname, CPUFREQ_NAME_LEN);
+    strncpy(scaling_governor, govname, CPUFREQ_NAME_LEN - 1);
     scaling_governor[CPUFREQ_NAME_LEN - 1] = '\0';
 
     return xc_sysctl(xch, &sysctl);
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/7] tools/misc: fix hypothetical buffer overflow in xen-lowmemd

[Marek Marczykowski-Górecki]

gcc-8 complains:

    xen-lowmemd.c: In function 'handle_low_mem':
    xen-lowmemd.c:80:55: error: '%s' directive output may be truncated writing up to 511 bytes into a region of size 489 [-Werror=format-truncation=]
             snprintf(error, BUFSZ,"Failed to write target %s to xenstore", data);
                                                           ^~               ~~~~
    xen-lowmemd.c:80:9: note: 'snprintf' output between 36 and 547 bytes into a destination of size 512
             snprintf(error, BUFSZ,"Failed to write target %s to xenstore", data);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In practice it wouldn't happen, because 'data' contains string
representation of 64-bit unsigned number (20 characters at most).
But place a limit to mute gcc warning.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/misc/xen-lowmemd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/misc/xen-lowmemd.c b/tools/misc/xen-lowmemd.c
index 865a54c..79ad34c 100644
--- a/tools/misc/xen-lowmemd.c
+++ b/tools/misc/xen-lowmemd.c
@@ -77,7 +77,7 @@ void handle_low_mem(void)
     if (!xs_write(xs_handle, XBT_NULL, 
             "/local/domain/0/memory/target", data, strlen(data)))
     {
-        snprintf(error, BUFSZ,"Failed to write target %s to xenstore", data);
+        snprintf(error, BUFSZ,"Failed to write target %.24s to xenstore", data);
         perror(error);
     }
 }
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 3/7] tools/blktap2: fix hypothetical buffer overflow

[Marek Marczykowski-Górecki]

gcc-8 complains:

    vhd-util-read.c: In function 'vhd_util_read':
    vhd-util-read.c:50:24: error: '%lu' directive output may be truncated writing between 1 and 20 bytes into a region of size 15 [-Werror=format-truncation=]
      snprintf(nbuf, nsize, "%" PRIu64, num);
                            ^~~
    vhd-util-read.c:50:25: note: format string is defined here
      snprintf(nbuf, nsize, "%" PRIu64, num);
    vhd-util-read.c:50:24: note: directive argument in the range [0, 18446744073709551614]
      snprintf(nbuf, nsize, "%" PRIu64, num);
                            ^~~
    vhd-util-read.c:50:2: note: 'snprintf' output between 2 and 21 bytes into a destination of size 15
      snprintf(nbuf, nsize, "%" PRIu64, num);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    vhd-util-read.c:43:24: error: '%#lx' directive output may be truncated writing between 1 and 18 bytes into a region of size 15 [-Werror=format-truncation=]
      snprintf(nbuf, nsize, "%#" PRIx64 , num);
                            ^~~~
    vhd-util-read.c:43:25: note: format string is defined here
      snprintf(nbuf, nsize, "%#" PRIx64 , num);
    vhd-util-read.c:43:24: note: directive argument in the range [0, 18446744073709551614]
      snprintf(nbuf, nsize, "%#" PRIx64 , num);
                            ^~~~
    vhd-util-read.c:43:2: note: 'snprintf' output between 2 and 19 bytes into a destination of size 15
      snprintf(nbuf, nsize, "%#" PRIx64 , num);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Make the buffer larger.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/blktap2/vhd/lib/vhd-util-read.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/blktap2/vhd/lib/vhd-util-read.c b/tools/blktap2/vhd/lib/vhd-util-read.c
index ac4d833..f290661 100644
--- a/tools/blktap2/vhd/lib/vhd-util-read.c
+++ b/tools/blktap2/vhd/lib/vhd-util-read.c
@@ -34,7 +34,7 @@
 #include "libvhd.h"
 #include "vhd-util.h"
 
-#define nsize     15
+#define nsize     24
 static char nbuf[nsize];
 
 static inline char *
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 4/7] tools/blktap2: fix possible '\0' truncation

[Marek Marczykowski-Górecki]

gcc-8 complains:

    tapdisk-vbd.c: In function 'tapdisk_vbd_resume_ring':
    tapdisk-vbd.c:1671:53: error: 'snprintf' output may be truncated before the last format character [-Werror=format-truncation=]
       snprintf(params.name, sizeof(params.name) - 1, "%s", message);
                                                         ^
    tapdisk-vbd.c:1671:3: note: 'snprintf' output between 1 and 256 bytes into a destination of size 255
       snprintf(params.name, sizeof(params.name) - 1, "%s", message);
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The "- 1" in buffer size should be actually applied to message, to leave
place for terminating '\0', not the other way around (truncate '\0' even
if it would fit).

    In function 'tapdisk_control_open_image',
        inlined from 'tapdisk_control_handle_request' at tapdisk-control.c:660:10:
    tapdisk-control.c:465:2: error: 'strncpy' specified bound 256 equals destination size [-Werror=stringop-truncation]
      strncpy(params.name, vbd->name, BLKTAP2_MAX_MESSAGE_LEN);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    In function 'tapdisk_control_create_socket',
        inlined from 'tapdisk_control_open' at tapdisk-control.c:836:9:
    tapdisk-control.c:793:2: error: 'strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
      strncpy(saddr.sun_path, td_control.path, sizeof(saddr.sun_path));
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    block-qcow.c: In function 'qcow_create':
    block-qcow.c:1216:5: error: 'strncpy' specified bound 4096 equals destination size [-Werror=stringop-truncation]
         strncpy(backing_filename, backing_file,
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          sizeof(backing_filename));
          ~~~~~~~~~~~~~~~~~~~~~~~~~

I those cases, reduce size of copied string and make sure final '\0' is
added.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/blktap2/drivers/block-qcow.c      | 3 ++-
 tools/blktap2/drivers/tapdisk-control.c | 5 +++--
 tools/blktap2/drivers/tapdisk-vbd.c     | 3 ++-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/blktap2/drivers/block-qcow.c b/tools/blktap2/drivers/block-qcow.c
index b45bcaa..ae43922 100644
--- a/tools/blktap2/drivers/block-qcow.c
+++ b/tools/blktap2/drivers/block-qcow.c
@@ -1214,7 +1214,8 @@ int qcow_create(const char *filename, uint64_t total_size,
 			if (p && (p - backing_file) >= 2) {
 				/* URL like but exclude "c:" like filenames */
 				strncpy(backing_filename, backing_file,
-					sizeof(backing_filename));
+					sizeof(backing_filename) - 1);
+				backing_filename[sizeof(backing_filename) - 1] = '\0';
 			} else {
 				if (realpath(backing_file, backing_filename) == NULL ||
 				    stat(backing_filename, &st) != 0) {
diff --git a/tools/blktap2/drivers/tapdisk-control.c b/tools/blktap2/drivers/tapdisk-control.c
index 0b5cf3c..3ca5713 100644
--- a/tools/blktap2/drivers/tapdisk-control.c
+++ b/tools/blktap2/drivers/tapdisk-control.c
@@ -462,7 +462,8 @@ tapdisk_control_open_image(struct tapdisk_control_connection *connection,
 
 	params.capacity = image.size;
 	params.sector_size = image.secsize;
-	strncpy(params.name, vbd->name, BLKTAP2_MAX_MESSAGE_LEN);
+	strncpy(params.name, vbd->name, BLKTAP2_MAX_MESSAGE_LEN - 1);
+	params.name[BLKTAP2_MAX_MESSAGE_LEN - 1] = '\0';
 
 	err = ioctl(vbd->ring.fd, BLKTAP2_IOCTL_CREATE_DEVICE, &params);
 	if (err && errno != EEXIST) {
@@ -790,7 +791,7 @@ tapdisk_control_create_socket(char **socket_path)
 	}
 
 	memset(&saddr, 0, sizeof(saddr));
-	strncpy(saddr.sun_path, td_control.path, sizeof(saddr.sun_path));
+	strncpy(saddr.sun_path, td_control.path, sizeof(saddr.sun_path) - 1);
 	saddr.sun_family = AF_UNIX;
 
 	err = bind(td_control.socket,
diff --git a/tools/blktap2/drivers/tapdisk-vbd.c b/tools/blktap2/drivers/tapdisk-vbd.c
index fd4999a..842a427 100644
--- a/tools/blktap2/drivers/tapdisk-vbd.c
+++ b/tools/blktap2/drivers/tapdisk-vbd.c
@@ -1668,7 +1668,8 @@ out:
 
 		params.sector_size = image.secsize;
 		params.capacity    = image.size;
-		snprintf(params.name, sizeof(params.name) - 1, "%s", message);
+		snprintf(params.name, sizeof(params.name),
+			 "%.*s", (int)sizeof(params.name) - 1, message);
 
 		ioctl(vbd->ring.fd, BLKTAP2_IOCTL_SET_PARAMS, &params);
 		td_flag_clear(vbd->state, TD_VBD_PAUSED);
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 5/7] tools/xenpmd: fix possible '\0' truncation

[Marek Marczykowski-Górecki]

gcc-8 complains:
    xenpmd.c:207:9: error: 'strncpy' specified bound 32 equals destination size [-Werror=stringop-truncation]
             strncpy(info->oem_info, attrib_value, 32);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    xenpmd.c:201:9: error: 'strncpy' specified bound 32 equals destination size [-Werror=stringop-truncation]
             strncpy(info->battery_type, attrib_value, 32);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    xenpmd.c:195:9: error: 'strncpy' specified bound 32 equals destination size [-Werror=stringop-truncation]
             strncpy(info->serial_number, attrib_value, 32);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    xenpmd.c:189:9: error: 'strncpy' specified bound 32 equals destination size [-Werror=stringop-truncation]
             strncpy(info->model_number, attrib_value, 32);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Copy 31 chars, then make sure terminating '\0' is present. Those fields
are passed to strlen and as '%s' for snprintf later.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/xenpmd/xenpmd.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/xenpmd/xenpmd.c b/tools/xenpmd/xenpmd.c
index 689c8fd..56412a9 100644
--- a/tools/xenpmd/xenpmd.c
+++ b/tools/xenpmd/xenpmd.c
@@ -186,25 +186,29 @@ void set_attribute_battery_info(char *attrib_name,
 
     if ( strstr(attrib_name, "model number") ) 
     {
-        strncpy(info->model_number, attrib_value, 32);
+        strncpy(info->model_number, attrib_value, 31);
+        info->model_number[31] = '\0';
         return;
     }
 
     if ( strstr(attrib_name, "serial number") ) 
     {
-        strncpy(info->serial_number, attrib_value, 32);
+        strncpy(info->serial_number, attrib_value, 31);
+        info->serial_number[31] = '\0';
         return;
     }
 
     if ( strstr(attrib_name, "battery type") ) 
     {
-        strncpy(info->battery_type, attrib_value, 32);
+        strncpy(info->battery_type, attrib_value, 31);
+        info->battery_type[31] = '\0';
         return;
     }
 
     if ( strstr(attrib_name, "OEM info") ) 
     {
-        strncpy(info->oem_info, attrib_value, 32);
+        strncpy(info->oem_info, attrib_value, 31);
+        info->oem_info[31] = '\0';
         return;
     }
 
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 6/7] tools/gdbsx: fix -Wstringop-truncation warning

[Marek Marczykowski-Górecki]

gcc-8 complains:

    gx_main.c: In function 'prepare_stop_reply':
    gx_main.c:385:9: error: 'strncpy' output truncated before terminating nul copying 6 bytes from a string of the same length [-Werror=stringop-truncation]
             strncpy(buf, "watch:", 6);
             ^~~~~~~~~~~~~~~~~~~~~~~~~

Since terminating '\0' isn't needed here at all, switch to memcpy.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/debugger/gdbsx/gx/gx_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/debugger/gdbsx/gx/gx_main.c b/tools/debugger/gdbsx/gx/gx_main.c
index a908c45..6dfa501 100644
--- a/tools/debugger/gdbsx/gx/gx_main.c
+++ b/tools/debugger/gdbsx/gx/gx_main.c
@@ -382,7 +382,7 @@ prepare_stop_reply(enum target_signal sig, char *buf, vcpuid_t vcpu)
 
     /* TBD: check if we stopped because of watchpoint */
     if (watchpoint_stop()) {
-        strncpy(buf, "watch:", 6);
+        memcpy(buf, "watch:", 6);
         buf += 6;
         /* TBD: **/
     }
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 7/7] tools/kdd: mute spurious gcc warning

[Marek Marczykowski-Górecki]

gcc-8 complains:

    kdd.c:698:13: error: 'memcpy' offset [-204, -717] is out of the bounds [0, 216] of object 'ctrl' with type 'kdd_ctrl' {aka 'union <anonymous>'} [-Werror=array-bounds]
                 memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    kdd.c: In function 'kdd_select_callback':
    kdd.c:642:14: note: 'ctrl' declared here
         kdd_ctrl ctrl;
                  ^~~~

But this is impossible - 'offset' is unsigned and correctly validated
few lines before.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/debugger/kdd/kdd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
index 1bd5dd5..61d769e 100644
--- a/tools/debugger/kdd/kdd.c
+++ b/tools/debugger/kdd/kdd.c
@@ -695,7 +695,10 @@ static void kdd_handle_read_ctrl(kdd_state *s)
             KDD_LOG(s, "Request outside of known control space\n");
             len = 0;
         } else {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
             memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
+#pragma GCC diagnostic pop
         }
     }
 
-- 
git-series 0.9.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/7] Fix warnings found by gcc 8

[Wei Liu]

On Thu, Apr 05, 2018 at 03:50:48AM +0200, Marek Marczykowski-Górecki wrote:
> A few patches enabling build with gcc 8.
> 
> Marek Marczykowski-Górecki (7):
>   tools/libxc: fix strncpy size
>   tools/misc: fix hypothetical buffer overflow in xen-lowmemd
>   tools/blktap2: fix hypothetical buffer overflow
>   tools/blktap2: fix possible '\0' truncation
>   tools/xenpmd: fix possible '\0' truncation
>   tools/gdbsx: fix -Wstringop-truncation warning
>   tools/kdd: mute spurious gcc warning

Acked-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 0/7] Fix warnings found by gcc 8

[Juergen Gross]

On 05/04/18 11:03, Wei Liu wrote:
> On Thu, Apr 05, 2018 at 03:50:48AM +0200, Marek Marczykowski-Górecki wrote:
>> A few patches enabling build with gcc 8.
>>
>> Marek Marczykowski-Górecki (7):
>>   tools/libxc: fix strncpy size
>>   tools/misc: fix hypothetical buffer overflow in xen-lowmemd
>>   tools/blktap2: fix hypothetical buffer overflow
>>   tools/blktap2: fix possible '\0' truncation
>>   tools/xenpmd: fix possible '\0' truncation
>>   tools/gdbsx: fix -Wstringop-truncation warning
>>   tools/kdd: mute spurious gcc warning
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Release-Acked-by: Juergen Gross <jgross@suse.com>


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 7/7] tools/kdd: mute spurious gcc warning

[Boris Ostrovsky]

On 04/04/2018 09:50 PM, Marek Marczykowski-Górecki wrote:
> gcc-8 complains:
>
>     kdd.c:698:13: error: 'memcpy' offset [-204, -717] is out of the bounds [0, 216] of object 'ctrl' with type 'kdd_ctrl' {aka 'union <anonymous>'} [-Werror=array-bounds]
>                  memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     kdd.c: In function 'kdd_select_callback':
>     kdd.c:642:14: note: 'ctrl' declared here
>          kdd_ctrl ctrl;
>                   ^~~~
>
> But this is impossible - 'offset' is unsigned and correctly validated
> few lines before.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  tools/debugger/kdd/kdd.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> index 1bd5dd5..61d769e 100644
> --- a/tools/debugger/kdd/kdd.c
> +++ b/tools/debugger/kdd/kdd.c
> @@ -695,7 +695,10 @@ static void kdd_handle_read_ctrl(kdd_state *s)
>              KDD_LOG(s, "Request outside of known control space\n");
>              len = 0;
>          } else {
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Warray-bounds"
>              memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> +#pragma GCC diagnostic pop
>          }
>      }
>  


Breaks 32-bit build, at least with my (ancient, gcc version 4.4.5
20101112 (Red Hat 4.4.5-2) (GCC)) compiler:



kdd.c: In function ‘kdd_handle_read_ctrl’:
kdd.c:698: error: #pragma GCC diagnostic not allowed inside functions
kdd.c:699: error: #pragma GCC diagnostic not allowed inside functions
kdd.c:701: error: #pragma GCC diagnostic not allowed inside functions
make[5]: *** [kdd.o] Error 1


-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 7/7] tools/kdd: mute spurious gcc warning

[Wei Liu]

On Fri, Apr 06, 2018 at 08:39:53AM -0400, Boris Ostrovsky wrote:
> On 04/04/2018 09:50 PM, Marek Marczykowski-Górecki wrote:
> > gcc-8 complains:
> >
> >     kdd.c:698:13: error: 'memcpy' offset [-204, -717] is out of the bounds [0, 216] of object 'ctrl' with type 'kdd_ctrl' {aka 'union <anonymous>'} [-Werror=array-bounds]
> >                  memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> >                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >     kdd.c: In function 'kdd_select_callback':
> >     kdd.c:642:14: note: 'ctrl' declared here
> >          kdd_ctrl ctrl;
> >                   ^~~~
> >
> > But this is impossible - 'offset' is unsigned and correctly validated
> > few lines before.
> >
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> >  tools/debugger/kdd/kdd.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> > index 1bd5dd5..61d769e 100644
> > --- a/tools/debugger/kdd/kdd.c
> > +++ b/tools/debugger/kdd/kdd.c
> > @@ -695,7 +695,10 @@ static void kdd_handle_read_ctrl(kdd_state *s)
> >              KDD_LOG(s, "Request outside of known control space\n");
> >              len = 0;
> >          } else {
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Warray-bounds"
> >              memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> > +#pragma GCC diagnostic pop
> >          }
> >      }
> >  
> 
> 
> Breaks 32-bit build, at least with my (ancient, gcc version 4.4.5
> 20101112 (Red Hat 4.4.5-2) (GCC)) compiler:
> 
> 
> 
> kdd.c: In function ‘kdd_handle_read_ctrl’:
> kdd.c:698: error: #pragma GCC diagnostic not allowed inside functions
> kdd.c:699: error: #pragma GCC diagnostic not allowed inside functions
> kdd.c:701: error: #pragma GCC diagnostic not allowed inside functions
> make[5]: *** [kdd.o] Error 1
> 

Does moving the relevant #pragma's outside of the function fix it?

Wei.

> 
> -boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 7/7] tools/kdd: mute spurious gcc warning

[Boris Ostrovsky]

On 04/06/2018 09:07 AM, Wei Liu wrote:
> On Fri, Apr 06, 2018 at 08:39:53AM -0400, Boris Ostrovsky wrote:
>> On 04/04/2018 09:50 PM, Marek Marczykowski-Górecki wrote:
>>> gcc-8 complains:
>>>
>>>     kdd.c:698:13: error: 'memcpy' offset [-204, -717] is out of the bounds [0, 216] of object 'ctrl' with type 'kdd_ctrl' {aka 'union <anonymous>'} [-Werror=array-bounds]
>>>                  memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
>>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>     kdd.c: In function 'kdd_select_callback':
>>>     kdd.c:642:14: note: 'ctrl' declared here
>>>          kdd_ctrl ctrl;
>>>                   ^~~~
>>>
>>> But this is impossible - 'offset' is unsigned and correctly validated
>>> few lines before.
>>>
>>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>>> ---
>>>  tools/debugger/kdd/kdd.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
>>> index 1bd5dd5..61d769e 100644
>>> --- a/tools/debugger/kdd/kdd.c
>>> +++ b/tools/debugger/kdd/kdd.c
>>> @@ -695,7 +695,10 @@ static void kdd_handle_read_ctrl(kdd_state *s)
>>>              KDD_LOG(s, "Request outside of known control space\n");
>>>              len = 0;
>>>          } else {
>>> +#pragma GCC diagnostic push
>>> +#pragma GCC diagnostic ignored "-Warray-bounds"
>>>              memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
>>> +#pragma GCC diagnostic pop
>>>          }
>>>      }
>>>  
>>
>> Breaks 32-bit build, at least with my (ancient, gcc version 4.4.5
>> 20101112 (Red Hat 4.4.5-2) (GCC)) compiler:
>>
>>
>>
>> kdd.c: In function ‘kdd_handle_read_ctrl’:
>> kdd.c:698: error: #pragma GCC diagnostic not allowed inside functions
>> kdd.c:699: error: #pragma GCC diagnostic not allowed inside functions
>> kdd.c:701: error: #pragma GCC diagnostic not allowed inside functions
>> make[5]: *** [kdd.o] Error 1
>>
> Does moving the relevant #pragma's outside of the function fix it?

The additional problem with these pragmas is that apparently push/pop
have been introduced in gcc 4.6.0:

https://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Diagnostic-Pragmas.html#Diagnostic-Pragmas

If you change release number to a lower one (e.g. 4.5.4) you won't see them.

So I can move "diagnostic ignored" from inside the function and that
will clear the "GCC diagnostic not allowed inside functions" error. But
then push/pop are not recognized:

cc1: warnings being treated as errors
kdd.c:639: error: expected [error|warning|ignored] after ‘#pragma GCC
diagnostic’
kdd.c:714: error: expected [error|warning|ignored] after ‘#pragma GCC
diagnostic’

(Interestingly, my 64-bit build completed without issues)

-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 7/7] tools/kdd: mute spurious gcc warning

[Wei Liu]

On Fri, Apr 06, 2018 at 09:39:50AM -0400, Boris Ostrovsky wrote:
> On 04/06/2018 09:07 AM, Wei Liu wrote:
> > On Fri, Apr 06, 2018 at 08:39:53AM -0400, Boris Ostrovsky wrote:
> >> On 04/04/2018 09:50 PM, Marek Marczykowski-Górecki wrote:
> >>> gcc-8 complains:
> >>>
> >>>     kdd.c:698:13: error: 'memcpy' offset [-204, -717] is out of the bounds [0, 216] of object 'ctrl' with type 'kdd_ctrl' {aka 'union <anonymous>'} [-Werror=array-bounds]
> >>>                  memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> >>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>     kdd.c: In function 'kdd_select_callback':
> >>>     kdd.c:642:14: note: 'ctrl' declared here
> >>>          kdd_ctrl ctrl;
> >>>                   ^~~~
> >>>
> >>> But this is impossible - 'offset' is unsigned and correctly validated
> >>> few lines before.
> >>>
> >>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> >>> ---
> >>>  tools/debugger/kdd/kdd.c | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> >>> index 1bd5dd5..61d769e 100644
> >>> --- a/tools/debugger/kdd/kdd.c
> >>> +++ b/tools/debugger/kdd/kdd.c
> >>> @@ -695,7 +695,10 @@ static void kdd_handle_read_ctrl(kdd_state *s)
> >>>              KDD_LOG(s, "Request outside of known control space\n");
> >>>              len = 0;
> >>>          } else {
> >>> +#pragma GCC diagnostic push
> >>> +#pragma GCC diagnostic ignored "-Warray-bounds"
> >>>              memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> >>> +#pragma GCC diagnostic pop
> >>>          }
> >>>      }
> >>>  
> >>
> >> Breaks 32-bit build, at least with my (ancient, gcc version 4.4.5
> >> 20101112 (Red Hat 4.4.5-2) (GCC)) compiler:
> >>
> >>
> >>
> >> kdd.c: In function ‘kdd_handle_read_ctrl’:
> >> kdd.c:698: error: #pragma GCC diagnostic not allowed inside functions
> >> kdd.c:699: error: #pragma GCC diagnostic not allowed inside functions
> >> kdd.c:701: error: #pragma GCC diagnostic not allowed inside functions
> >> make[5]: *** [kdd.o] Error 1
> >>
> > Does moving the relevant #pragma's outside of the function fix it?
> 
> The additional problem with these pragmas is that apparently push/pop
> have been introduced in gcc 4.6.0:
> 
> https://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Diagnostic-Pragmas.html#Diagnostic-Pragmas
> 
> If you change release number to a lower one (e.g. 4.5.4) you won't see them.
> 
> So I can move "diagnostic ignored" from inside the function and that
> will clear the "GCC diagnostic not allowed inside functions" error. But
> then push/pop are not recognized:
> 
> cc1: warnings being treated as errors
> kdd.c:639: error: expected [error|warning|ignored] after ‘#pragma GCC
> diagnostic’
> kdd.c:714: error: expected [error|warning|ignored] after ‘#pragma GCC
> diagnostic’
> 
> (Interestingly, my 64-bit build completed without issues)

Hmm... this is messy.

If you have information about which version does what we can try to
enclose the #pragma's with #if __GCC__.

Wei.

> 
> -boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 7/7] tools/kdd: mute spurious gcc warning

[Boris Ostrovsky]

On 04/06/2018 09:41 AM, Wei Liu wrote:
> On Fri, Apr 06, 2018 at 09:39:50AM -0400, Boris Ostrovsky wrote:
>> On 04/06/2018 09:07 AM, Wei Liu wrote:
>>> On Fri, Apr 06, 2018 at 08:39:53AM -0400, Boris Ostrovsky wrote:
>>>> On 04/04/2018 09:50 PM, Marek Marczykowski-Górecki wrote:
>>>>> gcc-8 complains:
>>>>>
>>>>>     kdd.c:698:13: error: 'memcpy' offset [-204, -717] is out of the bounds [0, 216] of object 'ctrl' with type 'kdd_ctrl' {aka 'union <anonymous>'} [-Werror=array-bounds]
>>>>>                  memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
>>>>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>     kdd.c: In function 'kdd_select_callback':
>>>>>     kdd.c:642:14: note: 'ctrl' declared here
>>>>>          kdd_ctrl ctrl;
>>>>>                   ^~~~
>>>>>
>>>>> But this is impossible - 'offset' is unsigned and correctly validated
>>>>> few lines before.
>>>>>
>>>>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>>>>> ---
>>>>>  tools/debugger/kdd/kdd.c | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
>>>>> index 1bd5dd5..61d769e 100644
>>>>> --- a/tools/debugger/kdd/kdd.c
>>>>> +++ b/tools/debugger/kdd/kdd.c
>>>>> @@ -695,7 +695,10 @@ static void kdd_handle_read_ctrl(kdd_state *s)
>>>>>              KDD_LOG(s, "Request outside of known control space\n");
>>>>>              len = 0;
>>>>>          } else {
>>>>> +#pragma GCC diagnostic push
>>>>> +#pragma GCC diagnostic ignored "-Warray-bounds"
>>>>>              memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
>>>>> +#pragma GCC diagnostic pop
>>>>>          }
>>>>>      }
>>>>>  
>>>> Breaks 32-bit build, at least with my (ancient, gcc version 4.4.5
>>>> 20101112 (Red Hat 4.4.5-2) (GCC)) compiler:
>>>>
>>>>
>>>>
>>>> kdd.c: In function ‘kdd_handle_read_ctrl’:
>>>> kdd.c:698: error: #pragma GCC diagnostic not allowed inside functions
>>>> kdd.c:699: error: #pragma GCC diagnostic not allowed inside functions
>>>> kdd.c:701: error: #pragma GCC diagnostic not allowed inside functions
>>>> make[5]: *** [kdd.o] Error 1
>>>>
>>> Does moving the relevant #pragma's outside of the function fix it?
>> The additional problem with these pragmas is that apparently push/pop
>> have been introduced in gcc 4.6.0:
>>
>> https://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Diagnostic-Pragmas.html#Diagnostic-Pragmas
>>
>> If you change release number to a lower one (e.g. 4.5.4) you won't see them.
>>
>> So I can move "diagnostic ignored" from inside the function and that
>> will clear the "GCC diagnostic not allowed inside functions" error. But
>> then push/pop are not recognized:
>>
>> cc1: warnings being treated as errors
>> kdd.c:639: error: expected [error|warning|ignored] after ‘#pragma GCC
>> diagnostic’
>> kdd.c:714: error: expected [error|warning|ignored] after ‘#pragma GCC
>> diagnostic’
>>
>> (Interestingly, my 64-bit build completed without issues)
> Hmm... this is messy.
>
> If you have information about which version does what we can try to
> enclose the #pragma's with #if __GCC__.


Can we instead pre-compute the pointer to pacify the compiler? I haven't
seen the original error so I can't test it, but something like

diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
index 61d769e..1b048ac 100644
--- a/tools/debugger/kdd/kdd.c
+++ b/tools/debugger/kdd/kdd.c
@@ -688,6 +688,7 @@ static void kdd_handle_read_ctrl(kdd_state *s)
     } else {
         /* 32-bit control-register space starts at 0x[2]cc, for 84 bytes */
         uint64_t offset = addr;
+        void *ptr = &ctrl.c32;
         if (offset > 0x200)
             offset -= 0x200;
         offset -= 0xcc;
@@ -695,10 +696,8 @@ static void kdd_handle_read_ctrl(kdd_state *s)
             KDD_LOG(s, "Request outside of known control space\n");
             len = 0;
         } else {
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Warray-bounds"
-            memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
-#pragma GCC diagnostic pop
+            ptr += offset;
+            memcpy(buf, ptr, len);
         }
     }
 
-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 7/7] tools/kdd: mute spurious gcc warning

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 5055 bytes --]

On Fri, Apr 06, 2018 at 09:56:05AM -0400, Boris Ostrovsky wrote:
> On 04/06/2018 09:41 AM, Wei Liu wrote:
> > On Fri, Apr 06, 2018 at 09:39:50AM -0400, Boris Ostrovsky wrote:
> >> On 04/06/2018 09:07 AM, Wei Liu wrote:
> >>> On Fri, Apr 06, 2018 at 08:39:53AM -0400, Boris Ostrovsky wrote:
> >>>> On 04/04/2018 09:50 PM, Marek Marczykowski-Górecki wrote:
> >>>>> gcc-8 complains:
> >>>>>
> >>>>>     kdd.c:698:13: error: 'memcpy' offset [-204, -717] is out of the bounds [0, 216] of object 'ctrl' with type 'kdd_ctrl' {aka 'union <anonymous>'} [-Werror=array-bounds]
> >>>>>                  memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> >>>>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>>>>     kdd.c: In function 'kdd_select_callback':
> >>>>>     kdd.c:642:14: note: 'ctrl' declared here
> >>>>>          kdd_ctrl ctrl;
> >>>>>                   ^~~~
> >>>>>
> >>>>> But this is impossible - 'offset' is unsigned and correctly validated
> >>>>> few lines before.
> >>>>>
> >>>>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> >>>>> ---
> >>>>>  tools/debugger/kdd/kdd.c | 3 +++
> >>>>>  1 file changed, 3 insertions(+)
> >>>>>
> >>>>> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> >>>>> index 1bd5dd5..61d769e 100644
> >>>>> --- a/tools/debugger/kdd/kdd.c
> >>>>> +++ b/tools/debugger/kdd/kdd.c
> >>>>> @@ -695,7 +695,10 @@ static void kdd_handle_read_ctrl(kdd_state *s)
> >>>>>              KDD_LOG(s, "Request outside of known control space\n");
> >>>>>              len = 0;
> >>>>>          } else {
> >>>>> +#pragma GCC diagnostic push
> >>>>> +#pragma GCC diagnostic ignored "-Warray-bounds"
> >>>>>              memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> >>>>> +#pragma GCC diagnostic pop
> >>>>>          }
> >>>>>      }
> >>>>>  
> >>>> Breaks 32-bit build, at least with my (ancient, gcc version 4.4.5
> >>>> 20101112 (Red Hat 4.4.5-2) (GCC)) compiler:
> >>>>
> >>>>
> >>>>
> >>>> kdd.c: In function ‘kdd_handle_read_ctrl’:
> >>>> kdd.c:698: error: #pragma GCC diagnostic not allowed inside functions
> >>>> kdd.c:699: error: #pragma GCC diagnostic not allowed inside functions
> >>>> kdd.c:701: error: #pragma GCC diagnostic not allowed inside functions
> >>>> make[5]: *** [kdd.o] Error 1
> >>>>
> >>> Does moving the relevant #pragma's outside of the function fix it?
> >> The additional problem with these pragmas is that apparently push/pop
> >> have been introduced in gcc 4.6.0:
> >>
> >> https://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Diagnostic-Pragmas.html#Diagnostic-Pragmas
> >>
> >> If you change release number to a lower one (e.g. 4.5.4) you won't see them.
> >>
> >> So I can move "diagnostic ignored" from inside the function and that
> >> will clear the "GCC diagnostic not allowed inside functions" error. But
> >> then push/pop are not recognized:
> >>
> >> cc1: warnings being treated as errors
> >> kdd.c:639: error: expected [error|warning|ignored] after ‘#pragma GCC
> >> diagnostic’
> >> kdd.c:714: error: expected [error|warning|ignored] after ‘#pragma GCC
> >> diagnostic’
> >>
> >> (Interestingly, my 64-bit build completed without issues)
> > Hmm... this is messy.
> >
> > If you have information about which version does what we can try to
> > enclose the #pragma's with #if __GCC__.
> 
> 
> Can we instead pre-compute the pointer to pacify the compiler? I haven't
> seen the original error so I can't test it, but something like

Nope, it doesn't help. But adding "if (offset > 0)" before that "+=
offset" does... 
For me it looks like a gcc bug. Not sure how to deal with this. Enclose
#pragma with #if __GNUC__ >= 8 ?

> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> index 61d769e..1b048ac 100644
> --- a/tools/debugger/kdd/kdd.c
> +++ b/tools/debugger/kdd/kdd.c
> @@ -688,6 +688,7 @@ static void kdd_handle_read_ctrl(kdd_state *s)
>      } else {
>          /* 32-bit control-register space starts at 0x[2]cc, for 84 bytes */
>          uint64_t offset = addr;
> +        void *ptr = &ctrl.c32;
>          if (offset > 0x200)
>              offset -= 0x200;
>          offset -= 0xcc;
> @@ -695,10 +696,8 @@ static void kdd_handle_read_ctrl(kdd_state *s)
>              KDD_LOG(s, "Request outside of known control space\n");
>              len = 0;
>          } else {
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Warray-bounds"
> -            memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> -#pragma GCC diagnostic pop
> +            ptr += offset;
> +            memcpy(buf, ptr, len);
>          }
>      }
>  
> -boris

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

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

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 7/7] tools/kdd: mute spurious gcc warning

[Wei Liu]

On Fri, Apr 06, 2018 at 04:32:42PM +0200, Marek Marczykowski-Górecki wrote:
> > >> cc1: warnings being treated as errors
> > >> kdd.c:639: error: expected [error|warning|ignored] after ‘#pragma GCC
> > >> diagnostic’
> > >> kdd.c:714: error: expected [error|warning|ignored] after ‘#pragma GCC
> > >> diagnostic’
> > >>
> > >> (Interestingly, my 64-bit build completed without issues)
> > > Hmm... this is messy.
> > >
> > > If you have information about which version does what we can try to
> > > enclose the #pragma's with #if __GCC__.
> > 
> > 
> > Can we instead pre-compute the pointer to pacify the compiler? I haven't
> > seen the original error so I can't test it, but something like
> 
> Nope, it doesn't help. But adding "if (offset > 0)" before that "+=
> offset" does... 
> For me it looks like a gcc bug. Not sure how to deal with this. Enclose
> #pragma with #if __GNUC__ >= 8 ?

I would be fine with this.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH] tools/kdd: use mute -Warray-bounds only on new gcc version

[Marek Marczykowski-Górecki]

"#pragma GCC diagnostic push" is supported only on gcc >= 4.6. But since
muting this the warning is needed only on gcc >= 8, do it only then,
instead of tricking the compiler about this code (and making it less
readable to the human too).

This fixes 5888eecca0 "tools/kdd: mute spurious gcc warning".

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/debugger/kdd/kdd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
index 61d769ece9..95c3a949ec 100644
--- a/tools/debugger/kdd/kdd.c
+++ b/tools/debugger/kdd/kdd.c
@@ -695,10 +695,14 @@ static void kdd_handle_read_ctrl(kdd_state *s)
             KDD_LOG(s, "Request outside of known control space\n");
             len = 0;
         } else {
+#if __GNUC__ >= 8
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Warray-bounds"
+#endif
             memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
+#if __GNUC__ >= 8
 #pragma GCC diagnostic pop
+#endif
         }
     }
 
-- 
2.13.6


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 7/7] tools/kdd: mute spurious gcc warning

[Tim Deegan]

Hi,

At 16:32 +0200 on 06 Apr (1523032362), Marek Marczykowski-Górecki wrote:
> On Fri, Apr 06, 2018 at 09:56:05AM -0400, Boris Ostrovsky wrote:
> > Can we instead pre-compute the pointer to pacify the compiler? I haven't
> > seen the original error so I can't test it, but something like
> 
> Nope, it doesn't help. But adding "if (offset > 0)" before that "+=
> offset" does... 
> For me it looks like a gcc bug. Not sure how to deal with this. Enclose
> #pragma with #if __GNUC__ >= 8 ?

If the logic can be reshuffled so that gcc8 is happy with, that would
be better than silencing the warning, IMO.  As far as I can see the
pointer is indeed safe[1], and if the compiler's going to decide
that it's not, I'd rather have the warning than have it silently
optimized out.

Cheers,

Tim.

[1] except a possible memcpy(dst, one-past-the-end-of-the-struct, 0),
    which is not what's being complained of here, and is a good deal
    too theological for me while I'm on my holiday.

> > diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> > index 61d769e..1b048ac 100644
> > --- a/tools/debugger/kdd/kdd.c
> > +++ b/tools/debugger/kdd/kdd.c
> > @@ -688,6 +688,7 @@ static void kdd_handle_read_ctrl(kdd_state *s)
> >      } else {
> >          /* 32-bit control-register space starts at 0x[2]cc, for 84 bytes */
> >          uint64_t offset = addr;
> > +        void *ptr = &ctrl.c32;
> >          if (offset > 0x200)
> >              offset -= 0x200;
> >          offset -= 0xcc;
> > @@ -695,10 +696,8 @@ static void kdd_handle_read_ctrl(kdd_state *s)
> >              KDD_LOG(s, "Request outside of known control space\n");
> >              len = 0;
> >          } else {
> > -#pragma GCC diagnostic push
> > -#pragma GCC diagnostic ignored "-Warray-bounds"
> > -            memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> > -#pragma GCC diagnostic pop
> > +            ptr += offset;
> > +            memcpy(buf, ptr, len);
> >          }
> >      }
> >  
> > -boris
> 
> -- 
> Best Regards,
> Marek Marczykowski-Górecki
> Invisible Things Lab
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/kdd: use mute -Warray-bounds only on new gcc version

[Wei Liu]

On Fri, Apr 06, 2018 at 05:32:57PM +0200, Marek Marczykowski-Górecki wrote:
> "#pragma GCC diagnostic push" is supported only on gcc >= 4.6. But since
> muting this the warning is needed only on gcc >= 8, do it only then,
> instead of tricking the compiler about this code (and making it less
> readable to the human too).
> 
> This fixes 5888eecca0 "tools/kdd: mute spurious gcc warning".
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  tools/debugger/kdd/kdd.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> index 61d769ece9..95c3a949ec 100644
> --- a/tools/debugger/kdd/kdd.c
> +++ b/tools/debugger/kdd/kdd.c
> @@ -695,10 +695,14 @@ static void kdd_handle_read_ctrl(kdd_state *s)
>              KDD_LOG(s, "Request outside of known control space\n");
>              len = 0;
>          } else {
> +#if __GNUC__ >= 8
>  #pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Warray-bounds"
> +#endif
>              memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> +#if __GNUC__ >= 8
>  #pragma GCC diagnostic pop
> +#endif

Oh thanks for the quick turnaround.

Since Tim thinks it is better to not disable the warning -- how about
using assert() to give the compiler a hint? Would that work?

There is no need to rush for the deadline, I think the fix to this can
be committed any time.

Wei.

>          }
>      }
>  
> -- 
> 2.13.6
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/kdd: use mute -Warray-bounds only on new gcc version

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 1915 bytes --]

On Fri, Apr 06, 2018 at 06:12:50PM +0100, Wei Liu wrote:
> On Fri, Apr 06, 2018 at 05:32:57PM +0200, Marek Marczykowski-Górecki wrote:
> > "#pragma GCC diagnostic push" is supported only on gcc >= 4.6. But since
> > muting this the warning is needed only on gcc >= 8, do it only then,
> > instead of tricking the compiler about this code (and making it less
> > readable to the human too).
> > 
> > This fixes 5888eecca0 "tools/kdd: mute spurious gcc warning".
> > 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> >  tools/debugger/kdd/kdd.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> > index 61d769ece9..95c3a949ec 100644
> > --- a/tools/debugger/kdd/kdd.c
> > +++ b/tools/debugger/kdd/kdd.c
> > @@ -695,10 +695,14 @@ static void kdd_handle_read_ctrl(kdd_state *s)
> >              KDD_LOG(s, "Request outside of known control space\n");
> >              len = 0;
> >          } else {
> > +#if __GNUC__ >= 8
> >  #pragma GCC diagnostic push
> >  #pragma GCC diagnostic ignored "-Warray-bounds"
> > +#endif
> >              memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
> > +#if __GNUC__ >= 8
> >  #pragma GCC diagnostic pop
> > +#endif
> 
> Oh thanks for the quick turnaround.
> 
> Since Tim thinks it is better to not disable the warning -- how about
> using assert() to give the compiler a hint? Would that work?

No, it doesn't.

Changing offset type to uint32_t, or unsigned int works. Also adding
"offset &= 0x2ff" helps (but changes behavior). And now I wonder if
this warning isn't legitimate - maybe there is some int overflow case
that I don't see?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

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

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/kdd: use mute -Warray-bounds only on new gcc version

[Tim Deegan]

At 00:39 +0200 on 07 Apr (1523061555), Marek Marczykowski-Górecki wrote:
> On Fri, Apr 06, 2018 at 06:12:50PM +0100, Wei Liu wrote:
> > On Fri, Apr 06, 2018 at 05:32:57PM +0200, Marek Marczykowski-Górecki wrote:
> > Oh thanks for the quick turnaround.
> > 
> > Since Tim thinks it is better to not disable the warning -- how about
> > using assert() to give the compiler a hint? Would that work?
> 
> No, it doesn't.
> 
> Changing offset type to uint32_t, or unsigned int works. Also adding
> "offset &= 0x2ff" helps (but changes behavior). And now I wonder if
> this warning isn't legitimate - maybe there is some int overflow case
> that I don't see?

Huh.  I don't see an overflow - if anything, having offset be 64bit
should make it safer.  But having offset be uint32_t should be fine
too, so if it makes gcc happy, that's fine.

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel