x86: fix clang .macro retention check
diff mbox series

Message ID c46e975b-ef68-f09a-2790-3c4fb503cbf9@suse.com
State New
Headers show
Series
  • x86: fix clang .macro retention check
Related show

Commit Message

Jan Beulich Nov. 13, 2019, 5:01 p.m. UTC
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.

Comments

Roger Pau Monne Nov. 14, 2019, 9:38 a.m. UTC | #1
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.
Jan Beulich Nov. 14, 2019, 11:43 a.m. UTC | #2
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
Roger Pau Monne Nov. 14, 2019, 1:12 p.m. UTC | #3
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.
Jan Beulich Nov. 14, 2019, 3:56 p.m. UTC | #4
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)
Roger Pau Monne Nov. 14, 2019, 6:10 p.m. UTC | #5
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.
Jan Beulich Nov. 15, 2019, 9:44 a.m. UTC | #6
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
Roger Pau Monne Nov. 15, 2019, 9:58 a.m. UTC | #7
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.

Patch
diff mbox series

--- 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