Patchwork [U-Boot] sandbox: Crash on startup

login
register
mail settings
Submitter Matthias Weisser
Date Nov. 1, 2011, 9:50 a.m.
Message ID <4EAFC0D8.6030407@arcor.de>
Download mbox | patch
Permalink /patch/123050/
State Superseded
Delegated to: Mike Frysinger
Headers show

Comments

Matthias Weisser - Nov. 1, 2011, 9:50 a.m.
Dear Simon

I just wanted to play around with the sandbox "arch" of u-boot maybe
adding tun/tap support. Current head compiled successfully but crashed
immediately after startup in board_init_f:

	gd = malloc(sizeof(gd_t));
	assert(gd);

	memset((void *)gd, 0, sizeof(gd_t));

The simple reason was that malloc refers to u-boots internal malloc
which is not initialized at this point. I added the following snippet

building u-boot from current head? Wouldn't it be a better approach to
use the internal malloc of u-boot and acquire some memory from the
system using mmap?

Regards
Matthias
Simon Glass - Nov. 1, 2011, 2:01 p.m.
Mi Matthias,

On Tue, Nov 1, 2011 at 2:50 AM, Matthias Weisser <weisserm@arcor.de> wrote:
> Dear Simon
>
> I just wanted to play around with the sandbox "arch" of u-boot maybe
> adding tun/tap support. Current head compiled successfully but crashed
> immediately after startup in board_init_f:
>
>        gd = malloc(sizeof(gd_t));
>        assert(gd);
>
>        memset((void *)gd, 0, sizeof(gd_t));
>
> The simple reason was that malloc refers to u-boots internal malloc
> which is not initialized at this point. I added the following snippet

That is odd. This commit is supposed to switch over to the system malloc():

fe34107 sandbox: Disable built-in malloc

Can you please check that it is there?

>
> diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
> index 685793e..c98ca61 100644
> --- a/arch/sandbox/cpu/start.c
> +++ b/arch/sandbox/cpu/start.c
> @@ -21,8 +21,12 @@
>
>  #include <common.h>
>
> +static uint8_t malloc_area[1024*1024*256];
> +
>  int main(int argc, char *argv[])
>  {
> +    mem_malloc_init(malloc_area, sizeof(malloc_area));
> +
>        /*
>
> and got the console working. Can you tell me what I am doing wrong when
> building u-boot from current head? Wouldn't it be a better approach to
> use the internal malloc of u-boot and acquire some memory from the
> system using mmap?
>

I would first check that dlmalloc.o is not being linked, and that
mem_malloc_init() is undefined.

The current setup works OK for me, but yes it would be nice use
U-Boot's internal malloc(). The initial effort was to get a baseline
implementation into U-Boot and there are a number of areas where it
can be expanded. We want to test as much as possible of the code, and
dlmalloc is no exception.

Regards,
Simon

> Regards
> Matthias
>
Matthias Weisser - Nov. 1, 2011, 2:18 p.m.
Am 01.11.2011 15:01, schrieb Simon Glass:
> Mi Matthias,
> 
> On Tue, Nov 1, 2011 at 2:50 AM, Matthias Weisser <weisserm@arcor.de> wrote:
>> Dear Simon
>>
>> I just wanted to play around with the sandbox "arch" of u-boot maybe
>> adding tun/tap support. Current head compiled successfully but crashed
>> immediately after startup in board_init_f:
>>
>>        gd = malloc(sizeof(gd_t));
>>        assert(gd);
>>
>>        memset((void *)gd, 0, sizeof(gd_t));
>>
>> The simple reason was that malloc refers to u-boots internal malloc
>> which is not initialized at this point. I added the following snippet
> 
> That is odd. This commit is supposed to switch over to the system malloc():
> 
> fe34107 sandbox: Disable built-in malloc
> 
> Can you please check that it is there?

This commit is there. But it seems to be ignored. dlmalloc.c is build
and linked on my machine here.

> I would first check that dlmalloc.o is not being linked, and that
> mem_malloc_init() is undefined.
> 
> The current setup works OK for me, but yes it would be nice use
> U-Boot's internal malloc(). The initial effort was to get a baseline
> implementation into U-Boot and there are a number of areas where it
> can be expanded. We want to test as much as possible of the code, and
> dlmalloc is no exception.

Please see http://patchwork.ozlabs.org/patch/123074/ I just posted. This
may be a solution for this issue and we can use dlmalloc in sandbox.

Regards
Matthias
Mike Frysinger - Nov. 1, 2011, 6:54 p.m.
On Tuesday 01 November 2011 10:01:53 Simon Glass wrote:
> On Tue, Nov 1, 2011 at 2:50 AM, Matthias Weisser wrote:
> > I just wanted to play around with the sandbox "arch" of u-boot maybe
> > adding tun/tap support. Current head compiled successfully but crashed
> > immediately after startup in board_init_f:
> > 
> >        gd = malloc(sizeof(gd_t));
> >        assert(gd);
> > 
> >        memset((void *)gd, 0, sizeof(gd_t));
> > 
> > The simple reason was that malloc refers to u-boots internal malloc
> > which is not initialized at this point. I added the following snippet
> 
> That is odd. This commit is supposed to switch over to the system malloc():
> 
> fe34107 sandbox: Disable built-in malloc
> 
> Can you please check that it is there?

hmm, this was working for me, but now it's not.  so something has changed ...

but probably best to cut over to the mmap/virtual mapping route proposed 
elsewhere so that we can test the internal u-boot malloc and not worry about 
people accidentally breaking sandbox like this in the future.
-mike
Simon Glass - Nov. 2, 2011, 9:01 p.m.
On Tue, Nov 1, 2011 at 11:54 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Tuesday 01 November 2011 10:01:53 Simon Glass wrote:
>> On Tue, Nov 1, 2011 at 2:50 AM, Matthias Weisser wrote:
>> > I just wanted to play around with the sandbox "arch" of u-boot maybe
>> > adding tun/tap support. Current head compiled successfully but crashed
>> > immediately after startup in board_init_f:
>> >
>> >        gd = malloc(sizeof(gd_t));
>> >        assert(gd);
>> >
>> >        memset((void *)gd, 0, sizeof(gd_t));
>> >
>> > The simple reason was that malloc refers to u-boots internal malloc
>> > which is not initialized at this point. I added the following snippet
>>
>> That is odd. This commit is supposed to switch over to the system malloc():
>>
>> fe34107 sandbox: Disable built-in malloc
>>
>> Can you please check that it is there?
>
> hmm, this was working for me, but now it's not.  so something has changed ...
>
> but probably best to cut over to the mmap/virtual mapping route proposed
> elsewhere so that we can test the internal u-boot malloc and not worry about
> people accidentally breaking sandbox like this in the future.
> -mike
>

Yes definitely - disabling dlmalloc was only a stopgap to reduce the
size of the initial patch set.

Regards,
Simon

Patch

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 685793e..c98ca61 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -21,8 +21,12 @@ 

 #include <common.h>

+static uint8_t malloc_area[1024*1024*256];
+
 int main(int argc, char *argv[])
 {
+    mem_malloc_init(malloc_area, sizeof(malloc_area));
+
        /*

and got the console working. Can you tell me what I am doing wrong when