Re: RFA: V850: Separate DATA and HEAP memory areas

[Jeff Johnston <jjohnstn@redhat.com>]

I have no issues with the change.  Please go ahead.

-- Jeff J.

On 05/23/2011 05:54 AM, Nick Clifton wrote:
> Hi Jeff,
>
>    I would like permission to apply the patch below to the V850 version
>    of the sbrk() function in newlib and libgloss.  It does two things.
>    Firstly it tidies up the code.  Secondly it changes the start of the
>    heap to be based on the symbol "heap_start" rather than the symbol
>    "end".  The importance of the second change is that it allows the heap
>    to be placed somewhere other than at the end of data.  This means that
>    on a V850 board with two separate banks of memory, for example, the
>    stack and heap can be in one bank and the data in another.  Eg:
>
>      MEMORY
>      {
>        ROM       : ORIGIN = 0x00001000, LENGTH = 0x001ff000 /* 2Mb */
>        RAM2 (w)  : ORIGIN = 0xfebf0000, LENGTH = 0x00010000 /* 64k */
>        STACK (w) : ORIGIN = 0xfebffff0, LENGTH = 16         /* Top of RAM2 */
>        RAM (w)   : ORIGIN = 0xfedf0000, LENGTH = 0x00010000 /* 64k */
>      }
>
>      SECTIONS
>      {
>        .text :
>        {
>          *(.text)
>        }>ROM =0
>
>        .data :
>        {
>          PROVIDE (__datastart = .);
>          *(.data)
>        }>RAM AT>ROM
>
>        .bss :
>        {
> 	*(.bss)
> 	PROVIDE (end = .);
>        }>RAM
>
>        .heap :
>        {
> 	KEEP (*(.heap))
> 	PROVIDE (_heap_start = .);
>        }>RAM2
> 	
>        .stack :
>        {
> 	__stack = .;
> 	KEEP ( *(.stack) )
>        }>  STACK
>      }
>
>    Ok to apply ?
>
> Cheers
>    Nick
>
> newlib/ChangeLog
> 2011-05-23  Nick Clifton<nickc@redhat.com>
>
> 	* libc/sys/sysnecv850/sbrk.c (_sbrk): Tidy code.
>          Base start of heap on the "heap_start" symbol.
>
> libgloss/ChangeLog
> 2011-05-23  Nick Clifton<nickc@redhat.com>
>
> 	* v8500/sbrk.c (_sbrk): Tidy code.
>          Base start of heap on the "heap_start" symbol.
>
>
> Index: newlib/libc/sys/sysnecv850/sbrk.c
> ===================================================================
> RCS file: /cvs/src/src/newlib/libc/sys/sysnecv850/sbrk.c,v
> retrieving revision 1.1.1.1
> diff -u -3 -p -r1.1.1.1 sbrk.c
> --- newlib/libc/sys/sysnecv850/sbrk.c	17 Feb 2000 19:39:50 -0000	1.1.1.1
> +++ newlib/libc/sys/sysnecv850/sbrk.c	23 May 2011 09:38:00 -0000
> @@ -3,34 +3,27 @@
>   #include<sys/stat.h>
>   #include "sys/syscall.h"
>
> -int errno;
> -
> -int __trap0 (int function, int p1, int p2, int p3);
> -
> -#define TRAP0(f, p1, p2, p3) __trap0(f, (int)(p1), (int)(p2), (int)(p3))
> -
>   caddr_t
>   _sbrk (int incr)
>   {
> -  extern char end;		/* Defined by the linker */
> -  static char *heap_end;
> -  char *prev_heap_end;
> -#if 0
> -  char *sp = (char *)stack_ptr;
> -#else
> -  char *sp = (char *)&sp;
> -#endif
> +  extern char   heap_start;	/* Defined by the linker script.  */
> +  static char * heap_end = NULL;
> +  char *        prev_heap_end;
> +  char *        sp = (char *)&  sp;
> +
> +  if (heap_end == NULL)
> +    heap_end =&  heap_start;
>
> -  if (heap_end == 0)
> -    {
> -      heap_end =&end;
> -    }
>     prev_heap_end = heap_end;
> +
>     if (heap_end + incr>  sp)
>       {
> -      _write (1, "Heap and stack collision\n", 25);
> +#define MESSAGE "Heap and stack collision\n"
> +      _write (1, MESSAGE, sizeof MESSAGE);
>         abort ();
>       }
> +
>     heap_end += incr;
> +
>     return (caddr_t) prev_heap_end;
>   }
> Index: libgloss/v850/sbrk.c
> ===================================================================
> RCS file: /cvs/src/src/libgloss/v850/sbrk.c,v
> retrieving revision 1.1
> diff -u -3 -p -r1.1 sbrk.c
> --- libgloss/v850/sbrk.c	23 Jul 2010 17:52:37 -0000	1.1
> +++ libgloss/v850/sbrk.c	23 May 2011 09:38:00 -0000
> @@ -3,34 +3,27 @@
>   #include<sys/stat.h>
>   #include "sys/syscall.h"
>
> -int errno;
> -
> -int __trap0 (int function, int p1, int p2, int p3);
> -
> -#define TRAP0(f, p1, p2, p3) __trap0(f, (int)(p1), (int)(p2), (int)(p3))
> -
>   caddr_t
>   _sbrk (int incr)
>   {
> -  extern char end;		/* Defined by the linker */
> -  static char *heap_end;
> -  char *prev_heap_end;
> -#if 0
> -  char *sp = (char *)stack_ptr;
> -#else
> -  char *sp = (char *)&sp;
> -#endif
> +  extern char   heap_start;		/* Defined by the linker script. */
> +  static char * heap_end = NULL;
> +  char *        prev_heap_end;
> +  char *        sp = (char *)&  sp;
> +
> +  if (heap_end == NULL)
> +    heap_end =&  heap_start;
>
> -  if (heap_end == 0)
> -    {
> -      heap_end =&end;
> -    }
>     prev_heap_end = heap_end;
> +
>     if (heap_end + incr>  sp)
>       {
> -      _write (1, "Heap and stack collision\n", 25);
> +#define MESSAGE "Heap and stack collision\n"
> +      _write (1, MESSAGE, sizeof MESSAGE);
>         abort ();
>       }
> +
>     heap_end += incr;
> +
>     return (caddr_t) prev_heap_end;
>   }
>
> PPS. Assuming that you approve the newlib/libgloss patch, I will apply
> this patch to the linker sources:
>
> Index: ld/scripttempl/v850.sc
> ===================================================================
> RCS file: /cvs/src/src/ld/scripttempl/v850.sc,v
> retrieving revision 1.10
> diff -u -3 -p -r1.10 v850.sc
> --- ld/scripttempl/v850.sc	10 Feb 2011 08:18:58 -0000	1.10
> +++ ld/scripttempl/v850.sc	23 May 2011 09:38:00 -0000
> @@ -180,6 +180,7 @@ SECTIONS
>
>     ${RELOCATING+_end = . ;}
>     ${RELOCATING+PROVIDE (end = .);}
> +  ${RELOCATING+PROVIDE (_heap_start = .);}
>
>     /* Stabs debugging sections.  */
>     .stab 0		: { *(.stab) }
>