[PATCH 0/3] stdinc-related build fixes.

[PATCH 0/3] stdinc-related build fixes.

[Tim Deegan]

Hi,

These three patches tidy some of the xen build runes around the use of
nostdinc.

Patch 3/3 hasn't yet been tested on an actual NetBSD box,
so I'd appreciate it if Patrick would give it a go.  Apart from that,
I've tested with every GCC I can lay my hands on (and clang 3.0),
building x86 and ARM.

I rather suspect that FreeBSD and Solaris ought to be treated the same
way -- currently FreeBSD has no workarounds (but on a FreeBSD 5 box I
see stdarg.h in /usr/include) and Solaris just drops the nostdinc
(I have no Solaris box handy to test on).

Cheers,

Tim.

[PATCH 1/3] xen: move some arch CFLAGS into the common Rules.mk.

[Tim Deegan]

These are the same on all current architectures, so move them
into xen/Rules.mk.  Lay them out a little more neatly too.

Signed-off-by: Tim Deegan <tim@xen.org>
---
 xen/Rules.mk          |    4 ++++
 xen/arch/arm/Rules.mk |    2 --
 xen/arch/x86/Rules.mk |    2 --
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 26e5bb6..c432ad6 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -41,6 +41,10 @@ ALL_OBJS-y               += $(BASEDIR)/xsm/built_in.o
 ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(x86)          += $(BASEDIR)/crypto/built_in.o
 
+CFLAGS-y                += -fno-builtin -fno-common
+CFLAGS-y                += -Werror -Wredundant-decls -Wno-pointer-arith
+CFLAGS-y                += -pipe
+CFLAGS-y                += -iwithprefix include
 CFLAGS-y                += -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
 CFLAGS-$(XSM_ENABLE)    += -DXSM_ENABLE
 CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE -DXSM_MAGIC=0xf97cff8c
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index a18e7fd..d80dfce 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -10,8 +10,6 @@ HAS_DEVICE_TREE := y
 HAS_VIDEO := y
 HAS_ARM_HDLCD := y
 
-CFLAGS += -fno-builtin -fno-common -Wredundant-decls
-CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
 CFLAGS += -I$(BASEDIR)/include
 
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 0a9d68d..f629dff 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -25,8 +25,6 @@ ifneq ($(XEN_OS),SunOS)
 CFLAGS-$(gcc) += -nostdinc
 endif
 
-CFLAGS += -fno-builtin -fno-common -Wredundant-decls
-CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
 CFLAGS += -I$(BASEDIR)/include 
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
-- 
1.7.10.4

[PATCH 2/3] xen: move -nostdinc into common Rules.mk.

[Tim Deegan]

Previously we didn't use it at all the on ARM ports or for clang builds.

For ARM, I think this is just an oversight.

For clang, this used not to work, because '-withprefix include' didn't
let us see stdarg.h, but that's fixed in clang v3.0.

Also move the '-withprefix include' to beside -nostdinc as it's only
needed with -nostdinc anyway.

Signed-off-by: Tim Deegan <tim@xen.org>
---
 xen/Rules.mk          |    6 +++++-
 xen/arch/x86/Rules.mk |    6 ------
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index c432ad6..bbfc1ac 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -44,8 +44,12 @@ ALL_OBJS-$(x86)          += $(BASEDIR)/crypto/built_in.o
 CFLAGS-y                += -fno-builtin -fno-common
 CFLAGS-y                += -Werror -Wredundant-decls -Wno-pointer-arith
 CFLAGS-y                += -pipe
-CFLAGS-y                += -iwithprefix include
 CFLAGS-y                += -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
+# Solaris puts stdarg.h &c in the system include directory.
+ifneq ($(XEN_OS),SunOS)
+CFLAGS-y                += -nostdinc -iwithprefix include
+endif
+
 CFLAGS-$(XSM_ENABLE)    += -DXSM_ENABLE
 CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE -DXSM_MAGIC=0xf97cff8c
 CFLAGS-$(FLASK_ENABLE)  += -DFLASK_DEVELOP -DFLASK_BOOTPARAM -DFLASK_AVC_STATS
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index f629dff..04c1a90 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -19,12 +19,6 @@ 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
-
 CFLAGS += -I$(BASEDIR)/include 
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
 CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default
-- 
1.7.10.4

[PATCH 3/3] xen: Add stdbool.h workaround for BSD.

[Tim Deegan]

On *BSD, stdbool.h lives in /usr/include, but we don't want to have
that on the search path in case we pick up any headers from the build
host's C libraries.

Copy the equivalent hack already in place for stdarg.h: on all
supported compilers the contents of stdbool.h are trivial, so just
supply the things we need in a xen/stdbool.h header.

Signed-off-by: Tim Deegan <tim@xen.org>
---
 xen/include/xen/libelf.h  |    4 ++--
 xen/include/xen/stdbool.h |   13 +++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)
 create mode 100644 xen/include/xen/stdbool.h

diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index e65db6d..6393040 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -29,8 +29,6 @@
 #error define architectural endianness
 #endif
 
-#include <stdbool.h>
-
 typedef int elf_errorstatus; /* 0: ok; -ve (normally -1): error */
 typedef int elf_negerrnoval; /* 0: ok; -EFOO: error */
 
@@ -39,12 +37,14 @@ typedef int elf_negerrnoval; /* 0: ok; -EFOO: error */
 #ifdef __XEN__
 #include <public/elfnote.h>
 #include <public/features.h>
+#include <xen/stdbool.h>
 #include <xen/string.h>
 #else
 #include <xen/elfnote.h>
 #include <xen/features.h>
 
 #include <stdarg.h>
+#include <stdbool.h>
 #include <string.h>
 
 struct elf_binary;
diff --git a/xen/include/xen/stdbool.h b/xen/include/xen/stdbool.h
new file mode 100644
index 0000000..2eecd52
--- /dev/null
+++ b/xen/include/xen/stdbool.h
@@ -0,0 +1,13 @@
+#ifndef __XEN_STDBOOL_H__
+#define __XEN_STDBOOL_H__
+
+#if defined(__OpenBSD__) || defined(__NetBSD__)
+#  define bool _Bool
+#  define true 1
+#  define false 0
+#  define __bool_true_false_are_defined   1
+#else
+#  include <stdbool.h>
+#endif
+
+#endif /* __XEN_STDBOOL_H__ */
-- 
1.7.10.4

Re: [PATCH 0/3] stdinc-related build fixes.

[Jan Beulich]

>>> On 15.08.13 at 15:25, Tim Deegan <tim@xen.org> wrote:
> These three patches tidy some of the xen build runes around the use of
> nostdinc.
> 
> Patch 3/3 hasn't yet been tested on an actual NetBSD box,
> so I'd appreciate it if Patrick would give it a go.  Apart from that,
> I've tested with every GCC I can lay my hands on (and clang 3.0),
> building x86 and ARM.
> 
> I rather suspect that FreeBSD and Solaris ought to be treated the same
> way -- currently FreeBSD has no workarounds (but on a FreeBSD 5 box I
> see stdarg.h in /usr/include) and Solaris just drops the nostdinc
> (I have no Solaris box handy to test on).

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

Re: [PATCH 1/3] xen: move some arch CFLAGS into the common Rules.mk.

[Ian Campbell]

On Thu, 2013-08-15 at 14:25 +0100, Tim Deegan wrote:
> These are the same on all current architectures, so move them
> into xen/Rules.mk.  Lay them out a little more neatly too.
> 
> Signed-off-by: Tim Deegan <tim@xen.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  xen/Rules.mk          |    4 ++++
>  xen/arch/arm/Rules.mk |    2 --
>  xen/arch/x86/Rules.mk |    2 --
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 26e5bb6..c432ad6 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -41,6 +41,10 @@ ALL_OBJS-y               += $(BASEDIR)/xsm/built_in.o
>  ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
>  ALL_OBJS-$(x86)          += $(BASEDIR)/crypto/built_in.o
>  
> +CFLAGS-y                += -fno-builtin -fno-common
> +CFLAGS-y                += -Werror -Wredundant-decls -Wno-pointer-arith
> +CFLAGS-y                += -pipe
> +CFLAGS-y                += -iwithprefix include
>  CFLAGS-y                += -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
>  CFLAGS-$(XSM_ENABLE)    += -DXSM_ENABLE
>  CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE -DXSM_MAGIC=0xf97cff8c
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index a18e7fd..d80dfce 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -10,8 +10,6 @@ HAS_DEVICE_TREE := y
>  HAS_VIDEO := y
>  HAS_ARM_HDLCD := y
>  
> -CFLAGS += -fno-builtin -fno-common -Wredundant-decls
> -CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
>  CFLAGS += -I$(BASEDIR)/include
>  
>  $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
> index 0a9d68d..f629dff 100644
> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -25,8 +25,6 @@ ifneq ($(XEN_OS),SunOS)
>  CFLAGS-$(gcc) += -nostdinc
>  endif
>  
> -CFLAGS += -fno-builtin -fno-common -Wredundant-decls
> -CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
>  CFLAGS += -I$(BASEDIR)/include 
>  CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
>  CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default

