All of lore.kernel.org
 help / color / mirror / Atom feed
* Compile errors with gcc 4.8
@ 2013-02-06 20:37 M A Young
  2013-02-07  0:55 ` Andrew Cooper
  2013-02-07 10:02 ` Jan Beulich
  0 siblings, 2 replies; 14+ messages in thread
From: M A Young @ 2013-02-06 20:37 UTC (permalink / raw)
  To: xen-devel

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1706 bytes --]

Fedora rawhide has just moved onto gcc 4.8, and I have had to apply the 
attached patch to get it to build (I constructed the patch against 4.2.1 
but I believe it will apply to xen-unstable).

There are two types of problem, the first is from 
xen/common/compat/memory.c where the error is

memory.c: In function 'compat_memory_op':
/builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:35:33: 
error: typedef '__guest_handle_const_compat_memory_exchange_t' locally 
defined but not used [-Werror=unused-local-typedefs]
      typedef struct { type *p; } __guest_handle_ ## name
                                  ^
/builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:43:5: 
note: in expansion of macro '___DEFINE_XEN_GUEST_HANDLE'
      ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
      ^
/builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:44:41: 
note: in expansion of macro '__DEFINE_XEN_GUEST_HANDLE'
  #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name, 
name)
                                          ^
memory.c:261:13: note: in expansion of macro 'DEFINE_XEN_GUEST_HANDLE'
              DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t);
              ^

so we are defining something that isn't used.

Secondly gcc 4.8 objects to lines like

memset(ctxt, 0, sizeof(ctxt));

where I think you are just zeroing a pointer's worth of memory. There are 
a few cases of this in the xen code fixed in the patch, and I am also 
suspicious of line 630 of 
stubdom/grub-upstream/stage2/fsys_reiserfs.c which is

memset (INFO->blocks, 0, sizeof (INFO->blocks));

which is noted in the build log but not flagged as an error.

 	Michael Young

[-- Attachment #2: Type: TEXT/PLAIN, Size: 3328 bytes --]

--- xen-4.2.1/xen/common/compat/memory.c.orig	2012-12-17 15:01:54.000000000 +0000
+++ xen-4.2.1/xen/common/compat/memory.c	2013-01-28 21:24:36.971241229 +0000
@@ -258,7 +258,7 @@
 
         case XENMEM_exchange:
         {
-            DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t);
+            DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t) __attribute__((unused));
             int order_delta;
 
             BUG_ON(split >= 0 && rc);
--- xen-4.2.1/tools/libxc/xc_dom_boot.c.orig	2012-12-17 15:00:48.000000000 +0000
+++ xen-4.2.1/tools/libxc/xc_dom_boot.c	2013-01-28 22:21:13.215782329 +0000
@@ -266,7 +266,7 @@
         return rc;
 
     /* let the vm run */
-    memset(ctxt, 0, sizeof(ctxt));
+    memset(ctxt, 0, sizeof(*ctxt));
     if ( (rc = dom->arch_hooks->vcpu(dom, ctxt)) != 0 )
         return rc;
     xc_dom_unmap_all(dom);
--- xen-4.2.1/tools/blktap2/drivers/md5.c.orig	2012-12-17 15:00:11.000000000 +0000
+++ xen-4.2.1/tools/blktap2/drivers/md5.c	2013-01-28 23:49:51.940289123 +0000
@@ -174,7 +174,7 @@
     MD5Transform(ctx->buf, (uint32_t *) ctx->in);
     byteReverse((unsigned char *) ctx->buf, 4);
     memcpy(digest, ctx->buf, 16);
-    memset(ctx, 0, sizeof(ctx));     /* In case it's sensitive */
+    memset(ctx, 0, sizeof(*ctx));     /* In case it's sensitive */
 }
 
 /* The four core functions - F1 is optimized somewhat */
--- xen-4.2.1/tools/libxl/libxl_qmp.c.orig	2012-12-17 15:01:09.000000000 +0000
+++ xen-4.2.1/tools/libxl/libxl_qmp.c	2013-01-29 21:28:27.763650073 +0000
@@ -377,7 +377,7 @@
     ret = libxl_fd_set_cloexec(qmp->ctx, qmp->qmp_fd, 1);
     if (ret) return -1;
 
-    memset(&qmp->addr, 0, sizeof (&qmp->addr));
+    memset(&qmp->addr, 0, sizeof (qmp->addr));
     qmp->addr.sun_family = AF_UNIX;
     strncpy(qmp->addr.sun_path, qmp_socket_path,
             sizeof (qmp->addr.sun_path));
