| Message ID | 20220125110103.3527686-4-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: Build system improvements, now with out-of-tree build! | expand |
On 25.01.2022 12:00, Anthony PERARD wrote: > Exporting a variable with a dash doesn't work reliably, they may be > striped from the environment when calling a sub-make or sub-shell. > > CFLAGS-stack-boundary start to be removed from env in patch "build: > set ALL_OBJS in main Makefile; move prelink.o to main Makefile" when > running `make "ALL_OBJS=.."` due to the addition of the quote. At > least in my empirical tests. > > Fixes: 2740d96efd ("xen/build: have the root Makefile generates the CFLAGS") > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
On 25.01.2022 12:00, Anthony PERARD wrote: > Exporting a variable with a dash doesn't work reliably, they may be > striped from the environment when calling a sub-make or sub-shell. > > CFLAGS-stack-boundary start to be removed from env in patch "build: > set ALL_OBJS in main Makefile; move prelink.o to main Makefile" when > running `make "ALL_OBJS=.."` due to the addition of the quote. At > least in my empirical tests. > > Fixes: 2740d96efd ("xen/build: have the root Makefile generates the CFLAGS") > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> While I did commit this, I'm still somewhat confused. How would quoting of elements on a make command line make a difference to which variables get exported? In any event I understand the description that prior to the subsequent change there's not actually any issue. Hence I'm not going to queue this for backporting despite the Fixes: tag. Unless of course I'm told otherwise (with justification). Jan
On Fri, Jan 28, 2022 at 12:14:58PM +0100, Jan Beulich wrote: > On 25.01.2022 12:00, Anthony PERARD wrote: > > Exporting a variable with a dash doesn't work reliably, they may be > > striped from the environment when calling a sub-make or sub-shell. > > > > CFLAGS-stack-boundary start to be removed from env in patch "build: > > set ALL_OBJS in main Makefile; move prelink.o to main Makefile" when > > running `make "ALL_OBJS=.."` due to the addition of the quote. At > > least in my empirical tests. > > > > Fixes: 2740d96efd ("xen/build: have the root Makefile generates the CFLAGS") > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > While I did commit this, I'm still somewhat confused. How would quoting > of elements on a make command line make a difference to which variables > get exported? I don't know, maybe without quote, make run sub-make directly, but with double-quote, a shell is used. But after reading the manual, I didn't expect the variable to be passed down sub-make at all: 5.7.2 Communicating Variables to a Sub-make Except by explicit request, make exports a variable only if it is either defined in the environment initially or set on the command line, and if its name consists only of letters, numbers, and underscores. But somehow, sub-makes also export the non-shell-exportable variables, unless the command line in a recipe invoking make has double-quotes. I've tried again, and checked the process list: - when running `$(MAKE) -C foo`; make just execute make - when running `$(MAKE) -C 'foo'`; make just execute make - when running `$(MAKE) -C "foo"`; make execute /bin/sh -c ... - when running `$(MAKE) -C foo | tee`; make execute /bin/sh -c ... So, CFLAGS-stack-boundary disappear when /bin/sh is involved. > In any event I understand the description that prior to the subsequent > change there's not actually any issue. Hence I'm not going to queue > this for backporting despite the Fixes: tag. Unless of course I'm told > otherwise (with justification). Justification would be that it's not supposed to work, based on information from make's manual. Thanks,
On 28.01.2022 16:04, Anthony PERARD wrote: > On Fri, Jan 28, 2022 at 12:14:58PM +0100, Jan Beulich wrote: >> On 25.01.2022 12:00, Anthony PERARD wrote: >>> Exporting a variable with a dash doesn't work reliably, they may be >>> striped from the environment when calling a sub-make or sub-shell. >>> >>> CFLAGS-stack-boundary start to be removed from env in patch "build: >>> set ALL_OBJS in main Makefile; move prelink.o to main Makefile" when >>> running `make "ALL_OBJS=.."` due to the addition of the quote. At >>> least in my empirical tests. >>> >>> Fixes: 2740d96efd ("xen/build: have the root Makefile generates the CFLAGS") >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >> >> While I did commit this, I'm still somewhat confused. How would quoting >> of elements on a make command line make a difference to which variables >> get exported? > > I don't know, maybe without quote, make run sub-make directly, but with > double-quote, a shell is used. > > But after reading the manual, I didn't expect the variable to be passed > down sub-make at all: > > 5.7.2 Communicating Variables to a Sub-make > > Except by explicit request, make exports a variable only if it is > either defined in the environment initially or set on the command > line, and if its name consists only of letters, numbers, and > underscores. > > But somehow, sub-makes also export the non-shell-exportable variables, > unless the command line in a recipe invoking make has double-quotes. > > > I've tried again, and checked the process list: > - when running `$(MAKE) -C foo`; make just execute make > - when running `$(MAKE) -C 'foo'`; make just execute make > - when running `$(MAKE) -C "foo"`; make execute /bin/sh -c ... > - when running `$(MAKE) -C foo | tee`; make execute /bin/sh -c ... > > So, CFLAGS-stack-boundary disappear when /bin/sh is involved. Very "interesting" behavior. >> In any event I understand the description that prior to the subsequent >> change there's not actually any issue. Hence I'm not going to queue >> this for backporting despite the Fixes: tag. Unless of course I'm told >> otherwise (with justification). > > Justification would be that it's not supposed to work, based on > information from make's manual. Okay, I'll queue it then, but in case it doesn't backport straightforwardly I'll still consider leaving it out. Jan
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk index 56fe22c979ea..7aef93f5f3a0 100644 --- a/xen/arch/x86/Rules.mk +++ b/xen/arch/x86/Rules.mk @@ -6,5 +6,5 @@ object_label_flags = '-D__OBJECT_LABEL__=$(subst $(BASEDIR)/,,$(CURDIR))/$@' else object_label_flags = '-D__OBJECT_LABEL__=$(subst /,$$,$(subst -,_,$(subst $(BASEDIR)/,,$(CURDIR))/$@))' endif -c_flags += $(object_label_flags) $(CFLAGS-stack-boundary) -a_flags += $(object_label_flags) $(CFLAGS-stack-boundary) +c_flags += $(object_label_flags) $(CFLAGS_stack_boundary) +a_flags += $(object_label_flags) $(CFLAGS_stack_boundary) diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk index a93fa6d2e4c9..fa7cf3844362 100644 --- a/xen/arch/x86/arch.mk +++ b/xen/arch/x86/arch.mk @@ -49,8 +49,8 @@ CFLAGS-$(CONFIG_INDIRECT_THUNK) += -fno-jump-tables # If supported by the compiler, reduce stack alignment to 8 bytes. But allow # this to be overridden elsewhere. -$(call cc-option-add,CFLAGS-stack-boundary,CC,-mpreferred-stack-boundary=3) -export CFLAGS-stack-boundary +$(call cc-option-add,CFLAGS_stack_boundary,CC,-mpreferred-stack-boundary=3) +export CFLAGS_stack_boundary ifeq ($(CONFIG_UBSAN),y) # Don't enable alignment sanitisation. x86 has efficient unaligned accesses, diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile index 87b927ed865b..abae493bf344 100644 --- a/xen/arch/x86/efi/Makefile +++ b/xen/arch/x86/efi/Makefile @@ -12,7 +12,7 @@ EFIOBJ-y := boot.init.o pe.init.o ebmalloc.o runtime.o EFIOBJ-$(CONFIG_COMPAT) += compat.o $(call cc-option-add,cflags-stack-boundary,CC,-mpreferred-stack-boundary=4) -$(EFIOBJ-y): CFLAGS-stack-boundary := $(cflags-stack-boundary) +$(EFIOBJ-y): CFLAGS_stack_boundary := $(cflags-stack-boundary) obj-y := stub.o obj-$(XEN_BUILD_EFI) := $(filter-out %.init.o,$(EFIOBJ-y))
Exporting a variable with a dash doesn't work reliably, they may be striped from the environment when calling a sub-make or sub-shell. CFLAGS-stack-boundary start to be removed from env in patch "build: set ALL_OBJS in main Makefile; move prelink.o to main Makefile" when running `make "ALL_OBJS=.."` due to the addition of the quote. At least in my empirical tests. Fixes: 2740d96efd ("xen/build: have the root Makefile generates the CFLAGS") Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v9: - new patch xen/arch/x86/Rules.mk | 4 ++-- xen/arch/x86/arch.mk | 4 ++-- xen/arch/x86/efi/Makefile | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)