Re: [PATCH 2/3] xen: move -nostdinc into common Rules.mk.

[Ian Campbell]

On Thu, 2013-08-15 at 14:25 +0100, Tim Deegan wrote:
> Previously we didn't use it at all the on ARM ports or for clang builds.
> 
> For ARM, I think this is just an oversight.

Yes.

> 
> For clang, this used not to work, because '-withprefix include' didn't
> let us see stdarg.h, but that's fixed in clang v3.0.
> 
> Also move the '-withprefix include' to beside -nostdinc as it's only
> needed with -nostdinc anyway.
> 
> Signed-off-by: Tim Deegan <tim@xen.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  xen/Rules.mk          |    6 +++++-
>  xen/arch/x86/Rules.mk |    6 ------
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index c432ad6..bbfc1ac 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -44,8 +44,12 @@ ALL_OBJS-$(x86)          += $(BASEDIR)/crypto/built_in.o
>  CFLAGS-y                += -fno-builtin -fno-common
>  CFLAGS-y                += -Werror -Wredundant-decls -Wno-pointer-arith
>  CFLAGS-y                += -pipe
> -CFLAGS-y                += -iwithprefix include
>  CFLAGS-y                += -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
> +# Solaris puts stdarg.h &c in the system include directory.
> +ifneq ($(XEN_OS),SunOS)
> +CFLAGS-y                += -nostdinc -iwithprefix include
> +endif
> +
>  CFLAGS-$(XSM_ENABLE)    += -DXSM_ENABLE
>  CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE -DXSM_MAGIC=0xf97cff8c
>  CFLAGS-$(FLASK_ENABLE)  += -DFLASK_DEVELOP -DFLASK_BOOTPARAM -DFLASK_AVC_STATS
> diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
> index f629dff..04c1a90 100644
> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -19,12 +19,6 @@ 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
> -
>  CFLAGS += -I$(BASEDIR)/include 
>  CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-generic
>  CFLAGS += -I$(BASEDIR)/include/asm-x86/mach-default

Re: [PATCH 3/3] xen: Add stdbool.h workaround for BSD.

[Ian Campbell]

On Thu, 2013-08-15 at 14:25 +0100, Tim Deegan wrote:
> On *BSD, stdbool.h lives in /usr/include, but we don't want to have
> that on the search path in case we pick up any headers from the build
> host's C libraries.
> 
> Copy the equivalent hack already in place for stdarg.h: on all
> supported compilers the contents of stdbool.h are trivial, so just
> supply the things we need in a xen/stdbool.h header.
> 
> Signed-off-by: Tim Deegan <tim@xen.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>
(or Reviewed-by if you prefer)

I suppose we ought to start prefering xen/stdbool.h+bool to xen/types.h
+bool_t in new code and think about migrating uses of bool_t over.

Ian.

Re: [PATCH 0/3] stdinc-related build fixes.

[Keir Fraser]

On 15/08/2013 14:51, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 15.08.13 at 15:25, Tim Deegan <tim@xen.org> wrote:
>> These three patches tidy some of the xen build runes around the use of
>> nostdinc.
>> 
>> Patch 3/3 hasn't yet been tested on an actual NetBSD box,
>> so I'd appreciate it if Patrick would give it a go.  Apart from that,
>> I've tested with every GCC I can lay my hands on (and clang 3.0),
>> building x86 and ARM.
>> 
>> I rather suspect that FreeBSD and Solaris ought to be treated the same
>> way -- currently FreeBSD has no workarounds (but on a FreeBSD 5 box I
>> see stdarg.h in /usr/include) and Solaris just drops the nostdinc
>> (I have no Solaris box handy to test on).
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Keir Fraser <keir@xen.org>

Solaris / FreeBSD builds (was Re: [PATCH 0/3] stdinc-related build fixes.)

[Tim Deegan]

At 14:25 +0100 on 15 Aug (1376576704), Tim Deegan wrote:
> I rather suspect that FreeBSD and Solaris ought to be treated the same
> way -- currently FreeBSD has no workarounds (but on a FreeBSD 5 box I
> see stdarg.h in /usr/include)

Yep, FreeBSD works with the same handling as the other BSDs.  (I just
sent a patch to do that.)

> and Solaris just drops the nostdinc (I have no Solaris box handy to test on).

Playing with a Solaris 10 VM (and gcc &c from opencsw) it looks like
'-nostdinc -iwithprefix include' should work fine there, i.e.:

diff --git a/xen/Rules.mk b/xen/Rules.mk
index bbfc1ac..bf870da 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -45,10 +45,7 @@ CFLAGS-y                += -fno-builtin -fno-common
 CFLAGS-y                += -Werror -Wredundant-decls -Wno-pointer-arith
 CFLAGS-y                += -pipe
 CFLAGS-y                += -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
-# Solaris puts stdarg.h &c in the system include directory.
-ifneq ($(XEN_OS),SunOS)
 CFLAGS-y                += -nostdinc -iwithprefix include
-endif
 
 CFLAGS-$(XSM_ENABLE)    += -DXSM_ENABLE
 CFLAGS-$(FLASK_ENABLE)  += -DFLASK_ENABLE -DXSM_MAGIC=0xf97cff8c


Sadly all I could find at short notice was 32-bit install media so I can't
actually test the x86_64 build.  

Does anyone from Oracle want to try it? :)

Cheers,

Tim.

Re: [PATCH 0/3] stdinc-related build fixes.

[Patrick Welche]

On Thu, Aug 15, 2013 at 02:25:04PM +0100, Tim Deegan wrote:
> These three patches tidy some of the xen build runes around the use of
> nostdinc.
> 
> Patch 3/3 hasn't yet been tested on an actual NetBSD box,
> so I'd appreciate it if Patrick would give it a go.  Apart from that,
> I've tested with every GCC I can lay my hands on (and clang 3.0),
> building x86 and ARM.

Your changes allowed a successful "gmake xen", thank you!

If we want to carry on helping smooth out the build, I was surprised
by "gmake install xen" giving

  [ -d //boot ] || install -d -m0755 -p //boot
  install: //boot exists but is not a directory

given that I had configured with --prefix=/usr/local/xen.

/boot isn't a directory, it's my second stage bootloader, so I
would rather not loose it ;-) Start another thread?

Cheers,

Patrick

Re: [PATCH 0/3] stdinc-related build fixes.

[Andrew Cooper]

On 15/08/13 20:49, Patrick Welche wrote:
> On Thu, Aug 15, 2013 at 02:25:04PM +0100, Tim Deegan wrote:
>> These three patches tidy some of the xen build runes around the use of
>> nostdinc.
>>
>> Patch 3/3 hasn't yet been tested on an actual NetBSD box,
>> so I'd appreciate it if Patrick would give it a go.  Apart from that,
>> I've tested with every GCC I can lay my hands on (and clang 3.0),
>> building x86 and ARM.
> Your changes allowed a successful "gmake xen", thank you!
>
> If we want to carry on helping smooth out the build, I was surprised
> by "gmake install xen" giving
>
>   [ -d //boot ] || install -d -m0755 -p //boot
>   install: //boot exists but is not a directory
>
> given that I had configured with --prefix=/usr/local/xen.
>
> /boot isn't a directory, it's my second stage bootloader, so I
> would rather not loose it ;-) Start another thread?
>
> Cheers,
>
> Patrick

As for your boot problem, that will be something not correctly setting
the DESTDIR make variable.

Is gmake having a fit with the "local variable" syntax in xen/Makefile

