Samuel Thibault
2008-Mar-25 17:25 UTC
[Xen-devel] [PATCH] unnamed union structure members are dangerous to expose in public headers
Hello,
We have been struggling with Jean because of a bug in the headers:
#include <stdio.h>
#include <stdint.h>
#include <xen/foreign/x86_64.h>
int main(int argc, char *argv[]) {
printf("%#lx\n", sizeof(vcpu_guest_context_x86_64_t));
return 0;
}
will return 0x13e0 when compiled without gcc -std=c99, while the
expected value is 0x1430, resulting to seg faults and all kinds of
tricks. Compiling with
#include "/usr/include/xen/foreign/x86_64.h"
shows why:
/usr/include/xen/foreign/x86_64.h:88: warning: declaration does not
declare anything
The special case of an unnamed union field is just a gcc extension, and
thus in C99 mode it considers Xen''s __DECL_REG() just as an unnamed
union declaration, and does not reserve data in the structure. The
patch below fixes this by telling gcc that it should use its gcc
extension.
Prefix unnamed union structure fields with __extension__ to make it work
properly even in e.g. C99 standard mode.
Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>
diff -r be3025f6af2e xen/include/public/arch-ia64.h
--- a/xen/include/public/arch-ia64.h Mon Mar 24 18:24:03 2008 +0000
+++ b/xen/include/public/arch-ia64.h Tue Mar 25 17:05:53 2008 +0000
@@ -165,11 +165,11 @@ struct mapped_regs {
unsigned long reserved1[29];
unsigned long vhpi;
unsigned long reserved2[95];
- union {
+ __extension__ union {
unsigned long vgr[16];
unsigned long bank1_regs[16]; // bank1 regs (r16-r31) when bank0 active
};
- union {
+ __extension__ union {
unsigned long vbgr[16];
unsigned long bank0_regs[16]; // bank0 regs (r16-r31) when bank1 active
};
@@ -180,7 +180,7 @@ struct mapped_regs {
unsigned long vpsr;
unsigned long vpr;
unsigned long reserved4[76];
- union {
+ __extension__ union {
unsigned long vcr[128];
struct {
unsigned long dcr; // CR0
@@ -214,7 +214,7 @@ struct mapped_regs {
unsigned long rsv6[46];
};
};
- union {
+ __extension__ union {
unsigned long reserved5[128];
struct {
unsigned long precover_ifs;
@@ -608,7 +608,7 @@ struct xen_ia64_opt_feature {
struct xen_ia64_opt_feature {
unsigned long cmd; /* Which feature */
unsigned char on; /* Switch feature on/off */
- union {
+ __extension__ union {
struct {
/* The page protection bit mask of the pte.
* This will be or''ed with the pte. */
diff -r be3025f6af2e xen/include/public/arch-x86/xen-x86_64.h
--- a/xen/include/public/arch-x86/xen-x86_64.h Mon Mar 24 18:24:03 2008 +0000
+++ b/xen/include/public/arch-x86/xen-x86_64.h Tue Mar 25 17:05:53 2008 +0000
@@ -142,7 +142,7 @@ struct iret_context {
#ifdef __GNUC__
/* Anonymous union includes both 32- and 64-bit names (e.g., eax/rax). */
-#define __DECL_REG(name) union { \
+#define __DECL_REG(name) __extension__ union { \
uint64_t r ## name, e ## name; \
uint32_t _e ## name; \
}
diff -r be3025f6af2e tools/include/xen-foreign/mkheader.py
--- a/tools/include/xen-foreign/mkheader.py Mon Mar 24 18:24:03 2008 +0000
+++ b/tools/include/xen-foreign/mkheader.py Tue Mar 25 17:05:53 2008 +0000
@@ -38,7 +38,7 @@ inttypes["x86_64"] = {
};
header["x86_64"] = """
#ifdef __GNUC__
-# define __DECL_REG(name) union { uint64_t r ## name, e ## name; }
+# define __DECL_REG(name) __extension__ union { uint64_t r ## name, e ## name;
}
# define __align8__ __attribute__((aligned (8)))
#else
# define __DECL_REG(name) uint64_t r ## name
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2008-Mar-25 17:33 UTC
Re: [Xen-devel] [PATCH] unnamed union structure members are dangerous to expose in public headers
On 25/3/08 17:25, "Samuel Thibault" <samuel.thibault@eu.citrix.com> wrote:> will return 0x13e0 when compiled without gcc -std=c99, while the > expected value is 0x1430, resulting to seg faults and all kinds of > tricks. Compiling with > > #include "/usr/include/xen/foreign/x86_64.h" > > shows why: > > /usr/include/xen/foreign/x86_64.h:88: warning: declaration does not > declare anythingThis raises two obvious questions: 1. Why building with -std=c99? 2. Why building without -Werror? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Samuel Thibault
2008-Mar-25 17:42 UTC
Re: [Xen-devel] [PATCH] unnamed union structure members are dangerous to expose in public headers
Keir Fraser, le Tue 25 Mar 2008 17:33:53 +0000, a écrit :> On 25/3/08 17:25, "Samuel Thibault" <samuel.thibault@eu.citrix.com> wrote: > > > will return 0x13e0 when compiled without gcc -std=c99, while the > > expected value is 0x1430, resulting to seg faults and all kinds of > > tricks. Compiling with > > > > #include "/usr/include/xen/foreign/x86_64.h" > > > > shows why: > > > > /usr/include/xen/foreign/x86_64.h:88: warning: declaration does not > > declare anything > > This raises two obvious questions: > 1. Why building with -std=c99?Because in the wild public world, people may want to.> 2. Why building without -Werror?That wouldn''t produce an error: the warning only appears when including with "" instead of <>, so that gcc spits warnings of the header itself. Samuel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel