https://github.com/llvm/llvm-project/issues/60958

While trying to work around a different Clang-IAS bug, I stubled onto

  In file included from arch/x86/asm-macros.c:3:
  ./arch/x86/include/asm/spec_ctrl_asm.h:144:19: error: \u used with
  no following hex digits; treating as '\' followed by identifier [-Werror,-Wunicode]
  .L\@_fill_rsb_loop\uniq:
                    ^

It turns out that Clang -E is sensitive to the file extension of the source
file it is processing.

asm-macros should really have been .S from the outset, as it is ultimately
generating assembly, not C.  Rename it, which causes Clang not to complain.

We need to introduce rules for generating a .i file from .S, and substituting
c_flags for a_flags lets us drop the now-redundant -D__ASSEMBLY__.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Rules.mk                                | 6 ++++++
 xen/arch/x86/Makefile                       | 2 +-
 xen/arch/x86/{asm-macros.c => asm-macros.S} | 0
 3 files changed, 7 insertions(+), 1 deletion(-)
 rename xen/arch/x86/{asm-macros.c => asm-macros.S} (100%)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index d6b7cec0a882..59072ae8dff2 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -273,6 +273,9 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): $(obj)/%.init.o: $(obj)/%.o
 quiet_cmd_cpp_i_c = CPP     $@
 cmd_cpp_i_c = $(CPP) $(call cpp_flags,$(c_flags)) -MQ $@ -o $@ $<
 
+quiet_cmd_cpp_i_S = CPP     $@
+cmd_cpp_i_S = $(CPP) $(call cpp_flags,$(a_flags)) -MQ $@ -o $@ $<
+
 quiet_cmd_cc_s_c = CC      $@
 cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@
 
@@ -282,6 +285,9 @@ cmd_cpp_s_S = $(CPP) $(call cpp_flags,$(a_flags)) -MQ $@ -o $@ $<
 $(obj)/%.i: $(src)/%.c FORCE
 	$(call if_changed_dep,cpp_i_c)
 
+$(obj)/%.i: $(src)/%.S FORCE
+	$(call if_changed_dep,cpp_i_S)
+
 $(obj)/%.s: $(src)/%.c FORCE
 	$(call if_changed_dep,cc_s_c)
 
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 177a2ff74272..5accbe4c6746 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -240,7 +240,7 @@ $(obj)/efi/buildid.o $(obj)/efi/relocs-dummy.o: ;
 .PHONY: include
 include: $(objtree)/arch/x86/include/asm/asm-macros.h
 
-$(obj)/asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -P
+$(obj)/asm-macros.i: CFLAGS-y += -P
 
 $(objtree)/arch/x86/include/asm/asm-macros.h: $(obj)/asm-macros.i $(src)/Makefile
 	$(call filechk,asm-macros.h)
diff --git a/xen/arch/x86/asm-macros.c b/xen/arch/x86/asm-macros.S
similarity index 100%
rename from xen/arch/x86/asm-macros.c
rename to xen/arch/x86/asm-macros.S
-- 
2.30.2

On 23.02.2023 23:03, Andrew Cooper wrote:
> https://github.com/llvm/llvm-project/issues/60958
> 
> While trying to work around a different Clang-IAS bug, I stubled onto
> 
>   In file included from arch/x86/asm-macros.c:3:
>   ./arch/x86/include/asm/spec_ctrl_asm.h:144:19: error: \u used with
>   no following hex digits; treating as '\' followed by identifier [-Werror,-Wunicode]
>   .L\@_fill_rsb_loop\uniq:
>                     ^
> 
> It turns out that Clang -E is sensitive to the file extension of the source
> file it is processing.

I'm inclined to say there's no bug there in Clang. Gcc, while not affected
in this specific regard, also treats .c and .S differently in some perhaps
subtle ways.

> asm-macros should really have been .S from the outset, as it is ultimately
> generating assembly, not C.  Rename it, which causes Clang not to complain.
> 
> We need to introduce rules for generating a .i file from .S, and substituting
> c_flags for a_flags lets us drop the now-redundant -D__ASSEMBLY__.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