.PHONY: _install
_install: D=$(DESTDIR)
_install: T=$(notdir $(TARGET))
_install: Z=$(CONFIG_XEN_INSTALL_SUFFIX)
_install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
        [ -d $(D)/boot ] || $(INSTALL_DIR) $(D)/boot
        $(INSTALL_DATA) $(TARGET)$(Z) $(D)/boot/$(T)-$(XEN_FULLVERSION)$(Z)

or is something else not setting DESTDIR higher up?

~Andrew

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

Re: [PATCH 0/3] stdinc-related build fixes.

[Patrick Welche]

On Thu, Aug 15, 2013 at 08:58:44PM +0100, Andrew Cooper wrote:
> > given that I had configured with --prefix=/usr/local/xen.
> 
> As for your boot problem, that will be something not correctly setting
> the DESTDIR make variable.

Right - I'm just being silly - what is in the "xen" directory is untouched
by configure, so setting --prefix isn't going change anything for a
"gmake install xen". A "gmake -j8 install xen DESTDIR=/usr/local/xen44"
was fine as you would expect...

Cheers,

Patrick

Re: [PATCH 0/3] stdinc-related build fixes.

[Tim Deegan]

At 20:49 +0100 on 15 Aug (1376599776), Patrick Welche wrote:
> On Thu, Aug 15, 2013 at 02:25:04PM +0100, Tim Deegan wrote:
> > These three patches tidy some of the xen build runes around the use of
> > nostdinc.
> > 
> > Patch 3/3 hasn't yet been tested on an actual NetBSD box,
> > so I'd appreciate it if Patrick would give it a go.  Apart from that,
> > I've tested with every GCC I can lay my hands on (and clang 3.0),
> > building x86 and ARM.
> 
> Your changes allowed a successful "gmake xen", thank you!

Great; thanks for testing.  I've pushed them to the staging tree now.

Jan, I think 3/3 (7b9685ca4ed2fd723600ce66eb20a6d0c115b6cb) is a
candidate for backporting as far as 4.1.

Tim.

clang compilation

[Patrick Welche]

On Thu, Aug 15, 2013 at 10:15:31PM +0100, Tim Deegan wrote:
> > > I've tested with every GCC I can lay my hands on (and clang 3.0),
> > > building x86 and ARM.

Joerg has a "clang fix" for warnings for xen/arch/x86/time.c, essentially
mul to mull or mulq - did you see such warnings when testing with clang?

http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/sysutils/xenkernel42/patches/patch-xen_arch_x86_time.c

Cheers,

Patrick

Re: clang compilation

[Tim Deegan]

At 08:17 +0100 on 16 Aug (1376641071), Patrick Welche wrote:
> On Thu, Aug 15, 2013 at 10:15:31PM +0100, Tim Deegan wrote:
> > > > I've tested with every GCC I can lay my hands on (and clang 3.0),
> > > > building x86 and ARM.
> 
> Joerg has a "clang fix" for warnings for xen/arch/x86/time.c, essentially
> mul to mull or mulq - did you see such warnings when testing with clang?
> 
> http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/sysutils/xenkernel42/patches/patch-xen_arch_x86_time.c

We already have one of those changes upstream:
http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=59a28b5f045331641cbf0c1fc8d5d67afe328939

Jan, can that be backported as far as 4.2 as a build fix?

I'm not sure why I didn't trip over the mul in mul_frac() too, but
we can probably take that if you'd like to send a patch.

The other chunk in that file is against 32-bit code, which is gone in
4.3 and 4.4.  I guess we might take that as a fix directly against 4.2.
Jan, what do you think?

The patch-xen_include_xen_lib.h in the same directory looks OK to me too
- is that also a clang fix?

The $(EXTRA_CFLAGS) runes I'm less sure of; I know people have strong
feelings about the Right Way to do that.  You seem to be using it to
pass clang-specific options -- it might be better to build with
'clang=y' and add any more warning suppressions in Config.mk where
CFLAGS-$(clang) is set.

Cheers,

Tim.

Re: [PATCH 0/3] stdinc-related build fixes.

[Jan Beulich]

