[U-Boot] [PATCH] Makefile: use $(shell ...) for determining file_size
file_size was being calculated using back-ticks but map_size uses $(shell ...). Update the file_size calculation to use $(shell ...).
Signed-off-by: Chris Packham judge.packham@gmail.com --- The back ticks didn't work in my environment (GNU Make 3.81). Updating to use $(shell ...) makes sense from a consistency view even if the problem is my environment.
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index ca212b5..f6a0f68 100644 --- a/Makefile +++ b/Makefile @@ -786,7 +786,7 @@ u-boot.hex u-boot.srec: u-boot FORCE OBJCOPYFLAGS_u-boot.bin := -O binary
binary_size_check: u-boot.bin System.map FORCE - @file_size=`stat -c %s u-boot.bin` ; \ + file_size=$(shell stat -c %s u-boot.bin) ; \ map_size=$(shell cat System.map | \ awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \ | bc); \
On 22 July 2014 18:08, Chris Packham judge.packham@gmail.com wrote:
file_size was being calculated using back-ticks but map_size uses $(shell ...). Update the file_size calculation to use $(shell ...).
Signed-off-by: Chris Packham judge.packham@gmail.com
Acked-by: Simon Glass sjg@chromium.org
But you might want to look at this.
http://patchwork.ozlabs.org/patch/371936/
Regards, Simon
On Wed, Jul 23, 2014 at 04:27:36AM -0600, Simon Glass wrote:
On 22 July 2014 18:08, Chris Packham judge.packham@gmail.com wrote:
file_size was being calculated using back-ticks but map_size uses $(shell ...). Update the file_size calculation to use $(shell ...).
Signed-off-by: Chris Packham judge.packham@gmail.com
Acked-by: Simon Glass sjg@chromium.org
But you might want to look at this.
Yeah, Jeroen, can we get a v2 of your patch that uses $(shell ... )? Thanks!
Hi,
On 23-07-14 20:03, Tom Rini wrote:
On Wed, Jul 23, 2014 at 04:27:36AM -0600, Simon Glass wrote:
On 22 July 2014 18:08, Chris Packham judge.packham@gmail.com wrote:
file_size was being calculated using back-ticks but map_size uses $(shell ...). Update the file_size calculation to use $(shell ...).
Signed-off-by: Chris Packham judge.packham@gmail.com
Acked-by: Simon Glass sjg@chromium.org
But you might want to look at this.
Yeah, Jeroen, can we get a v2 of your patch that uses $(shell ... )? Thanks!
no problem. For the record I have not seen any issues with mentioned gmake version. But using $(shell .. ) seem like a sane thing to do. v2 is on its way, running MAKEALL for arm (can we keep it?).
Chris: blackfin targets seem to rely on the same construct btw...
Regards, Jeroen
On Wed, Jul 23, 2014 at 09:24:00PM +0200, Jeroen Hofstee wrote:
Hi,
On 23-07-14 20:03, Tom Rini wrote:
On Wed, Jul 23, 2014 at 04:27:36AM -0600, Simon Glass wrote:
On 22 July 2014 18:08, Chris Packham judge.packham@gmail.com wrote:
file_size was being calculated using back-ticks but map_size uses $(shell ...). Update the file_size calculation to use $(shell ...).
Signed-off-by: Chris Packham judge.packham@gmail.com
Acked-by: Simon Glass sjg@chromium.org
But you might want to look at this.
Yeah, Jeroen, can we get a v2 of your patch that uses $(shell ... )? Thanks!
no problem. For the record I have not seen any issues with mentioned gmake version. But using $(shell .. ) seem like a sane thing to do. v2 is on its way, running MAKEALL for arm (can we keep it?).
MAKEALL isn't going away for v2014.10 but please reply to Simon's RFC about it. I do have to admit that switching all of my stuff to buildman is still on my TODO list.
Hi Simon,
On Wed, Jul 23, 2014 at 10:27 PM, Simon Glass sjg@chromium.org wrote:
On 22 July 2014 18:08, Chris Packham judge.packham@gmail.com wrote:
file_size was being calculated using back-ticks but map_size uses $(shell ...). Update the file_size calculation to use $(shell ...).
Signed-off-by: Chris Packham judge.packham@gmail.com
Acked-by: Simon Glass sjg@chromium.org
But you might want to look at this.
Thanks. I've re-submitted a version with Jeroen's change included and it seems a v2 of his patch is imminent too. Either way I'm not fussed.
But this has highlighted another issue I'm experiencing related to this code. Originally I thought it was because I was doing something a bit weird with a custom u-boot.lds but actually the problem is my u-boot is setup so that it finishes exactly at the end of a 32bit address space so _image_binary_end is just off the end of it which screws up the size calculation.
$ grep _image_binary_end u-boot.map 0x0000000100000000 _image_binary_end = .
$ grep _image_binary_end System.map 00000000 A _image_binary_end
$ make binary_size_check ... System.map shows a binary size of -4294180864 but u-boot.bin shows 786432 make: *** [binary_size_check] Error 1
I think it should be possible to change binary_size_check to use u-boot.map instead of System.map but would that be OK for all architectures?
u-boot.map is generated automatically by the compiler and more importantly can handle addresses >4GB. --- On Thu, Jul 24, 2014 at 5:14 PM, Chris Packham judge.packham@gmail.com wrote:
Hi Simon,
On Wed, Jul 23, 2014 at 10:27 PM, Simon Glass sjg@chromium.org wrote:
On 22 July 2014 18:08, Chris Packham judge.packham@gmail.com wrote:
file_size was being calculated using back-ticks but map_size uses $(shell ...). Update the file_size calculation to use $(shell ...).
Signed-off-by: Chris Packham judge.packham@gmail.com
Acked-by: Simon Glass sjg@chromium.org
But you might want to look at this.
Thanks. I've re-submitted a version with Jeroen's change included and it seems a v2 of his patch is imminent too. Either way I'm not fussed.
But this has highlighted another issue I'm experiencing related to this code. Originally I thought it was because I was doing something a bit weird with a custom u-boot.lds but actually the problem is my u-boot is setup so that it finishes exactly at the end of a 32bit address space so _image_binary_end is just off the end of it which screws up the size calculation.
$ grep _image_binary_end u-boot.map 0x0000000100000000 _image_binary_end = .
$ grep _image_binary_end System.map 00000000 A _image_binary_end
$ make binary_size_check ... System.map shows a binary size of -4294180864 but u-boot.bin shows 786432 make: *** [binary_size_check] Error 1
I think it should be possible to change binary_size_check to use u-boot.map instead of System.map but would that be OK for all architectures?
Something like this works for me but maybe there is a way to get nm to handle addresses >4GB.
Makefile | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile index b98a80a..3ed02a4 100644 --- a/Makefile +++ b/Makefile @@ -785,14 +785,15 @@ u-boot.hex u-boot.srec: u-boot FORCE
OBJCOPYFLAGS_u-boot.bin := -O binary
-binary_size_check: u-boot.bin System.map FORCE +binary_size_check: u-boot.bin FORCE @file_size=$(shell wc -c u-boot.bin | awk '{print $$1}') ; \ - map_size=$(shell cat System.map | \ + map_size=$(shell cat u-boot.map | \ awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \ + | sed 's/0X//g' \ | bc); \ if [ "" != "$$map_size" ]; then \ if test $$map_size -ne $$file_size; then \ - echo "System.map shows a binary size of $$map_size" >&2 ; \ + echo "u-boot.map shows a binary size of $$map_size" >&2 ; \ echo " but u-boot.bin shows $$file_size" >&2 ; \ exit 1; \ fi \
Dear Chris Packham,
In message 1406179627-9496-1-git-send-email-judge.packham@gmail.com you wrote:
u-boot.map is generated automatically by the compiler and more importantly can handle addresses >4GB.
...
I think it should be possible to change binary_size_check to use u-boot.map instead of System.map but would that be OK for all architectures?
...
Something like this works for me but maybe there is a way to get nm to handle addresses >4GB.
...
- map_size=$(shell cat u-boot.map | \ awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \
| sed 's/0X//g' \
Do we _really_ need all of this?
This looks very much like creaping featurism to me...
Best regards,
Wolfgang Denk
On Thu, Jul 24, 2014 at 10:31:14AM +0200, Wolfgang Denk wrote:
Dear Chris Packham,
In message 1406179627-9496-1-git-send-email-judge.packham@gmail.com you wrote:
u-boot.map is generated automatically by the compiler and more importantly can handle addresses >4GB.
...
I think it should be possible to change binary_size_check to use u-boot.map instead of System.map but would that be OK for all architectures?
...
Something like this works for me but maybe there is a way to get nm to handle addresses >4GB.
...
- map_size=$(shell cat u-boot.map | \ awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \
| sed 's/0X//g' \Do we _really_ need all of this?
This looks very much like creaping featurism to me...
So it's a safetey check. The various linker script clean-ups we've been doing have introduced a problem from time to time and binary_check_size gives a sanity check. This just fixes a corner case in the test.
Hi Wolfgang, Tom,
On Fri, Jul 25, 2014 at 1:50 AM, Tom Rini trini@ti.com wrote:
On Thu, Jul 24, 2014 at 10:31:14AM +0200, Wolfgang Denk wrote:
Dear Chris Packham,
In message 1406179627-9496-1-git-send-email-judge.packham@gmail.com you wrote:
u-boot.map is generated automatically by the compiler and more importantly can handle addresses >4GB.
...
I think it should be possible to change binary_size_check to use u-boot.map instead of System.map but would that be OK for all architectures?
...
Something like this works for me but maybe there is a way to get nm to handle addresses >4GB.
...
- map_size=$(shell cat u-boot.map | \ awk '/_image_copy_start/ {start = $$1} /_image_binary_end/ {end = $$1} END {if (start != "" && end != "") print "ibase=16; " toupper(end) " - " toupper(start)}' \
| sed 's/0X//g' \Do we _really_ need all of this?
This looks very much like creaping featurism to me...
Do we need it? Maybe not I'd defer that judgement to you.
My problem is binary_size_check is on by default and my particular platform (an out of tree ARM board) fails to build because of it. If there was a way of opting out I'd happily use it.
So it's a safetey check. The various linker script clean-ups we've been doing have introduced a problem from time to time and binary_check_size gives a sanity check. This just fixes a corner case in the test.
I wouldn't say this is a corner case. Certainly for the ARM target I'm building it is unusual but I know plenty of powerpc boards would hit the same problem becuase they have a jump instruction at the top of their address space.
-- Tom
On Thu, Jul 24, 2014 at 05:27:07PM +1200, Chris Packham wrote:
u-boot.map is generated automatically by the compiler and more importantly can handle addresses >4GB.
Applied to u-boot/master, thanks!
Hi Chris,
On 24 July 2014 06:14, Chris Packham judge.packham@gmail.com wrote:
Hi Simon,
On Wed, Jul 23, 2014 at 10:27 PM, Simon Glass sjg@chromium.org wrote:
On 22 July 2014 18:08, Chris Packham judge.packham@gmail.com wrote:
file_size was being calculated using back-ticks but map_size uses $(shell ...). Update the file_size calculation to use $(shell ...).
Signed-off-by: Chris Packham judge.packham@gmail.com
Acked-by: Simon Glass sjg@chromium.org
But you might want to look at this.
Thanks. I've re-submitted a version with Jeroen's change included and it seems a v2 of his patch is imminent too. Either way I'm not fussed.
But this has highlighted another issue I'm experiencing related to this code. Originally I thought it was because I was doing something a bit weird with a custom u-boot.lds but actually the problem is my u-boot is setup so that it finishes exactly at the end of a 32bit address space so _image_binary_end is just off the end of it which screws up the size calculation.
$ grep _image_binary_end u-boot.map 0x0000000100000000 _image_binary_end = .
$ grep _image_binary_end System.map 00000000 A _image_binary_end
$ make binary_size_check ... System.map shows a binary size of -4294180864 but u-boot.bin shows 786432 make: *** [binary_size_check] Error 1
I think it should be possible to change binary_size_check to use u-boot.map instead of System.map but would that be OK for all architectures?
That's unfortunate - it's just a little check. It does protect again a breakage but we need to keep it simple. Maybe in this case we can mask with 0xffffffff and get the right answer?
Regards, Simon
participants (5)
-
Chris Packham -
Jeroen Hofstee -
Simon Glass -
Tom Rini -
Wolfgang Denk