--- xen-4.2.1/tools/xenstat/libxenstat/src/xenstat_linux.c.orig	2012-12-17 15:01:35.000000000 +0000
+++ xen-4.2.1/tools/xenstat/libxenstat/src/xenstat_linux.c	2013-01-29 21:43:46.044169987 +0000
@@ -113,7 +113,7 @@
 
 	/* Initialize all variables called has passed as non-NULL to zeros */
 	if (iface != NULL)
-		memset(iface, 0, sizeof(iface));
+		memset(iface, 0, sizeof(*iface));
 	if (rxBytes != NULL)
 		*rxBytes = 0;
 	if (rxPackets != NULL)
--- xen-4.2.1/tools/debugger/kdd/kdd-xen.c.orig	2012-12-17 15:00:22.000000000 +0000
+++ xen-4.2.1/tools/debugger/kdd/kdd-xen.c	2013-01-29 21:45:12.652087239 +0000
@@ -333,7 +333,7 @@
     if (!cpu) 
         return -1;
 
-    memset(r, 0, sizeof(r));
+    memset(r, 0, sizeof(*r));
     
     if (w64)
         kdd_get_regs_x86_64(cpu, &r->r64);
--- xen-4.2.1/tools/python/xen/lowlevel/netlink/libnetlink.c.orig	2012-12-17 15:01:24.000000000 +0000
+++ xen-4.2.1/tools/python/xen/lowlevel/netlink/libnetlink.c	2013-01-29 21:47:59.524001053 +0000
@@ -37,7 +37,7 @@
        int sndbuf = 32768;
        int rcvbuf = 32768;
 
-       memset(rth, 0, sizeof(rth));
+       memset(rth, 0, sizeof(*rth));
 
        rth->fd = socket(AF_NETLINK, SOCK_RAW, protocol);
        if (rth->fd < 0) {
@@ -264,7 +264,7 @@
                return -1;
        }
 
-       memset(buf,0,sizeof(buf));
+       memset(buf,0,sizeof(*buf));
 
        iov.iov_base = buf;
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Compile errors with gcc 4.8
  2013-02-06 20:37 Compile errors with gcc 4.8 M A Young
@ 2013-02-07  0:55 ` Andrew Cooper
  2013-02-07  8:59   ` M A Young
  2013-02-07  9:08   ` Frediano Ziglio
  2013-02-07 10:02 ` Jan Beulich
  1 sibling, 2 replies; 14+ messages in thread
From: Andrew Cooper @ 2013-02-07  0:55 UTC (permalink / raw)
  To: M A Young; +Cc: xen-devel

On 06/02/2013 20:37, M A Young wrote:
> Fedora rawhide has just moved onto gcc 4.8, and I have had to apply the 
> attached patch to get it to build (I constructed the patch against 4.2.1 
> but I believe it will apply to xen-unstable).
>
> There are two types of problem, the first is from 
> xen/common/compat/memory.c where the error is
>
> memory.c: In function 'compat_memory_op':
> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:35:33: 
> error: typedef '__guest_handle_const_compat_memory_exchange_t' locally 
> defined but not used [-Werror=unused-local-typedefs]
>       typedef struct { type *p; } __guest_handle_ ## name
>                                   ^
> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:43:5: 
> note: in expansion of macro '___DEFINE_XEN_GUEST_HANDLE'
>       ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
>       ^
> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:44:41: 
> note: in expansion of macro '__DEFINE_XEN_GUEST_HANDLE'
>   #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name, 
> name)
>                                           ^
> memory.c:261:13: note: in expansion of macro 'DEFINE_XEN_GUEST_HANDLE'
>               DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t);
>               ^
>
> so we are defining something that isn't used.

But it is used, by guest_handle_cast, albeit through several layers of
macros.

I would tentativly object to the use of "__attribute__((unused));" to
work around what appears to be a gcc bug.  If 4.8 support is desperately
wanted, then a better solution would be to specify
-Wno-unused-local-typedefs to disable the buggy feature.

~Andrew

