Clang/LLVM version requirements

Clang/LLVM version requirements

[Jeffrey Karrels]

Is there a required Clang and LLVM version one needs to do a 'make
xen-dist clang=y'?

I recently tried and the process failed. I haven't begun debugging as
I figured I would ask the obvious first. The build failed with the
following:

...
make[4]: Entering directory `/home/builder/xen-unstable/xen/drivers'
make -f /home/builder/xen-unstable/xen/Rules.mk -C char built_in.o
make[5]: Entering directory `/home/builder/xen-unstable/xen/drivers/char'
clang -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing
-std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement
-fno-builtin -fno-common -Wredundant-decls -iwithprefix include
-Werror -Wno-pointer-arith -pipe
-I/home/builder/xen-unstable/xen/include
-I/home/builder/xen-unstable/xen/include/asm-x86/mach-generic
-I/home/builder/xen-unstable/xen/include/asm-x86/mach-default
-msoft-float -fno-stack-protector -fno-exceptions -Wnested-externs
-mno-red-zone -mno-sse -fpic -fno-asynchronous-unwind-tables
-DGCC_HAS_VISIBILITY_ATTRIBUTE -Wno-parentheses -Wno-format
-Wno-unused-value -g -D__XEN__ -include
/home/builder/xen-unstable/xen/include/xen/config.h -DVERBOSE
-DHAS_ACPI -DHAS_GDBSX -DHAS_PASSTHROUGH -fno-omit-frame-pointer
-DCONFIG_FRAME_POINTER -MMD -MF .console.o.d -c console.c -o console.o
...
"error in backend: Invalid operand for inline asm constraint 'i'!"

Here are my details:
[builder@xenbuild1 xen-unstable]$ uname -a
Linux xenbuild1.cyberlab 2.6.32-220.el6.x86_64 #1 SMP Tue Dec 6
19:48:22 GMT 2011 x86_64 x86_64 x86_64 GNU/Linux

[builder@xenbuild1 xen-unstable]$ hg summary
parent: 25813:9dc729b75595 tip

[builder@xenbuild1 char]$ clang -v
clang version 2.8 (branches/release_28)
Target: x86_64-redhat-linux-gnu
Thread model: posix

Re: Clang/LLVM version requirements

[Tim Deegan]

At 15:32 -0700 on 06 Sep (1346945573), Jeffrey Karrels wrote:
> Is there a required Clang and LLVM version one needs to do a 'make
> xen-dist clang=y'?
> 
> I recently tried and the process failed. I haven't begun debugging as
> I figured I would ask the obvious first. The build failed with the
> following:

Looks like the clang build has bitrotted a little - sorry.  It's too
late to fix this for 4.2 now but we can sort it out after we branch
(i.e. next week) and backport any build fixes for 4.2.1.

I have: 
  Debian clang version 3.0-6 (tags/RELEASE_30/final) (based on LLVM 3.0)
  Target: x86_64-pc-linux-gnu
  Thread model: posix

which can build _almost_ everything but chokes on a new .subsection rune
in inline asm.  It builds and boots with the change below.  Building
lto=y isn't working (GOLD is choking on the final link).  I'll look into
that next week.

Jan, would you object to some other way of checking for .bss in reloc.c?
Could we just objcopy it out so any BSS symbols make it fail at link
time?  In any case we probably ought to check for stray .data symbols too.

Cheers,

Tim.

