[Xen-devel] [PATCH 0/5] build improvements/fixes after b41666f2c1

[Xen-devel] [PATCH 0/5] build improvements/fixes after b41666f2c1

[Roger Pau Monne]

Hello,

The following fixes arise from the travis-ci fallout caused by
b41666f2c1 ('config: don't hardcode toolchain binaries'). First patches
aim to simplify and group together the place where toolchain binaries to
be used by the build system. Last patch changes the travis-ci build
script to accommodate the changes introduced by b41666f2c1.

Thanks, Roger.

Roger Pau Monne (5):
  make: simplify setting HOST{CC/CXX}
  kconfig: don't pass ARCH and SRCARCH on the sub-make call
  kconfig: include default toolchain values
  kconfig: disable non-literal format string warnings
  travis: pass a correct CC/CXX if CROSS_COMPILE is defined

 Config.mk                          | 10 ----------
 config/StdGNU.mk                   |  4 ++++
 scripts/travis-build               |  8 ++++++++
 xen/Makefile                       | 10 +++++-----
 xen/tools/kconfig/Makefile.kconfig | 12 ++++++++----
 5 files changed, 25 insertions(+), 19 deletions(-)

-- 
2.20.1 (Apple Git-117)


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

[Xen-devel] [PATCH 1/5] make: simplify setting HOST{CC/CXX}

[Roger Pau Monne]

Infer the values of HOST{CC/CXX} from CC/CXX if unset, do this in
StdGNU.mk, together with the rest of the toolchain variables.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wl@xen.org>
---
 Config.mk        | 10 ----------
 config/StdGNU.mk |  4 ++++
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/Config.mk b/Config.mk
index 417039d7f6..1a1cc09881 100644
--- a/Config.mk
+++ b/Config.mk
@@ -39,22 +39,12 @@ DESTDIR     ?= /
 # Allow phony attribute to be listed as dependency rather than fake target
 .PHONY: .phony
 
-# If we are not cross-compiling, default HOSTC{C/XX} to C{C/XX}
-ifeq ($(XEN_TARGET_ARCH), $(XEN_COMPILE_ARCH))
-HOSTCC ?= $(CC)
-HOSTCXX ?= $(CXX)
-endif
-
 # Use Clang/LLVM instead of GCC?
 clang ?= n
 ifeq ($(clang),n)
 gcc := y
-HOSTCC ?= gcc
-HOSTCXX ?= g++
 else
 gcc := n
-HOSTCC ?= clang
-HOSTCXX ?= clang++
 endif
 
 DEPS_INCLUDE = $(addsuffix .d2, $(basename $(wildcard $(DEPS))))
diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index 490ebdf23c..7b7dfe0440 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -9,6 +9,10 @@ CC        ?= $(CROSS_COMPILE)gcc
 CXX       ?= $(CROSS_COMPILE)g++
 LD_LTO    ?= $(CROSS_COMPILE)ld
 endif
+
+HOSTCC    ?= $(CC)
+HOSTCXX   ?= $(CXX)
+
 CPP       ?= $(CC) -E
 AR        ?= $(CROSS_COMPILE)ar
 RANLIB    ?= $(CROSS_COMPILE)ranlib
-- 
2.20.1 (Apple Git-117)


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

[Xen-devel] [PATCH 2/5] kconfig: don't pass ARCH and SRCARCH on the sub-make call

[Roger Pau Monne]

and instead export them from the top-level Xen makefile.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wl@xen.org>
---
 xen/Makefile | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index c80914c31d..3e3d08d1cc 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -21,8 +21,8 @@ MAKEFLAGS += -rR
 
 EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
 
-ARCH=$(XEN_TARGET_ARCH)
-SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
+export ARCH=$(XEN_TARGET_ARCH)
+export SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
 
 # Don't break if the build process wasn't called from the top level
 # we need XEN_TARGET_ARCH to generate the proper config
@@ -267,14 +267,14 @@ kconfig := silentoldconfig oldconfig config menuconfig defconfig \
 	randconfig $(notdir $(wildcard arch/$(SRCARCH)/configs/*_defconfig))
 .PHONY: $(kconfig)
 $(kconfig):
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
 
 include/config/%.conf: include/config/auto.conf.cmd $(KCONFIG_CONFIG)
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" silentoldconfig
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" silentoldconfig
 
 # Allow people to just run `make` as before and not force them to configure
 $(KCONFIG_CONFIG):
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" defconfig
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" defconfig
 
 # Break the dependency chain for the first run
 include/config/auto.conf.cmd: ;
-- 
2.20.1 (Apple Git-117)


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

[Xen-devel] [PATCH 3/5] kconfig: include default toolchain values

[Roger Pau Monne]

Include config/$(OS).mk which contains the default values for the
toolchain variables. This removes the need to pass HOST{CC/CXX} as
parameters from the high level make target.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wl@xen.org>
Cc: Doug Goldstein <cardoe@cardoe.com>
---
 xen/Makefile                       | 6 +++---
 xen/tools/kconfig/Makefile.kconfig | 7 +++----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index 3e3d08d1cc..620af7883c 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -267,14 +267,14 @@ kconfig := silentoldconfig oldconfig config menuconfig defconfig \
 	randconfig $(notdir $(wildcard arch/$(SRCARCH)/configs/*_defconfig))
 .PHONY: $(kconfig)
 $(kconfig):
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig $@
 
 include/config/%.conf: include/config/auto.conf.cmd $(KCONFIG_CONFIG)
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" silentoldconfig
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig silentoldconfig
 
 # Allow people to just run `make` as before and not force them to configure
 $(KCONFIG_CONFIG):
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" defconfig
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig defconfig
 
 # Break the dependency chain for the first run
 include/config/auto.conf.cmd: ;
diff --git a/xen/tools/kconfig/Makefile.kconfig b/xen/tools/kconfig/Makefile.kconfig
index dbd8912015..138bf3f1b7 100644
--- a/xen/tools/kconfig/Makefile.kconfig
+++ b/xen/tools/kconfig/Makefile.kconfig
@@ -35,15 +35,14 @@ KBUILD_DEFCONFIG := $(ARCH)_defconfig
 # provide our shell
 CONFIG_SHELL := $(SHELL)
 
-# provide the host compiler
-HOSTCC ?= gcc
-HOSTCXX ?= g++
-
 # force target
 PHONY += FORCE
 
 FORCE:
 
+# Sets toolchain binaries to use
+include $(XEN_ROOT)/config/$(shell uname -s).mk
+
 # include the original Makefile and Makefile.host from Linux
 include $(src)/Makefile
 include $(src)/Makefile.host
-- 
2.20.1 (Apple Git-117)


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

[Xen-devel] [PATCH 4/5] kconfig: disable non-literal format string warnings

[Roger Pau Monne]

Kconfig makes heavy use of non-literals as format strings, disable
compiler warnings since this is expected usage.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Doug Goldstein <cardoe@cardoe.com>
---
 xen/tools/kconfig/Makefile.kconfig | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/tools/kconfig/Makefile.kconfig b/xen/tools/kconfig/Makefile.kconfig
index 138bf3f1b7..763de37a14 100644
--- a/xen/tools/kconfig/Makefile.kconfig
+++ b/xen/tools/kconfig/Makefile.kconfig
@@ -43,6 +43,11 @@ FORCE:
 # Sets toolchain binaries to use
 include $(XEN_ROOT)/config/$(shell uname -s).mk
 
+# Disable format warnings, kconfig makes heavy use of non-literal format
+# strings.
+HOSTCFLAGS += -Wno-format
+HOSTCXXFLAGS += -Wno-format
+
 # include the original Makefile and Makefile.host from Linux
 include $(src)/Makefile
 include $(src)/Makefile.host
-- 
2.20.1 (Apple Git-117)


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

[Xen-devel] [PATCH 5/5] travis: pass a correct CC/CXX if CROSS_COMPILE is defined

[Roger Pau Monne]

After b41666f2c1 Xen no longer overwrites the names of the build
toolchain utilities required during the build process, and instead
uses the values from the environment.

In that case, if the user wants to define CC or other variables to
point to specific toolchain utilities to use it must also take into
account that such variables must be prefixed with CROSS_COMPILE, since
Xen will no longer do this.

Fixes: b41666f2c1 ('config: don't hardcode toolchain binaries')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Doug Goldstein <cardoe@cardoe.com>
---
 scripts/travis-build | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/travis-build b/scripts/travis-build
index 0cb15a89e4..a264e286b2 100755
--- a/scripts/travis-build
+++ b/scripts/travis-build
@@ -1,6 +1,14 @@
 #!/bin/bash -ex
 
+# Set HOST{CC/CXX} in case we are cross building
+export HOSTCC=${CC}
+export HOSTCXX=${CXX}
+# Prefix environment CC/CXX with CROSS_COMPILE if present
+export CC=${CROSS_COMPILE}${CC}
+export CXX=${CROSS_COMPILE}${CXX}
+
 $CC --version
+[[ "${CC}" != "${HOSTCC}" ]] && $HOSTCC --version
 
 # random config or default config
 if [[ "${RANDCONFIG}" == "y" ]]; then
-- 
2.20.1 (Apple Git-117)


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

Re: [Xen-devel] [PATCH 4/5] kconfig: disable non-literal format string warnings

[Jan Beulich]

>>> On 26.06.19 at 15:55, <roger.pau@citrix.com> wrote:
> Kconfig makes heavy use of non-literals as format strings, disable
> compiler warnings since this is expected usage.

I've never seen any with any version of gcc - are there more
aspects to be mentioned here?

> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Doug Goldstein <cardoe@cardoe.com>
> ---
>  xen/tools/kconfig/Makefile.kconfig | 5 +++++
>  1 file changed, 5 insertions(+)

You Cc list looks to be too short for this change.

> --- a/xen/tools/kconfig/Makefile.kconfig
> +++ b/xen/tools/kconfig/Makefile.kconfig
> @@ -43,6 +43,11 @@ FORCE:
>  # Sets toolchain binaries to use
>  include $(XEN_ROOT)/config/$(shell uname -s).mk
>  
> +# Disable format warnings, kconfig makes heavy use of non-literal format
> +# strings.
> +HOSTCFLAGS += -Wno-format
> +HOSTCXXFLAGS += -Wno-format

But this disables far more warnings than ones for non-literal format
strings, at least afaict.

Jan


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

Re: [Xen-devel] [PATCH 4/5] kconfig: disable non-literal format string warnings

[Roger Pau Monné]

On Wed, Jun 26, 2019 at 08:45:27AM -0600, Jan Beulich wrote:
> >>> On 26.06.19 at 15:55, <roger.pau@citrix.com> wrote:
> > Kconfig makes heavy use of non-literals as format strings, disable
> > compiler warnings since this is expected usage.
> 
> I've never seen any with any version of gcc - are there more
> aspects to be mentioned here?

Oh, I've always seen them with clang. Not sure why gcc doesn't show
such warnings.

clang -Wp,-MD,tools/kconfig/.conf.o.d    -DCURSES_LOC="<ncurses.h>" -DLOCALE -DKBUILD_NO_NLS  -c -o tools/kconfig/conf.o tools/kconfig/conf.c
tools/kconfig/conf.c:77:10: warning: format string is not a string literal (potentially insecure)
      [-Wformat-security]
                printf(_("aborted!\n\n"));
                       ^~~~~~~~~~~~~~~~~
tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
#define _(text) gettext(text)
                ^~~~~~~~~~~~~
tools/kconfig/conf.c:77:10: note: treat the string as an argument to avoid this
                printf(_("aborted!\n\n"));
                       ^
                       "%s",
tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
#define _(text) gettext(text)
                ^
tools/kconfig/conf.c:78:10: warning: format string is not a string literal (potentially insecure)
      [-Wformat-security]
                printf(_("Console input/output is redirected. "));
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
#define _(text) gettext(text)
                ^~~~~~~~~~~~~
tools/kconfig/conf.c:78:10: note: treat the string as an argument to avoid this
                printf(_("Console input/output is redirected. "));
                       ^
                       "%s",
tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
#define _(text) gettext(text)
                ^
[...]

> 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Doug Goldstein <cardoe@cardoe.com>
> > ---
> >  xen/tools/kconfig/Makefile.kconfig | 5 +++++
> >  1 file changed, 5 insertions(+)
> 
> You Cc list looks to be too short for this change.

That's what get_maintainer.pl has given me. Maybe the syntax in
MAINTAINERS is not correct, or get_maintainer.pl needs adjustments.

> 
> > --- a/xen/tools/kconfig/Makefile.kconfig
> > +++ b/xen/tools/kconfig/Makefile.kconfig
> > @@ -43,6 +43,11 @@ FORCE:
> >  # Sets toolchain binaries to use
> >  include $(XEN_ROOT)/config/$(shell uname -s).mk
> >  
> > +# Disable format warnings, kconfig makes heavy use of non-literal format
> > +# strings.
> > +HOSTCFLAGS += -Wno-format
> > +HOSTCXXFLAGS += -Wno-format
> 
> But this disables far more warnings than ones for non-literal format
> strings, at least afaict.

Sorry, it should be -Wno-format-security. I think I dropped the
-security part while copying.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH 4/5] kconfig: disable non-literal format string warnings

[Andrew Cooper]

On 26/06/2019 16:20, Roger Pau Monné wrote:
> On Wed, Jun 26, 2019 at 08:45:27AM -0600, Jan Beulich wrote:
>>>>> On 26.06.19 at 15:55, <roger.pau@citrix.com> wrote:
>>> Kconfig makes heavy use of non-literals as format strings, disable
>>> compiler warnings since this is expected usage.
>> I've never seen any with any version of gcc - are there more
>> aspects to be mentioned here?
> Oh, I've always seen them with clang. Not sure why gcc doesn't show
> such warnings.
>
> clang -Wp,-MD,tools/kconfig/.conf.o.d    -DCURSES_LOC="<ncurses.h>" -DLOCALE -DKBUILD_NO_NLS  -c -o tools/kconfig/conf.o tools/kconfig/conf.c
> tools/kconfig/conf.c:77:10: warning: format string is not a string literal (potentially insecure)
>       [-Wformat-security]
>                 printf(_("aborted!\n\n"));
>                        ^~~~~~~~~~~~~~~~~
> tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
> #define _(text) gettext(text)
>                 ^~~~~~~~~~~~~
> tools/kconfig/conf.c:77:10: note: treat the string as an argument to avoid this
>                 printf(_("aborted!\n\n"));
>                        ^
>                        "%s",
> tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
> #define _(text) gettext(text)
>                 ^
> tools/kconfig/conf.c:78:10: warning: format string is not a string literal (potentially insecure)
>       [-Wformat-security]
>                 printf(_("Console input/output is redirected. "));
>                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
> #define _(text) gettext(text)
>                 ^~~~~~~~~~~~~
> tools/kconfig/conf.c:78:10: note: treat the string as an argument to avoid this
>                 printf(_("Console input/output is redirected. "));
>                        ^
>                        "%s",
> tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
> #define _(text) gettext(text)
>                 ^

Clang is correct and GCC is wrong.  This code is plain buggy.

Its trivial to arrange for gettext to return a string with a % in it.

These want fixing to "%s", _(), or to use puts().

It looks like Linux has dropped the use of gettext in the first place. 
Look like c/s 694c49a7c01cc87194be40cb26404b58b68c291c wants backporting.

~Andrew

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

Re: [Xen-devel] [PATCH 2/5] kconfig: don't pass ARCH and SRCARCH on the sub-make call

[Roger Pau Monné]

On Fri, Jun 28, 2019 at 09:40:59AM +0000, Jan Beulich wrote:
>  >>> On 26.06.19 at 15:55, <roger.pau@citrix.com> wrote:
>  > and instead export them from the top-level Xen makefile.
> 
> Would be helpful to know what problem this solves. Adding
> "random" exports _can_ have undesirable side effects, so we
> shouldn't make such a change without reason.

It solves having to pass ARCH and SRCARCH explicitly to the kconfig
calls. I can leave this out in the next round, since it's not related
to the toolchain mess that I'm trying to untangle.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH 2/5] kconfig: don't pass ARCH and SRCARCH on the sub-make call

[Jan Beulich]

 >>> On 26.06.19 at 15:55, <roger.pau@citrix.com> wrote:
 > and instead export them from the top-level Xen makefile.

Would be helpful to know what problem this solves. Adding
"random" exports _can_ have undesirable side effects, so we
shouldn't make such a change without reason.

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