Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Christoph Egger]

On 05/27/11 11:21, Keir Fraser wrote:
> On 27/05/2011 08:55, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>>> This stuff can be moderately fragile, and we've now had no actual bug
>>> reports about it in a long while.
>>
>> Here is a full error message:
>
> Ah, okay, actually I see our stdarg support is a bit inconsistent already.
> How about the attached alternative fix? A few notes about it:
>
> Firstly, in xen/stdarg.h I handle NetBSD the same as OpenBSD -- I go and
> grab /usr/include/stdarg.h directly. I don't know if that is really better
> than including the compiler intrinsics directly as you did. If you prefer we
> could do that instead, but whichever I think it makes sense to do the same
> for both BSD variants.

No, the assumption what works for one BSD works for all BSDs is wrong.

In the NetBSD case I get the error
that <machine/ansi.h> and <sys/featuretest.h> are not found.
That way <stdarg.h> pulls in machine specific definitions on NetBSD.

>
> Secondly, I wonder whether *BSD would be happier if we did not include
> -nostdinc on the command line? See arch/x86/Rules.mk where SunOS is already
> special cased. I don't know whether that would work for BSD too.

It works for NetBSD at least.

> Finally, rather than referencing a hardcoded /usr/include path or including
> gcc intrinsics in xen/stdarg.h, would *BSD prefer a -isystem command-line
> option to include the gcc headers in the search path? Then we could perhaps
> even get rid of xen/stdarg.h completely.

NetBSD is about to add clang/llvm support. If -isystem works for both
gcc and clang then this should be fine.

Christoph