>>> On 15.08.13 at 23:15, Tim Deegan <tim@xen.org> wrote:
> At 20:49 +0100 on 15 Aug (1376599776), Patrick Welche wrote:
>> On Thu, Aug 15, 2013 at 02:25:04PM +0100, Tim Deegan wrote:
>> > These three patches tidy some of the xen build runes around the use of
>> > nostdinc.
>> > 
>> > Patch 3/3 hasn't yet been tested on an actual NetBSD box,
>> > so I'd appreciate it if Patrick would give it a go.  Apart from that,
>> > I've tested with every GCC I can lay my hands on (and clang 3.0),
>> > building x86 and ARM.
>> 
>> Your changes allowed a successful "gmake xen", thank you!
> 
> Great; thanks for testing.  I've pushed them to the staging tree now.
> 
> Jan, I think 3/3 (7b9685ca4ed2fd723600ce66eb20a6d0c115b6cb) is a
> candidate for backporting as far as 4.1.

Okay, taking note of it. Will do as soon as the test infrastructure
works again and this gets pushed through.

Jan

Re: clang compilation

[Jan Beulich]

>>> On 16.08.13 at 10:17, Tim Deegan <tim@xen.org> wrote:
> At 08:17 +0100 on 16 Aug (1376641071), Patrick Welche wrote:
>> On Thu, Aug 15, 2013 at 10:15:31PM +0100, Tim Deegan wrote:
>> > > > I've tested with every GCC I can lay my hands on (and clang 3.0),
>> > > > building x86 and ARM.
>> 
>> Joerg has a "clang fix" for warnings for xen/arch/x86/time.c, essentially
>> mul to mull or mulq - did you see such warnings when testing with clang?
>> 
>> 
> http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/sysutils/xenkernel42/patches/ 
> patch-xen_arch_x86_time.c
> 
> We already have one of those changes upstream:
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=59a28b5f045331641cbf 
> 0c1fc8d5d67afe328939
> 
> Jan, can that be backported as far as 4.2 as a build fix?

Sure, even more so that this is not just a build fix - the generated
code would end up being wrong if the input "delta" was put in
memory instead of a register.

> I'm not sure why I didn't trip over the mul in mul_frac() too, but
> we can probably take that if you'd like to send a patch.

And that should relax the constraint of multiplier to "rm" at once.

> The other chunk in that file is against 32-bit code, which is gone in
> 4.3 and 4.4.  I guess we might take that as a fix directly against 4.2.
> Jan, what do you think?

I'd add this as part of the backport of 59a28b5f, as logically the 64-bit
change ought to be extended to 32-bit.

Yet with all of this I wonder what kind of broken assembler they're
using - deferring the operand size from register operands should
work consistently for all instructions or none, i.e. needing explicit
suffixes on mul but not any of the other .

Jan

Re: clang compilation

[Tim Deegan]

At 09:48 +0100 on 16 Aug (1376646500), Jan Beulich wrote:
> Yet with all of this I wonder what kind of broken assembler they're
> using - deferring the operand size from register operands should
> work consistently for all instructions or none, i.e. needing explicit
> suffixes on mul but not any of the other .

It's not just mul (see 794d4b9e85047aacfe23b852d3a03a8eff920aec in the
original clang series) but it is certainly odd that it's inconsistent
Presumably in some cases the choice is constrained by some other choice
the optimizer has already made.

Maybe one day I'll have the time to audit all the inline asm for
implicit operand sizes. :)

Tim.

Re: clang compilation

[Jan Beulich]

>>> On 16.08.13 at 10:54, Tim Deegan <tim@xen.org> wrote:
> At 09:48 +0100 on 16 Aug (1376646500), Jan Beulich wrote:
>> Yet with all of this I wonder what kind of broken assembler they're
>> using - deferring the operand size from register operands should
>> work consistently for all instructions or none, i.e. needing explicit
>> suffixes on mul but not any of the other .
> 
> It's not just mul (see 794d4b9e85047aacfe23b852d3a03a8eff920aec in the
> original clang series) but it is certainly odd that it's inconsistent
> Presumably in some cases the choice is constrained by some other choice
> the optimizer has already made.

No, from my pov that other change has nothing to do with operand
size deduction. Especially the missing 's' suffixes on the fi...
instructions were an outright bug.

> Maybe one day I'll have the time to audit all the inline asm for
> implicit operand sizes. :)