diff -r ec23c2a11f6f xen/arch/x86/boot/reloc.c
--- a/xen/arch/x86/boot/reloc.c	Thu Sep 06 17:08:44 2012 +0100
+++ b/xen/arch/x86/boot/reloc.c	Fri Sep 07 09:30:13 2012 +0100
@@ -18,10 +18,7 @@ asm (
     "    call 1f                       \n"
     "1:  pop  %ebx                     \n"
     "    mov  %eax,alloc-1b(%ebx)      \n"
-    "    mov  $_end,%ecx               \n"  /* check that BSS is empty! */
-    "    sub  $__bss_start,%ecx        \n"
-    "    jz reloc                      \n"
-    "1:  jmp 1b                        \n"
+    "    jmp  reloc                    \n"
     );
 
 /* This is our data.  Because the code must be relocatable, no BSS is
@@ -30,9 +27,6 @@ asm (
 asm (
     "alloc:                            \n"
     "    .long 0                       \n"
-    "    .subsection 1                 \n"
-    "    .p2align 4, 0xcc              \n"
-    "    .subsection 0                 \n"
     );
 
 typedef unsigned int u32;

Re: Clang/LLVM version requirements

[Jan Beulich]

>>> On 07.09.12 at 10:50, Tim Deegan <tim@xen.org> wrote:
> Jan, would you object to some other way of checking for .bss in reloc.c?

No, if you have a better idea.

> Could we just objcopy it out so any BSS symbols make it fail at link
> time?

Would that reliably fail with all linker versions?

> In any case we probably ought to check for stray .data symbols too.

Not really, no. Those would still be present in reloc.bin, and
hence make it into reloc.S.

Jan

Re: Clang/LLVM version requirements

[Jeffrey Karrels]

> Looks like the clang build has bitrotted a little - sorry.  It's too
> late to fix this for 4.2 now but we can sort it out after we branch
> (i.e. next week) and backport any build fixes for 4.2.1.
>

No worries. I can try and help out on this. Do you think it is
worthwhile to start a Wiki page for this type of work. I was working
with cppcheck, sparse, clang on/for xen...

Jeff

Re: Clang/LLVM version requirements

[Jeffrey Karrels]

On Fri, Sep 7, 2012 at 8:58 AM, Jeffrey Karrels <karrelsj@gmail.com> wrote:
>> Looks like the clang build has bitrotted a little - sorry.  It's too
>> late to fix this for 4.2 now but we can sort it out after we branch
>> (i.e. next week) and backport any build fixes for 4.2.1.

Just to keep the knowledge in the collective... I updated my clang and
llvm to the latest trunk:

[builder@xenbuild1 xen-unstable]$ /usr/local/bin/clang -v
clang version 3.2 (trunk 163631)
Target: x86_64-unknown-linux-gnu
Thread model: posix

After that I applied the patch that Tim previously posted and tried
another make. The xen build succeeded.

Re: Clang/LLVM version requirements

[Stefano Stabellini]

On Tue, 11 Sep 2012, Jeffrey Karrels wrote:
> On Fri, Sep 7, 2012 at 8:58 AM, Jeffrey Karrels <karrelsj@gmail.com> wrote:
> >> Looks like the clang build has bitrotted a little - sorry.  It's too
> >> late to fix this for 4.2 now but we can sort it out after we branch
> >> (i.e. next week) and backport any build fixes for 4.2.1.
> 
> Just to keep the knowledge in the collective... I updated my clang and
> llvm to the latest trunk:
> 
> [builder@xenbuild1 xen-unstable]$ /usr/local/bin/clang -v
> clang version 3.2 (trunk 163631)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> 
> After that I applied the patch that Tim previously posted and tried
> another make. The xen build succeeded.

time to add a clang build target to the test system?

Re: Clang/LLVM version requirements

[Tim Deegan]

At 08:58 -0700 on 07 Sep (1347008330), Jeffrey Karrels wrote:
> > Looks like the clang build has bitrotted a little - sorry.  It's too
> > late to fix this for 4.2 now but we can sort it out after we branch
> > (i.e. next week) and backport any build fixes for 4.2.1.
> >
> 
> No worries. I can try and help out on this. Do you think it is
> worthwhile to start a Wiki page for this type of work.

Yes, I think so.  Lars Kurth can sort you out with whatever
accounts/permissions you need.

> I was working with cppcheck, sparse, clang on/for xen...

That all sounds great.  I'm in favour of any more analysis we can get,
at least if it doesn't need intrusive annotations.

At 11:43 -0700 on 11 Sep (1347363829), Jeffrey Karrels wrote:
> Just to keep the knowledge in the collective... I updated my clang and
> llvm to the latest trunk:
> 
> [builder@xenbuild1 xen-unstable]$ /usr/local/bin/clang -v
> clang version 3.2 (trunk 163631)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> 
> After that I applied the patch that Tim previously posted and tried
> another make. The xen build succeeded.

Glad to hear it.  I'll sort out a better version of that patch
(i.e. that doesn't just remove the code but reimplements it in a
clang-friendly way) tomorrow.

Tim.

Re: Clang/LLVM version requirements

[Ian Campbell]

On Wed, 2012-09-12 at 11:31 +0100, Tim Deegan wrote:
> At 08:58 -0700 on 07 Sep (1347008330), Jeffrey Karrels wrote:
> > > Looks like the clang build has bitrotted a little - sorry.  It's too
> > > late to fix this for 4.2 now but we can sort it out after we branch
> > > (i.e. next week) and backport any build fixes for 4.2.1.
> > >
> > 
> > No worries. I can try and help out on this. Do you think it is
> > worthwhile to start a Wiki page for this type of work.
> 
> Yes, I think so.  Lars Kurth can sort you out with whatever
> accounts/permissions you need.

I think you just need to sign up. The new wiki has captcha and such so
no need to be approved by anyone etc like with the old wiki.

Ian.

Re: Clang/LLVM version requirements

[Tim Deegan]

At 11:04 +0100 on 07 Sep (1347015863), Jan Beulich wrote:
> >>> On 07.09.12 at 10:50, Tim Deegan <tim@xen.org> wrote:
> > Jan, would you object to some other way of checking for .bss in reloc.c?
> 
> No, if you have a better idea.
> 
> > Could we just objcopy it out so any BSS symbols make it fail at link
> > time?
> 
> Would that reliably fail with all linker versions?
> 
> > In any case we probably ought to check for stray .data symbols too.
> 
> Not really, no. Those would still be present in reloc.bin, and
> hence make it into reloc.S.

But they would break the test, because the magic alignment happens in
.text, right?  Anyway, compile-time failure seems more useful.  How
about this, based on the similar checks for init-only files?


# HG changeset patch
# Parent 5691e4cc17da7fe8664a67f1d07c3755c0ca34ed
x86: check for BSS in reloc code at compile time.

This is a more helpful failure mode than hanging at boot time, and
incidentally fixes the clang/LLVM build by removing a .subsection rune.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r 5691e4cc17da xen/arch/x86/boot/build32.mk
--- a/xen/arch/x86/boot/build32.mk	Thu Sep 13 10:23:17 2012 +0200
+++ b/xen/arch/x86/boot/build32.mk	Thu Sep 13 11:04:29 2012 +0100
@@ -20,6 +20,15 @@ CFLAGS := $(filter-out -flto,$(CFLAGS))
 
 %.o: %.c
 	$(CC) $(CFLAGS) -c -fpic $< -o $@
+	$(OBJDUMP) -h $@ | sed -n '/[0-9]/{s,00*,0,g;p}' |\
+		while read idx name sz rest; do \
+			case "$$name" in \
+			.bss|.bss.*) \
+				test $$sz != 0 || continue; \
+				echo "Error: non-empty BSS: 0x$$sz" >&2; \
+				exit $$(expr $$idx + 1);; \
+			esac; \
+		done
 
 reloc.o: $(BASEDIR)/include/asm-x86/config.h
 .PRECIOUS: %.bin %.lnk
diff -r 5691e4cc17da xen/arch/x86/boot/reloc.c
--- a/xen/arch/x86/boot/reloc.c	Thu Sep 13 10:23:17 2012 +0200
+++ b/xen/arch/x86/boot/reloc.c	Thu Sep 13 11:04:29 2012 +0100
@@ -18,10 +18,7 @@ asm (
     "    call 1f                       \n"
     "1:  pop  %ebx                     \n"
     "    mov  %eax,alloc-1b(%ebx)      \n"
-    "    mov  $_end,%ecx               \n"  /* check that BSS is empty! */
-    "    sub  $__bss_start,%ecx        \n"
-    "    jz reloc                      \n"
-    "1:  jmp 1b                        \n"
+    "    jmp  reloc                    \n"
     );
 
 /* This is our data.  Because the code must be relocatable, no BSS is
@@ -30,9 +27,6 @@ asm (
 asm (
     "alloc:                            \n"
     "    .long 0                       \n"
-    "    .subsection 1                 \n"
-    "    .p2align 4, 0xcc              \n"
-    "    .subsection 0                 \n"
     );
 
 typedef unsigned int u32;

Re: Clang/LLVM version requirements

[Jan Beulich]

>>> On 13.09.12 at 12:11, Tim Deegan <tim@xen.org> wrote:
> At 11:04 +0100 on 07 Sep (1347015863), Jan Beulich wrote:
>> >>> On 07.09.12 at 10:50, Tim Deegan <tim@xen.org> wrote:
>> > In any case we probably ought to check for stray .data symbols too.
>> 
>> Not really, no. Those would still be present in reloc.bin, and
>> hence make it into reloc.S.
> 
> But they would break the test, because the magic alignment happens in
> .text, right?  Anyway, compile-time failure seems more useful.  How
> about this, based on the similar checks for init-only files?

Yes, it's happening in .text currently. The problem, iirc, really was
that _end and __bss_start didn't always match (depending on
compiler version), apparently because at the linking stage _end
got aligned more strictly than __bss_start.

So the patch is fine by me if it covers that misalignment case.
But it seems a little heavy handed - I'd think that instead of the
sub-section, we could just create an arbitrary other section, or
even allow uninitialized variable (it's unclear to me why Paolo
wrote the comment - in c/s 25479:61dfb3da56b - regarding BSS
the way it is now) - after all we only need to make sure that
- the space gets properly allocated in trampoline.S, i.e. also in
  reloc.bin
- all accesses are PC-relative
Neither has anything to do with use of uninitialized variables.

Jan

Re: Clang/LLVM version requirements

[Tim Deegan]

At 12:52 +0100 on 13 Sep (1347540769), Jan Beulich wrote:
> >>> On 13.09.12 at 12:11, Tim Deegan <tim@xen.org> wrote:
> > At 11:04 +0100 on 07 Sep (1347015863), Jan Beulich wrote:
> >> >>> On 07.09.12 at 10:50, Tim Deegan <tim@xen.org> wrote:
> >> > In any case we probably ought to check for stray .data symbols too.
> >> 
> >> Not really, no. Those would still be present in reloc.bin, and
> >> hence make it into reloc.S.
> > 
> > But they would break the test, because the magic alignment happens in
> > .text, right?  Anyway, compile-time failure seems more useful.  How
> > about this, based on the similar checks for init-only files?
> 
> Yes, it's happening in .text currently. The problem, iirc, really was
> that _end and __bss_start didn't always match (depending on
> compiler version), apparently because at the linking stage _end
> got aligned more strictly than __bss_start.
> 
> So the patch is fine by me if it covers that misalignment case.
> But it seems a little heavy handed - I'd think that instead of the
> sub-section, we could just create an arbitrary other section, or
> even allow uninitialized variable (it's unclear to me why Paolo
> wrote the comment - in c/s 25479:61dfb3da56b - regarding BSS
> the way it is now) - after all we only need to make sure that
> - the space gets properly allocated in trampoline.S, i.e. also in
>   reloc.bin
> - all accesses are PC-relative
> Neither has anything to do with use of uninitialized variables.

Allowing BSS would just need a few extra runes (AFAICS,
"--set-section-flags .bss=alloc,load,contents") in the objcopy that
makes reloc.bin.  But I'm not sure how to make sure everything is
rip-relative, do if that's the real concern I'm inclined to go with
this compile-time check and exclude .[ro]data[.*] as well.
We can always fix it up to allow bss and data sections if we ever
actually need them.

Tim.

Re: Clang/LLVM version requirements

[Paolo Bonzini]

Il 13/09/2012 13:52, Jan Beulich ha scritto:
> 
> So the patch is fine by me if it covers that misalignment case.
> But it seems a little heavy handed - I'd think that instead of the
> sub-section, we could just create an arbitrary other section, or
> even allow uninitialized variable (it's unclear to me why Paolo
> wrote the comment - in c/s 25479:61dfb3da56b - regarding BSS
> the way it is now) - after all we only need to make sure that
> - the space gets properly allocated in trampoline.S, i.e. also in
>   reloc.bin
> - all accesses are PC-relative
> Neither has anything to do with use of uninitialized variables.

We cannot use BSS because it doesn't appear in reloc.S.

Apart from BSS, there would be no benefit for reloc to move itself to
BOOT_TRAMPOLINE, because BOOT_TRAMPOLINE is not a known location
anymore.  So it was easier to just run it in place from where we put it,
in the middle of head.S, but this means BSS disappears after extracting
reloc.bin.

So it's not really because the code must be relocatable, but more
because of the way we extract the binary data and put it in the middle
of head.S.

Initialized data works as long as you pass -fno-zero-initialized-in-bss
to the compiler or it is eaten.  I used assembly to declare the alloc
variable, because I wasn't sure of which GCC versions need the option,
and whether older versions are supported for compiling Xen.

Paolo

Re: Clang/LLVM version requirements

[Jan Beulich]

>>> On 13.09.12 at 14:21, Tim Deegan <tim@xen.org> wrote:
> At 12:52 +0100 on 13 Sep (1347540769), Jan Beulich wrote:
>> >>> On 13.09.12 at 12:11, Tim Deegan <tim@xen.org> wrote:
>> > At 11:04 +0100 on 07 Sep (1347015863), Jan Beulich wrote:
>> >> >>> On 07.09.12 at 10:50, Tim Deegan <tim@xen.org> wrote:
>> >> > In any case we probably ought to check for stray .data symbols too.
>> >> 
>> >> Not really, no. Those would still be present in reloc.bin, and
>> >> hence make it into reloc.S.
>> > 
>> > But they would break the test, because the magic alignment happens in
>> > .text, right?  Anyway, compile-time failure seems more useful.  How
>> > about this, based on the similar checks for init-only files?
>> 
>> Yes, it's happening in .text currently. The problem, iirc, really was
>> that _end and __bss_start didn't always match (depending on
>> compiler version), apparently because at the linking stage _end
>> got aligned more strictly than __bss_start.
>> 
>> So the patch is fine by me if it covers that misalignment case.
>> But it seems a little heavy handed - I'd think that instead of the
>> sub-section, we could just create an arbitrary other section, or
>> even allow uninitialized variable (it's unclear to me why Paolo
>> wrote the comment - in c/s 25479:61dfb3da56b - regarding BSS
>> the way it is now) - after all we only need to make sure that
>> - the space gets properly allocated in trampoline.S, i.e. also in
>>   reloc.bin
>> - all accesses are PC-relative
>> Neither has anything to do with use of uninitialized variables.
> 
> Allowing BSS would just need a few extra runes (AFAICS,
> "--set-section-flags .bss=alloc,load,contents") in the objcopy that
> makes reloc.bin.
>  But I'm not sure how to make sure everything is
> rip-relative, do if that's the real concern I'm inclined to go with
> this compile-time check and exclude .[ro]data[.*] as well.
> We can always fix it up to allow bss and data sections if we ever
> actually need them.

As said, I'm fine with any approach as long as it works with all
supported tool chains. So feel free to go the route you're
proposing.

Jan

Re: Clang/LLVM version requirements

[Jan Beulich]

>>> On 13.09.12 at 14:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 13/09/2012 13:52, Jan Beulich ha scritto:
>> 
>> So the patch is fine by me if it covers that misalignment case.
>> But it seems a little heavy handed - I'd think that instead of the
>> sub-section, we could just create an arbitrary other section, or
>> even allow uninitialized variable (it's unclear to me why Paolo
>> wrote the comment - in c/s 25479:61dfb3da56b - regarding BSS
>> the way it is now) - after all we only need to make sure that
>> - the space gets properly allocated in trampoline.S, i.e. also in
>>   reloc.bin
>> - all accesses are PC-relative
>> Neither has anything to do with use of uninitialized variables.
> 
> We cannot use BSS because it doesn't appear in reloc.S.

That, imo, would be a binutils bug.

Jan

> Apart from BSS, there would be no benefit for reloc to move itself to
> BOOT_TRAMPOLINE, because BOOT_TRAMPOLINE is not a known location
> anymore.  So it was easier to just run it in place from where we put it,
> in the middle of head.S, but this means BSS disappears after extracting
> reloc.bin.
> 
> So it's not really because the code must be relocatable, but more
> because of the way we extract the binary data and put it in the middle
> of head.S.
> 
> Initialized data works as long as you pass -fno-zero-initialized-in-bss
> to the compiler or it is eaten.  I used assembly to declare the alloc
> variable, because I wasn't sure of which GCC versions need the option,
> and whether older versions are supported for compiling Xen.
> 
> Paolo

Re: Clang/LLVM version requirements

[Paolo Bonzini]

Il 13/09/2012 16:04, Jan Beulich ha scritto:
>>> >> So the patch is fine by me if it covers that misalignment case.
>>> >> But it seems a little heavy handed - I'd think that instead of the
>>> >> sub-section, we could just create an arbitrary other section, or
>>> >> even allow uninitialized variable (it's unclear to me why Paolo
>>> >> wrote the comment - in c/s 25479:61dfb3da56b - regarding BSS
>>> >> the way it is now) - after all we only need to make sure that
>>> >> - the space gets properly allocated in trampoline.S, i.e. also in
>>> >>   reloc.bin
>>> >> - all accesses are PC-relative
>>> >> Neither has anything to do with use of uninitialized variables.
>> > 
>> > We cannot use BSS because it doesn't appear in reloc.S.
> That, imo, would be a binutils bug.

It's just because the flags say so.  Tim's flag magic should do it (my
reply crossed with his).

Paolo

Re: Clang/LLVM version requirements

[Jan Beulich]

>>> On 13.09.12 at 16:13, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 13/09/2012 16:04, Jan Beulich ha scritto:
>>>> >> So the patch is fine by me if it covers that misalignment case.
>>>> >> But it seems a little heavy handed - I'd think that instead of the
>>>> >> sub-section, we could just create an arbitrary other section, or
>>>> >> even allow uninitialized variable (it's unclear to me why Paolo
>>>> >> wrote the comment - in c/s 25479:61dfb3da56b - regarding BSS
>>>> >> the way it is now) - after all we only need to make sure that
>>>> >> - the space gets properly allocated in trampoline.S, i.e. also in
>>>> >>   reloc.bin
>>>> >> - all accesses are PC-relative
>>>> >> Neither has anything to do with use of uninitialized variables.
>>> > 
>>> > We cannot use BSS because it doesn't appear in reloc.S.
>> That, imo, would be a binutils bug.
> 
> It's just because the flags say so.  Tim's flag magic should do it (my
> reply crossed with his).

Flags magic shouldn't be required either, since I don't think you'll
find a .o with a .bss section with the alloc flag clear. The alloc flag
set, however, means it ought to be present in the output.

Jan

Re: Clang/LLVM version requirements

[Tim Deegan]

At 15:01 +0100 on 13 Sep (1347548504), Jan Beulich wrote:
> >>> On 13.09.12 at 14:21, Tim Deegan <tim@xen.org> wrote:
> > Allowing BSS would just need a few extra runes (AFAICS,
> > "--set-section-flags .bss=alloc,load,contents") in the objcopy that
> > makes reloc.bin.
> >  But I'm not sure how to make sure everything is
> > rip-relative, do if that's the real concern I'm inclined to go with
> > this compile-time check and exclude .[ro]data[.*] as well.
> > We can always fix it up to allow bss and data sections if we ever
> > actually need them.
> 
> As said, I'm fine with any approach as long as it works with all
> supported tool chains. So feel free to go the route you're
> proposing.

OK.  The patch below works for me on gccs 4.1 to 4.7 and clang 3.0.
I tries gcc 3.4 but the build already fails in a few other places.
Do we really still support gcc 3.4 like the README says?

Can I have an Ack, or would you like to apply it yourself?

Tim.

# HG changeset patch
# Parent 9e46d90d0182979a7220314ca19d2525e338aa5d
x86: check for data and BSS in reloc code at compile time.

This is a more useful failure mode than hanging at boot time, and
incidentally fixes the clang/LLVM build by removing a .subsection rune.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r 9e46d90d0182 xen/arch/x86/boot/build32.mk
--- a/xen/arch/x86/boot/build32.mk	Thu Sep 13 15:00:26 2012 +0100
+++ b/xen/arch/x86/boot/build32.mk	Thu Sep 13 15:04:32 2012 +0100
@@ -20,6 +20,15 @@ CFLAGS := $(filter-out -flto,$(CFLAGS))
 
 %.o: %.c
 	$(CC) $(CFLAGS) -c -fpic $< -o $@
+	$(OBJDUMP) -h $@ | sed -n '/[0-9]/{s,00*,0,g;p}' |\
+		while read idx name sz rest; do \
+			case "$$name" in \
+			.data|.data.*|.rodata|.rodata.*|.bss|.bss.*) \
+				test $$sz != 0 || continue; \
+				echo "Error: non-empty $$name: 0x$$sz" >&2; \
+				exit $$(expr $$idx + 1);; \
+			esac; \
+		done
 
 reloc.o: $(BASEDIR)/include/asm-x86/config.h
 .PRECIOUS: %.bin %.lnk
diff -r 9e46d90d0182 xen/arch/x86/boot/reloc.c
--- a/xen/arch/x86/boot/reloc.c	Thu Sep 13 15:00:26 2012 +0100
+++ b/xen/arch/x86/boot/reloc.c	Thu Sep 13 15:04:32 2012 +0100
@@ -18,10 +18,7 @@ asm (
     "    call 1f                       \n"
     "1:  pop  %ebx                     \n"
     "    mov  %eax,alloc-1b(%ebx)      \n"
-    "    mov  $_end,%ecx               \n"  /* check that BSS is empty! */
-    "    sub  $__bss_start,%ecx        \n"
-    "    jz reloc                      \n"
-    "1:  jmp 1b                        \n"
+    "    jmp  reloc                    \n"
     );
 
 /* This is our data.  Because the code must be relocatable, no BSS is
@@ -30,9 +27,6 @@ asm (
 asm (
     "alloc:                            \n"
     "    .long 0                       \n"
-    "    .subsection 1                 \n"
-    "    .p2align 4, 0xcc              \n"
-    "    .subsection 0                 \n"
     );
 
 typedef unsigned int u32;

Re: Clang/LLVM version requirements

[Keir Fraser]

On 13/09/2012 15:55, "Tim Deegan" <tim@xen.org> wrote:

> At 15:01 +0100 on 13 Sep (1347548504), Jan Beulich wrote:
>>>>> On 13.09.12 at 14:21, Tim Deegan <tim@xen.org> wrote:
>>> Allowing BSS would just need a few extra runes (AFAICS,
>>> "--set-section-flags .bss=alloc,load,contents") in the objcopy that
>>> makes reloc.bin.
>>>  But I'm not sure how to make sure everything is
>>> rip-relative, do if that's the real concern I'm inclined to go with
>>> this compile-time check and exclude .[ro]data[.*] as well.
>>> We can always fix it up to allow bss and data sections if we ever
>>> actually need them.
>> 
>> As said, I'm fine with any approach as long as it works with all
>> supported tool chains. So feel free to go the route you're
>> proposing.
> 
> OK.  The patch below works for me on gccs 4.1 to 4.7 and clang 3.0.
> I tries gcc 3.4 but the build already fails in a few other places.
> Do we really still support gcc 3.4 like the README says?

It's been a long while since we updated our compiler support. In general
I've been happy to say we support each gcc release only for 2-3 years. In
this case that would mean we could *even* update our support to be only gcc
4.2 and later.

What do people think about us forcing this? It might even let us get rid of
GCC_HAS_VISIBILITY_ATTRIBUTE?

 -- Keir

> Can I have an Ack, or would you like to apply it yourself?
> 
> Tim.
> 
> # HG changeset patch
> # Parent 9e46d90d0182979a7220314ca19d2525e338aa5d
> x86: check for data and BSS in reloc code at compile time.
> 
> This is a more useful failure mode than hanging at boot time, and
> incidentally fixes the clang/LLVM build by removing a .subsection rune.
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
> 
> diff -r 9e46d90d0182 xen/arch/x86/boot/build32.mk
> --- a/xen/arch/x86/boot/build32.mk Thu Sep 13 15:00:26 2012 +0100
> +++ b/xen/arch/x86/boot/build32.mk Thu Sep 13 15:04:32 2012 +0100
> @@ -20,6 +20,15 @@ CFLAGS := $(filter-out -flto,$(CFLAGS))
>  
>  %.o: %.c
> $(CC) $(CFLAGS) -c -fpic $< -o $@
> + $(OBJDUMP) -h $@ | sed -n '/[0-9]/{s,00*,0,g;p}' |\
> +  while read idx name sz rest; do \
> +   case "$$name" in \
> +   .data|.data.*|.rodata|.rodata.*|.bss|.bss.*) \
> +    test $$sz != 0 || continue; \
> +    echo "Error: non-empty $$name: 0x$$sz" >&2; \
> +    exit $$(expr $$idx + 1);; \
> +   esac; \
> +  done
>  
>  reloc.o: $(BASEDIR)/include/asm-x86/config.h
>  .PRECIOUS: %.bin %.lnk
> diff -r 9e46d90d0182 xen/arch/x86/boot/reloc.c
> --- a/xen/arch/x86/boot/reloc.c Thu Sep 13 15:00:26 2012 +0100
> +++ b/xen/arch/x86/boot/reloc.c Thu Sep 13 15:04:32 2012 +0100
> @@ -18,10 +18,7 @@ asm (
>      "    call 1f                       \n"
>      "1:  pop  %ebx                     \n"
>      "    mov  %eax,alloc-1b(%ebx)      \n"
> -    "    mov  $_end,%ecx               \n"  /* check that BSS is empty! */
> -    "    sub  $__bss_start,%ecx        \n"
> -    "    jz reloc                      \n"
> -    "1:  jmp 1b                        \n"
> +    "    jmp  reloc                    \n"
>      );
>  
>  /* This is our data.  Because the code must be relocatable, no BSS is
> @@ -30,9 +27,6 @@ asm (
>  asm (
>      "alloc:                            \n"
>      "    .long 0                       \n"
> -    "    .subsection 1                 \n"
> -    "    .p2align 4, 0xcc              \n"
> -    "    .subsection 0                 \n"
>      );
>  
>  typedef unsigned int u32;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: Clang/LLVM version requirements

[Jan Beulich]

>>> On 13.09.12 at 16:55, Tim Deegan <tim@xen.org> wrote:
> At 15:01 +0100 on 13 Sep (1347548504), Jan Beulich wrote:
>> >>> On 13.09.12 at 14:21, Tim Deegan <tim@xen.org> wrote:
>> > Allowing BSS would just need a few extra runes (AFAICS,
>> > "--set-section-flags .bss=alloc,load,contents") in the objcopy that
>> > makes reloc.bin.
>> >  But I'm not sure how to make sure everything is
>> > rip-relative, do if that's the real concern I'm inclined to go with
>> > this compile-time check and exclude .[ro]data[.*] as well.
>> > We can always fix it up to allow bss and data sections if we ever
>> > actually need them.
>> 
>> As said, I'm fine with any approach as long as it works with all
>> supported tool chains. So feel free to go the route you're
>> proposing.
> 
> OK.  The patch below works for me on gccs 4.1 to 4.7 and clang 3.0.
> I tries gcc 3.4 but the build already fails in a few other places.
> Do we really still support gcc 3.4 like the README says?
> 
> Can I have an Ack, or would you like to apply it yourself?

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

> # HG changeset patch
> # Parent 9e46d90d0182979a7220314ca19d2525e338aa5d
> x86: check for data and BSS in reloc code at compile time.
> 
> This is a more useful failure mode than hanging at boot time, and
> incidentally fixes the clang/LLVM build by removing a .subsection rune.
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
> 
> diff -r 9e46d90d0182 xen/arch/x86/boot/build32.mk
> --- a/xen/arch/x86/boot/build32.mk	Thu Sep 13 15:00:26 2012 +0100
> +++ b/xen/arch/x86/boot/build32.mk	Thu Sep 13 15:04:32 2012 +0100
> @@ -20,6 +20,15 @@ CFLAGS := $(filter-out -flto,$(CFLAGS))
>  
>  %.o: %.c
>  	$(CC) $(CFLAGS) -c -fpic $< -o $@
> +	$(OBJDUMP) -h $@ | sed -n '/[0-9]/{s,00*,0,g;p}' |\
> +		while read idx name sz rest; do \
> +			case "$$name" in \
> +			.data|.data.*|.rodata|.rodata.*|.bss|.bss.*) \
> +				test $$sz != 0 || continue; \
> +				echo "Error: non-empty $$name: 0x$$sz" >&2; \
> +				exit $$(expr $$idx + 1);; \
> +			esac; \
> +		done
>  
>  reloc.o: $(BASEDIR)/include/asm-x86/config.h
>  .PRECIOUS: %.bin %.lnk
> diff -r 9e46d90d0182 xen/arch/x86/boot/reloc.c
> --- a/xen/arch/x86/boot/reloc.c	Thu Sep 13 15:00:26 2012 +0100
> +++ b/xen/arch/x86/boot/reloc.c	Thu Sep 13 15:04:32 2012 +0100
> @@ -18,10 +18,7 @@ asm (
>      "    call 1f                       \n"
>      "1:  pop  %ebx                     \n"
>      "    mov  %eax,alloc-1b(%ebx)      \n"
> -    "    mov  $_end,%ecx               \n"  /* check that BSS is empty! */
> -    "    sub  $__bss_start,%ecx        \n"
> -    "    jz reloc                      \n"
> -    "1:  jmp 1b                        \n"
> +    "    jmp  reloc                    \n"
>      );
>  
>  /* This is our data.  Because the code must be relocatable, no BSS is
> @@ -30,9 +27,6 @@ asm (
>  asm (
>      "alloc:                            \n"
>      "    .long 0                       \n"
> -    "    .subsection 1                 \n"
> -    "    .p2align 4, 0xcc              \n"
> -    "    .subsection 0                 \n"
>      );
>  
>  typedef unsigned int u32;

Re: Clang/LLVM version requirements

[Keir Fraser]

On 13/09/2012 16:05, "Keir Fraser" <keir.xen@gmail.com> wrote:

> On 13/09/2012 15:55, "Tim Deegan" <tim@xen.org> wrote:
> 
>> At 15:01 +0100 on 13 Sep (1347548504), Jan Beulich wrote:
>>>>>> On 13.09.12 at 14:21, Tim Deegan <tim@xen.org> wrote:
>>>> Allowing BSS would just need a few extra runes (AFAICS,
>>>> "--set-section-flags .bss=alloc,load,contents") in the objcopy that
>>>> makes reloc.bin.
>>>>  But I'm not sure how to make sure everything is
>>>> rip-relative, do if that's the real concern I'm inclined to go with
>>>> this compile-time check and exclude .[ro]data[.*] as well.
>>>> We can always fix it up to allow bss and data sections if we ever
>>>> actually need them.
>>> 
>>> As said, I'm fine with any approach as long as it works with all
>>> supported tool chains. So feel free to go the route you're
>>> proposing.
>> 
>> OK.  The patch below works for me on gccs 4.1 to 4.7 and clang 3.0.
>> I tries gcc 3.4 but the build already fails in a few other places.
>> Do we really still support gcc 3.4 like the README says?
> 
> It's been a long while since we updated our compiler support. In general
> I've been happy to say we support each gcc release only for 2-3 years. In
> this case that would mean we could *even* update our support to be only gcc
> 4.2 and later.
> 
> What do people think about us forcing this? It might even let us get rid of
> GCC_HAS_VISIBILITY_ATTRIBUTE?

I should add, this is mainly a question of how aggressive we should be. I'm
quite happy to retire gcc-3.4 support if it happens it is now broken. In
that case, x86/Rules.mk should have its gcc version check updated. And
perhaps arch/arm may as well do the same? I would be happy to Ack a patch to
that effect.

 -- Keir

>  -- Keir
> 
>> Can I have an Ack, or would you like to apply it yourself?
>> 
>> Tim.
>> 
>> # HG changeset patch
>> # Parent 9e46d90d0182979a7220314ca19d2525e338aa5d
>> x86: check for data and BSS in reloc code at compile time.
>> 
>> This is a more useful failure mode than hanging at boot time, and
>> incidentally fixes the clang/LLVM build by removing a .subsection rune.
>> 
>> Signed-off-by: Tim Deegan <tim@xen.org>
>> 
>> diff -r 9e46d90d0182 xen/arch/x86/boot/build32.mk
>> --- a/xen/arch/x86/boot/build32.mk Thu Sep 13 15:00:26 2012 +0100
>> +++ b/xen/arch/x86/boot/build32.mk Thu Sep 13 15:04:32 2012 +0100
>> @@ -20,6 +20,15 @@ CFLAGS := $(filter-out -flto,$(CFLAGS))
>>  
>>  %.o: %.c
>> $(CC) $(CFLAGS) -c -fpic $< -o $@
>> + $(OBJDUMP) -h $@ | sed -n '/[0-9]/{s,00*,0,g;p}' |\
>> +  while read idx name sz rest; do \
>> +   case "$$name" in \
>> +   .data|.data.*|.rodata|.rodata.*|.bss|.bss.*) \
>> +    test $$sz != 0 || continue; \
>> +    echo "Error: non-empty $$name: 0x$$sz" >&2; \
>> +    exit $$(expr $$idx + 1);; \
>> +   esac; \
>> +  done
>>  
>>  reloc.o: $(BASEDIR)/include/asm-x86/config.h
>>  .PRECIOUS: %.bin %.lnk
>> diff -r 9e46d90d0182 xen/arch/x86/boot/reloc.c
>> --- a/xen/arch/x86/boot/reloc.c Thu Sep 13 15:00:26 2012 +0100
>> +++ b/xen/arch/x86/boot/reloc.c Thu Sep 13 15:04:32 2012 +0100
>> @@ -18,10 +18,7 @@ asm (
>>      "    call 1f                       \n"
>>      "1:  pop  %ebx                     \n"
>>      "    mov  %eax,alloc-1b(%ebx)      \n"
>> -    "    mov  $_end,%ecx               \n"  /* check that BSS is empty! */
>> -    "    sub  $__bss_start,%ecx        \n"
>> -    "    jz reloc                      \n"
>> -    "1:  jmp 1b                        \n"
>> +    "    jmp  reloc                    \n"
>>      );
>>  
>>  /* This is our data.  Because the code must be relocatable, no BSS is
>> @@ -30,9 +27,6 @@ asm (
>>  asm (
>>      "alloc:                            \n"
>>      "    .long 0                       \n"
>> -    "    .subsection 1                 \n"
>> -    "    .p2align 4, 0xcc              \n"
>> -    "    .subsection 0                 \n"
>>      );
>>  
>>  typedef unsigned int u32;
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
> 
> 

Re: Clang/LLVM version requirements

[Jan Beulich]

>>> On 13.09.12 at 17:05, Keir Fraser <keir.xen@gmail.com> wrote:
> On 13/09/2012 15:55, "Tim Deegan" <tim@xen.org> wrote:
> 
>> At 15:01 +0100 on 13 Sep (1347548504), Jan Beulich wrote:
>>>>>> On 13.09.12 at 14:21, Tim Deegan <tim@xen.org> wrote:
>>>> Allowing BSS would just need a few extra runes (AFAICS,
>>>> "--set-section-flags .bss=alloc,load,contents") in the objcopy that
>>>> makes reloc.bin.
>>>>  But I'm not sure how to make sure everything is
>>>> rip-relative, do if that's the real concern I'm inclined to go with
>>>> this compile-time check and exclude .[ro]data[.*] as well.
>>>> We can always fix it up to allow bss and data sections if we ever
>>>> actually need them.
>>> 
>>> As said, I'm fine with any approach as long as it works with all
>>> supported tool chains. So feel free to go the route you're
>>> proposing.
>> 
>> OK.  The patch below works for me on gccs 4.1 to 4.7 and clang 3.0.
>> I tries gcc 3.4 but the build already fails in a few other places.
>> Do we really still support gcc 3.4 like the README says?
> 
> It's been a long while since we updated our compiler support. In general
> I've been happy to say we support each gcc release only for 2-3 years. In
> this case that would mean we could *even* update our support to be only gcc
> 4.2 and later.

For the purpose of being able to build on SLE10 without overrides,
I'd like to ask to consider keeping this at 4.1.x for the next while.

Jan

Re: Clang/LLVM version requirements

[Ian Campbell]

On Thu, 2012-09-13 at 16:09 +0100, Keir Fraser wrote:
> On 13/09/2012 16:05, "Keir Fraser" <keir.xen@gmail.com> wrote:
> 
> > On 13/09/2012 15:55, "Tim Deegan" <tim@xen.org> wrote:
> > 
> >> At 15:01 +0100 on 13 Sep (1347548504), Jan Beulich wrote:
> >>>>>> On 13.09.12 at 14:21, Tim Deegan <tim@xen.org> wrote:
> >>>> Allowing BSS would just need a few extra runes (AFAICS,
> >>>> "--set-section-flags .bss=alloc,load,contents") in the objcopy that
> >>>> makes reloc.bin.
> >>>>  But I'm not sure how to make sure everything is
> >>>> rip-relative, do if that's the real concern I'm inclined to go with
> >>>> this compile-time check and exclude .[ro]data[.*] as well.
> >>>> We can always fix it up to allow bss and data sections if we ever
> >>>> actually need them.
> >>> 
> >>> As said, I'm fine with any approach as long as it works with all
> >>> supported tool chains. So feel free to go the route you're
> >>> proposing.
> >> 
> >> OK.  The patch below works for me on gccs 4.1 to 4.7 and clang 3.0.
> >> I tries gcc 3.4 but the build already fails in a few other places.
> >> Do we really still support gcc 3.4 like the README says?
> > 
> > It's been a long while since we updated our compiler support. In general
> > I've been happy to say we support each gcc release only for 2-3 years. In
> > this case that would mean we could *even* update our support to be only gcc
> > 4.2 and later.
> > 
> > What do people think about us forcing this? It might even let us get rid of
> > GCC_HAS_VISIBILITY_ATTRIBUTE?
> 
> I should add, this is mainly a question of how aggressive we should be. I'm
> quite happy to retire gcc-3.4 support if it happens it is now broken. In
> that case, x86/Rules.mk should have its gcc version check updated. And
> perhaps arch/arm may as well do the same? I would be happy to Ack a patch to
> that effect.

Some data points: Debian Squeeze (current stable) has gcc 4.4 as the
default (but ships a bunch of others) and Lenny (previous stable) had
4.3. AFAICT RHEL5 and SLES10 both shipped 4.1, SLES11 shipped 4.3 and
RHEL 6 4.4.

Do we really want/need to support host OSes older than RHEL5/SLES10? We
are talking 2006/7 vintage there.

Ian.

Re: Clang/LLVM version requirements

[Keir Fraser]

On 13/09/2012 16:22, "Jan Beulich" <JBeulich@suse.com> wrote:

>>> OK.  The patch below works for me on gccs 4.1 to 4.7 and clang 3.0.
>>> I tries gcc 3.4 but the build already fails in a few other places.
>>> Do we really still support gcc 3.4 like the README says?
>> 
>> It's been a long while since we updated our compiler support. In general
>> I've been happy to say we support each gcc release only for 2-3 years. In
>> this case that would mean we could *even* update our support to be only gcc
>> 4.2 and later.
> 
> For the purpose of being able to build on SLE10 without overrides,
> I'd like to ask to consider keeping this at 4.1.x for the next while.

Fine. Unless I hear otherwise I will update the check to gcc-4.1 and later,
tomorrow.

 -- Keir

Re: Clang/LLVM version requirements

[Keir Fraser]

On 13/09/2012 16:27, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:

>> I should add, this is mainly a question of how aggressive we should be. I'm
>> quite happy to retire gcc-3.4 support if it happens it is now broken. In
>> that case, x86/Rules.mk should have its gcc version check updated. And
>> perhaps arch/arm may as well do the same? I would be happy to Ack a patch to
>> that effect.
> 
> Some data points: Debian Squeeze (current stable) has gcc 4.4 as the
> default (but ships a bunch of others) and Lenny (previous stable) had
> 4.3. AFAICT RHEL5 and SLES10 both shipped 4.1, SLES11 shipped 4.3 and
> RHEL 6 4.4.
> 
> Do we really want/need to support host OSes older than RHEL5/SLES10? We
> are talking 2006/7 vintage there.

Yes, I think that is where we are going to draw the line. Thanks.

Re: Clang/LLVM version requirements

[Ian Jackson]

Stefano Stabellini writes ("Re: [Xen-devel] Clang/LLVM version requirements"):
> On Tue, 11 Sep 2012, Jeffrey Karrels wrote:
> > [builder@xenbuild1 xen-unstable]$ /usr/local/bin/clang -v
> > clang version 3.2 (trunk 163631)
> > Target: x86_64-unknown-linux-gnu
> > Thread model: posix
> > 
> > After that I applied the patch that Tim previously posted and tried
> > another make. The xen build succeeded.
> 
> time to add a clang build target to the test system?

This should be fairly straightforward.

What do I need to do on an amd64 Debian squeeze system to build with
clang ?

Ian.

Re: Clang/LLVM version requirements

[Tim Deegan]

At 17:15 +0100 on 13 Sep (1347556552), Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] Clang/LLVM version requirements"):
> > On Tue, 11 Sep 2012, Jeffrey Karrels wrote:
> > > [builder@xenbuild1 xen-unstable]$ /usr/local/bin/clang -v
> > > clang version 3.2 (trunk 163631)
> > > Target: x86_64-unknown-linux-gnu
> > > Thread model: posix
> > > 
> > > After that I applied the patch that Tim previously posted and tried
> > > another make. The xen build succeeded.
> > 
> > time to add a clang build target to the test system?
> 
> This should be fairly straightforward.
> 
> What do I need to do on an amd64 Debian squeeze system to build with
> clang ?

You need clang 3.0-6, which is in wheezy but not squeeze.  Then just
"make dist-xen clang=y".

The tools don't build with clang, only Xen.
I presume that if we want to build the tools with clang in future it
would be done with autofoo rather than clang=y.

Cheers,

Tim.

Re: Clang/LLVM version requirements

[Ian Jackson]

Tim Deegan writes ("Re: [Xen-devel] Clang/LLVM version requirements"):
> At 17:15 +0100 on 13 Sep (1347556552), Ian Jackson wrote:
> > What do I need to do on an amd64 Debian squeeze system to build with
> > clang ?
> 
> You need clang 3.0-6, which is in wheezy but not squeeze.  Then just
> "make dist-xen clang=y".

Ah.  Well I guess I could install wheezy but I would prefer not to
make the test system depend on wheezy until wheezy is released.  So
this should wait.

> The tools don't build with clang, only Xen.
> I presume that if we want to build the tools with clang in future it
> would be done with autofoo rather than clang=y.

Probably, yes.

Ian.