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
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;
>>> 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
> 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
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.
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?
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.
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.
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;
>>> 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
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.
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
>>> 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
>>> 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
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
>>> 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
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;
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
>>> 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;
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 > >
>>> 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
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.
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
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.
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.
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.
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.