>
> Secondly gcc 4.8 objects to lines like
>
> memset(ctxt, 0, sizeof(ctxt));
>
> where I think you are just zeroing a pointer's worth of memory. There are 
> a few cases of this in the xen code fixed in the patch, and I am also 
> suspicious of line 630 of 
> stubdom/grub-upstream/stage2/fsys_reiserfs.c which is
>
> memset (INFO->blocks, 0, sizeof (INFO->blocks));
>
> which is noted in the build log but not flagged as an error.
>
>  	Michael Young

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Compile errors with gcc 4.8
  2013-02-07  0:55 ` Andrew Cooper
@ 2013-02-07  8:59   ` M A Young
  2013-02-07  9:06     ` Ian Campbell
  2013-02-07  9:07     ` M A Young
  2013-02-07  9:08   ` Frediano Ziglio
  1 sibling, 2 replies; 14+ messages in thread
From: M A Young @ 2013-02-07  8:59 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel

On Thu, 7 Feb 2013, Andrew Cooper wrote:

> On 06/02/2013 20:37, M A Young wrote:
>
>> There are two types of problem, the first is from
>> xen/common/compat/memory.c where the error is
>>
>> memory.c: In function 'compat_memory_op':
>> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:35:33:
>> error: typedef '__guest_handle_const_compat_memory_exchange_t' locally
>> defined but not used [-Werror=unused-local-typedefs]
>>       typedef struct { type *p; } __guest_handle_ ## name
>>                                   ^
>> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:43:5:
>> note: in expansion of macro '___DEFINE_XEN_GUEST_HANDLE'
>>       ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
>>       ^
>> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:44:41:
>> note: in expansion of macro '__DEFINE_XEN_GUEST_HANDLE'
>>   #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name,
>> name)
>>                                           ^
>> memory.c:261:13: note: in expansion of macro 'DEFINE_XEN_GUEST_HANDLE'
>>               DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t);
>>               ^
>>
>> so we are defining something that isn't used.
>
> But it is used, by guest_handle_cast, albeit through several layers of
> macros.
>
> I would tentativly object to the use of "__attribute__((unused));" to
> work around what appears to be a gcc bug.  If 4.8 support is desperately
> wanted, then a better solution would be to specify
> -Wno-unused-local-typedefs to disable the buggy feature.

It looked to me as if the DEFINE macro does two typedefs and the code only 
uses the first, so gcc 4.8 was correct in its objections. The suggested 
workaround of using -Wno-unused-local-typedefs isn't very portible as 
earlier versions of gcc don't recognize it.

 	Michael Young

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Compile errors with gcc 4.8
  2013-02-07  8:59   ` M A Young
@ 2013-02-07  9:06     ` Ian Campbell
  2013-02-07  9:57       ` Jan Beulich
  2013-02-07  9:07     ` M A Young
  1 sibling, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2013-02-07  9:06 UTC (permalink / raw)
  To: M A Young; +Cc: Andrew Cooper, xen-devel

On Thu, 2013-02-07 at 08:59 +0000, M A Young wrote:
> On Thu, 7 Feb 2013, Andrew Cooper wrote:
> 
> > On 06/02/2013 20:37, M A Young wrote:
> >
> >> There are two types of problem, the first is from
> >> xen/common/compat/memory.c where the error is
> >>
> >> memory.c: In function 'compat_memory_op':
> >> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:35:33:
> >> error: typedef '__guest_handle_const_compat_memory_exchange_t' locally
> >> defined but not used [-Werror=unused-local-typedefs]
> >>       typedef struct { type *p; } __guest_handle_ ## name
> >>                                   ^
> >> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:43:5:
> >> note: in expansion of macro '___DEFINE_XEN_GUEST_HANDLE'
> >>       ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
> >>       ^
> >> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:44:41:
> >> note: in expansion of macro '__DEFINE_XEN_GUEST_HANDLE'
> >>   #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name,
> >> name)
> >>                                           ^
> >> memory.c:261:13: note: in expansion of macro 'DEFINE_XEN_GUEST_HANDLE'
> >>               DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t);
> >>               ^
> >>
> >> so we are defining something that isn't used.
> >
> > But it is used, by guest_handle_cast, albeit through several layers of
> > macros.
> >
> > I would tentativly object to the use of "__attribute__((unused));" to
> > work around what appears to be a gcc bug.  If 4.8 support is desperately
> > wanted, then a better solution would be to specify
> > -Wno-unused-local-typedefs to disable the buggy feature.
> 
> It looked to me as if the DEFINE macro does two typedefs and the code only 
> uses the first, so gcc 4.8 was correct in its objections. The suggested 
> workaround of using -Wno-unused-local-typedefs isn't very portible as 
> earlier versions of gcc don't recognize it.

We have a cc-option macro in the makefiles to enable these sorts of
options to be enabled conditionally.

However it seems a bit odd to be using DEFINE_XEN_GUEST_HANDLE in this
manner, should it not be declared in some header? I can see a
DEFINE_COMPAT_HANDLE(compat_memory_exchange_t) in
xen/include/compat/memory.h. Perhaps the one in memory.c can just be
dropped or otherwise modified to use this?

Ian.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Compile errors with gcc 4.8
  2013-02-07  8:59   ` M A Young
  2013-02-07  9:06     ` Ian Campbell
@ 2013-02-07  9:07     ` M A Young
  1 sibling, 0 replies; 14+ messages in thread
From: M A Young @ 2013-02-07  9:07 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel

On Thu, 7 Feb 2013, M A Young wrote:

> It looked to me as if the DEFINE macro does two typedefs and the code only 
> uses the first, so gcc 4.8 was correct in its objections. The suggested 
> workaround of using -Wno-unused-local-typedefs isn't very portible as earlier 
> versions of gcc don't recognize it.

Actually, I could be wrong about -Wno-unused-local-typedefs as I think I 
am confusing it with the flag to ignore the other error which I attempted 
to use while tracing the occurrences for that problem.

 	Michael Young

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Compile errors with gcc 4.8
  2013-02-07  0:55 ` Andrew Cooper
  2013-02-07  8:59   ` M A Young
@ 2013-02-07  9:08   ` Frediano Ziglio
  1 sibling, 0 replies; 14+ messages in thread
From: Frediano Ziglio @ 2013-02-07  9:08 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel, m.a.young

On Thu, 2013-02-07 at 00:55 +0000, Andrew Cooper wrote:
> On 06/02/2013 20:37, M A Young wrote:
> > Fedora rawhide has just moved onto gcc 4.8, and I have had to apply the 
> > attached patch to get it to build (I constructed the patch against 4.2.1 
> > but I believe it will apply to xen-unstable).
> >
> > There are two types of problem, the first is from 
> > xen/common/compat/memory.c where the error is
> >
> > memory.c: In function 'compat_memory_op':
> > /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:35:33: 
> > error: typedef '__guest_handle_const_compat_memory_exchange_t' locally 
> > defined but not used [-Werror=unused-local-typedefs]
> >       typedef struct { type *p; } __guest_handle_ ## name
> >                                   ^
> > /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:43:5: 
> > note: in expansion of macro '___DEFINE_XEN_GUEST_HANDLE'
> >       ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
> >       ^
> > /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:44:41: 
> > note: in expansion of macro '__DEFINE_XEN_GUEST_HANDLE'
> >   #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name, 
> > name)
> >                                           ^
> > memory.c:261:13: note: in expansion of macro 'DEFINE_XEN_GUEST_HANDLE'
> >               DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t);
> >               ^
> >
> > so we are defining something that isn't used.
> 
> But it is used, by guest_handle_cast, albeit through several layers of
> macros.
> 
> I would tentativly object to the use of "__attribute__((unused));" to
> work around what appears to be a gcc bug.  If 4.8 support is desperately
> wanted, then a better solution would be to specify
> -Wno-unused-local-typedefs to disable the buggy feature.
> 

A workaround would be to put the defined outside the function. At the
end it's just a typedef.

> ~Andrew
> 
> >
> > Secondly gcc 4.8 objects to lines like
> >
> > memset(ctxt, 0, sizeof(ctxt));
> >
> > where I think you are just zeroing a pointer's worth of memory. There are 
> > a few cases of this in the xen code fixed in the patch, and I am also 
> > suspicious of line 630 of 
> > stubdom/grub-upstream/stage2/fsys_reiserfs.c which is
> >
> > memset (INFO->blocks, 0, sizeof (INFO->blocks));
> >
> > which is noted in the build log but not flagged as an error.
> >
> >  	Michael Young
> 

This seems real problems even to me. The md5 code is know and wrong to
me for sure.

Frediano

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Compile errors with gcc 4.8
  2013-02-07  9:06     ` Ian Campbell
@ 2013-02-07  9:57       ` Jan Beulich
  2013-02-07 11:55         ` Keir Fraser
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2013-02-07  9:57 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Andrew Cooper, xen-devel, M A Young

>>> On 07.02.13 at 10:06, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2013-02-07 at 08:59 +0000, M A Young wrote:
>> On Thu, 7 Feb 2013, Andrew Cooper wrote:
>> 
>> > On 06/02/2013 20:37, M A Young wrote:
>> >
>> >> There are two types of problem, the first is from
>> >> xen/common/compat/memory.c where the error is
>> >>
>> >> memory.c: In function 'compat_memory_op':
>> >> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:35:33:
>> >> error: typedef '__guest_handle_const_compat_memory_exchange_t' locally
>> >> defined but not used [-Werror=unused-local-typedefs]
>> >>       typedef struct { type *p; } __guest_handle_ ## name
>> >>                                   ^
>> >> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:43:5:
>> >> note: in expansion of macro '___DEFINE_XEN_GUEST_HANDLE'
>> >>       ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
>> >>       ^
>> >> /builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:44:41:
>> >> note: in expansion of macro '__DEFINE_XEN_GUEST_HANDLE'
>> >>   #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name,
>> >> name)
>> >>                                           ^
>> >> memory.c:261:13: note: in expansion of macro 'DEFINE_XEN_GUEST_HANDLE'
>> >>               DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t);
>> >>               ^
>> >>
>> >> so we are defining something that isn't used.
>> >
>> > But it is used, by guest_handle_cast, albeit through several layers of
>> > macros.
>> >
>> > I would tentativly object to the use of "__attribute__((unused));" to
>> > work around what appears to be a gcc bug.  If 4.8 support is desperately
>> > wanted, then a better solution would be to specify
>> > -Wno-unused-local-typedefs to disable the buggy feature.
>> 
>> It looked to me as if the DEFINE macro does two typedefs and the code only 
>> uses the first, so gcc 4.8 was correct in its objections. The suggested 
>> workaround of using -Wno-unused-local-typedefs isn't very portible as 
>> earlier versions of gcc don't recognize it.
> 
> We have a cc-option macro in the makefiles to enable these sorts of
> options to be enabled conditionally.
> 
> However it seems a bit odd to be using DEFINE_XEN_GUEST_HANDLE in this
> manner, should it not be declared in some header? I can see a
> DEFINE_COMPAT_HANDLE(compat_memory_exchange_t) in
> xen/include/compat/memory.h. Perhaps the one in memory.c can just be
> dropped or otherwise modified to use this?

