Re: [Xen-devel] [PATCH] x86: fix clang .macro retention check

[Jan Beulich <jbeulich@xxxxxxxx>]

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

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