| Message ID | 20230223220358.2611252-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Work around Clang -E vs -Wunicode bug | expand |
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
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
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%)