* [PATCH] ARM: Fix cross compilation broken by failing size command
@ 2011-12-16 10:42 ` Janusz Krzysztofik
0 siblings, 0 replies; 21+ messages in thread
From: Janusz Krzysztofik @ 2011-12-16 10:42 UTC (permalink / raw)
To: linux-arm-kernel
Since commit 5ffb04f6690d71fab241b3562ebf52b893ac4ff1, "ARM: zImage:
make sure appended DTB doesn't get overwritten by kernel .bss", the
native 'size' command, which is now always used for calculation of the
kernel bss size, may break in selected cross compilation environments
with error messages like:
size: arch/arm/boot/compressed/../../../../vmlinux: File format is ambiguous
size: Matching formats: elf32-littlearm elf32-littlearm-symbian elf32-littlearm-vxworks
As a consequence, the KBSS_SZ variable extracted from the size output is
empty, and the the final linker command, provided with incorrectly
formatted arguments, also fails:
LD arch/arm/boot/compressed/vmlinux
arm-angstrom-linux-uclibcgnueabi-ld:--defsym _kernel_bss_size=: syntax error
Don't append the '_kernel_bss_size=$(KBSS_SZ)' argument to the linker
command line if that variable is empty because of the failing size
command. Moreover, use $(CROSS_COMPILE)size if available instead of
native size.
Created and tested against linux-3.2-rc5.
Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
---
This patch supersedes my previously submitted "ARM: fix $(CROSS_COMPILE)
prefix missing from size invocation". Thanks to Tony Lindgren for his
comments on $(CROSS_COMPILE)size limited availability.
Unfortunately, I've already pushed that old version to the patch system
at http://www.arm.linux.org.uk/developer/patches/, which I know is not
a review system. Apparently, I was not patient enough, not waiting more
than a week with that regression fix for a single reply to my initial
submission to Russell King, the ARM PORT maintainer, Cc: Nicolas Pitre,
the regression introducing commit author, and the linux-arm-kernel list.
Sorry for that, please drop the old version from the Incoming queue, and
I'll not be pushing anything to that patch system without prior positive
review response any more.
Thanks,
Janusz
arch/arm/boot/compressed/Makefile | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 21f56ff..17972e9 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -126,8 +126,11 @@ ccflags-y := -fpic -fno-builtin -I$(obj)
asflags-y := -Wa,-march=all
# Supply kernel BSS size to the decompressor via a linker symbol.
-KBSS_SZ = $(shell size $(obj)/../../../../vmlinux | awk 'END{print $$3}')
+KBSS_SZ = $(shell if $(CROSS_COMPILE)size $(obj)/../../../../vmlinux; then :; \
+ else size $(obj)/../../../../vmlinux; fi | awk 'END{print $$3}')
+ifneq ($(KBSS_SZ),)
LDFLAGS_vmlinux = --defsym _kernel_bss_size=$(KBSS_SZ)
+endif
# Supply ZRELADDR to the decompressor via a linker symbol.
ifneq ($(CONFIG_AUTO_ZRELADDR),y)
LDFLAGS_vmlinux += --defsym zreladdr=$(ZRELADDR)
--
1.7.3.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH] ARM: Fix cross compilation broken by failing size command
2011-12-16 10:42 ` Janusz Krzysztofik
@ 2011-12-17 0:30 ` Tony Lindgren
-1 siblings, 0 replies; 21+ messages in thread
From: Tony Lindgren @ 2011-12-17 0:30 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Russell King, Nicolas Pitre, linux-arm-kernel, linux-kernel
* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111216 02:12]:
> Since commit 5ffb04f6690d71fab241b3562ebf52b893ac4ff1, "ARM: zImage:
> make sure appended DTB doesn't get overwritten by kernel .bss", the
> native 'size' command, which is now always used for calculation of the
> kernel bss size, may break in selected cross compilation environments
> with error messages like:
>
> size: arch/arm/boot/compressed/../../../../vmlinux: File format is ambiguous
> size: Matching formats: elf32-littlearm elf32-littlearm-symbian elf32-littlearm-vxworks
>
> As a consequence, the KBSS_SZ variable extracted from the size output is
> empty, and the the final linker command, provided with incorrectly
> formatted arguments, also fails:
>
> LD arch/arm/boot/compressed/vmlinux
> arm-angstrom-linux-uclibcgnueabi-ld:--defsym _kernel_bss_size=: syntax error
>
> Don't append the '_kernel_bss_size=$(KBSS_SZ)' argument to the linker
> command line if that variable is empty because of the failing size
> command. Moreover, use $(CROSS_COMPILE)size if available instead of
> native size.
>
> Created and tested against linux-3.2-rc5.
>
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
This seems like a good solution to me:
Acked-by: Tony Lindgren <tony@atomide.com>
> ---
> This patch supersedes my previously submitted "ARM: fix $(CROSS_COMPILE)
> prefix missing from size invocation". Thanks to Tony Lindgren for his
> comments on $(CROSS_COMPILE)size limited availability.
>
> Unfortunately, I've already pushed that old version to the patch system
> at http://www.arm.linux.org.uk/developer/patches/, which I know is not
> a review system. Apparently, I was not patient enough, not waiting more
> than a week with that regression fix for a single reply to my initial
> submission to Russell King, the ARM PORT maintainer, Cc: Nicolas Pitre,
> the regression introducing commit author, and the linux-arm-kernel list.
> Sorry for that, please drop the old version from the Incoming queue, and
> I'll not be pushing anything to that patch system without prior positive
> review response any more.
Heh :) The patch system is pretty arcane.. You can move the
patch there to superseded yourself btw.
Tony
> Thanks,
> Janusz
>
> arch/arm/boot/compressed/Makefile | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index 21f56ff..17972e9 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -126,8 +126,11 @@ ccflags-y := -fpic -fno-builtin -I$(obj)
> asflags-y := -Wa,-march=all
>
> # Supply kernel BSS size to the decompressor via a linker symbol.
> -KBSS_SZ = $(shell size $(obj)/../../../../vmlinux | awk 'END{print $$3}')
> +KBSS_SZ = $(shell if $(CROSS_COMPILE)size $(obj)/../../../../vmlinux; then :; \
> + else size $(obj)/../../../../vmlinux; fi | awk 'END{print $$3}')
> +ifneq ($(KBSS_SZ),)
> LDFLAGS_vmlinux = --defsym _kernel_bss_size=$(KBSS_SZ)
> +endif
> # Supply ZRELADDR to the decompressor via a linker symbol.
> ifneq ($(CONFIG_AUTO_ZRELADDR),y)
> LDFLAGS_vmlinux += --defsym zreladdr=$(ZRELADDR)
> --
> 1.7.3.4
>
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH] ARM: Fix cross compilation broken by failing size command
@ 2011-12-17 0:30 ` Tony Lindgren
0 siblings, 0 replies; 21+ messages in thread
From: Tony Lindgren @ 2011-12-17 0:30 UTC (permalink / raw)
To: linux-arm-kernel
* Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> [111216 02:12]:
> Since commit 5ffb04f6690d71fab241b3562ebf52b893ac4ff1, "ARM: zImage:
> make sure appended DTB doesn't get overwritten by kernel .bss", the
> native 'size' command, which is now always used for calculation of the
> kernel bss size, may break in selected cross compilation environments
> with error messages like:
>
> size: arch/arm/boot/compressed/../../../../vmlinux: File format is ambiguous
> size: Matching formats: elf32-littlearm elf32-littlearm-symbian elf32-littlearm-vxworks
>
> As a consequence, the KBSS_SZ variable extracted from the size output is
> empty, and the the final linker command, provided with incorrectly
> formatted arguments, also fails:
>
> LD arch/arm/boot/compressed/vmlinux
> arm-angstrom-linux-uclibcgnueabi-ld:--defsym _kernel_bss_size=: syntax error
>
> Don't append the '_kernel_bss_size=$(KBSS_SZ)' argument to the linker
> command line if that variable is empty because of the failing size
> command. Moreover, use $(CROSS_COMPILE)size if available instead of
> native size.
>
> Created and tested against linux-3.2-rc5.
>
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
This seems like a good solution to me:
Acked-by: Tony Lindgren <tony@atomide.com>
> ---
> This patch supersedes my previously submitted "ARM: fix $(CROSS_COMPILE)
> prefix missing from size invocation". Thanks to Tony Lindgren for his
> comments on $(CROSS_COMPILE)size limited availability.
>
> Unfortunately, I've already pushed that old version to the patch system
> at http://www.arm.linux.org.uk/developer/patches/, which I know is not
> a review system. Apparently, I was not patient enough, not waiting more
> than a week with that regression fix for a single reply to my initial
> submission to Russell King, the ARM PORT maintainer, Cc: Nicolas Pitre,
> the regression introducing commit author, and the linux-arm-kernel list.
> Sorry for that, please drop the old version from the Incoming queue, and
> I'll not be pushing anything to that patch system without prior positive
> review response any more.
Heh :) The patch system is pretty arcane.. You can move the
patch there to superseded yourself btw.
Tony
> Thanks,
> Janusz
>
> arch/arm/boot/compressed/Makefile | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index 21f56ff..17972e9 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -126,8 +126,11 @@ ccflags-y := -fpic -fno-builtin -I$(obj)
> asflags-y := -Wa,-march=all
>
> # Supply kernel BSS size to the decompressor via a linker symbol.
> -KBSS_SZ = $(shell size $(obj)/../../../../vmlinux | awk 'END{print $$3}')
> +KBSS_SZ = $(shell if $(CROSS_COMPILE)size $(obj)/../../../../vmlinux; then :; \
> + else size $(obj)/../../../../vmlinux; fi | awk 'END{print $$3}')
> +ifneq ($(KBSS_SZ),)
> LDFLAGS_vmlinux = --defsym _kernel_bss_size=$(KBSS_SZ)
> +endif
> # Supply ZRELADDR to the decompressor via a linker symbol.
> ifneq ($(CONFIG_AUTO_ZRELADDR),y)
> LDFLAGS_vmlinux += --defsym zreladdr=$(ZRELADDR)
> --
> 1.7.3.4
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] ARM: Fix cross compilation broken by failing size command
2011-12-16 10:42 ` Janusz Krzysztofik
@ 2011-12-17 10:57 ` Russell King - ARM Linux
-1 siblings, 0 replies; 21+ messages in thread
From: Russell King - ARM Linux @ 2011-12-17 10:57 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Nicolas Pitre, Tony Lindgren, linux-arm-kernel, linux-kernel
On Fri, Dec 16, 2011 at 11:42:26AM +0100, Janusz Krzysztofik wrote:
> Unfortunately, I've already pushed that old version to the patch system
> at http://www.arm.linux.org.uk/developer/patches/, which I know is not
> a review system. Apparently, I was not patient enough, not waiting more
> than a week with that regression fix for a single reply to my initial
I'll disagree with you there: this is not a regression fix. A regression
fix implies that it was something that worked before, a change happened
to the kernel, and now it's broken.
That is not the case: a change happened to binutils which meant 'size'
no longer parses foreign ELF formats. This is not a change to the
kernel, and therefore it is not a kernel regression.
Therefore, I consider it lower priority than true kernel regression
fixes, and also lower priority than normal bug fixes to the kernel:
it's actually a feature change - changing the kernel to work with a
new toolchain.
Since the original proposed change causes _me_ problems (I'm missing
an arm-linux-size for some unknown reason - which I've yet to resolve
one way or other) I've not yet considered the change any further than
noting that fact.
If it turns out that older versions of binutils do not supply a 'size'
command, then we need something like your v2 patch. If it's the case
that for some reason it never got transferred when I changed machines,
then your v1 patch is appropriate and I need to fix that. But that
requires investigation on my side, and I've not been able to do that
yet.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH] ARM: Fix cross compilation broken by failing size command
@ 2011-12-17 10:57 ` Russell King - ARM Linux
0 siblings, 0 replies; 21+ messages in thread
From: Russell King - ARM Linux @ 2011-12-17 10:57 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Dec 16, 2011 at 11:42:26AM +0100, Janusz Krzysztofik wrote:
> Unfortunately, I've already pushed that old version to the patch system
> at http://www.arm.linux.org.uk/developer/patches/, which I know is not
> a review system. Apparently, I was not patient enough, not waiting more
> than a week with that regression fix for a single reply to my initial
I'll disagree with you there: this is not a regression fix. A regression
fix implies that it was something that worked before, a change happened
to the kernel, and now it's broken.
That is not the case: a change happened to binutils which meant 'size'
no longer parses foreign ELF formats. This is not a change to the
kernel, and therefore it is not a kernel regression.
Therefore, I consider it lower priority than true kernel regression
fixes, and also lower priority than normal bug fixes to the kernel:
it's actually a feature change - changing the kernel to work with a
new toolchain.
Since the original proposed change causes _me_ problems (I'm missing
an arm-linux-size for some unknown reason - which I've yet to resolve
one way or other) I've not yet considered the change any further than
noting that fact.
If it turns out that older versions of binutils do not supply a 'size'
command, then we need something like your v2 patch. If it's the case
that for some reason it never got transferred when I changed machines,
then your v1 patch is appropriate and I need to fix that. But that
requires investigation on my side, and I've not been able to do that
yet.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] ARM: Fix cross compilation broken by failing size command
2011-12-17 10:57 ` Russell King - ARM Linux
@ 2011-12-17 13:01 ` Janusz Krzysztofik
-1 siblings, 0 replies; 21+ messages in thread
From: Janusz Krzysztofik @ 2011-12-17 13:01 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Nicolas Pitre, Tony Lindgren, linux-arm-kernel, linux-kernel
On Saturday 17 of December 2011 at 11:57:37, Russell King - ARM Linux wrote:
> On Fri, Dec 16, 2011 at 11:42:26AM +0100, Janusz Krzysztofik wrote:
> > Unfortunately, I've already pushed that old version to the patch system
> > at http://www.arm.linux.org.uk/developer/patches/, which I know is not
> > a review system. Apparently, I was not patient enough, not waiting more
> > than a week with that regression fix for a single reply to my initial
>
> I'll disagree with you there: this is not a regression fix. A regression
> fix implies that it was something that worked before, a change happened
> to the kernel, and now it's broken.
>
> That is not the case: a change happened to binutils
Not to mine, I'm still using the same version as before.
> which meant 'size'
> no longer parses foreign ELF formats.
But 'size' was not there before v3.2-rc1! The problem has arised because
arch/arm/boot/compress/Makefile, which belongs to the kernel sources,
not to binutils you're trying to blame, was changed. The 'size' call was
introduced into that Makefile, i.e. the file that belongs to the kernel
sources, whithout taking care of what can happen if that command fails.
And I was just lucky enough to find out it may fail in certains build
environments, like mine.
> This is not a change to the
> kernel,
but a change to a file in the kernel source tree. Isn't it the same?
> and therefore it is not a kernel regression.
Than, you say that if a kernel Makefile is broken, effectively breaking
the kernel compilation in certain build environments, then it's not a
kernel regression and can be released as is? Funny.
> Therefore, I consider it lower priority than true kernel regression
> fixes, and also lower priority than normal bug fixes to the kernel:
> it's actually a feature change - changing the kernel to work with a
> new toolchain.
I repeat: I haven't changed my toolchain. A file that belongs to the
kernel sources was changed.
> Since the original proposed change causes _me_ problems (I'm missing
> an arm-linux-size for some unknown reason - which I've yet to resolve
> one way or other) I've not yet considered the change any further than
> noting that fact.
If you were so kind to share your findings once discovered, you would
give me a chance to correct that problem with missing
$(CROSS_COMPILE)size tool already several weeks ago. Fortunately, Tony
was kind enough to comment, better late than never.
> If it turns out that older versions of binutils do not supply a 'size'
> command, then we need something like your v2 patch. If it's the case
> that for some reason it never got transferred when I changed machines,
> then your v1 patch is appropriate and I need to fix that. But that
> requires investigation on my side, and I've not been able to do that
> yet.
Thanks to Tony's help, you now have two versions of the patch to choose
from, the initial one, with its own issue, and the corrected one, with
Tony's Ack.
If you are sure this is not a regression and can wait while the stable
kernel version is released with that issue not corrected - your choice.
You are the ARM port maintainer, not me, and I respect your rights to
manage things the way you like. I only disagree with your opinion about
this issue not being a regression.
I'm not representing any third party interests, only my own (and public
to some extent, I thought), and even if not happy, I'm willing to handle
that issue myslef in my build environment as long as it exists in the
official kernel sources, in order to be able to still help solving other
kernel issues I happen to find out from time to time.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH] ARM: Fix cross compilation broken by failing size command
@ 2011-12-17 13:01 ` Janusz Krzysztofik
0 siblings, 0 replies; 21+ messages in thread
From: Janusz Krzysztofik @ 2011-12-17 13:01 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 17 of December 2011 at 11:57:37, Russell King - ARM Linux wrote:
> On Fri, Dec 16, 2011 at 11:42:26AM +0100, Janusz Krzysztofik wrote:
> > Unfortunately, I've already pushed that old version to the patch system
> > at http://www.arm.linux.org.uk/developer/patches/, which I know is not
> > a review system. Apparently, I was not patient enough, not waiting more
> > than a week with that regression fix for a single reply to my initial
>
> I'll disagree with you there: this is not a regression fix. A regression
> fix implies that it was something that worked before, a change happened
> to the kernel, and now it's broken.
>
> That is not the case: a change happened to binutils
Not to mine, I'm still using the same version as before.
> which meant 'size'
> no longer parses foreign ELF formats.
But 'size' was not there before v3.2-rc1! The problem has arised because
arch/arm/boot/compress/Makefile, which belongs to the kernel sources,
not to binutils you're trying to blame, was changed. The 'size' call was
introduced into that Makefile, i.e. the file that belongs to the kernel
sources, whithout taking care of what can happen if that command fails.
And I was just lucky enough to find out it may fail in certains build
environments, like mine.
> This is not a change to the
> kernel,
but a change to a file in the kernel source tree. Isn't it the same?
> and therefore it is not a kernel regression.
Than, you say that if a kernel Makefile is broken, effectively breaking
the kernel compilation in certain build environments, then it's not a
kernel regression and can be released as is? Funny.
> Therefore, I consider it lower priority than true kernel regression
> fixes, and also lower priority than normal bug fixes to the kernel:
> it's actually a feature change - changing the kernel to work with a
> new toolchain.
I repeat: I haven't changed my toolchain. A file that belongs to the
kernel sources was changed.
> Since the original proposed change causes _me_ problems (I'm missing
> an arm-linux-size for some unknown reason - which I've yet to resolve
> one way or other) I've not yet considered the change any further than
> noting that fact.
If you were so kind to share your findings once discovered, you would
give me a chance to correct that problem with missing
$(CROSS_COMPILE)size tool already several weeks ago. Fortunately, Tony
was kind enough to comment, better late than never.
> If it turns out that older versions of binutils do not supply a 'size'
> command, then we need something like your v2 patch. If it's the case
> that for some reason it never got transferred when I changed machines,
> then your v1 patch is appropriate and I need to fix that. But that
> requires investigation on my side, and I've not been able to do that
> yet.
Thanks to Tony's help, you now have two versions of the patch to choose
from, the initial one, with its own issue, and the corrected one, with
Tony's Ack.
If you are sure this is not a regression and can wait while the stable
kernel version is released with that issue not corrected - your choice.
You are the ARM port maintainer, not me, and I respect your rights to
manage things the way you like. I only disagree with your opinion about
this issue not being a regression.
I'm not representing any third party interests, only my own (and public
to some extent, I thought), and even if not happy, I'm willing to handle
that issue myslef in my build environment as long as it exists in the
official kernel sources, in order to be able to still help solving other
kernel issues I happen to find out from time to time.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] ARM: Fix cross compilation broken by failing size command
2011-12-17 13:01 ` Janusz Krzysztofik
@ 2011-12-17 16:38 ` Nicolas Pitre
-1 siblings, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2011-12-17 16:38 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Russell King - ARM Linux, Tony Lindgren, linux-arm-kernel, linux-kernel
On Sat, 17 Dec 2011, Janusz Krzysztofik wrote:
> On Saturday 17 of December 2011 at 11:57:37, Russell King - ARM Linux wrote:
> > On Fri, Dec 16, 2011 at 11:42:26AM +0100, Janusz Krzysztofik wrote:
> > > Unfortunately, I've already pushed that old version to the patch system
> > > at http://www.arm.linux.org.uk/developer/patches/, which I know is not
> > > a review system. Apparently, I was not patient enough, not waiting more
> > > than a week with that regression fix for a single reply to my initial
> >
> > I'll disagree with you there: this is not a regression fix. A regression
> > fix implies that it was something that worked before, a change happened
> > to the kernel, and now it's broken.
> >
> > That is not the case: a change happened to binutils
>
> Not to mine, I'm still using the same version as before.
>
> > which meant 'size'
> > no longer parses foreign ELF formats.
>
> But 'size' was not there before v3.2-rc1! The problem has arised because
> arch/arm/boot/compress/Makefile, which belongs to the kernel sources,
> not to binutils you're trying to blame, was changed. The 'size' call was
> introduced into that Makefile, i.e. the file that belongs to the kernel
> sources, whithout taking care of what can happen if that command fails.
> And I was just lucky enough to find out it may fail in certains build
> environments, like mine.
OK let's stop arguing in circles please.
I introduced the call to 'size' so by definition I'm the one who broke
the build for you.
However I don't have any problem using the native 'size' command with a
foreign architecture binary. My binutils version is 2.21.51.0.6 on
X86-64. And clearly I'm not alone in that situation otherwise we would
have been overwhelmed by complaints from a much bigger crowd by now. So
your installation must be "special".
OTOH all cross-compiler installations I've seen had $(CROSS_COMPILE)size
available. It is therefore well possible that Russell's cross compiler
installation is also "special".
Given that the build works for the vast majority of people now, there is
a risk that applying your first patch might break the build for more
people (currently only one known) than the number of people for whom
this is an improvement (currently only one known).
I suggested a simple fix for Russell's installation already:
ln -s size arm-linux-size
I don't know what is special about yours though, and maybe it is worth
investigating given that, as I said, you appear to be alone so far
having problems with the current Makefile which is rather intriguing.
If it is clear with references to facts (binutils release notes, etc.)
that the problem is something that will persist in the future then we
have a more solid basis for choosing either of your solutions.
Otherwise we can't call it a "regression" just yet.
Nicolas
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH] ARM: Fix cross compilation broken by failing size command
@ 2011-12-17 16:38 ` Nicolas Pitre
0 siblings, 0 replies; 21+ messages in thread
From: Nicolas Pitre @ 2011-12-17 16:38 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, 17 Dec 2011, Janusz Krzysztofik wrote:
> On Saturday 17 of December 2011 at 11:57:37, Russell King - ARM Linux wrote:
> > On Fri, Dec 16, 2011 at 11:42:26AM +0100, Janusz Krzysztofik wrote:
> > > Unfortunately, I've already pushed that old version to the patch system
> > > at http://www.arm.linux.org.uk/developer/patches/, which I know is not
> > > a review system. Apparently, I was not patient enough, not waiting more
> > > than a week with that regression fix for a single reply to my initial
> >
> > I'll disagree with you there: this is not a regression fix. A regression
> > fix implies that it was something that worked before, a change happened
> > to the kernel, and now it's broken.
> >
> > That is not the case: a change happened to binutils
>
> Not to mine, I'm still using the same version as before.
>
> > which meant 'size'
> > no longer parses foreign ELF formats.
>
> But 'size' was not there before v3.2-rc1! The problem has arised because
> arch/arm/boot/compress/Makefile, which belongs to the kernel sources,
> not to binutils you're trying to blame, was changed. The 'size' call was
> introduced into that Makefile, i.e. the file that belongs to the kernel
> sources, whithout taking care of what can happen if that command fails.
> And I was just lucky enough to find out it may fail in certains build
> environments, like mine.
OK let's stop arguing in circles please.
I introduced the call to 'size' so by definition I'm the one who broke
the build for you.
However I don't have any problem using the native 'size' command with a
foreign architecture binary. My binutils version is 2.21.51.0.6 on
X86-64. And clearly I'm not alone in that situation otherwise we would
have been overwhelmed by complaints from a much bigger crowd by now. So
your installation must be "special".
OTOH all cross-compiler installations I've seen had $(CROSS_COMPILE)size
available. It is therefore well possible that Russell's cross compiler
installation is also "special".
Given that the build works for the vast majority of people now, there is
a risk that applying your first patch might break the build for more
people (currently only one known) than the number of people for whom
this is an improvement (currently only one known).
I suggested a simple fix for Russell's installation already:
ln -s size arm-linux-size
I don't know what is special about yours though, and maybe it is worth
investigating given that, as I said, you appear to be alone so far
having problems with the current Makefile which is rather intriguing.
If it is clear with references to facts (binutils release notes, etc.)
that the problem is something that will persist in the future then we
have a more solid basis for choosing either of your solutions.
Otherwise we can't call it a "regression" just yet.
Nicolas
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] ARM: Fix cross compilation broken by failing size command
2011-12-17 16:38 ` Nicolas Pitre
@ 2011-12-17 17:15 ` Russell King - ARM Linux
-1 siblings, 0 replies; 21+ messages in thread
From: Russell King - ARM Linux @ 2011-12-17 17:15 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Janusz Krzysztofik, Tony Lindgren, linux-arm-kernel, linux-kernel
On Sat, Dec 17, 2011 at 11:38:34AM -0500, Nicolas Pitre wrote:
> Given that the build works for the vast majority of people now, there is
> a risk that applying your first patch might break the build for more
> people (currently only one known) than the number of people for whom
> this is an improvement (currently only one known).
>
> I suggested a simple fix for Russell's installation already:
>
> ln -s size arm-linux-size
One which I've refused to do until I've investigated _why_ I don't have
this command. And I've now done that (having waited the half-hour for
the fsck of the old machine to complete.) It appears that several of
the binutils commands weren't transferred to the new machine from the
old machine.
That's now fixed - and it suggests that there isn't a version of
binutils which missed out on this command, so I think we can go with
the original patch.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH] ARM: Fix cross compilation broken by failing size command
@ 2011-12-17 17:15 ` Russell King - ARM Linux
0 siblings, 0 replies; 21+ messages in thread
From: Russell King - ARM Linux @ 2011-12-17 17:15 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Dec 17, 2011 at 11:38:34AM -0500, Nicolas Pitre wrote:
> Given that the build works for the vast majority of people now, there is
> a risk that applying your first patch might break the build for more
> people (currently only one known) than the number of people for whom
> this is an improvement (currently only one known).
>
> I suggested a simple fix for Russell's installation already:
>
> ln -s size arm-linux-size
One which I've refused to do until I've investigated _why_ I don't have
this command. And I've now done that (having waited the half-hour for
the fsck of the old machine to complete.) It appears that several of
the binutils commands weren't transferred to the new machine from the
old machine.
That's now fixed - and it suggests that there isn't a version of
binutils which missed out on this command, so I think we can go with
the original patch.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] ARM: Fix cross compilation broken by failing size command
2011-12-17 16:38 ` Nicolas Pitre
@ 2011-12-17 18:57 ` Janusz Krzysztofik
-1 siblings, 0 replies; 21+ messages in thread
From: Janusz Krzysztofik @ 2011-12-17 18:57 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Russell King - ARM Linux, Tony Lindgren, linux-arm-kernel, linux-kernel
On Saturday 17 of December 2011 at 17:38:34, Nicolas Pitre wrote:
> On Sat, 17 Dec 2011, Janusz Krzysztofik wrote:
>
> > On Saturday 17 of December 2011 at 11:57:37, Russell King - ARM Linux wrote:
> > > On Fri, Dec 16, 2011 at 11:42:26AM +0100, Janusz Krzysztofik wrote:
> > > > Unfortunately, I've already pushed that old version to the patch system
> > > > at http://www.arm.linux.org.uk/developer/patches/, which I know is not
> > > > a review system. Apparently, I was not patient enough, not waiting more
> > > > than a week with that regression fix for a single reply to my initial
> > >
> > > I'll disagree with you there: this is not a regression fix. A regression
> > > fix implies that it was something that worked before, a change happened
> > > to the kernel, and now it's broken.
> > >
> > > That is not the case: a change happened to binutils
> >
> > Not to mine, I'm still using the same version as before.
> >
> > > which meant 'size'
> > > no longer parses foreign ELF formats.
> >
> > But 'size' was not there before v3.2-rc1! The problem has arised because
> > arch/arm/boot/compress/Makefile, which belongs to the kernel sources,
> > not to binutils you're trying to blame, was changed. The 'size' call was
> > introduced into that Makefile, i.e. the file that belongs to the kernel
> > sources, whithout taking care of what can happen if that command fails.
> > And I was just lucky enough to find out it may fail in certains build
> > environments, like mine.
>
> OK let's stop arguing in circles please.
>
> I introduced the call to 'size' so by definition I'm the one who broke
> the build for you.
I don't care who broke it, this might happen to anyone, including me.
Now I see my fault was not examining the linux-arm-kernel archives a few
months deep before submitting my patch, which was apparently a
(independently developed) duplicate for the one already submitted by
Thomas Weber 6 weeks earlier.
> However I don't have any problem using the native 'size' command with a
> foreign architecture binary. My binutils version is 2.21.51.0.6 on
> X86-64. And clearly I'm not alone in that situation otherwise we would
> have been overwhelmed by complaints from a much bigger crowd by now. So
> your installation must be "special".
2.20.1.20100303 on X86-64 here, built with --enable-targets=all for a
reason I'm not able to recall now.
> OTOH all cross-compiler installations I've seen had $(CROSS_COMPILE)size
> available. It is therefore well possible that Russell's cross compiler
> installation is also "special".
>
> Given that the build works for the vast majority of people now, there is
> a risk that applying your first patch might break the build for more
> people (currently only one known) than the number of people for whom
> this is an improvement (currently only one known).
Tony already told me the same, and I came out with v2 as a solution.
> I suggested a simple fix for Russell's installation already:
>
> ln -s size arm-linux-size
>
> I don't know what is special about yours though, and maybe it is worth
> investigating given that, as I said, you appear to be alone so far
> having problems with the current Makefile which is rather intriguing.
> If it is clear with references to facts (binutils release notes, etc.)
> that the problem is something that will persist in the future then we
> have a more solid basis for choosing either of your solutions.
> Otherwise we can't call it a "regression" just yet.
After Tony's comments I changed my approach in that the core regression
here was not calling the native 'size', but passing an additional
argument to the linker without checking for its correctness first. The
core problem was not a broken 'size', but a broken linker command after
all. Broken 'size' consequences should be limited to at most making the
change your patch was about ineffective, but not the whole compilation
broken. That's what my v2 was about in the first place.
YMMV.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH] ARM: Fix cross compilation broken by failing size command
@ 2011-12-17 18:57 ` Janusz Krzysztofik
0 siblings, 0 replies; 21+ messages in thread
From: Janusz Krzysztofik @ 2011-12-17 18:57 UTC (permalink / raw)
To: linux-arm-kernel
On Saturday 17 of December 2011 at 17:38:34, Nicolas Pitre wrote:
> On Sat, 17 Dec 2011, Janusz Krzysztofik wrote:
>
> > On Saturday 17 of December 2011 at 11:57:37, Russell King - ARM Linux wrote:
> > > On Fri, Dec 16, 2011 at 11:42:26AM +0100, Janusz Krzysztofik wrote:
> > > > Unfortunately, I've already pushed that old version to the patch system
> > > > at http://www.arm.linux.org.uk/developer/patches/, which I know is not
> > > > a review system. Apparently, I was not patient enough, not waiting more
> > > > than a week with that regression fix for a single reply to my initial
> > >
> > > I'll disagree with you there: this is not a regression fix. A regression
> > > fix implies that it was something that worked before, a change happened
> > > to the kernel, and now it's broken.
> > >
> > > That is not the case: a change happened to binutils
> >
> > Not to mine, I'm still using the same version as before.
> >
> > > which meant 'size'
> > > no longer parses foreign ELF formats.
> >
> > But 'size' was not there before v3.2-rc1! The problem has arised because
> > arch/arm/boot/compress/Makefile, which belongs to the kernel sources,
> > not to binutils you're trying to blame, was changed. The 'size' call was
> > introduced into that Makefile, i.e. the file that belongs to the kernel
> > sources, whithout taking care of what can happen if that command fails.
> > And I was just lucky enough to find out it may fail in certains build
> > environments, like mine.
>
> OK let's stop arguing in circles please.
>
> I introduced the call to 'size' so by definition I'm the one who broke
> the build for you.
I don't care who broke it, this might happen to anyone, including me.
Now I see my fault was not examining the linux-arm-kernel archives a few
months deep before submitting my patch, which was apparently a
(independently developed) duplicate for the one already submitted by
Thomas Weber 6 weeks earlier.
> However I don't have any problem using the native 'size' command with a
> foreign architecture binary. My binutils version is 2.21.51.0.6 on
> X86-64. And clearly I'm not alone in that situation otherwise we would
> have been overwhelmed by complaints from a much bigger crowd by now. So
> your installation must be "special".
2.20.1.20100303 on X86-64 here, built with --enable-targets=all for a
reason I'm not able to recall now.
> OTOH all cross-compiler installations I've seen had $(CROSS_COMPILE)size
> available. It is therefore well possible that Russell's cross compiler
> installation is also "special".
>
> Given that the build works for the vast majority of people now, there is
> a risk that applying your first patch might break the build for more
> people (currently only one known) than the number of people for whom
> this is an improvement (currently only one known).
Tony already told me the same, and I came out with v2 as a solution.
> I suggested a simple fix for Russell's installation already:
>
> ln -s size arm-linux-size
>
> I don't know what is special about yours though, and maybe it is worth
> investigating given that, as I said, you appear to be alone so far
> having problems with the current Makefile which is rather intriguing.
> If it is clear with references to facts (binutils release notes, etc.)
> that the problem is something that will persist in the future then we
> have a more solid basis for choosing either of your solutions.
> Otherwise we can't call it a "regression" just yet.
After Tony's comments I changed my approach in that the core regression
here was not calling the native 'size', but passing an additional
argument to the linker without checking for its correctness first. The
core problem was not a broken 'size', but a broken linker command after
all. Broken 'size' consequences should be limited to at most making the
change your patch was about ineffective, but not the whole compilation
broken. That's what my v2 was about in the first place.
YMMV.
Thanks,
Janusz
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] ARM: Fix cross compilation broken by failing size command
2011-12-16 10:42 ` Janusz Krzysztofik
@ 2011-12-19 8:27 ` Igor Grinberg
-1 siblings, 0 replies; 21+ messages in thread
From: Igor Grinberg @ 2011-12-19 8:27 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Russell King, Nicolas Pitre, Tony Lindgren, linux-kernel,
linux-arm-kernel
Hi Janusz,
On 12/16/11 12:42, Janusz Krzysztofik wrote:
> Since commit 5ffb04f6690d71fab241b3562ebf52b893ac4ff1, "ARM: zImage:
> make sure appended DTB doesn't get overwritten by kernel .bss", the
> native 'size' command, which is now always used for calculation of the
> kernel bss size, may break in selected cross compilation environments
> with error messages like:
>
> size: arch/arm/boot/compressed/../../../../vmlinux: File format is ambiguous
> size: Matching formats: elf32-littlearm elf32-littlearm-symbian elf32-littlearm-vxworks
>
> As a consequence, the KBSS_SZ variable extracted from the size output is
> empty, and the the final linker command, provided with incorrectly
> formatted arguments, also fails:
>
> LD arch/arm/boot/compressed/vmlinux
> arm-angstrom-linux-uclibcgnueabi-ld:--defsym _kernel_bss_size=: syntax error
>
> Don't append the '_kernel_bss_size=$(KBSS_SZ)' argument to the linker
> command line if that variable is empty because of the failing size
> command. Moreover, use $(CROSS_COMPILE)size if available instead of
> native size.
>
> Created and tested against linux-3.2-rc5.
>
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
> ---
Tested-by: Igor Grinberg <grinberg@compulab.co.il>
Thanks for the patch, it fixes the build for me, so you are
not the only one having this issue...
I currently, use CodeSourcery 2010q1, but have multiple
cross tool chains installed.
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH] ARM: Fix cross compilation broken by failing size command
@ 2011-12-19 8:27 ` Igor Grinberg
0 siblings, 0 replies; 21+ messages in thread
From: Igor Grinberg @ 2011-12-19 8:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi Janusz,
On 12/16/11 12:42, Janusz Krzysztofik wrote:
> Since commit 5ffb04f6690d71fab241b3562ebf52b893ac4ff1, "ARM: zImage:
> make sure appended DTB doesn't get overwritten by kernel .bss", the
> native 'size' command, which is now always used for calculation of the
> kernel bss size, may break in selected cross compilation environments
> with error messages like:
>
> size: arch/arm/boot/compressed/../../../../vmlinux: File format is ambiguous
> size: Matching formats: elf32-littlearm elf32-littlearm-symbian elf32-littlearm-vxworks
>
> As a consequence, the KBSS_SZ variable extracted from the size output is
> empty, and the the final linker command, provided with incorrectly
> formatted arguments, also fails:
>
> LD arch/arm/boot/compressed/vmlinux
> arm-angstrom-linux-uclibcgnueabi-ld:--defsym _kernel_bss_size=: syntax error
>
> Don't append the '_kernel_bss_size=$(KBSS_SZ)' argument to the linker
> command line if that variable is empty because of the failing size
> command. Moreover, use $(CROSS_COMPILE)size if available instead of
> native size.
>
> Created and tested against linux-3.2-rc5.
>
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@tis.icnet.pl>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
> ---
Tested-by: Igor Grinberg <grinberg@compulab.co.il>
Thanks for the patch, it fixes the build for me, so you are
not the only one having this issue...
I currently, use CodeSourcery 2010q1, but have multiple
cross tool chains installed.
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 21+ messages in thread