diff mbox

[U-Boot,4/5] arm: ensure u-boot only uses relative relocations

Message ID 1368561780-19104-5-git-send-email-albert.u.boot@aribaud.net
State Superseded
Delegated to: Albert ARIBAUD
Headers show

Commit Message

Albert ARIBAUD May 14, 2013, 8:02 p.m. UTC
Add a Makefile target ('checkarmreloc') which
fails of the ELF binary contains relocation records
of types other than R_ARM_RELATIVE.

Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
 Makefile           |    7 +++++++
 arch/arm/config.mk |    5 +++++
 2 files changed, 12 insertions(+)

Comments

Benoît Thébaudeau May 14, 2013, 10:12 p.m. UTC | #1
Hi Albert,

On Tuesday, May 14, 2013 10:02:59 PM, Albert ARIBAUD wrote:
> Add a Makefile target ('checkarmreloc') which
> fails of the ELF binary contains relocation records
> of types other than R_ARM_RELATIVE.
> 
> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> ---
>  Makefile           |    7 +++++++
>  arch/arm/config.mk |    5 +++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index c52f0f1..9aa5755 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
>  	$(MAKE) -C $@ all
>  endif	# config.mk
>  
> +# ARM relocations should all be R_ARM_RELATIVE.
> +checkarmreloc: $(obj)u-boot
> +	@if test "R_ARM_RELATIVE" != \
> +		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort -u`"; \
                             ^
                             or $$< to avoid a duplicate?

> +		then echo "$(obj)u-boot contains relocations other than \
                           ^
                           or $$< too, or no $(obj) prefix at all for this message?

> +		R_ARM_RELATIVE"; false; fi
> +
>  $(VERSION_FILE):
>  		@mkdir -p $(dir $(VERSION_FILE))
>  		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \
> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> index 461899e..5b7a602 100644
> --- a/arch/arm/config.mk
> +++ b/arch/arm/config.mk
> @@ -106,3 +106,8 @@ ifeq ($(GAS_BUG_12532),y)
>  PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
>  endif
>  endif
> +
> +# check that only R_ARM_RELATIVE relocations are generated
> +ifneq ($(CONFIG_SPL_BUILD),y)
> +ALL-y	+= checkarmreloc
> +endif
> --
> 1.7.10.4

Best regards,
Benoît
Albert ARIBAUD May 15, 2013, 7:46 a.m. UTC | #2
Hi Benoît,

On Wed, 15 May 2013 00:12:24 +0200 (CEST), Benoît Thébaudeau
<benoit.thebaudeau@advansee.com> wrote:

> Hi Albert,

> > --- a/Makefile
> > +++ b/Makefile
> > @@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
> >  	$(MAKE) -C $@ all
> >  endif	# config.mk
> >  
> > +# ARM relocations should all be R_ARM_RELATIVE.
> > +checkarmreloc: $(obj)u-boot
> > +	@if test "R_ARM_RELATIVE" != \
> > +		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort -u`"; \
>                              ^
>                              or $$< to avoid a duplicate?

Will fix as suggested.

> > +		then echo "$(obj)u-boot contains relocations other than \
>                            ^
>                            or $$< too, or no $(obj) prefix at all for this message?

I prefer leaving the prefix so that failures during out-of-tree builds
or during MAKEALL builds with BUILD_NBUILDS>1 log the correct path.

> Best regards,
> Benoît

Thanks!

Amicalement,
Albert ARIBAUD May 15, 2013, 9:38 a.m. UTC | #3
Hi again Benoît,

On Wed, 15 May 2013 09:46:17 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:

> Hi Benoît,
> 
> On Wed, 15 May 2013 00:12:24 +0200 (CEST), Benoît Thébaudeau
> <benoit.thebaudeau@advansee.com> wrote:
> 
> > Hi Albert,
> 
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
> > >  	$(MAKE) -C $@ all
> > >  endif	# config.mk
> > >  
> > > +# ARM relocations should all be R_ARM_RELATIVE.
> > > +checkarmreloc: $(obj)u-boot
> > > +	@if test "R_ARM_RELATIVE" != \
> > > +		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort -u`"; \
> >                              ^
> >                              or $$< to avoid a duplicate?
> 
> Will fix as suggested.

> > > +		then echo "$(obj)u-boot contains relocations other than \
> >                            ^
> >                            or $$< too, or no $(obj) prefix at all for this message?
> 
> I prefer leaving the prefix so that failures during out-of-tree builds
> or during MAKEALL builds with BUILD_NBUILDS>1 log the correct path.

Actually $$< does not work within backquotes unless escaped as a less
legible \$\$<, and does not work properly at all within double quotes,
whether escaped or not.

Do you prefer that I change only the first $(obj)u-boot into \$\$< and
leave the second one untouched, or that I leave both $(obj)u-boot
instances as-is for the sake of homogeneity?

> > Best regards,
> > Benoît

Amicalement,
Benoît Thébaudeau May 15, 2013, 1:49 p.m. UTC | #4
Hi Albert,

On Wednesday, May 15, 2013 11:38:37 AM, Albert ARIBAUD wrote:
> Hi again Benoît,
> 
> On Wed, 15 May 2013 09:46:17 +0200, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
> 
> > Hi Benoît,
> > 
> > On Wed, 15 May 2013 00:12:24 +0200 (CEST), Benoît Thébaudeau
> > <benoit.thebaudeau@advansee.com> wrote:
> > 
> > > Hi Albert,
> > 
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
> > > >  	$(MAKE) -C $@ all
> > > >  endif	# config.mk
> > > >  
> > > > +# ARM relocations should all be R_ARM_RELATIVE.
> > > > +checkarmreloc: $(obj)u-boot
> > > > +	@if test "R_ARM_RELATIVE" != \
> > > > +		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort
> > > > -u`"; \
> > >                              ^
> > >                              or $$< to avoid a duplicate?
> > 
> > Will fix as suggested.
> 
> > > > +		then echo "$(obj)u-boot contains relocations other than \
> > >                            ^
> > >                            or $$< too, or no $(obj) prefix at all for
> > >                            this message?
> > 
> > I prefer leaving the prefix so that failures during out-of-tree builds
> > or during MAKEALL builds with BUILD_NBUILDS>1 log the correct path.
> 
> Actually $$< does not work within backquotes unless escaped as a less
> legible \$\$<, and does not work properly at all within double quotes,
> whether escaped or not.
> 
> Do you prefer that I change only the first $(obj)u-boot into \$\$< and
> leave the second one untouched, or that I leave both $(obj)u-boot
> instances as-is for the sake of homogeneity?