>   -- Keir
>
>> gcc -O2 -I/usr/include -I/usr/pkg/include -O2 -fomit-frame-pointer -m64
>> -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
>> -Wno-unused-value -Wdeclaration-after-statement  -O2
>> -fomit-frame-pointer -m64 -fno-strict-aliasing -std=gnu99 -Wall
>> -Wstrict-prototypes -Wno-unused-value -Wdeclaration-after-statement  -O2
>> -fomit-frame-pointer -m64 -fno-strict-aliasing -std=gnu99 -Wall
>> -Wstrict-prototypes -Wno-unused-value -Wdeclaration-after-statement
>> -DNDEBUG -nostdinc -fno-builtin -fno-common -Wredundant-decls
>> -iwithprefix include -Werror -Wno-pointer-arith -pipe
>> -I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include
>> -I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm-x86/
>> mach-generic
>> -I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm-x86/
>> mach-default
>> -msoft-float -fno-stack-protector -fno-exceptions -mno-red-zone
>>    -fpic -fno-asynchronous-unwind-tables -DGCC_HAS_VISIBILITY_ATTRIBUTE
>> -g -D__XEN__ -MMD -MF .xen.d -O2 -fomit-frame-pointer -m64
>> -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
>> -Wno-unused-value -Wdeclaration-after-statement  -DNDEBUG -nostdinc
>> -fno-builtin -fno-common -Wredundant-decls -iwithprefix include -Werror
>> -Wno-pointer-arith -pipe
>> -I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include
>> -I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm-x86/
>> mach-generic
>> -I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm-x86/
>> mach-default
>> -msoft-float -fno-stack-protector -fno-exceptions -mno-red-zone -fpic
>> -fno-asynchronous-unwind-tables -DGCC_HAS_VISIBILITY_ATTRIBUTE -g
>> -D__XEN__ -MMD -MF .asm-offsets.s.d -S -o asm-offsets.s x86_64/asm-offsets.c
>> In file included from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/lib.h:
>> 5,
>>                    from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm/system
>> .h:4,
>>                    from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/spinlo
>> ck.h:5,
>>                    from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/sched.
>> h:7,
>>                    from x86_64/asm-offsets.c:10:
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/stdarg
>> .h:4:22:
>> error: stdarg.h: No such file or directory
>> In file included from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm/system
>> .h:4,
>>                    from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/spinlo
>> ck.h:5,
>>                    from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/sched.
>> h:7,
>>                    from x86_64/asm-offsets.c:10:
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/lib.h:
>> 86:
>> error: expected declaration specifiers or '...' before 'va_list'
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/lib.h:
>> 90:
>> error: expected declaration specifiers or '...' before 'va_list'
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/lib.h:
>> 94:
>> error: expected declaration specifiers or '...' before 'va_list'
>> In file included from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/acpi/acpi.
>> h:61,
>>                    from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/acpi.h
>> :34,
>>                    from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/acpi/apei.
>> h:8,
>>                    from
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm/fixmap
>> .h:23,
>>                    from x86_64/asm-offsets.c:12:
>> /tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/acpi/acpio
>> sxf.h:257:
>> error: expected declaration specifiers or '...' before 'va_list'
>> gmake[3]: *** [asm-offsets.s] Error 1
>> gmake[3]: Leaving directory
>> `/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/arch/x86'
>> gmake[2]: ***
>> [/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/xen] Error 2
>> gmake[2]: Leaving directory
>> `/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen'
>> gmake[1]: *** [install] Error 2
>> gmake[1]: Leaving directory
>> `/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen'
>>
>>
>>>
>>>    -- Keir
>>>
>>>>> Use the external header when we compile the tools.
>>>>> Adapt libelf-loader.c which is compiled in the kernel
>>>>> and with the tools.
>>>>
>>>> -#include<stdarg.h>
>>>> +#include "../../include/xen/stdarg.h"
>>>>
>>>> Firstly, is this correct for all uses of libelf-loader ?  This file is
>>>> compiled both for use inside Xen and for use on the host.  In the
>>>> latter case it /should/ use the system's stdarg.h.
>>>>
>>>> Also, this kind of relative path should not be necessary.  The
>>>> relevant files should be on the include path.
>>>>
>>>> So I think this should be fixed by arranging that your new stdarg.h is
>>>> on the relevant include path, not by changing the .c file.
>>>>
>>>> Ian.


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Keir Fraser]

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

On 27/05/2011 10:48, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> Finally, rather than referencing a hardcoded /usr/include path or including
>> gcc intrinsics in xen/stdarg.h, would *BSD prefer a -isystem command-line
>> option to include the gcc headers in the search path? Then we could perhaps
>> even get rid of xen/stdarg.h completely.
> 
> NetBSD is about to add clang/llvm support. If -isystem works for both
> gcc and clang then this should be fine.

Something like the attached patch? I don't know whether this would work for
clang too -- cc'ing Tim who will know or can try it out.

I wouldn't backport this one to 4.1, but my first patch was a cleanup plus
BSD-specific changes, so a variant on that would be suitable for 4.0/4.1,
with (if it works) this new one in unstable.

 -- Keir


[-- Attachment #2: 01-stdarg-fixes --]
[-- Type: application/octet-stream, Size: 1985 bytes --]

diff -r fc963f9f2808 xen/arch/x86/Rules.mk
--- a/xen/arch/x86/Rules.mk	Fri May 27 08:56:47 2011 +0100
+++ b/xen/arch/x86/Rules.mk	Fri May 27 14:08:55 2011 +0100
@@ -11,11 +11,8 @@ xenoprof := y
 #
 supervisor_mode_kernel ?= n
 
-# Solaris grabs stdarg.h and friends from the system include directory.
-# Clang likewise.
-ifneq ($(XEN_OS),SunOS)
-CFLAGS-$(gcc) += -nostdinc
-endif
+SYS_INCDIR := $(shell $(CC) -print-file-name=include)
+CFLAGS += -nostdinc -isystem $(SYS_INCDIR)
 
 CFLAGS += -fno-builtin -fno-common -Wredundant-decls
 CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
diff -r fc963f9f2808 xen/common/libelf/libelf-loader.c
--- a/xen/common/libelf/libelf-loader.c	Fri May 27 08:56:47 2011 +0100
+++ b/xen/common/libelf/libelf-loader.c	Fri May 27 14:08:55 2011 +0100
@@ -16,8 +16,6 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
-#include <stdarg.h>
-
 #include "libelf-private.h"
 
 /* ------------------------------------------------------------------------ */
diff -r fc963f9f2808 xen/common/libelf/libelf-private.h
--- a/xen/common/libelf/libelf-private.h	Fri May 27 08:56:47 2011 +0100
+++ b/xen/common/libelf/libelf-private.h	Fri May 27 14:08:55 2011 +0100
@@ -40,6 +40,7 @@
 
 #else /* !__XEN__ */
 
+#include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
diff -r fc963f9f2808 xen/common/vsprintf.c
--- a/xen/common/vsprintf.c	Fri May 27 08:56:47 2011 +0100
+++ b/xen/common/vsprintf.c	Fri May 27 14:08:55 2011 +0100
@@ -16,7 +16,6 @@
  * - scnprintf and vscnprintf
  */
 
-#include <xen/stdarg.h>
 #include <xen/ctype.h>
 #include <xen/lib.h>
 #include <asm/div64.h>
diff -r fc963f9f2808 xen/include/xen/stdarg.h
--- a/xen/include/xen/stdarg.h	Fri May 27 08:56:47 2011 +0100
+++ b/xen/include/xen/stdarg.h	Fri May 27 14:08:55 2011 +0100
@@ -1,5 +1,1 @@
-#if defined(__OpenBSD__)
-#  include "/usr/include/stdarg.h"
-#else
-#  include <stdarg.h>
-#endif
+#include <stdarg.h>

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

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

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Christoph Egger]

On 05/27/11 15:13, Keir Fraser wrote:
> On 27/05/2011 10:48, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>>> Finally, rather than referencing a hardcoded /usr/include path or including
>>> gcc intrinsics in xen/stdarg.h, would *BSD prefer a -isystem command-line
>>> option to include the gcc headers in the search path? Then we could perhaps
>>> even get rid of xen/stdarg.h completely.
>>
>> NetBSD is about to add clang/llvm support. If -isystem works for both
>> gcc and clang then this should be fine.
>
> Something like the attached patch? I don't know whether this would work for
> clang too -- cc'ing Tim who will know or can try it out.
>
> I wouldn't backport this one to 4.1, but my first patch was a cleanup plus
> BSD-specific changes, so a variant on that would be suitable for 4.0/4.1,
> with (if it works) this new one in unstable.

$ clang -print-file-name=include
include
$ gcc -print-file-name=include
include
$ gcc -print-file-name=a
a
$

I got two variants working:
a) use -nostdinc and map the stdarg usage to the gcc builtins
b) do not use -nostdinc on NetBSD

Both 0?-stdarg-fixes work with a minor
modification to match case a) or case b).

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Keir Fraser]

On 27/05/2011 15:05, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> Something like the attached patch? I don't know whether this would work for
>> clang too -- cc'ing Tim who will know or can try it out.
>> 
>> I wouldn't backport this one to 4.1, but my first patch was a cleanup plus
>> BSD-specific changes, so a variant on that would be suitable for 4.0/4.1,
>> with (if it works) this new one in unstable.
> 
> $ clang -print-file-name=include
> include
> $ gcc -print-file-name=include
> include
> $ gcc -print-file-name=a
> a

Well that's quite broken...

 -- Keir

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Keir Fraser]

On 27/05/2011 15:05, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> Something like the attached patch? I don't know whether this would work for
>> clang too -- cc'ing Tim who will know or can try it out.
>> 
>> I wouldn't backport this one to 4.1, but my first patch was a cleanup plus
>> BSD-specific changes, so a variant on that would be suitable for 4.0/4.1,
>> with (if it works) this new one in unstable.
> 
> $ clang -print-file-name=include
> include
> $ gcc -print-file-name=include
> include

Hm, well, I don't care that much about this, my first patch was good enough.
I've checked in a variant of it as c/s 23432. Please check it works okay for
you. It's simple enough to be a candidate for 4.1.1 also.

 -- Keir

> $ gcc -print-file-name=a
> a

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Christoph Egger]

On 05/27/11 16:51, Keir Fraser wrote:
> On 27/05/2011 15:05, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>>> Something like the attached patch? I don't know whether this would work for
>>> clang too -- cc'ing Tim who will know or can try it out.
>>>
>>> I wouldn't backport this one to 4.1, but my first patch was a cleanup plus
>>> BSD-specific changes, so a variant on that would be suitable for 4.0/4.1,
>>> with (if it works) this new one in unstable.
>>
>> $ clang -print-file-name=include
>> include
>> $ gcc -print-file-name=include
>> include
>
> Hm, well, I don't care that much about this, my first patch was good enough.
> I've checked in a variant of it as c/s 23432. Please check it works okay for
> you. It's simple enough to be a candidate for 4.1.1 also.

Yes, it works. Please apply for Xen 4.1.1, too.

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Tim Deegan]

At 11:48 +0200 on 27 May (1306496895), Christoph Egger wrote:
> On 05/27/11 11:21, Keir Fraser wrote:
> >Secondly, I wonder whether *BSD would be happier if we did not include
> >-nostdinc on the command line? See arch/x86/Rules.mk where SunOS is already
> >special cased. I don't know whether that would work for BSD too.
> 
> It works for NetBSD at least.
> 
> >Finally, rather than referencing a hardcoded /usr/include path or including
> >gcc intrinsics in xen/stdarg.h, would *BSD prefer a -isystem command-line
> >option to include the gcc headers in the search path? Then we could perhaps
> >even get rid of xen/stdarg.h completely.
> 
> NetBSD is about to add clang/llvm support. If -isystem works for both
> gcc and clang then this should be fine.

Building Xen with clang I had to remove the -nostdinc because I couldn't
find a sensible way of automatically finding the clang include paths. 
It may be that I just wasn't looking in the right place.

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Keir Fraser]

On 27/05/2011 14:16, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:

>> NetBSD is about to add clang/llvm support. If -isystem works for both
>> gcc and clang then this should be fine.
> 
> Building Xen with clang I had to remove the -nostdinc because I couldn't
> find a sensible way of automatically finding the clang include paths.
> It may be that I just wasn't looking in the right place.

Well, keeping the -nostdinc -isystem line as CFLAGS-$(gcc) wouldn't be so
bad really. I wasn't sure whether clang would have support for these gcc
command line options for dumping spec.

 -- Keir

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Tim Deegan]

At 14:19 +0100 on 27 May (1306505941), Keir Fraser wrote:
> On 27/05/2011 14:16, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:
> 
> >> NetBSD is about to add clang/llvm support. If -isystem works for both
> >> gcc and clang then this should be fine.
> > 
> > Building Xen with clang I had to remove the -nostdinc because I couldn't
> > find a sensible way of automatically finding the clang include paths.
> > It may be that I just wasn't looking in the right place.
> 
> Well, keeping the -nostdinc -isystem line as CFLAGS-$(gcc) wouldn't be so
> bad really. I wasn't sure whether clang would have support for these gcc
> command line options for dumping spec.

My current clang seems happy enough with this, actually, though wierdly
it reports /usr/lib/gcc/x86_64-linux-gnu/4.5.2/include, so lord knows
what would happen on a machine with clang but not gcc installed.

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Ian Campbell]

On Fri, 2011-05-27 at 14:19 +0100, Keir Fraser wrote:
> On 27/05/2011 14:16, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:
> 
> >> NetBSD is about to add clang/llvm support. If -isystem works for both
> >> gcc and clang then this should be fine.
> > 
> > Building Xen with clang I had to remove the -nostdinc because I couldn't
> > find a sensible way of automatically finding the clang include paths.
> > It may be that I just wasn't looking in the right place.
> 
> Well, keeping the -nostdinc -isystem line as CFLAGS-$(gcc) wouldn't be so
> bad really. I wasn't sure whether clang would have support for these gcc
> command line options for dumping spec.

$ clang -print-file-name=include
/usr/bin/../lib/clang/2.8/include
$ ls /usr/bin/../lib/clang/2.8/include/stdarg.h 
/usr/bin/../lib/clang/2.8/include/stdarg.h

... on recentish Debian at least.

Ian.

[PATCH] xen: Deal with stdarg.h and -nostdinc

[Christoph Egger]

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


For compiling the xen kernel we use -nostdinc which means
we don't want to rely on any external headers.
That also counts vor stdarg.h.

Use the external header when we compile the tools.
Adapt libelf-loader.c which is compiled in the kernel
and with the tools.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_stdarg.diff --]
[-- Type: text/plain, Size: 992 bytes --]

diff -r 24b01e02ea17 xen/common/libelf/libelf-loader.c
--- a/xen/common/libelf/libelf-loader.c	Thu May 26 11:34:49 2011 +0200
+++ b/xen/common/libelf/libelf-loader.c	Thu May 26 16:45:38 2011 +0200
@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
-#include <stdarg.h>
+#include "../../include/xen/stdarg.h"
 
 #include "libelf-private.h"
 
diff -r 24b01e02ea17 xen/include/xen/stdarg.h
--- a/xen/include/xen/stdarg.h	Thu May 26 11:34:49 2011 +0200
+++ b/xen/include/xen/stdarg.h	Thu May 26 16:45:38 2011 +0200
@@ -1,5 +1,13 @@
-#if defined(__OpenBSD__)
-#  include "/usr/include/stdarg.h"
+#ifndef _XEN_STDARG_H_
+#define _XEN_STDARG_H_
+
+#if defined(__XEN__)
+typedef __builtin_va_list va_list;
+#define va_start(ap, last)    __builtin_stdarg_start((ap), (last))
+#define va_end(ap)            __builtin_va_end(ap)
+#define va_arg                __builtin_va_arg
 #else
 #  include <stdarg.h>
 #endif
+
+#endif	/* _XEN_STDARG_H_ */

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

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

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Christoph Egger]

On 05/26/11 16:54, Christoph Egger wrote:
>
> For compiling the xen kernel we use -nostdinc which means
> we don't want to rely on any external headers.
> That also counts vor stdarg.h.
>
> Use the external header when we compile the tools.
> Adapt libelf-loader.c which is compiled in the kernel
> and with the tools.
>
> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>
>

Forgot to mention: Please apply this to xen-4.1-testing, too.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Keir Fraser]

On 26/05/2011 15:54, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> 
> For compiling the xen kernel we use -nostdinc which means
> we don't want to rely on any external headers.
> That also counts vor stdarg.h.
> 
> Use the external header when we compile the tools.
> Adapt libelf-loader.c which is compiled in the kernel
> and with the tools.
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

Does the current approach not work for you?

 -- Keir

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Jan Beulich]

>>> On 26.05.11 at 16:54, Christoph Egger <Christoph.Egger@amd.com> wrote:

> For compiling the xen kernel we use -nostdinc which means
> we don't want to rely on any external headers.
> That also counts vor stdarg.h.

I think using compiler provided headers is fine, and I have to admit
I don't like the open-coding of various definitions to gcc builtins - if
gcc changes anything here, we'll break (which after all is why the
compiler provides this header instead of the platform).

Jan

> Use the external header when we compile the tools.
> Adapt libelf-loader.c which is compiled in the kernel
> and with the tools.
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Ian Jackson]

Christoph Egger writes ("[Xen-devel] [PATCH] xen: Deal with stdarg.h and -nostdinc"):
> For compiling the xen kernel we use -nostdinc which means
> we don't want to rely on any external headers.
> That also counts vor stdarg.h.

Right, OK, but:

> Use the external header when we compile the tools.
> Adapt libelf-loader.c which is compiled in the kernel
> and with the tools.

-#include <stdarg.h>
+#include "../../include/xen/stdarg.h"

Firstly, is this correct for all uses of libelf-loader ?  This file is
compiled both for use inside Xen and for use on the host.  In the
latter case it /should/ use the system's stdarg.h.

Also, this kind of relative path should not be necessary.  The
relevant files should be on the include path.

So I think this should be fixed by arranging that your new stdarg.h is
on the relevant include path, not by changing the .c file.

Ian.

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Keir Fraser]

On 26/05/2011 17:11, "Ian Jackson" <Ian.Jackson@eu.citrix.com> wrote:

> Christoph Egger writes ("[Xen-devel] [PATCH] xen: Deal with stdarg.h and
> -nostdinc"):
>> For compiling the xen kernel we use -nostdinc which means
>> we don't want to rely on any external headers.
>> That also counts vor stdarg.h.
> 
> Right, OK, but:

Not really. We are only doing the same as Linux right now. I do note that
they add an -isystem option to their command line, with a gcc include path
that they presumably dump out of gcc in some standard way. However, our
current approach seems to work, if it fails to work in some situation it is
likely to be pretty obvious (build or link failure), and we can fix it then.
This stuff can be moderately fragile, and we've now had no actual bug
reports about it in a long while.

 -- Keir

>> Use the external header when we compile the tools.
>> Adapt libelf-loader.c which is compiled in the kernel
>> and with the tools.
> 
> -#include <stdarg.h>
> +#include "../../include/xen/stdarg.h"
> 
> Firstly, is this correct for all uses of libelf-loader ?  This file is
> compiled both for use inside Xen and for use on the host.  In the
> latter case it /should/ use the system's stdarg.h.
> 
> Also, this kind of relative path should not be necessary.  The
> relevant files should be on the include path.
> 
> So I think this should be fixed by arranging that your new stdarg.h is
> on the relevant include path, not by changing the .c file.
> 
> Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Christoph Egger]

On 05/26/11 19:08, Keir Fraser wrote:
> On 26/05/2011 17:11, "Ian Jackson"<Ian.Jackson@eu.citrix.com>  wrote:
>
>> Christoph Egger writes ("[Xen-devel] [PATCH] xen: Deal with stdarg.h and
>> -nostdinc"):
>>> For compiling the xen kernel we use -nostdinc which means
>>> we don't want to rely on any external headers.
>>> That also counts vor stdarg.h.
>>
>> Right, OK, but:
>
> Not really. We are only doing the same as Linux right now. I do note that
> they add an -isystem option to their command line, with a gcc include path
> that they presumably dump out of gcc in some standard way. However, our
> current approach seems to work, if it fails to work in some situation it is
> likely to be pretty obvious (build or link failure), and we can fix it then.

The cross-compiling facility in NetBSD has been further improved which
made gcc more pendantic for this. -nostdinc is used and -isystem is 
*not* used.

> This stuff can be moderately fragile, and we've now had no actual bug
> reports about it in a long while.

Here is a full error message:

gcc -O2 -I/usr/include -I/usr/pkg/include -O2 -fomit-frame-pointer -m64 
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes 
-Wno-unused-value -Wdeclaration-after-statement  -O2 
-fomit-frame-pointer -m64 -fno-strict-aliasing -std=gnu99 -Wall 
-Wstrict-prototypes -Wno-unused-value -Wdeclaration-after-statement  -O2 
-fomit-frame-pointer -m64 -fno-strict-aliasing -std=gnu99 -Wall 
-Wstrict-prototypes -Wno-unused-value -Wdeclaration-after-statement 
-DNDEBUG -nostdinc -fno-builtin -fno-common -Wredundant-decls 
-iwithprefix include -Werror -Wno-pointer-arith -pipe 
-I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include 
-I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm-x86/mach-generic 
-I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm-x86/mach-default 
-msoft-float -fno-stack-protector -fno-exceptions -mno-red-zone
  -fpic -fno-asynchronous-unwind-tables -DGCC_HAS_VISIBILITY_ATTRIBUTE 
-g -D__XEN__ -MMD -MF .xen.d -O2 -fomit-frame-pointer -m64 
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes 
-Wno-unused-value -Wdeclaration-after-statement  -DNDEBUG -nostdinc 
-fno-builtin -fno-common -Wredundant-decls -iwithprefix include -Werror 
-Wno-pointer-arith -pipe 
-I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include 
-I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm-x86/mach-generic 
-I/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm-x86/mach-default 
-msoft-float -fno-stack-protector -fno-exceptions -mno-red-zone -fpic 
-fno-asynchronous-unwind-tables -DGCC_HAS_VISIBILITY_ATTRIBUTE -g 
-D__XEN__ -MMD -MF .asm-offsets.s.d -S -o asm-offsets.s x86_64/asm-offsets.c
In file included from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/lib.h:5,
                  from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm/system.h:4,
                  from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/spinlock.h:5,
                  from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/sched.h:7,
                  from x86_64/asm-offsets.c:10:
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/stdarg.h:4:22: 
error: stdarg.h: No such file or directory
In file included from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm/system.h:4,
                  from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/spinlock.h:5,
                  from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/sched.h:7,
                  from x86_64/asm-offsets.c:10:
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/lib.h:86: 
error: expected declaration specifiers or '...' before 'va_list'
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/lib.h:90: 
error: expected declaration specifiers or '...' before 'va_list'
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/lib.h:94: 
error: expected declaration specifiers or '...' before 'va_list'
In file included from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/acpi/acpi.h:61,
                  from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/xen/acpi.h:34,
                  from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/acpi/apei.h:8,
                  from 
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/asm/fixmap.h:23,
                  from x86_64/asm-offsets.c:12:
/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/include/acpi/acpiosxf.h:257: 
error: expected declaration specifiers or '...' before 'va_list'
gmake[3]: *** [asm-offsets.s] Error 1
gmake[3]: Leaving directory 
`/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/arch/x86'
gmake[2]: *** 
[/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen/xen] Error 2
gmake[2]: Leaving directory 
`/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen'
gmake[1]: *** [install] Error 2
gmake[1]: Leaving directory 
`/tmp/pkgsrc-obj/sysutils/xenkernel41/work.ixp/xen-4.1.0/xen'