On 24/02/2023 7:27 am, Jan Beulich wrote:
> On 23.02.2023 23:03, Andrew Cooper wrote:
>> https://github.com/llvm/llvm-project/issues/60958
>>
>> While trying to work around a different Clang-IAS bug, I stubled onto
>>
>>   In file included from arch/x86/asm-macros.c:3:
>>   ./arch/x86/include/asm/spec_ctrl_asm.h:144:19: error: \u used with
>>   no following hex digits; treating as '\' followed by identifier [-Werror,-Wunicode]
>>   .L\@_fill_rsb_loop\uniq:
>>                     ^
>>
>> It turns out that Clang -E is sensitive to the file extension of the source
>> file it is processing.
> I'm inclined to say there's no bug there in Clang. Gcc, while not affected
> in this specific regard, also treats .c and .S differently in some perhaps
> subtle ways.

This part was just an observation.

Whether .c and .S should be treated the same or not, this -Wunicode
diagnostic is given against something which isn't in a char/string
literal, and that is a bug.

>
>> asm-macros should really have been .S from the outset, as it is ultimately
>> generating assembly, not C.  Rename it, which causes Clang not to complain.
>>
>> We need to introduce rules for generating a .i file from .S, and substituting
>> c_flags for a_flags lets us drop the now-redundant -D__ASSEMBLY__.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

~Andrew

On 24.02.2023 11:50, Andrew Cooper wrote:
> On 24/02/2023 7:27 am, Jan Beulich wrote:
>> On 23.02.2023 23:03, Andrew Cooper wrote:
>>> https://github.com/llvm/llvm-project/issues/60958
>>>
>>> While trying to work around a different Clang-IAS bug, I stubled onto
>>>
>>>   In file included from arch/x86/asm-macros.c:3:
>>>   ./arch/x86/include/asm/spec_ctrl_asm.h:144:19: error: \u used with
>>>   no following hex digits; treating as '\' followed by identifier [-Werror,-Wunicode]
>>>   .L\@_fill_rsb_loop\uniq:
>>>                     ^
>>>
>>> It turns out that Clang -E is sensitive to the file extension of the source
>>> file it is processing.
>> I'm inclined to say there's no bug there in Clang. Gcc, while not affected
>> in this specific regard, also treats .c and .S differently in some perhaps
>> subtle ways.
> 
> This part was just an observation.
> 
> Whether .c and .S should be treated the same or not, this -Wunicode
> diagnostic is given against something which isn't in a char/string
> literal, and that is a bug.

Why are you thinking of only string literals? Gcc is quite okay with

void test\u0024ä\u00F6ü(void) {}

yielding "test$äöü" in the object file (I haven't understood yet why it
dislikes \u0040, i.e. @.)

Jan

On 24/02/2023 11:17 am, Jan Beulich wrote:
> On 24.02.2023 11:50, Andrew Cooper wrote:
>> On 24/02/2023 7:27 am, Jan Beulich wrote:
>>> On 23.02.2023 23:03, Andrew Cooper wrote:
>>>> https://github.com/llvm/llvm-project/issues/60958
>>>>
>>>> While trying to work around a different Clang-IAS bug, I stubled onto
>>>>
>>>>   In file included from arch/x86/asm-macros.c:3:
>>>>   ./arch/x86/include/asm/spec_ctrl_asm.h:144:19: error: \u used with
>>>>   no following hex digits; treating as '\' followed by identifier [-Werror,-Wunicode]
>>>>   .L\@_fill_rsb_loop\uniq:
>>>>                     ^
>>>>
>>>> It turns out that Clang -E is sensitive to the file extension of the source
>>>> file it is processing.
>>> I'm inclined to say there's no bug there in Clang. Gcc, while not affected
>>> in this specific regard, also treats .c and .S differently in some perhaps
>>> subtle ways.
>> This part was just an observation.
>>
>> Whether .c and .S should be treated the same or not, this -Wunicode
>> diagnostic is given against something which isn't in a char/string
>> literal, and that is a bug.
> Why are you thinking of only string literals? Gcc is quite okay with
>
> void test\u0024ä\u00F6ü(void) {}
>
> yielding "test$äöü" in the object file (I haven't understood yet why it
> dislikes \u0040, i.e. @.)

Oh - it seems universal-character-name is an explicitly permitted part
of identifier-nondigit.

In which case, Clang is arguably correct given a semantic expectation of
being a C file.  I'll reword the commit message seen as I've not quite
pushed it yet.

~Andrew