Actually, I'm of the opposite opinion - it should be in other places
too that handles don't get needlessly defined by the public
headers. They should get defined only when there actually is a
use for them. Everything else can be defined where actually
consumed, as in this case: For one, a handle of a compat_*
type can't be defined in a public header anyway, as the compat
types don't get defined there. And then, as you say, the oddity
of this type makes it desirable to scope restrict it as much as
possible.

Now, for the actual solution, I'd prefer the -Wno-... option
suggested above, or as a second best choice generalizing the
solution suggested by Michael, applying the attribute in
DEFINE_XEN_GUEST_HANDLE() itself. This is the more that
attaching it in to the use of the macro just happens to work,
but isn't guaranteed to in the future: Switching around the
order of the two lines of the expansion

#define __DEFINE_XEN_GUEST_HANDLE(name, type) \
    ___DEFINE_XEN_GUEST_HANDLE(name, type);   \
    ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)

or adding a third (say, volatile) one would re-expose the
problem.

Jan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Compile errors with gcc 4.8
  2013-02-06 20:37 Compile errors with gcc 4.8 M A Young
  2013-02-07  0:55 ` Andrew Cooper
@ 2013-02-07 10:02 ` Jan Beulich
  2013-02-07 23:25   ` [Patch 2/2] " M A Young
  1 sibling, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2013-02-07 10:02 UTC (permalink / raw)
  To: M A Young; +Cc: xen-devel

>>> On 06.02.13 at 21:37, M A Young <m.a.young@durham.ac.uk> wrote:
>@@ -264,7 +264,7 @@
>                return -1;
>        }
> 
>-       memset(buf,0,sizeof(buf));
>+       memset(buf,0,sizeof(*buf));

While everything else looks right, this one for sure is wrong - buf
is an array, and hence sizeof(*buf) would clear just the first array
element.

Also, we'd need you Signed-off-by to get the patch committed,
and I'd suggest splitting off the public header related change
from the (tools side) rest.

Jan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Compile errors with gcc 4.8
  2013-02-07  9:57       ` Jan Beulich
@ 2013-02-07 11:55         ` Keir Fraser
  2013-02-07 23:20           ` [PATCH 1/2] " M A Young
  0 siblings, 1 reply; 14+ messages in thread
From: Keir Fraser @ 2013-02-07 11:55 UTC (permalink / raw)
  To: Jan Beulich, Ian Campbell; +Cc: Andrew Cooper, M A Young, xen-devel

On 07/02/2013 09:57, "Jan Beulich" <JBeulich@suse.com> wrote:

> Actually, I'm of the opposite opinion - it should be in other places
> too that handles don't get needlessly defined by the public
> headers. They should get defined only when there actually is a
> use for them. Everything else can be defined where actually
> consumed, as in this case: For one, a handle of a compat_*
> type can't be defined in a public header anyway, as the compat
> types don't get defined there. And then, as you say, the oddity
> of this type makes it desirable to scope restrict it as much as
> possible.
> 
> Now, for the actual solution, I'd prefer the -Wno-... Option

+1. It's not a compiler warning that I care about.

 -- Keir

