Re: [Xen-devel] [PATCH 02/34] xen: clang: Disable built-in assembler

[Tim Deegan <tim@xxxxxxx>]

At 15:30 +0000 on 26 Mar (1395844252), Jan Beulich wrote:
> >>> On 26.03.14 at 16:08, <julien.grall@xxxxxxxxxx> wrote:
> > Hi Tim,
> > 
> > On 03/26/2014 01:16 PM, Tim Deegan wrote:
> >> At 11:53 +0000 on 26 Mar (1395831232), Jan Beulich wrote:
> >>>>>> On 25.03.14 at 17:55, <julien.grall@xxxxxxxxxx> wrote:
> >>>> --- a/xen/Rules.mk
> >>>> +++ b/xen/Rules.mk
> >>>> @@ -74,6 +74,7 @@ AFLAGS-y                += -D__ASSEMBLY__ -include 
> >>>> $(BASEDIR)/include/xen/config
> >>>>  
> >>>>  # Clang's built-in assembler can't handle .code16/.code32/.code64 yet
> >>>>  AFLAGS-$(clang)         += -no-integrated-as
> >>>> +CFLAGS-$(clang)         += -no-integrated-as
> >>>
> >>> Iirc Tim had found and worked around other built-in assembler issues
> >>> in the past, so if this is to be done unconditionally I wonder whether
> >>> we shouldn't then drop those workarounds.
> >> 
> >> I would prefer, wherever possible, to make things work with the clang
> >> assembler rather than rely on the binutils one forever.
> > 
> > The clang integrated assembler is too powerful for some part of Xen :).
> > Every inline assembly code is parsing by the assembler to check the syntax.
> > 
> > This will result to failure to generate asm-offsets.c because of the ->
> > in the code (see arch/arm/arm32/asm-offsets.c: DEFINE/BLANK macros).
> > Indeed, the -> is not a valid assembler syntax.
> 
> But what business does the compiler have to pass the assembly
> code to its internal assembler when all it was asked was to output
> assembly? That may be acceptable as an optional internal
> consistency check (verifying the compiler produced valid assembly),
> but shouldn't constrain the user from using the compiler for things
> where it's known the output isn't going to be valid assembly.
> 
> That said, I think it wouldn't be too difficult to change the
> asm-offsets logic to produce something that does look like valid
> assembly.

The patch below works for me (at least as far as building
asm-offsets.h on x86) by wrapping everything in a string.  I did try
just prefixing with '#' but clang 3.5 also strips the comments out.
That seems unhelpful, since I know some people put comments in their
inline assembler too. :(

---8<---

asm-offsets: encode magic asm-offset runes as strings.

Newer versions of clang attempt to parse inline assembler even when
not asked to assemble it.  Wrap our not-for-assembly runes as strings
of the form ``.ascii "==>MAGIC RUNES<=="'' so clang doesn't choke on them.

Reported-by: Julien Grall <julien.grall@xxxxxxxxxx>
Suggested-by: Jan Beulich <JBeulich@xxxxxxxx>
Signed-off-by: Tim Deegan <tim@xxxxxxx>

---
It should be possible to go futher and assemble the #define in
the C macro, just leaving sed to handle extracting them, but that's for
another day.

diff --git a/xen/Makefile b/xen/Makefile
index 79fa8f2..11aef27 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -150,7 +150,7 @@ include/asm-$(TARGET_ARCH)/asm-offsets.h: 
arch/$(TARGET_ARCH)/asm-offsets.s
          echo "#ifndef __ASM_OFFSETS_H__"; \
          echo "#define __ASM_OFFSETS_H__"; \
          echo ""; \
-         sed -ne "/^->/{s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 
/* \3 */:; s:->::; p;}"; \
+         sed -ne "/==>/{s:^.*==>\(.*\)<==.*:\1:; s:^\([^ ]*\) [\$$#]*\([^ ]*\) 
\(.*\):#define \1 \2 /* \3 */:; p;}"; \
          echo ""; \
          echo "#endif") <$< >$@
 
diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c
index ac628c0..db4bced 100644
--- a/xen/arch/arm/arm32/asm-offsets.c
+++ b/xen/arch/arm/arm32/asm-offsets.c
@@ -14,9 +14,9 @@
 #include <asm/procinfo.h>
 
 #define DEFINE(_sym, _val) \
-    __asm__ __volatile__ ( "\n->" #_sym " %0 " #_val : : "i" (_val) )
+    __asm__ __volatile__ ( "\n.ascii\"==>" #_sym " %0 " #_val "<==\"": : "i" 
(_val) )
 #define BLANK() \
-    __asm__ __volatile__ ( "\n->" : : )
+    __asm__ __volatile__ ( "\n.ascii\"==><==\"" : : )
 #define OFFSET(_sym, _str, _mem) \
     DEFINE(_sym, offsetof(_str, _mem));
 
diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
index d7572fa..769416e 100644
--- a/xen/arch/arm/arm64/asm-offsets.c
+++ b/xen/arch/arm/arm64/asm-offsets.c
@@ -13,9 +13,9 @@
 #include <asm/current.h>
 
 #define DEFINE(_sym, _val) \
-    __asm__ __volatile__ ( "\n->" #_sym " %0 " #_val : : "i" (_val) )
+    __asm__ __volatile__ ( "\n.ascii\"==>" #_sym " %0 " #_val "<==\"": : "i" 
(_val) )
 #define BLANK() \
-    __asm__ __volatile__ ( "\n->" : : )
+    __asm__ __volatile__ ( "\n.ascii\"==><==\"" : : )
 #define OFFSET(_sym, _str, _mem) \
     DEFINE(_sym, offsetof(_str, _mem));
 
diff --git a/xen/arch/x86/x86_64/asm-offsets.c 
b/xen/arch/x86/x86_64/asm-offsets.c
index b0098b3..e677214 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -15,9 +15,9 @@
 #include <xen/multiboot.h>
 
 #define DEFINE(_sym, _val) \
-    __asm__ __volatile__ ( "\n->" #_sym " %0 " #_val : : "i" (_val) )
+    __asm__ __volatile__ ( "\n.ascii\"==>" #_sym " %0 " #_val "<==\"": : "i" 
(_val) )
 #define BLANK() \
-    __asm__ __volatile__ ( "\n->" : : )
+    __asm__ __volatile__ ( "\n.ascii\"==><==\"" : : )
 #define OFFSET(_sym, _str, _mem) \
     DEFINE(_sym, offsetof(_str, _mem));
 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel