[PATCH] [nosys] ARM heap overflow check in libnosys

[Richard Earnshaw]

On 28/02/14 10:27, Richard Earnshaw wrote:
> On 27/02/14 04:43, Joey Ye wrote:
>> Original libnosys doesn't check heap overflow. This patch adds an ARM target
>> specific check there, without breaking other targets.
>>
>> OK to upstream?
>>
>>
>> ChangeLog (libgloss):
>>
>> 2014-02-27  Joey Ye  <joey.ye@arm.com>
>>         libnosys/sbrk.c (errno.h): New include.
>>         (errno): Redefine.
>>         (STACK_PTR): Define macro for __arm__.
>>         (_sbrk): Check STACK_PTR.
>>
> 
> I can't approve or reject this, since it's a change to generic code.
> However, some comments:
> 
> 1) You assume that the stack grows downwards and the heap upwards.  On
> some machines the stack can grow upwards.  I'm not sure if machines like
> this are interesting in this case.
> 2) You assume that there's a single stack and that the stack is not
> allocated within the heap (threaded code often does this)
> 3) It would be nice if the machine-specific definition could be moved
> out into a machine-specific header.
> 4) I don't think it's safe to undef errno like that.
> 

And one more:

5) You leave no grace space between the stack and the heap.  Unless you
are in the innermost frame at this time (deepest stack usage) you'll
likely later end up with a conflict when the stack moves down into the heap.

R.

> R.
> 
>> diff --git a/libgloss/libnosys/sbrk.c b/libgloss/libnosys/sbrk.c
>> index 86c130a..910db20 100644
>> --- a/libgloss/libnosys/sbrk.c
>> +++ b/libgloss/libnosys/sbrk.c
>> @@ -2,6 +2,16 @@
>>  
>>  #include "config.h"
>>  #include <_syslist.h>
>> +#include <errno.h>
>> +#undef errno
>> +extern int errno;
>> +
>> +#undef STACK_PTR
>> +#ifdef __arm__
>> +/* Register name faking - works in collusion with the linker.  */
>> +register char *stack_ptr asm ("sp");
>> +#define STACK_PTR stack_ptr
>> +#endif
>>  
>>  void *
>>  _sbrk (incr)
>> @@ -15,6 +25,15 @@ _sbrk (incr)
>>       heap_end = & end;
>>  
>>     prev_heap_end = heap_end;
>> +
>> +#ifdef STACK_PTR
>> +  if (heap_end + incr > STACK_PTR)
>> +    {
>> +      errno = ENOMEM;
>> +      return (void *) -1;
>> +    }
>> +#endif
>> +
>>     heap_end += incr;
>>  
>>     return (void *) prev_heap_end;=
>>
>>