> suggested above, or as a second best choice generalizing the
> solution suggested by Michael, applying the attribute in
> DEFINE_XEN_GUEST_HANDLE() itself. This is the more that
> attaching it in to the use of the macro just happens to work,
> but isn't guaranteed to in the future: Switching around the
> order of the two lines of the expansion
> 
> #define __DEFINE_XEN_GUEST_HANDLE(name, type) \
>     ___DEFINE_XEN_GUEST_HANDLE(name, type);   \
>     ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
> 
> or adding a third (say, volatile) one would re-expose the
> problem.
> 
> Jan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] Compile errors with gcc 4.8
  2013-02-07 11:55         ` Keir Fraser
@ 2013-02-07 23:20           ` M A Young
  2013-02-08  8:11             ` Jan Beulich
  0 siblings, 1 reply; 14+ messages in thread
From: M A Young @ 2013-02-07 23:20 UTC (permalink / raw)
  To: Keir Fraser; +Cc: Andrew Cooper, Ian Campbell, Jan Beulich, xen-devel

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1453 bytes --]

On Thu, 7 Feb 2013, Keir Fraser wrote:

> On 07/02/2013 09:57, "Jan Beulich" <JBeulich@suse.com> wrote:
>
>> Actually, I'm of the opposite opinion - it should be in other places
>> too that handles don't get needlessly defined by the public
>> headers. They should get defined only when there actually is a
>> use for them. Everything else can be defined where actually
>> consumed, as in this case: For one, a handle of a compat_*
>> type can't be defined in a public header anyway, as the compat
>> types don't get defined there. And then, as you say, the oddity
>> of this type makes it desirable to scope restrict it as much as
>> possible.
>>
>> Now, for the actual solution, I'd prefer the -Wno-... Option
>
> +1. It's not a compiler warning that I care about.
>
> -- Keir
>
>> suggested above, or as a second best choice generalizing the
>> solution suggested by Michael, applying the attribute in
>> DEFINE_XEN_GUEST_HANDLE() itself. This is the more that
>> attaching it in to the use of the macro just happens to work,
>> but isn't guaranteed to in the future: Switching around the
>> order of the two lines of the expansion
>>
>> #define __DEFINE_XEN_GUEST_HANDLE(name, type) \
>>     ___DEFINE_XEN_GUEST_HANDLE(name, type);   \
>>     ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
>>
>> or adding a third (say, volatile) one would re-expose the
>> problem.
>>
>> Jan

I am attaching a patch that turns off this check.

 	Michael Young

[-- Attachment #2: Type: TEXT/PLAIN, Size: 1548 bytes --]

gcc 4.8 gives the following error when building xen/common/compat/memory.c

memory.c: In function 'compat_memory_op':
/builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:35:33: error: typedef '__guest_handle_const_compat_memory_exchange_t' locally defined but not used [-Werror=unused-local-typedefs]
     typedef struct { type *p; } __guest_handle_ ## name
                                 ^
/builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:43:5: note: in expansion of macro '___DEFINE_XEN_GUEST_HANDLE'
     ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
     ^
/builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:44:41: note: in expansion of macro '__DEFINE_XEN_GUEST_HANDLE'
 #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name, name)
                                         ^
memory.c:261:13: note: in expansion of macro 'DEFINE_XEN_GUEST_HANDLE'
             DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t);
             ^

The error is harmless in this case so turn the check off.

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>

--- xen-4.2.1/Config.mk.orig	2013-02-07 21:11:51.603970255 +0000
+++ xen-4.2.1/Config.mk	2013-02-07 21:51:44.992048787 +0000
@@ -156,7 +156,7 @@
 
 CFLAGS += -std=gnu99
 
-CFLAGS += -Wall -Wstrict-prototypes
+CFLAGS += -Wall -Wstrict-prototypes -Wno-unused-local-typedefs
 
 # Clang complains about macros that expand to 'if ( ( foo == bar ) ) ...'
 # and is over-zealous with the printf format lint

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Patch 2/2] Compile errors with gcc 4.8
  2013-02-07 10:02 ` Jan Beulich
@ 2013-02-07 23:25   ` M A Young
  2013-02-08  8:13     ` Jan Beulich
  0 siblings, 1 reply; 14+ messages in thread
From: M A Young @ 2013-02-07 23:25 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

[-- Attachment #1: Type: TEXT/PLAIN, Size: 588 bytes --]

On Thu, 7 Feb 2013, Jan Beulich wrote:

>>>> On 06.02.13 at 21:37, M A Young <m.a.young@durham.ac.uk> wrote:
>> @@ -264,7 +264,7 @@
>>                return -1;
>>        }
>>
>> -       memset(buf,0,sizeof(buf));
>> +       memset(buf,0,sizeof(*buf));
>
> While everything else looks right, this one for sure is wrong - buf
> is an array, and hence sizeof(*buf) would clear just the first array
> element.

Yes, I now agree it is wrong (I found the other example in that file and 
assumed this one was broken as well). I am attaching a patch that fixes 
the other cases.

 	Michael Young

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2921 bytes --]

