Re: linux-next: Tree for March 8 (BROKEN:arch/x86/kernel/entry_32.S? Debian's binutils/as?)
From: Ingo Molnar
Date: Wed Mar 09 2011 - 03:52:10 EST
* H.J. Lu <hjl.tools@xxxxxxxxx> wrote:
> On Tue, Mar 8, 2011 at 12:33 PM, Sedat Dilek <sedat.dilek@xxxxxxxxxxxxxx> wrote:
> > On Tue, Mar 8, 2011 at 9:25 PM, Alexander van Heukelum
> > <heukelum@xxxxxxxxxxx> wrote:
> >> On Tue, 08 Mar 2011 18:53 +0100, "Sedat Dilek" <sedat.dilek@xxxxxxxxxxxxxx> wrote:
> >>> On Tue, Mar 8, 2011 at 6:27 PM, Alexander van Heukelum
> >>> <heukelum@xxxxxxxxxxx> wrote:
> >>> > On Tue, 08 Mar 2011 16:42 +0100, "Sedat Dilek" <sedat.dilek@xxxxxxxxxxxxxx> wrote:
> >>> >> On 3/8/11, Sedat Dilek <sedat.dilek@xxxxxxxxxxxxxx> wrote:
> >>> >> > On 3/8/11, H.J. Lu <hjl.tools@xxxxxxxxx> wrote:
> >>> >> >> On Tue, Mar 8, 2011 at 2:44 AM, Sedat Dilek <sedat.dilek@xxxxxxxxxxxxxx>
> >>> >> >> wrote:
> >>> >> >>> Hi,
> >>> >> >>>
> >>> >> >>> my build of linux-next (next-20110308, the same with the one from
> >>> >> >>> yesterday) is broken.
> >>> >> >>> (I translated the German output.)
> >>> >> >>>
> >>> >> >>> [ build.log ]
> >>> >> >>> AS arch/x86/kernel/entry_32.o
> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
> >>> >> >>> Assembler messages:
> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
> >>> >> >>> Error: .size expression does not evaluate to a constant
> >>> >> >>> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
> >>> >> >>> make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
> >>> >> >>> make[4]: *** [arch/x86] Fehler 2 (Error 2)
> >>> >> >>> make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
> >>> >> >>> unfinished jobs...)
> >>> >> >>>
> >>> >> >>
> >>> >> >> This is a kernel bug. Please use the latest binutils from CVS.
> >>> >> >> It will tell you which symbol causes this.
> >>> >> >>
> >>> >> >>
> >>> >> >> --
> >>> >> >> H.J.
> >>> >> >>
> >>> >> >
> >>> >> > Yeah, I have cherry-picked these two upstream commits before you have
> >>> >> > mentionned it...
> >>> >> >
> >>> >> > 0001-Mention-symbol-name-in-non-constant-.size-expression.patch
> >>> >> > (Cherry-picked from commit b9521fc0be7945fc842ce1197e241a023378125d)
> >>> >> > 0002-Revert-the-last-change-on-gas-elf-bad-size.err.patch
> >>> >> > (Cherry-picked from commit cbd141bb69f791de7ea1581abe7afb34f0c61288)
> >>> >> >
> >>> >> > ... and have built with them a new binutils Debian package.
> >>> >> >
> >>> >> > The error looks now like this (sorry for the German output):
> >>> >> > ...
> >>> >> > AS arch/x86/kernel/entry_32.o
> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
> >>> >> > Assembler messages:
> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
> >>> >> > Error: .size expression with symbol `apf_page_fault' does not evaluate
> >>> >> > to a constant
> >>> >> > make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1
> >>> >> > make[5]: *** [arch/x86/kernel] Fehler 2
> >>> >> > make[5]: *** Warte auf noch nicht beendete Prozesse...
> >>> >> >
> >>> >> > Anyway, before more riddling around it would be very helpful to have a
> >>> >> > clear pointer if there is a fix around... That building, testing and
> >>> >> > installing took me now several hours.
> >>> >> > And... yeah, backports to 2.21-branch appreciated.
> >>> >> >
> >>> >> > - Sedat -
> >>> >> >
> >>> >>
> >>> >> After a quick look into the source, it seems attached patch fixes the
> >>> >> issue.
> >>> >> Is that OK?
> >>> >
> >>> > Hi Sedat,
> >>> >
> >>> > The patch ( https://lkml.org/lkml/2011/3/8/203 ) is ok, feel free to add
> >>> > Acked-by: Alexander van Heukelum <heukelum@xxxxxxxxxxx>
> >>> >
> >>> > Better description might be something like:
> >>> >
> >>> > i386: Fix mismatched ENTRY/END pair.
> >>> >
> >>> > Under CONFIG_KVM_GUEST=y, the following part of entry_32.S causes a compile failure.
> >>> >
> >>> > 1409 #ifdef CONFIG_KVM_GUEST
> >>> > 1410 ENTRY(async_page_fault)
> >>> > 1411 RING0_EC_FRAME
> >>> > 1412 pushl $do_async_page_fault
> >>> > 1413 CFI_ADJUST_CFA_OFFSET 4
> >>> > 1414 jmp error_code
> >>> > 1415 CFI_ENDPROC
> >>> > 1416 END(apf_page_fault)
> >>> > 1417 #endif
> >>> >
> >>> > Replace apf_page_fault with async_page_fault, as intended.
> >>> >
> >>> > Greetings,
> >>> > Alexander
> >>> >
> >>> >> - Sedat -
> >>> >>
> >>> >> Email had 1 attachment:
> >>> >> + 0001-x86-Fix-build-failure-with-binutils-as-from-upstream.patch
> >>> >> 1k (text/x-patch)
> >>> >
> >>>
> >>> As I said, quick view on the code, quick fix :-).
> >>>
> >>> Your description is definitive more meaningful.
> >>> I can refresh my patch and add your ACK.
> >>>
> >>> Anyway, I continued after dinner and with the above patch I ran into
> >>> the next problem:
> >>> [ build.log ]
> >>> ...
> >>> AS arch/x86/kernel/acpi/wakeup_rm.o
> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:
> >>> Assembler messages:
> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:12:
> >>> Error: .size expression with symbol `wakeup_code_start' does not
> >>> evaluate to a constant
> >>
> >> No idea what's wrong there. But my version of wakeup_rm.S has only 10 lines...
> >>
> >> 1 /*
> >> 2 * Wrapper script for the realmode binary as a transport object
> >> 3 * before copying to low memory.
> >> 4 */
> >> 5 .section ".rodata","a"
> >> 6 .globl wakeup_code_start, wakeup_code_end
> >> 7 wakeup_code_start:
> >> 8 .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
> >> 9 wakeup_code_end:
> >> 10 .size wakeup_code_start, .-wakeup_code_start
> >>
> >> And it compiles just fine.
> >> The fix for entry_32.S is valid, though, and necessary for mainline.
> >>
> >> Greetings,
> >> Alexander
> >>
> >>> I am unsure how to fix that and open for feedback.
> >>>
> >>> - Sedat -
> >>>
> >>
> >
> > The file in linux-next (next-20110308) looks different (the above code
> > looks more logical to me)
> >
> > [ arch/x86/kernel/acpi/wakeup_rm.S ]
> >
> > /*
> > * Wrapper script for the realmode binary as a transport object
> > * before copying to low memory.
> > */
> > #include <asm/page_types.h>
> >
> > .section ".x86_trampoline","a"
> > .balign PAGE_SIZE
> > .globl acpi_wakeup_code
> > acpi_wakeup_code:
> > .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
> > .size wakeup_code_start, .-wakeup_code_start
> >
>
> Those are simply wrong. 2.6.38-rc8 is OK.
2.6.37-rc8 is not OK: for example commit 631bc4878220932fe67fc46fc7cf7cccdb1ec597 is
already upstream and if you enable KVM you see a broken kernel build with new
binutils. This is from 2.6.38-rc8:
#ifdef CONFIG_KVM_GUEST
ENTRY(async_page_fault)
RING0_EC_FRAME
pushl $do_async_page_fault
CFI_ADJUST_CFA_OFFSET 4
jmp error_code
CFI_ENDPROC
END(apf_page_fault)
#endif
Yes, the .size directive not matching up is technically a bug, but it was not
checked by binutils before, for *years* - and you cannot just flip a switch and
break who knows how much code.
You need to at least emit a warning for some time to give people a *chance* to fix
bugs - not just stuff an incompatible binutils down their throat and break the
kernel build for thousands of commits and break bisection.
This binutils change is breaking numerous upstream kernel builds (and is making
bisection with new binutils impossible) for no particular good reason: binutils was
capable to figure out the symbol name before this change.
At minimum you need to *understand* that what you are doing is an incompatible
change and is disruptive to others ...
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/