Patchwork [U-Boot] spl: remove forced linking of commands into SPL

login
register
mail settings
Submitter Tyler Olmstead
Date Aug. 9, 2012, 2:24 a.m.
Message ID <1344479053-18404-1-git-send-email-tyler.j.olmstead@gmail.com>
Download mbox | patch
Permalink /patch/175989/
State Accepted
Commit be7e41efdab086d15108020de8eeada8e7baac8d
Delegated to: Tom Rini
Headers show

Comments

Tyler Olmstead - Aug. 9, 2012, 2:24 a.m.
Remove linker command line options from the SPL makefile
that force the inclusion of unreferenced command code from
linked object files. As commands are not used in the SPL,
these options resulted in an unnecessary increase in the
image size, in addition to introducing the possibility of
tricky link errors in the case where the command code
contained symbols that were not resolved by linking in the
limited objects compiled in the SPL build.

Signed-off-by: Tyler Olmstead <tyler.j.olmstead@gmail.com>
---
 spl/Makefile |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
Tom Rini - Aug. 9, 2012, 2:35 p.m.
On 08/08/2012 07:24 PM, Tyler Olmstead wrote:

> Remove linker command line options from the SPL makefile
> that force the inclusion of unreferenced command code from
> linked object files. As commands are not used in the SPL,
> these options resulted in an unnecessary increase in the
> image size, in addition to introducing the possibility of
> tricky link errors in the case where the command code
> contained symbols that were not resolved by linking in the
> limited objects compiled in the SPL build.
> 
> Signed-off-by: Tyler Olmstead <tyler.j.olmstead@gmail.com>

Without either figuring out how to make the strings be garbage collected
automatically (I'm talking with a toolchain friend of mine about how to
try and do this) or whacking U_BOOT_CMD to do this for us, we will start
adding bloat to SPL after we do this.
Tyler Olmstead - Aug. 9, 2012, 5:37 p.m.
Hi Tom,

On Thu, Aug 9, 2012 at 7:35 AM, Tom Rini <trini@ti.com> wrote:
> On 08/08/2012 07:24 PM, Tyler Olmstead wrote:
>
>> Remove linker command line options from the SPL makefile
>> that force the inclusion of unreferenced command code from
>> linked object files. As commands are not used in the SPL,
>> these options resulted in an unnecessary increase in the
>> image size, in addition to introducing the possibility of
>> tricky link errors in the case where the command code
>> contained symbols that were not resolved by linking in the
>> limited objects compiled in the SPL build.
>>
>> Signed-off-by: Tyler Olmstead <tyler.j.olmstead@gmail.com>
>
> Without either figuring out how to make the strings be garbage collected
> automatically (I'm talking with a toolchain friend of mine about how to
> try and do this) or whacking U_BOOT_CMD to do this for us, we will start
> adding bloat to SPL after we do this.
>
> --
> Tom

I absolutely agree that bloat in the SPL should be avoided wherever
possible, but I don't understand how this patch would increase the
image size by *not* forcing commands to be linked in. There are
numerous examples where the linker is relied upon to not include
unused code into SPL. Is there something unique about U_BOOT_CMD that
makes it especially troublesome?

Thanks for your insight.
-- Tyler
Tom Rini - Aug. 9, 2012, 5:55 p.m.
On Thu, Aug 09, 2012 at 10:37:43AM -0700, Tyler Olmstead wrote:
> Hi Tom,
> 
> On Thu, Aug 9, 2012 at 7:35 AM, Tom Rini <trini@ti.com> wrote:
> > On 08/08/2012 07:24 PM, Tyler Olmstead wrote:
> >
> >> Remove linker command line options from the SPL makefile
> >> that force the inclusion of unreferenced command code from
> >> linked object files. As commands are not used in the SPL,
> >> these options resulted in an unnecessary increase in the
> >> image size, in addition to introducing the possibility of
> >> tricky link errors in the case where the command code
> >> contained symbols that were not resolved by linking in the
> >> limited objects compiled in the SPL build.
> >>
> >> Signed-off-by: Tyler Olmstead <tyler.j.olmstead@gmail.com>
> >
> > Without either figuring out how to make the strings be garbage collected
> > automatically (I'm talking with a toolchain friend of mine about how to
> > try and do this) or whacking U_BOOT_CMD to do this for us, we will start
> > adding bloat to SPL after we do this.
> >
> 
> I absolutely agree that bloat in the SPL should be avoided wherever
> possible, but I don't understand how this patch would increase the
> image size by *not* forcing commands to be linked in. There are
> numerous examples where the linker is relied upon to not include
> unused code into SPL. Is there something unique about U_BOOT_CMD that
> makes it especially troublesome?

