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

[Jan Beulich <jbeulich@xxxxxxxx>]

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)


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