>
>   -- Keir
>
>>> Use the external header when we compile the tools.
>>> Adapt libelf-loader.c which is compiled in the kernel
>>> and with the tools.
>>
>> -#include<stdarg.h>
>> +#include "../../include/xen/stdarg.h"
>>
>> Firstly, is this correct for all uses of libelf-loader ?  This file is
>> compiled both for use inside Xen and for use on the host.  In the
>> latter case it /should/ use the system's stdarg.h.
>>
>> Also, this kind of relative path should not be necessary.  The
>> relevant files should be on the include path.
>>
>> So I think this should be fixed by arranging that your new stdarg.h is
>> on the relevant include path, not by changing the .c file.
>>
>> Ian.
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
>
>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Keir Fraser]

On 27/05/2011 08:55, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> Not really. We are only doing the same as Linux right now. I do note that
>> they add an -isystem option to their command line, with a gcc include path
>> that they presumably dump out of gcc in some standard way. However, our
>> current approach seems to work, if it fails to work in some situation it is
>> likely to be pretty obvious (build or link failure), and we can fix it then.
> 
> The cross-compiling facility in NetBSD has been further improved which
> made gcc more pendantic for this. -nostdinc is used and -isystem is
> *not* used.

So where does NetBSD get its stdarg.h from in this case? It's somewhat bogus
to roll your own, as it contains compiler-specific intrinsics.

 -- Keir

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Christoph Egger]

