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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

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