Uglifying the code especially for those - like me - who come from
the Intel syntax world, which doesn't know suffixes except for
operands which have no way of expressing their size in non-
ambiguous ways.

Jan

Re: clang compilation

[Jan Beulich]

>>> On 16.08.13 at 10:17, Tim Deegan <tim@xen.org> wrote:
> At 08:17 +0100 on 16 Aug (1376641071), Patrick Welche wrote:
>> On Thu, Aug 15, 2013 at 10:15:31PM +0100, Tim Deegan wrote:
>> > > > I've tested with every GCC I can lay my hands on (and clang 3.0),
>> > > > building x86 and ARM.
>> 
>> Joerg has a "clang fix" for warnings for xen/arch/x86/time.c, essentially
>> mul to mull or mulq - did you see such warnings when testing with clang?
>> 
>> 
> http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/sysutils/xenkernel42/patches/ 
> patch-xen_arch_x86_time.c
> 
> We already have one of those changes upstream:
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=59a28b5f045331641cbf 
> 0c1fc8d5d67afe328939
> 
> Jan, can that be backported as far as 4.2 as a build fix?

Any reason why you ask for 4.2 but not 4.1? Oh, wait - there was
no clang support there yet. But as said earlier, this isn't just a
build fix, and hence I guess I'll apply the 64-bit part of it to 4.1
anyway.

Jan

Re: [PATCH 3/3] xen: Add stdbool.h workaround for BSD.

[Ian Jackson]

Tim Deegan writes ("[PATCH 3/3] xen: Add stdbool.h workaround for BSD."):
> On *BSD, stdbool.h lives in /usr/include, but we don't want to have
> that on the search path in case we pick up any headers from the build
> host's C libraries.
> 
> Copy the equivalent hack already in place for stdarg.h: on all
> supported compilers the contents of stdbool.h are trivial, so just
> supply the things we need in a xen/stdbool.h header.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

The only alternative is to try to reconstruct the compiler-specific
include directory using gcc --print-libgcc-file-name and seddery.

Ian.

Re: [PATCH 3/3] xen: Add stdbool.h workaround for BSD.

[Tim Deegan]

At 14:48 +0100 on 19 Aug (1376923719), Ian Jackson wrote:
> Tim Deegan writes ("[PATCH 3/3] xen: Add stdbool.h workaround for BSD."):
> > On *BSD, stdbool.h lives in /usr/include, but we don't want to have
> > that on the search path in case we pick up any headers from the build
> > host's C libraries.
> > 
> > Copy the equivalent hack already in place for stdarg.h: on all
> > supported compilers the contents of stdbool.h are trivial, so just
> > supply the things we need in a xen/stdbool.h header.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> The only alternative is to try to reconstruct the compiler-specific
> include directory using gcc --print-libgcc-file-name and seddery.

Sadly, not even that.  In these BSD systems, the compiler's headers and
libraries are bundled in with the rest of the system libraries,  e.g.:

ocelot:tjd$ gcc --print-libgcc-file-name
/usr/lib/libgcc.a

so it would be a matter of finding the headers we want in /usr/include
and pulling on that string to grab all the headers they #include, and
copying them somewhere on the Xen build's include path.  Something like:

ocelot:tjd$ gcc -E -include stdarg.h -x c /dev/null | grep '^# 1 "/usr'
# 1 "/usr/include/stdarg.h" 1 3 4
# 1 "/usr/include/sys/cdefs.h" 1 3 4
# 1 "/usr/include/sys/_types.h" 1 3 4
# 1 "/usr/include/machine/_types.h" 1 3 4

Bleah. :(

I don't feel too bad about just defining up the bits of stdarg and
stdbool that we need; after all we do that for a bunch of other
compiler-specific things.

Another approach would be to put those definitions into compiler.h and
stop including the compiler's headers altogether, even on linux -- after
all we don't link against libgcc.a either.

Tim.

Re: [PATCH 3/3] xen: Add stdbool.h workaround for BSD.

[Ian Jackson]

Tim Deegan writes ("Re: [PATCH 3/3] xen: Add stdbool.h workaround for BSD."):
> Bleah. :(

Quite so.

> I don't feel too bad about just defining up the bits of stdarg and
> stdbool that we need; after all we do that for a bunch of other
> compiler-specific things.

Yes.

Ian.