gcc 4.8 identifies several places where code of the form
memset (x, 0, sizeof(x));
is used incorrectly, meaning that less memory is set to zero than required.

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>

--- xen-4.2.1/tools/libxc/xc_dom_boot.c.orig	2012-12-17 15:00:48.000000000 +0000
+++ xen-4.2.1/tools/libxc/xc_dom_boot.c	2013-01-28 22:21:13.215782329 +0000
@@ -266,7 +266,7 @@
         return rc;
 
     /* let the vm run */
-    memset(ctxt, 0, sizeof(ctxt));
+    memset(ctxt, 0, sizeof(*ctxt));
     if ( (rc = dom->arch_hooks->vcpu(dom, ctxt)) != 0 )
         return rc;
     xc_dom_unmap_all(dom);
--- xen-4.2.1/tools/blktap2/drivers/md5.c.orig	2012-12-17 15:00:11.000000000 +0000
+++ xen-4.2.1/tools/blktap2/drivers/md5.c	2013-01-28 23:49:51.940289123 +0000
@@ -174,7 +174,7 @@
     MD5Transform(ctx->buf, (uint32_t *) ctx->in);
     byteReverse((unsigned char *) ctx->buf, 4);
     memcpy(digest, ctx->buf, 16);
-    memset(ctx, 0, sizeof(ctx));     /* In case it's sensitive */
+    memset(ctx, 0, sizeof(*ctx));     /* In case it's sensitive */
 }
 
 /* The four core functions - F1 is optimized somewhat */
--- xen-4.2.1/tools/libxl/libxl_qmp.c.orig	2012-12-17 15:01:09.000000000 +0000
+++ xen-4.2.1/tools/libxl/libxl_qmp.c	2013-01-29 21:28:27.763650073 +0000
@@ -377,7 +377,7 @@
     ret = libxl_fd_set_cloexec(qmp->ctx, qmp->qmp_fd, 1);
     if (ret) return -1;
 
-    memset(&qmp->addr, 0, sizeof (&qmp->addr));
+    memset(&qmp->addr, 0, sizeof (qmp->addr));
     qmp->addr.sun_family = AF_UNIX;
     strncpy(qmp->addr.sun_path, qmp_socket_path,
             sizeof (qmp->addr.sun_path));
--- xen-4.2.1/tools/xenstat/libxenstat/src/xenstat_linux.c.orig	2012-12-17 15:01:35.000000000 +0000
+++ xen-4.2.1/tools/xenstat/libxenstat/src/xenstat_linux.c	2013-01-29 21:43:46.044169987 +0000
@@ -113,7 +113,7 @@
 
 	/* Initialize all variables called has passed as non-NULL to zeros */
 	if (iface != NULL)
-		memset(iface, 0, sizeof(iface));
+		memset(iface, 0, sizeof(*iface));
 	if (rxBytes != NULL)
 		*rxBytes = 0;
 	if (rxPackets != NULL)
--- xen-4.2.1/tools/debugger/kdd/kdd-xen.c.orig	2012-12-17 15:00:22.000000000 +0000
+++ xen-4.2.1/tools/debugger/kdd/kdd-xen.c	2013-01-29 21:45:12.652087239 +0000
@@ -333,7 +333,7 @@
     if (!cpu) 
         return -1;
 
-    memset(r, 0, sizeof(r));
+    memset(r, 0, sizeof(*r));
     
     if (w64)
         kdd_get_regs_x86_64(cpu, &r->r64);
--- xen-4.2.1/tools/python/xen/lowlevel/netlink/libnetlink.c.orig	2012-12-17 15:01:24.000000000 +0000
+++ xen-4.2.1/tools/python/xen/lowlevel/netlink/libnetlink.c	2013-01-29 21:47:59.524001053 +0000
@@ -37,7 +37,7 @@
        int sndbuf = 32768;
        int rcvbuf = 32768;
 
-       memset(rth, 0, sizeof(rth));
+       memset(rth, 0, sizeof(*rth));
 
        rth->fd = socket(AF_NETLINK, SOCK_RAW, protocol);
        if (rth->fd < 0) {

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] Compile errors with gcc 4.8
  2013-02-07 23:20           ` [PATCH 1/2] " M A Young
@ 2013-02-08  8:11             ` Jan Beulich
  2013-02-08 20:56               ` [PATCH 1/2 v2] " M A Young
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2013-02-08  8:11 UTC (permalink / raw)
  To: M A Young; +Cc: Andrew Cooper, Keir Fraser, Ian Campbell, xen-devel

>>> On 08.02.13 at 00:20, M A Young <m.a.young@durham.ac.uk> wrote:
> I am attaching a patch that turns off this check.

You were told how to do it and still did it wrong: You can't blindly
pass an option to the compiler that not all supported versions
recognize. Once again, please do it the same way as done for
other warnings (using cc-option-add).

Jan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Patch 2/2] Compile errors with gcc 4.8
  2013-02-07 23:25   ` [Patch 2/2] " M A Young