On 05/27/11 10:46, Keir Fraser wrote:
> On 27/05/2011 08:55, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>>> Not really. We are only doing the same as Linux right now. I do note that
>>> they add an -isystem option to their command line, with a gcc include path
>>> that they presumably dump out of gcc in some standard way. However, our
>>> current approach seems to work, if it fails to work in some situation it is
>>> likely to be pretty obvious (build or link failure), and we can fix it then.
>>
>> The cross-compiling facility in NetBSD has been further improved which
>> made gcc more pendantic for this. -nostdinc is used and -isystem is
>> *not* used.
>
> So where does NetBSD get its stdarg.h from in this case?

-isystem must be *explicitely* specified now.

> It's somewhat bogus to roll your own, as it contains compiler-specific intrinsics.
>
>   -- Keir


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Ian Jackson]

Christoph Egger writes ("Re: [Xen-devel] [PATCH] xen: Deal with stdarg.h and -nostdinc"):
> The cross-compiling facility in NetBSD has been further improved which
> made gcc more pendantic for this. -nostdinc is used and -isystem is 
> *not* used.

Compiling Xen with some kind of native "cross-compiling facility"
provided by NetBSD is NOT SUPPORTED.

Please do NOT send any more patches to Xen to try to make it work.

Ian.

Re: [PATCH] xen: Deal with stdarg.h and -nostdinc

[Christoph Egger]

On 05/26/11 18:11, Ian Jackson wrote:
> Christoph Egger writes ("[Xen-devel] [PATCH] xen: Deal with stdarg.h and -nostdinc"):
>> For compiling the xen kernel we use -nostdinc which means
>> we don't want to rely on any external headers.
>> That also counts for stdarg.h.
>
> Right, OK, but:
>
>> Use the external header when we compile the tools.
>> Adapt libelf-loader.c which is compiled in the kernel
>> and with the tools.
>
> -#include<stdarg.h>
> +#include "../../include/xen/stdarg.h"
>
> Firstly, is this correct for all uses of libelf-loader ?  This file is
> compiled both for use inside Xen and for use on the host.  In the
> latter case it /should/ use the system's stdarg.h.

This is exactly what happens in conjunction with the other diff.
In the first case there is __XEN__ defined and in the latter case
__XEN__ is *not* defined.

> Also, this kind of relative path should not be necessary.  The
> relevant files should be on the include path.

I tried <xen/stdarg.h> but then compiling the tools failed.

> So I think this should be fixed by arranging that your new stdarg.h is
> on the relevant include path, not by changing the .c file.
>
> Ian.
>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632