| Message ID | c46e975b-ef68-f09a-2790-3c4fb503cbf9@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: fix clang .macro retention check | expand |
On Wed, Nov 13, 2019 at 06:01:40PM +0100, Jan Beulich wrote: > There were two problems here: The first closing parentheses got parsed > by make to end the $(call invocation, and the escaping of the quotes > wasn't right either, as there's nowhere they would get un-escaped. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > This needs to be tested in an environment where this was actually found > to matter; I can't see how it can have worked in its former shape. I Well, I could swear this was working, but obviously I did something wrong. > also don't understand why the same commit introducing the check that > gets fixed here put the .skip check in xen/Rules.mk - the only use of > .skip that I can spot is in x86 code. I recall removing some usages of .skip, but TBH I'm not sure whether ARM was also involved. I guess it's fine to move it to x86/Rules.mk now, since I also have to fix an issue with it. > > --- a/Config.mk > +++ b/Config.mk > @@ -6,6 +6,8 @@ endif > > # Convenient variables > comma := , > +open := ( > +close := ) > squote := ' > #' Balancing squote, to help syntax highlighting > empty := > --- a/xen/arch/x86/Rules.mk > +++ b/xen/arch/x86/Rules.mk > @@ -82,6 +64,6 @@ $(call as-option-add,CFLAGS,CC,".include > # Check whether clang keeps .macro-s between asm()-s: > # https://bugs.llvm.org/show_bug.cgi?id=36110 > $(call as-option-add,CFLAGS,CC,\ > - ".macro FOO\n.endm\"); asm volatile (\".macro FOO\n.endm",\ > + ".macro FOO\n.endm"$$(close); asm volatile $$(open)".macro FOO\n.endm",\ Thanks, while here could you also replace the '\n' with a ';'? '\n' doesn't work properly and gives me the following error: <stdin>:1:32: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] void _(void) { asm volatile ( ".macro FOO ^ <stdin>:1:32: error: expected string literal in 'asm' <stdin>:3:6: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] .endm" ); } ^ <stdin>:3:12: error: expected ')' .endm" ); } ^ <stdin>:1:29: note: to match this '(' void _(void) { asm volatile ( ".macro FOO ^ <stdin>:3:12: error: expected '}' .endm" ); } ^ <stdin>:1:14: note: to match this '{' void _(void) { asm volatile ( ".macro FOO ^ Roger.
On 14.11.2019 10:38, Roger Pau Monné wrote: > On Wed, Nov 13, 2019 at 06:01:40PM +0100, Jan Beulich wrote: >> --- a/xen/arch/x86/Rules.mk >> +++ b/xen/arch/x86/Rules.mk >> @@ -82,6 +64,6 @@ $(call as-option-add,CFLAGS,CC,".include >> # Check whether clang keeps .macro-s between asm()-s: >> # https://bugs.llvm.org/show_bug.cgi?id=36110 >> $(call as-option-add,CFLAGS,CC,\ >> - ".macro FOO\n.endm\"); asm volatile (\".macro FOO\n.endm",\ >> + ".macro FOO\n.endm"$$(close); asm volatile $$(open)".macro FOO\n.endm",\ > > Thanks, while here could you also replace the '\n' with a ';'? '\n' > doesn't work properly and gives me the following error: > > <stdin>:1:32: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] > void _(void) { asm volatile ( ".macro FOO > ^ > <stdin>:1:32: error: expected string literal in 'asm' > <stdin>:3:6: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] > .endm" ); } > ^ > <stdin>:3:12: error: expected ')' > .endm" ); } > ^ > <stdin>:1:29: note: to match this '(' > void _(void) { asm volatile ( ".macro FOO > ^ > <stdin>:3:12: error: expected '}' > .endm" ); } > ^ > <stdin>:1:14: note: to match this '{' > void _(void) { asm volatile ( ".macro FOO > ^ So this must be yet another issue - I did specifically look at the what gets handed to the compiler, and I did not see the above. I wonder whether that's also related to the \" that I found necessary to drop - with what you say I'd expect the un-escaped double quotes won't work for you. I suppose though this un-escaping (or not) happens at a level other than the compiler, i.e. either a difference in shell or in make behavior. IOW I don't think just replacing \n by ; will do. Jan
On Thu, Nov 14, 2019 at 12:43:33PM +0100, Jan Beulich wrote: > On 14.11.2019 10:38, Roger Pau Monné wrote: > > On Wed, Nov 13, 2019 at 06:01:40PM +0100, Jan Beulich wrote: > >> --- a/xen/arch/x86/Rules.mk > >> +++ b/xen/arch/x86/Rules.mk > >> @@ -82,6 +64,6 @@ $(call as-option-add,CFLAGS,CC,".include > >> # Check whether clang keeps .macro-s between asm()-s: > >> # https://bugs.llvm.org/show_bug.cgi?id=36110 > >> $(call as-option-add,CFLAGS,CC,\ > >> - ".macro FOO\n.endm\"); asm volatile (\".macro FOO\n.endm",\ > >> + ".macro FOO\n.endm"$$(close); asm volatile $$(open)".macro FOO\n.endm",\ > > > > Thanks, while here could you also replace the '\n' with a ';'? '\n' > > doesn't work properly and gives me the following error: > > > > <stdin>:1:32: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] > > void _(void) { asm volatile ( ".macro FOO > > ^ > > <stdin>:1:32: error: expected string literal in 'asm' > > <stdin>:3:6: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] > > .endm" ); } > > ^ > > <stdin>:3:12: error: expected ')' > > .endm" ); } > > ^ > > <stdin>:1:29: note: to match this '(' > > void _(void) { asm volatile ( ".macro FOO > > ^ > > <stdin>:3:12: error: expected '}' > > .endm" ); } > > ^ > > <stdin>:1:14: note: to match this '{' > > void _(void) { asm volatile ( ".macro FOO > > ^ > > So this must be yet another issue - I did specifically look at the what > gets handed to the compiler, and I did not see the above. I wonder > whether that's also related to the \" that I found necessary to drop - > with what you say I'd expect the un-escaped double quotes won't work > for you. AFAIK those work fine. > I suppose though this un-escaping (or not) happens at a level > other than the compiler, i.e. either a difference in shell or in make > behavior. Maybe, I'm not an expert on shells or makefiles. This time I've tested with Debian 9.5 instead of FreeBSD, so it's likely that what was there worked fine on FreeBSD which I'm quite sure was what I testing against back when I added this check. This is what I used to test: GNU Make 4.1 GNU bash, version 4.4.12(1)-release (x86_64-pc-linux-gnu) Not sure whether there are other utilities involved in this behavior. > IOW I don't think just replacing \n by ; will do. I can give your patch a try with FreeBSD, but that's not going to explain the different behavior I'm afraid. Thanks, Roger.
On 14.11.2019 14:12, Roger Pau Monné wrote: > On Thu, Nov 14, 2019 at 12:43:33PM +0100, Jan Beulich wrote: >> On 14.11.2019 10:38, Roger Pau Monné wrote: >>> On Wed, Nov 13, 2019 at 06:01:40PM +0100, Jan Beulich wrote: >>>> --- a/xen/arch/x86/Rules.mk >>>> +++ b/xen/arch/x86/Rules.mk >>>> @@ -82,6 +64,6 @@ $(call as-option-add,CFLAGS,CC,".include >>>> # Check whether clang keeps .macro-s between asm()-s: >>>> # https://bugs.llvm.org/show_bug.cgi?id=36110 >>>> $(call as-option-add,CFLAGS,CC,\ >>>> - ".macro FOO\n.endm\"); asm volatile (\".macro FOO\n.endm",\ >>>> + ".macro FOO\n.endm"$$(close); asm volatile $$(open)".macro FOO\n.endm",\ >>> >>> Thanks, while here could you also replace the '\n' with a ';'? '\n' >>> doesn't work properly and gives me the following error: >>> >>> <stdin>:1:32: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] >>> void _(void) { asm volatile ( ".macro FOO >>> ^ >>> <stdin>:1:32: error: expected string literal in 'asm' >>> <stdin>:3:6: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] >>> .endm" ); } >>> ^ >>> <stdin>:3:12: error: expected ')' >>> .endm" ); } >>> ^ >>> <stdin>:1:29: note: to match this '(' >>> void _(void) { asm volatile ( ".macro FOO >>> ^ >>> <stdin>:3:12: error: expected '}' >>> .endm" ); } >>> ^ >>> <stdin>:1:14: note: to match this '{' >>> void _(void) { asm volatile ( ".macro FOO >>> ^ >> >> So this must be yet another issue - I did specifically look at the what >> gets handed to the compiler, and I did not see the above. I wonder >> whether that's also related to the \" that I found necessary to drop - >> with what you say I'd expect the un-escaped double quotes won't work >> for you. > > AFAIK those work fine. > >> I suppose though this un-escaping (or not) happens at a level >> other than the compiler, i.e. either a difference in shell or in make >> behavior. > > Maybe, I'm not an expert on shells or makefiles. This time I've tested > with Debian 9.5 instead of FreeBSD, so it's likely that what was there > worked fine on FreeBSD which I'm quite sure was what I testing against > back when I added this check. > > This is what I used to test: > > GNU Make 4.1 > GNU bash, version 4.4.12(1)-release (x86_64-pc-linux-gnu) > > Not sure whether there are other utilities involved in this behavior. > >> IOW I don't think just replacing \n by ; will do. > > I can give your patch a try with FreeBSD, but that's not going to > explain the different behavior I'm afraid. Let's approach this a different way. Below is a debugging patch (similar to something I did use yesterday). With my patch and yours on top (but with the \n restored for the purposes here, and with the block inserted first in the ifeq(,) it gets moved to) I get in .as-insn.1 void _(void) { asm volatile ( ".equ \"x\",1" ); } void _(void) { asm volatile ( ".L0:\n.L1:\n.skip (.L1 - .L0)" ); } void _(void) { asm volatile ( ".include \"asm/indirect_thunk_asm.h\"" ); } void _(void) { asm volatile ( ".macro FOO\n.endm"); asm volatile (".macro FOO\n.endm" ); } (you may see further lines here, as the context in which I ran into this was a patch to move out the determination of -DHAVE_AS_*, which also is in effect here) and in .as-insn.2 <stdin>:1:44: error: invalid number of bytes void _(void) { asm volatile ( ".L0:\n.L1:\n.skip (.L1 - .L0)" ); } ^ <inline asm>:3:7: note: instantiated into assembly here .skip (.L1 - .L0) ^ 1 error generated. /tmp/--8f887d.s: Assembler messages: /tmp/--8f887d.s:20: Error: Macro `foo' was already defined clang-5.0.1: error: assembler command failed with exit code 1 (use -v to see invocation) I'm curious in which aspects output on your system will differ. Jan --- unstable.orig/Config.mk +++ unstable/Config.mk @@ -162,8 +162,8 @@ endif # as-insn: Check whether assembler supports an instruction. # Usage: cflags-y += $(call as-insn,CC FLAGS,"insn",option-yes,option-no) as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \ - | $(filter-out -M% %.d -include %/config.h,$(1)) \ - -c -x c -o /dev/null - 2>&1),$(4),$(3)) + | tee -a .as-insn.1 | $(filter-out -M% %.d -include %/config.h,$(1)) \ + -c -x c -o /dev/null - 2>&1 | tee -a .as-insn.2),$(4),$(3)) # as-option-add: Conditionally add options to flags # Usage: $(call as-option-add,CFLAGS,CC,"insn",option-yes,option-no)
On Thu, Nov 14, 2019 at 04:56:35PM +0100, Jan Beulich wrote: > On 14.11.2019 14:12, Roger Pau Monné wrote: > > On Thu, Nov 14, 2019 at 12:43:33PM +0100, Jan Beulich wrote: > >> On 14.11.2019 10:38, Roger Pau Monné wrote: > >>> On Wed, Nov 13, 2019 at 06:01:40PM +0100, Jan Beulich wrote: > >>>> --- a/xen/arch/x86/Rules.mk > >>>> +++ b/xen/arch/x86/Rules.mk > >>>> @@ -82,6 +64,6 @@ $(call as-option-add,CFLAGS,CC,".include > >>>> # Check whether clang keeps .macro-s between asm()-s: > >>>> # https://bugs.llvm.org/show_bug.cgi?id=36110 > >>>> $(call as-option-add,CFLAGS,CC,\ > >>>> - ".macro FOO\n.endm\"); asm volatile (\".macro FOO\n.endm",\ > >>>> + ".macro FOO\n.endm"$$(close); asm volatile $$(open)".macro FOO\n.endm",\ > >>> > >>> Thanks, while here could you also replace the '\n' with a ';'? '\n' > >>> doesn't work properly and gives me the following error: > >>> > >>> <stdin>:1:32: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] > >>> void _(void) { asm volatile ( ".macro FOO > >>> ^ > >>> <stdin>:1:32: error: expected string literal in 'asm' > >>> <stdin>:3:6: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] > >>> .endm" ); } > >>> ^ > >>> <stdin>:3:12: error: expected ')' > >>> .endm" ); } > >>> ^ > >>> <stdin>:1:29: note: to match this '(' > >>> void _(void) { asm volatile ( ".macro FOO > >>> ^ > >>> <stdin>:3:12: error: expected '}' > >>> .endm" ); } > >>> ^ > >>> <stdin>:1:14: note: to match this '{' > >>> void _(void) { asm volatile ( ".macro FOO > >>> ^ > >> > >> So this must be yet another issue - I did specifically look at the what > >> gets handed to the compiler, and I did not see the above. I wonder > >> whether that's also related to the \" that I found necessary to drop - > >> with what you say I'd expect the un-escaped double quotes won't work > >> for you. > > > > AFAIK those work fine. > > > >> I suppose though this un-escaping (or not) happens at a level > >> other than the compiler, i.e. either a difference in shell or in make > >> behavior. > > > > Maybe, I'm not an expert on shells or makefiles. This time I've tested > > with Debian 9.5 instead of FreeBSD, so it's likely that what was there > > worked fine on FreeBSD which I'm quite sure was what I testing against > > back when I added this check. > > > > This is what I used to test: > > > > GNU Make 4.1 > > GNU bash, version 4.4.12(1)-release (x86_64-pc-linux-gnu) > > > > Not sure whether there are other utilities involved in this behavior. > > > >> IOW I don't think just replacing \n by ; will do. > > > > I can give your patch a try with FreeBSD, but that's not going to > > explain the different behavior I'm afraid. > > Let's approach this a different way. Below is a debugging patch > (similar to something I did use yesterday). With my patch and yours > on top (but with the \n restored for the purposes here, and with > the block inserted first in the ifeq(,) it gets moved to) I get in > .as-insn.1 > > void _(void) { asm volatile ( ".equ \"x\",1" ); } > void _(void) { asm volatile ( ".L0:\n.L1:\n.skip (.L1 - .L0)" ); } > void _(void) { asm volatile ( ".include \"asm/indirect_thunk_asm.h\"" ); } > void _(void) { asm volatile ( ".macro FOO\n.endm"); asm volatile (".macro FOO\n.endm" ); } The following data is from Debian GNU/Linux 9.5 (stretch), I haven't tried on FreeBSD but this output is already different from what you get. So this is what I have in .as-insn.1 (the relevant part): void _(void) { asm volatile ( ".equ \"x\",1" ); } void _(void) { asm volatile ( "invpcid (%rax),%rax" ); } void _(void) { asm volatile ( ".if ((1 > 0) < 0); .error \"\";.endif" ); } void _(void) { asm volatile ( ".L1: .L2: .nops (.L2 - .L1),9" ); } void _(void) { asm volatile ( ".L0: .L1: .skip (.L1 - .L0)" ); } void _(void) { asm volatile ( ".include \"asm/indirect_thunk_asm.h\"" ); } void _(void) { asm volatile ( ".macro FOO .endm"); asm volatile (".macro FOO .endm" ); } So my make/shell/whatever is expanding the \n. FTR the command I've used is `make -j8 xen clang=y CC=clang-8`. > > (you may see further lines here, as the context in which I ran into > this was a patch to move out the determination of -DHAVE_AS_*, which > also is in effect here) and in .as-insn.2 > > <stdin>:1:44: error: invalid number of bytes > void _(void) { asm volatile ( ".L0:\n.L1:\n.skip (.L1 - .L0)" ); } > ^ > <inline asm>:3:7: note: instantiated into assembly here > .skip (.L1 - .L0) > ^ > 1 error generated. > /tmp/--8f887d.s: Assembler messages: > /tmp/--8f887d.s:20: Error: Macro `foo' was already defined > clang-5.0.1: error: assembler command failed with exit code 1 (use -v to see invocation) And in .as-insn.2: <stdin>:1:32: error: void _(void) { asm volatile ( ".if ((1 > 0) < 0); .error \"\";.endif" ); } ^ <inline asm>:1:21: note: instantiated into assembly here .if ((1 > 0) < 0); .error "";.endif ^ 1 error generated. <stdin>:1:32: error: unknown directive void _(void) { asm volatile ( ".L1: .L2: .nops (.L2 - .L1),9" ); } ^ <inline asm>:1:12: note: instantiated into assembly here .L1: .L2: .nops (.L2 - .L1),9 ^ 1 error generated. <stdin>:1:31: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] void _(void) { asm volatile ( ".L0: ^ <stdin>:1:31: error: expected string literal in 'asm' <stdin>:3:18: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] .skip (.L1 - .L0)" ); } ^ <stdin>:3:24: error: expected ')' .skip (.L1 - .L0)" ); } ^ <stdin>:1:29: note: to match this '(' void _(void) { asm volatile ( ".L0: ^ <stdin>:3:24: error: expected '}' .skip (.L1 - .L0)" ); } ^ <stdin>:1:14: note: to match this '{' void _(void) { asm volatile ( ".L0: ^ 5 errors generated. <stdin>:1:32: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] void _(void) { asm volatile ( ".macro FOO ^ <stdin>:1:32: error: expected string literal in 'asm' <stdin>:3:6: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] .endm" ); } ^ <stdin>:3:12: error: expected ')' .endm" ); } ^ <stdin>:1:29: note: to match this '(' void _(void) { asm volatile ( ".macro FOO ^ <stdin>:3:12: error: expected '}' .endm" ); } ^ <stdin>:1:14: note: to match this '{' void _(void) { asm volatile ( ".macro FOO ^ 5 errors generated. Roger.
On 14.11.2019 19:10, Roger Pau Monné wrote: > On Thu, Nov 14, 2019 at 04:56:35PM +0100, Jan Beulich wrote: >> On 14.11.2019 14:12, Roger Pau Monné wrote: >>> On Thu, Nov 14, 2019 at 12:43:33PM +0100, Jan Beulich wrote: >>>> On 14.11.2019 10:38, Roger Pau Monné wrote: >>>>> On Wed, Nov 13, 2019 at 06:01:40PM +0100, Jan Beulich wrote: >>>>>> --- a/xen/arch/x86/Rules.mk >>>>>> +++ b/xen/arch/x86/Rules.mk >>>>>> @@ -82,6 +64,6 @@ $(call as-option-add,CFLAGS,CC,".include >>>>>> # Check whether clang keeps .macro-s between asm()-s: >>>>>> # https://bugs.llvm.org/show_bug.cgi?id=36110 >>>>>> $(call as-option-add,CFLAGS,CC,\ >>>>>> - ".macro FOO\n.endm\"); asm volatile (\".macro FOO\n.endm",\ >>>>>> + ".macro FOO\n.endm"$$(close); asm volatile $$(open)".macro FOO\n.endm",\ >>>>> >>>>> Thanks, while here could you also replace the '\n' with a ';'? '\n' >>>>> doesn't work properly and gives me the following error: >>>>> >>>>> <stdin>:1:32: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] >>>>> void _(void) { asm volatile ( ".macro FOO >>>>> ^ >>>>> <stdin>:1:32: error: expected string literal in 'asm' >>>>> <stdin>:3:6: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] >>>>> .endm" ); } >>>>> ^ >>>>> <stdin>:3:12: error: expected ')' >>>>> .endm" ); } >>>>> ^ >>>>> <stdin>:1:29: note: to match this '(' >>>>> void _(void) { asm volatile ( ".macro FOO >>>>> ^ >>>>> <stdin>:3:12: error: expected '}' >>>>> .endm" ); } >>>>> ^ >>>>> <stdin>:1:14: note: to match this '{' >>>>> void _(void) { asm volatile ( ".macro FOO >>>>> ^ >>>> >>>> So this must be yet another issue - I did specifically look at the what >>>> gets handed to the compiler, and I did not see the above. I wonder >>>> whether that's also related to the \" that I found necessary to drop - >>>> with what you say I'd expect the un-escaped double quotes won't work >>>> for you. >>> >>> AFAIK those work fine. >>> >>>> I suppose though this un-escaping (or not) happens at a level >>>> other than the compiler, i.e. either a difference in shell or in make >>>> behavior. >>> >>> Maybe, I'm not an expert on shells or makefiles. This time I've tested >>> with Debian 9.5 instead of FreeBSD, so it's likely that what was there >>> worked fine on FreeBSD which I'm quite sure was what I testing against >>> back when I added this check. >>> >>> This is what I used to test: >>> >>> GNU Make 4.1 >>> GNU bash, version 4.4.12(1)-release (x86_64-pc-linux-gnu) >>> >>> Not sure whether there are other utilities involved in this behavior. >>> >>>> IOW I don't think just replacing \n by ; will do. >>> >>> I can give your patch a try with FreeBSD, but that's not going to >>> explain the different behavior I'm afraid. >> >> Let's approach this a different way. Below is a debugging patch >> (similar to something I did use yesterday). With my patch and yours >> on top (but with the \n restored for the purposes here, and with >> the block inserted first in the ifeq(,) it gets moved to) I get in >> .as-insn.1 >> >> void _(void) { asm volatile ( ".equ \"x\",1" ); } >> void _(void) { asm volatile ( ".L0:\n.L1:\n.skip (.L1 - .L0)" ); } >> void _(void) { asm volatile ( ".include \"asm/indirect_thunk_asm.h\"" ); } >> void _(void) { asm volatile ( ".macro FOO\n.endm"); asm volatile (".macro FOO\n.endm" ); } > > The following data is from Debian GNU/Linux 9.5 (stretch), I haven't > tried on FreeBSD but this output is already different from what you > get. > > So this is what I have in .as-insn.1 (the relevant part): > > void _(void) { asm volatile ( ".equ \"x\",1" ); } > void _(void) { asm volatile ( "invpcid (%rax),%rax" ); } > void _(void) { asm volatile ( ".if ((1 > 0) < 0); .error \"\";.endif" ); } > void _(void) { asm volatile ( ".L1: .L2: .nops (.L2 - .L1),9" ); } > void _(void) { asm volatile ( ".L0: > .L1: > .skip (.L1 - .L0)" ); } > void _(void) { asm volatile ( ".include \"asm/indirect_thunk_asm.h\"" ); } > void _(void) { asm volatile ( ".macro FOO > .endm"); asm volatile (".macro FOO > .endm" ); } > > So my make/shell/whatever is expanding the \n. So to tell apart make and shell - what does a plain echo '".L0:\n.L1:\n.skip (.L1 - .L0)"' produce for you? Also, unless you pass -s to make, you ought to be able to see what make actually passes to echo. On the positive side the \" instances don't get changed, which makes it even more of a mystery how the .macro retention check would have worked originally. This would then suggest that we could get away with simply replacing the \n with ; as you did in your patch (and I should then follow suit in mine). Jan
On Fri, Nov 15, 2019 at 10:44:22AM +0100, Jan Beulich wrote: > On 14.11.2019 19:10, Roger Pau Monné wrote: > > On Thu, Nov 14, 2019 at 04:56:35PM +0100, Jan Beulich wrote: > >> On 14.11.2019 14:12, Roger Pau Monné wrote: > >>> On Thu, Nov 14, 2019 at 12:43:33PM +0100, Jan Beulich wrote: > >>>> On 14.11.2019 10:38, Roger Pau Monné wrote: > >>>>> On Wed, Nov 13, 2019 at 06:01:40PM +0100, Jan Beulich wrote: > >>>>>> --- a/xen/arch/x86/Rules.mk > >>>>>> +++ b/xen/arch/x86/Rules.mk > >>>>>> @@ -82,6 +64,6 @@ $(call as-option-add,CFLAGS,CC,".include > >>>>>> # Check whether clang keeps .macro-s between asm()-s: > >>>>>> # https://bugs.llvm.org/show_bug.cgi?id=36110 > >>>>>> $(call as-option-add,CFLAGS,CC,\ > >>>>>> - ".macro FOO\n.endm\"); asm volatile (\".macro FOO\n.endm",\ > >>>>>> + ".macro FOO\n.endm"$$(close); asm volatile $$(open)".macro FOO\n.endm",\ > >>>>> > >>>>> Thanks, while here could you also replace the '\n' with a ';'? '\n' > >>>>> doesn't work properly and gives me the following error: > >>>>> > >>>>> <stdin>:1:32: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] > >>>>> void _(void) { asm volatile ( ".macro FOO > >>>>> ^ > >>>>> <stdin>:1:32: error: expected string literal in 'asm' > >>>>> <stdin>:3:6: error: missing terminating '"' character [-Werror,-Winvalid-pp-token] > >>>>> .endm" ); } > >>>>> ^ > >>>>> <stdin>:3:12: error: expected ')' > >>>>> .endm" ); } > >>>>> ^ > >>>>> <stdin>:1:29: note: to match this '(' > >>>>> void _(void) { asm volatile ( ".macro FOO > >>>>> ^ > >>>>> <stdin>:3:12: error: expected '}' > >>>>> .endm" ); } > >>>>> ^ > >>>>> <stdin>:1:14: note: to match this '{' > >>>>> void _(void) { asm volatile ( ".macro FOO > >>>>> ^ > >>>> > >>>> So this must be yet another issue - I did specifically look at the what > >>>> gets handed to the compiler, and I did not see the above. I wonder > >>>> whether that's also related to the \" that I found necessary to drop - > >>>> with what you say I'd expect the un-escaped double quotes won't work > >>>> for you. > >>> > >>> AFAIK those work fine. > >>> > >>>> I suppose though this un-escaping (or not) happens at a level > >>>> other than the compiler, i.e. either a difference in shell or in make > >>>> behavior. > >>> > >>> Maybe, I'm not an expert on shells or makefiles. This time I've tested > >>> with Debian 9.5 instead of FreeBSD, so it's likely that what was there > >>> worked fine on FreeBSD which I'm quite sure was what I testing against > >>> back when I added this check. > >>> > >>> This is what I used to test: > >>> > >>> GNU Make 4.1 > >>> GNU bash, version 4.4.12(1)-release (x86_64-pc-linux-gnu) > >>> > >>> Not sure whether there are other utilities involved in this behavior. > >>> > >>>> IOW I don't think just replacing \n by ; will do. > >>> > >>> I can give your patch a try with FreeBSD, but that's not going to > >>> explain the different behavior I'm afraid. > >> > >> Let's approach this a different way. Below is a debugging patch > >> (similar to something I did use yesterday). With my patch and yours > >> on top (but with the \n restored for the purposes here, and with > >> the block inserted first in the ifeq(,) it gets moved to) I get in > >> .as-insn.1 > >> > >> void _(void) { asm volatile ( ".equ \"x\",1" ); } > >> void _(void) { asm volatile ( ".L0:\n.L1:\n.skip (.L1 - .L0)" ); } > >> void _(void) { asm volatile ( ".include \"asm/indirect_thunk_asm.h\"" ); } > >> void _(void) { asm volatile ( ".macro FOO\n.endm"); asm volatile (".macro FOO\n.endm" ); } > > > > The following data is from Debian GNU/Linux 9.5 (stretch), I haven't > > tried on FreeBSD but this output is already different from what you > > get. > > > > So this is what I have in .as-insn.1 (the relevant part): > > > > void _(void) { asm volatile ( ".equ \"x\",1" ); } > > void _(void) { asm volatile ( "invpcid (%rax),%rax" ); } > > void _(void) { asm volatile ( ".if ((1 > 0) < 0); .error \"\";.endif" ); } > > void _(void) { asm volatile ( ".L1: .L2: .nops (.L2 - .L1),9" ); } > > void _(void) { asm volatile ( ".L0: > > .L1: > > .skip (.L1 - .L0)" ); } > > void _(void) { asm volatile ( ".include \"asm/indirect_thunk_asm.h\"" ); } > > void _(void) { asm volatile ( ".macro FOO > > .endm"); asm volatile (".macro FOO > > .endm" ); } > > > > So my make/shell/whatever is expanding the \n. > > So to tell apart make and shell - what does a plain > > echo '".L0:\n.L1:\n.skip (.L1 - .L0)"' > > produce for you? # echo '".L0:\n.L1:\n.skip (.L1 - .L0)"' ".L0:\n.L1:\n.skip (.L1 - .L0)" So it seems \n is not expanded. > Also, unless you pass -s to make, you ought to be > able to see what make actually passes to echo. > > On the positive side the \" instances don't get changed, which makes > it even more of a mystery how the .macro retention check would have > worked originally. IIRC I tested that on FreeBSD instead of Debian, which might have a different behavior. I will test the resulting patches on FreeBSD also to ensure they work correctly. > This would then suggest that we could get away > with simply replacing the \n with ; as you did in your patch (and I > should then follow suit in mine). I can test that on Debian and FreeBSD, but I'm still quite puzzled by this different behavior. Thanks, Roger.
--- a/Config.mk +++ b/Config.mk @@ -6,6 +6,8 @@ endif # Convenient variables comma := , +open := ( +close := ) squote := ' #' Balancing squote, to help syntax highlighting empty := --- a/xen/arch/x86/Rules.mk +++ b/xen/arch/x86/Rules.mk @@ -82,6 +64,6 @@ $(call as-option-add,CFLAGS,CC,".include # Check whether clang keeps .macro-s between asm()-s: # https://bugs.llvm.org/show_bug.cgi?id=36110 $(call as-option-add,CFLAGS,CC,\ - ".macro FOO\n.endm\"); asm volatile (\".macro FOO\n.endm",\ + ".macro FOO\n.endm"$$(close); asm volatile $$(open)".macro FOO\n.endm",\ -no-integrated-as) endif
There were two problems here: The first closing parentheses got parsed by make to end the $(call invocation, and the escaping of the quotes wasn't right either, as there's nowhere they would get un-escaped. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- This needs to be tested in an environment where this was actually found to matter; I can't see how it can have worked in its former shape. I also don't understand why the same commit introducing the check that gets fixed here put the .skip check in xen/Rules.mk - the only use of .skip that I can spot is in x86 code.