diff mbox

[U-Boot] Makefile: use u-boot.map for binary_size_check

Message ID 1406179627-9496-1-git-send-email-judge.packham@gmail.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Chris Packham July 24, 2014, 5:27 a.m. UTC
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.
>>
>> http://patchwork.ozlabs.org/patch/371936/
>>
>
> 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(-)

Comments

Wolfgang Denk July 24, 2014, 8:31 a.m. UTC | #1
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
Tom Rini July 24, 2014, 1:50 p.m. UTC | #2
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.
Chris Packham July 24, 2014, 9:08 p.m. UTC | #3
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
Tom Rini Aug. 10, 2014, 10:22 p.m. UTC | #4
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!
diff mbox

Patch

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 \