@ 2013-02-08  8:13     ` Jan Beulich
  0 siblings, 0 replies; 14+ messages in thread
From: Jan Beulich @ 2013-02-08  8:13 UTC (permalink / raw)
  To: M A Young, Ian Jackson; +Cc: xen-devel

>>> On 08.02.13 at 00:25, M A Young <m.a.young@durham.ac.uk> wrote:
> On Thu, 7 Feb 2013, Jan Beulich wrote:
> 
>>>>> On 06.02.13 at 21:37, M A Young <m.a.young@durham.ac.uk> wrote:
>>> @@ -264,7 +264,7 @@
>>>                return -1;
>>>        }
>>>
>>> -       memset(buf,0,sizeof(buf));
>>> +       memset(buf,0,sizeof(*buf));
>>
>> While everything else looks right, this one for sure is wrong - buf
>> is an array, and hence sizeof(*buf) would clear just the first array
>> element.
> 
> Yes, I now agree it is wrong (I found the other example in that file and 
> assumed this one was broken as well). I am attaching a patch that fixes 
> the other cases.

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Ian - please also consider applying to the 4.x trees.

Jan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2 v2] Compile errors with gcc 4.8
  2013-02-08  8:11             ` Jan Beulich
@ 2013-02-08 20:56               ` M A Young
  0 siblings, 0 replies; 14+ messages in thread
From: M A Young @ 2013-02-08 20:56 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Keir Fraser, Ian Campbell, xen-devel

[-- Attachment #1: Type: TEXT/PLAIN, Size: 522 bytes --]

On Fri, 8 Feb 2013, Jan Beulich wrote:

>>>> On 08.02.13 at 00:20, M A Young <m.a.young@durham.ac.uk> wrote:
>> I am attaching a patch that turns off this check.
>
> You were told how to do it and still did it wrong: You can't blindly
> pass an option to the compiler that not all supported versions
> recognize. Once again, please do it the same way as done for
> other warnings (using cc-option-add).

Here is a revised version that adds the -Wno-unused-local-typedefs option 
in a cc-option-add clause.

 	Michael Young

[-- Attachment #2: Type: TEXT/PLAIN, Size: 1649 bytes --]

gcc 4.8 gives the following error when building xen/common/compat/memory.c

memory.c: In function 'compat_memory_op':
/builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:35:33: error: typedef '__guest_handle_const_compat_memory_exchange_t' locally defined but not used [-Werror=unused-local-typedefs]
     typedef struct { type *p; } __guest_handle_ ## name
                                 ^
/builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:43:5: note: in expansion of macro '___DEFINE_XEN_GUEST_HANDLE'
     ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
     ^
/builddir/build/BUILD/xen-4.2.1/xen/include/public/arch-x86/xen.h:44:41: note: in expansion of macro '__DEFINE_XEN_GUEST_HANDLE'
 #define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name, name)
                                         ^
memory.c:261:13: note: in expansion of macro 'DEFINE_XEN_GUEST_HANDLE'
             DEFINE_XEN_GUEST_HANDLE(compat_memory_exchange_t);
             ^

The error is harmless in this case so turn the check off.

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>

--- xen-4.2.1/Config.mk.orig	2013-02-08 20:07:25.006716975 +0000
+++ xen-4.2.1/Config.mk	2013-02-08 20:13:52.184876582 +0000
@@ -166,6 +166,7 @@
 $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
 $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
 $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable)
+$(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
 
 LDFLAGS += $(foreach i, $(EXTRA_LIB), -L$(i)) 
 CFLAGS += $(foreach i, $(EXTRA_INCLUDES), -I$(i))

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-02-08 20:56 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-06 20:37 Compile errors with gcc 4.8 M A Young
2013-02-07  0:55 ` Andrew Cooper
2013-02-07  8:59   ` M A Young
2013-02-07  9:06     ` Ian Campbell
2013-02-07  9:57       ` Jan Beulich
2013-02-07 11:55         ` Keir Fraser
2013-02-07 23:20           ` [PATCH 1/2] " M A Young
2013-02-08  8:11             ` Jan Beulich
2013-02-08 20:56               ` [PATCH 1/2 v2] " M A Young
2013-02-07  9:07     ` M A Young
2013-02-07  9:08   ` Frediano Ziglio
2013-02-07 10:02 ` Jan Beulich
2013-02-07 23:25   ` [Patch 2/2] " M A Young
2013-02-08  8:13     ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.