Actually, a single dollar sign (i.e. "$<") would be needed since it must have
been expanded by make before reaching the shell, and no shell backslash escape
sequences should be required.

If this still does not pass smoothly, then I prefer simplicity and homogeneity.

Best regards,
Benoît
Albert ARIBAUD May 15, 2013, 3:01 p.m. UTC | #5
Hi Benoît,

On Wed, 15 May 2013 15:49:10 +0200 (CEST), Benoît Thébaudeau
<benoit.thebaudeau@advansee.com> wrote:

> Hi Albert,
> 
> On Wednesday, May 15, 2013 11:38:37 AM, Albert ARIBAUD wrote:
> > Hi again Benoît,
> > 
> > On Wed, 15 May 2013 09:46:17 +0200, Albert ARIBAUD
> > <albert.u.boot@aribaud.net> wrote:
> > 
> > > Hi Benoît,
> > > 
> > > On Wed, 15 May 2013 00:12:24 +0200 (CEST), Benoît Thébaudeau
> > > <benoit.thebaudeau@advansee.com> wrote:
> > > 
> > > > Hi Albert,
> > > 
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -746,6 +746,13 @@ tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
> > > > >  	$(MAKE) -C $@ all
> > > > >  endif	# config.mk
> > > > >  
> > > > > +# ARM relocations should all be R_ARM_RELATIVE.
> > > > > +checkarmreloc: $(obj)u-boot
> > > > > +	@if test "R_ARM_RELATIVE" != \
> > > > > +		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort
> > > > > -u`"; \
> > > >                              ^
> > > >                              or $$< to avoid a duplicate?
> > > 
> > > Will fix as suggested.
> > 
> > > > > +		then echo "$(obj)u-boot contains relocations other than \
> > > >                            ^
> > > >                            or $$< too, or no $(obj) prefix at all for
> > > >                            this message?
> > > 
> > > I prefer leaving the prefix so that failures during out-of-tree builds
> > > or during MAKEALL builds with BUILD_NBUILDS>1 log the correct path.
> > 
> > Actually $$< does not work within backquotes unless escaped as a less
> > legible \$\$<, and does not work properly at all within double quotes,
> > whether escaped or not.
> > 
> > Do you prefer that I change only the first $(obj)u-boot into \$\$< and
> > leave the second one untouched, or that I leave both $(obj)u-boot
> > instances as-is for the sake of homogeneity?
> 
> Actually, a single dollar sign (i.e. "$<") would be needed since it must have
> been expanded by make before reaching the shell, and no shell backslash escape
> sequences should be required.
> 
> If this still does not pass smoothly, then I prefer simplicity and homogeneity.

Single unescaped $< works like a charm within backward as well as double
quotes, thanks!

> Best regards,
> Benoît

Amicalement,
diff mbox

Patch

diff --git a/Makefile b/Makefile
index c52f0f1..9aa5755 100644
--- a/Makefile
+++ b/Makefile
@@ -746,6 +746,13 @@  tools: $(VERSION_FILE) $(TIMESTAMP_FILE)
 	$(MAKE) -C $@ all
 endif	# config.mk
 
+# ARM relocations should all be R_ARM_RELATIVE.
+checkarmreloc: $(obj)u-boot
+	@if test "R_ARM_RELATIVE" != \
+		"`readelf -r $(obj)u-boot | cut -d ' ' -f 4 | grep R_ARM | sort -u`"; \
+		then echo "$(obj)u-boot contains relocations other than \
+		R_ARM_RELATIVE"; false; fi
+
 $(VERSION_FILE):
 		@mkdir -p $(dir $(VERSION_FILE))
 		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \
diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index 461899e..5b7a602 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -106,3 +106,8 @@  ifeq ($(GAS_BUG_12532),y)
 PLATFORM_RELFLAGS += -fno-optimize-sibling-calls
 endif
 endif
+
+# check that only R_ARM_RELATIVE relocations are generated
+ifneq ($(CONFIG_SPL_BUILD),y)
+ALL-y	+= checkarmreloc
+endif