[U-Boot] [PATCH v4 1/2] dlmalloc: fix malloc range at end of ram

[Tom Rini]

On Mon, Apr 29, 2019 at 03:06:39PM +0200, Heiko Schocher wrote:
> Hello Simon,
> 
> Am 25.04.2019 um 21:24 schrieb Simon Goldschmidt:
> >Am 25.04.2019 um 12:50 schrieb Tom Rini:
> >>On Thu, Apr 25, 2019 at 09:32:22AM +0200, Simon Goldschmidt wrote:
> >>>On Thu, Apr 25, 2019 at 1:59 AM Simon Glass <sjg at chromium.org> wrote:
> >>>>
> >>>>Hi,
> >>>>
> >>>>On Wed, 24 Apr 2019 at 05:53, Tom Rini <trini at konsulko.com> wrote:
> >>>>>
> >>>>>On Wed, Apr 24, 2019 at 01:49:52PM +0200, Simon Goldschmidt wrote:
> >>>>>>On Wed, Apr 24, 2019 at 1:27 PM Tom Rini <trini at konsulko.com> wrote:
> >>>>>>>
> >>>>>>>On Tue, Apr 23, 2019 at 09:54:10PM -0600, Simon Glass wrote:
> >>>>>>>>On Mon, 1 Apr 2019 at 14:01, Simon Goldschmidt
> >>>>>>>><simon.k.r.goldschmidt at gmail.com> wrote:
> >>>>>>>>>
> >>>>>>>>>If the malloc range passed to mem_malloc_init() is at the end of address
> >>>>>>>>>range and 'start + size' overflows to 0, following allocations fail as
> >>>>>>>>>mem_malloc_end is zero (which looks like uninitialized).
> >>>>>>>>>
> >>>>>>>>>Fix this by subtracting 1 of 'start + size' overflows to zero.
> >>>>>>>>>
> >>>>>>>>>Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> >>>>>>>>>---
> >>>>>>>>>
> >>>>>>>>>Changes in v4: None
> >>>>>>>>>Changes in v3: None
> >>>>>>>>>
> >>>>>>>>>  common/dlmalloc.c | 4 ++++
> >>>>>>>>>  1 file changed, 4 insertions(+)
> >>>>>>>>
> >>>>>>>>Reviewed-by: Simon Glass <sjg at chromium.org>
> >>>>>>>
> >>>>>>>So, the problem with this patch is that it increases the generic malloc
> >>>>>>>code size ever so slightly and blows up smartweb :(
> >>>>>>
> >>>>>>Ehrm, ok, so how do we proceed?
> >>>>>
> >>>>>A good question.  Take a look at spl/u-boot-spl.map on smartweb and see
> >>>>>if, of the malloc functions it doesn't discard there's something that
> >>>>>maybe could be optimized somewhere?
> >>>>
> >>>>I wonder if we should have a Kconfig option like SPL_CHECKS which
> >>>>enables these sorts of minor checks, which may only fix one board at
> >>>>the cost of code size?
> >>>>
> >>>>Then it could be enabled by default, but disabled on this board?
> >>>
> >>>For a bigger change, this might be an idea, but for a change that I can cut
> >>>down to 16 or even 8 bytes code size increasement, I don't think having a
> >>>new option would be good.
> >>>
> >>>Anyway, I just tried at work and I don't get the overflow. Tom, which gcc
> >>>are you using to get the size error? It works for me on Debian 9 but doesn't
> >>>work with Ubuntu (both times, default cross compiler toolchain installed).
> >>
> >>I'm using the gcc-7.3 from kernel.org that we use in travis/etc.
> >
> >Ok, so I have gcc-7.3 on my Ubuntu machine as well. I don't know why 6.3
> >seems to produce smaller binaries (I thought they were getting smaller
> >with new versions, not larger).
> >
> >However, I've stripped down that patch to +8 Bytes only and sent v5.
> 
> Thanks!
> 
> Sorry for digging so late in, but I was on vacation...
> 
> Hmm.. the smartweb board has only 4k sram for SPL, and I have no chance
> to convert it to DM to get rid of some compiler warnings ...
> 
> I am unsure what to do now with this hardware ...

Well, with regards to SPL + DM, this is one of the cases wherein we just
have-to allow for the SPL driver code at least to be a one-off.  If the
"whatever ROM loads of our code" stage can set things up enough such
that we can hand off to a full U-Boot, that's great.  If not, this is
then a case where TPL comes in to play, and that really is as one-off as
needed, to load a more general SPL and so forth.

But, I'm fine with saying smartweb keeps and maintains whatever SPL code
it needs to use.  It's just that in this case, it's not at all a DM
thing, it's a change in malloc.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190429/db4a845c/attachment.sig>