It doesn't bloat today, true.  It bloats as soon as someone else adds in
code that links in a new U_BOOT_CMD that wasn't guarded as now the
string literals that before would have been guarded and not linked in
will now be un-guarded and not garbage collected (while the command will
be).  The linker is in fact getting this wrong already in a number of
cases (run a 'strings' on an SPL binary) and this needs to be fixed too.
I don't want to make the problem potentially worse until we have a fix
for the general problem.

To see what I mean, build omap3_beagle with this patch and then un-guard
nandecc in arch/arm/cpu/armv7/omap3/board.c.  The command will be
collected but the command/help string will be in the binary.
Tom Rini - Aug. 9, 2012, 11 p.m.
On Wed, Aug 08, 2012 at 07:24:13PM -0700, Tyler Olmstead wrote:

> Remove linker command line options from the SPL makefile
> that force the inclusion of unreferenced command code from
> linked object files. As commands are not used in the SPL,
> these options resulted in an unnecessary increase in the
> image size, in addition to introducing the possibility of
> tricky link errors in the case where the command code
> contained symbols that were not resolved by linking in the
> limited objects compiled in the SPL build.
> 
> Signed-off-by: Tyler Olmstead <tyler.j.olmstead@gmail.com>

OK, I've done some more poking at the specific problem I found and,
well, we're doing everything right and I don't have a solution, but I
think this patch moves us forward.  Based on what I'm seeing so far,
new U_BOOT_CMDs would be removed correctly, including strings.  Or be
fixed when I fix the current problem.

Acked-by: Tom Rini <trini@ti.com>
Tyler Olmstead - Aug. 10, 2012, 12:42 a.m.
Thanks Tom. I've been looking at this some too, and while the commands
are removed from the build I've been playing with (hawkboard), they
seem to not be removed completely from omap3_beagle. This is confusing
as both use -ffunction-sections, -fdata-sections, and --gc-sections. I
noticed the hawkboard has it's own linker script for SPL builds, and
I'm wondering if this doesn't account for the difference.
spl/u-boot-spl.lds uses wild cards for data where u-boot-spl-hawk.lds
explicitly names the data sections to link. I'm far from an expert on
linker scripts, so I can't test this theory without fear of messing up
something.

-- Tyler

On Thu, Aug 9, 2012 at 4:00 PM, Tom Rini <trini@ti.com> wrote:
> On Wed, Aug 08, 2012 at 07:24:13PM -0700, Tyler Olmstead wrote:
>
>> Remove linker command line options from the SPL makefile
>> that force the inclusion of unreferenced command code from
>> linked object files. As commands are not used in the SPL,
>> these options resulted in an unnecessary increase in the
>> image size, in addition to introducing the possibility of
>> tricky link errors in the case where the command code
>> contained symbols that were not resolved by linking in the
>> limited objects compiled in the SPL build.
>>
>> Signed-off-by: Tyler Olmstead <tyler.j.olmstead@gmail.com>
>
> OK, I've done some more poking at the specific problem I found and,
> well, we're doing everything right and I don't have a solution, but I
> think this patch moves us forward.  Based on what I'm seeing so far,
> new U_BOOT_CMDs would be removed correctly, including strings.  Or be
> fixed when I fix the current problem.
>
> Acked-by: Tom Rini <trini@ti.com>
>
> --
> Tom
Tom Rini - Sept. 18, 2012, 7:04 p.m.
On Wed, Aug 08, 2012 at 04:24:13PM -0000, Tyler Olmstead wrote:

> Remove linker command line options from the SPL makefile
> that force the inclusion of unreferenced command code from
> linked object files. As commands are not used in the SPL,
> these options resulted in an unnecessary increase in the
> image size, in addition to introducing the possibility of
> tricky link errors in the case where the command code
> contained symbols that were not resolved by linking in the
> limited objects compiled in the SPL build.
> 
> Signed-off-by: Tyler Olmstead <tyler.j.olmstead@gmail.com>
> Acked-by: Tom Rini <trini@ti.com>

Applied to u-boot/master, thanks!

Patch

diff --git a/spl/Makefile b/spl/Makefile
index ea7d475..0c30a85 100644
--- a/spl/Makefile
+++ b/spl/Makefile
@@ -126,9 +126,7 @@  $(obj)u-boot-spl.bin:	$(obj)u-boot-spl
 	$(OBJCOPY) $(OBJCFLAGS) -O binary $< $@
 
 GEN_UBOOT = \
-	UNDEF_SYM=`$(OBJDUMP) -x $(LIBS) | \
-	sed  -n -e 's/.*\($(SYM_PREFIX)__u_boot_cmd_.*\)/-u\1/p'|sort|uniq`;\
-	cd $(obj) && $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) $$UNDEF_SYM $(__START) \
+	cd $(obj) && $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) $(__START) \
 		--start-group $(__LIBS) --end-group $(PLATFORM_LIBS) \
 		-Map u-boot-spl.map -o u-boot-spl