How to detect when no RAM left for a malloc? It seems to just keep going (Teensy 3.1)

Status
Not open for further replies.
S

stevethese

Member
Hi, I'm working on a project which utilises a lot of dynamic memory allocation. I want to be able to detect when there's none left. Logically, I supposed that calling malloc() might just return a NULL pointer if there is no more ram. Instead however, it appears to allocate me ram which was already in use, leading to crashes.

I made a simple example sketch to demonstrate this:

Code: 
void* pointer;
uint32_t counter = 0;

void setup() {
  delay(1000);
}

void loop() {
  pointer = malloc(1);
  counter++;
  if (pointer == NULL) Serial.println("got null");
  else Serial.println(String(counter) + " allocations made");
}

The sketch repeatedly tries to allocate one byte, and reports to the user each time it does this successfully. If it receives a NULL pointer instead, it reports this. However, when I try running this sketch, no NULL pointers are reported, and instead the sketch successfully makes 3825 allocations, and then stops responding - presumably having ended up overwriting something it shouldn't have.

Is there a way to stop this from happening, and somehow figure out that point where no more malloc()'s can be called?

Another question - I made another edit to the code to give me some more info, which indicated that each of the malloc(1)'s called was in fact allocating 16 bytes - hence giving me under 4,000 of them despite the Teensy 3.1's 65,536 byte memory. Is this expected? Shouldn't a malloc(1) allocate just one byte?

Thanks :)
 
Try to increase the amount of allocated memory per block. I'm not sure how Serial.println exactly works, but I can imagine the following: You allocate blocks of memory until only a small amount is left. Serial.println() needs space on the stack for the string, but the heap is in the way, so it crashes.

So try again with blocks of something like 128 bytes and report the results.
 
Thanks for your reply... that doesn't seem to be it though. Increasing the block size up to 256 bytes yielded the exact same problem.

I tried cutting Serial and Strings out of the equation entirely, and instead just having the LED turn on when a NULL pointer is finally returned:

Code: 
void* pointer;
uint8_t ledPin = 13;

void setup() {
  pinMode(ledPin, OUTPUT);
}

void loop() {
  pointer = malloc(1);
  if (pointer == NULL) digitalWrite(ledPin, HIGH);
}

The LED never turned on.

I wrote another sketch which repeatedly blinks the LED while doing malloc()'s, just to show me that something's still happening and the sketch hasn't crashed:

Code: 
void* pointer;
uint8_t ledPin = 13;
boolean ledOn = true;

void setup() {
  pinMode(ledPin, OUTPUT);
}

void loop() {
  pointer = malloc(1);
  digitalWrite(ledPin, ledOn);
  ledOn = !ledOn;
  delayMicroseconds(1000);
}

After a couple of seconds, the LED stopped blinking - the code had still crashed. Same result when I increased the block size back up to 256.

Any further ideas?
 
I've put this on my list of bugs to investigate and fix. This is probably going to involve some serious digging into newlib stuff, so it might take a while, but it's absolutely on my bug list now.
 
I did a quick search about malloc for MCUs and found this:
https://forum.sparkfun.com/viewtopic.php?f=4&t=37114
Check out the link that is referenced inside that link.

I also found this:
https://community.freescale.com/thread/312130
Not sure if that is related or not.

I find it interesting that people avoid malloc in safety critical systems. I am not sure if this is a legacy issue with older compilers or something intrinsic to the lack of an OS managing memory in the background.
 
The problem with malloc() and free() is indeed that extensive usage can lead to memory fragmentation. Fragmentation, however, can only occur if free() is used. The problem is not malloc, but the combination of malloc and free (or new and delete) in different contextual aspects of the application.

During initialization it's quite safe to use malloc, especially for lazy initializtation (construct on first use). As long as malloc only allocates memory which is actually available (we seem to have a problem in this regard), nothing will go wrong as long as failed allocations are detected.

During runtime it can be necessary to use malloc and free in dynamic contexts, like GUIs. I use new and delete a lot with widgets, but basically not for anything else (like containers). And that is important: If new and delete are always used in the same context, memory can definitely be reused. Example: A window is created, populated with widgets, and deleted again when it is not needed any more. Memory will not be fragmented by that. If, however, something created in that window has a longer lifetime than the window, memory will be fragmented afterwards. If a free fragment is useful later depends on the fragment's size - if it's large the probability of the fragment being useful later is higher.

So to sum it up, in order of likeliness to fail:
  • Don't use malloc if you want information about memory usage after linking
  • use malloc during startup, for objects with unlimited lifetime
  • use malloc after startup, but don't free anything
  • if you use malloc and free dynamically, make sure that they are used in a "symmetric" way and only for temporary objects
  • Do what you need and see if it crashes
 
Does anyone know where in the Teensyduino files the code for malloc() for Teensy 3.1 is? I'm thinking I'll have a try at debugging this problem... I can see that for Teensy 1 and 2 it seems to be in hardware/teensy/cores/teensy/malloc.c but I can't seem to work it out for Teensy 3.x.
 
I concur - the only safe way to use malloc() in a small micro (like T3) is do use it (or use new()) one time, at startup, and check return values.
Then, use it no more.
And don't use free()
 
is this still open?

christoph said:
@stevethese AFAIK, newlib a compiled library shipped with the toolchain in teensyduino, so it won't show up as sources in the cores folder. Paul has a toolchain repo though: https://github.com/PaulStoffregen/ARM-Toolchain/ and it might be possible for you to start with the newlib archive in there.
Click to expand...

has this ever been resolved?
I'm currently working with GFXcanvas from the Adafruit_GFX library to build a gauges library. GFXcanvas basically malloc()s a bunch of RAM as an off-screen bitmap.
I find that my code would just stop working after a few attempts.
Then, just trying to malloc() small chunks of memory, I find that that also just stops and never returns a false

Code: 
void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
  while(!Serial);
  uint32_t n=0;
  while(malloc(256)){
    Serial.print("--");
    Serial.println(n+=256);
    delay(10);
  }
  Serial.println("out of memory");
}

void loop() {
  // put your main code here, to run repeatedly:

}
(yes, I'm aware of the potential issues with Serial and println in this case, but I assume that 256 byte chunks should be good enough to leave some room).

updates would be appreciated.

pj
 
malloc() uses sbrk() to increase the heap size. The Teensy version of sbrk does no checking for a possible stack collision, so you end up corrupting the stack if you 'mallocate' too much memory.
 
tni said:
malloc() uses sbrk() to increase the heap size. The Teensy version of sbrk does no checking for a possible stack collision, so you end up corrupting the stack if you 'mallocate' too much memory.
Click to expand...

maybe I'm being naive, but wouldn't a rather simple check help keep at least 512 bytes for the Stack from being malloc()'d?

I have looked at the M4 stack model, seing that there are two different stacks, but at this time, I believe only the non - privileged one would be relevant, right?
so checking for __brkval+incr+512 >= MSP should do the job?

am I not getting the full picture here?

thanks

pj

going back to /teensy/avr/cores/malloc.c, I found
Code: 
        if (__brkval == 0)
                __brkval = __malloc_heap_start;
        cp = __malloc_heap_end;
        if (cp == 0)
                cp = STACK_POINTER() - __malloc_margin;
        if (cp <= __brkval)
          /*
           * Memory exhausted.
           */
          return 0;
        avail = cp - __brkval;

with
Code: 
#define STACK_POINTER() ((char *)AVR_STACK_POINTER_REG)

which would indicate that it might be even simpler by just re-defining this to an M4 compatible pointer. R13?
 
Last edited:
I just added this to malloc.c
Code: 
#ifdef __AVR__
#define STACK_POINTER() ((char *)AVR_STACK_POINTER_REG)
#elif defined(__arm__) && defined(TEENSYDUINO)
        #define STACK_POINTER(){
                void* sp;
                __asm__ __volatile__("mv %[result],r13"\n\t
                                     : [result] "=r" (sp)\n\t
                                     :);
        return sp;
               }
#endif

but it seems that the core files are not being recompiled by the Arduino IDE, will have to dig some more...

pj
 
'teensy/avr/cores/malloc.c' is not used by Teensy 3. It uses the stuff under 'arduino/hardware/teensy/avr/cores/teensy3'. For Teensy 3, malloc (but not sbrk) is part of the C library / newlib that comes with the toolchain.
 
tni said:
'teensy/avr/cores/malloc.c' is not used by Teensy 3. It uses the stuff under 'arduino/hardware/teensy/avr/cores/teensy3'. For Teensy 3, malloc (but not sbrk) is part of the C library / newlib that comes with the toolchain.
Click to expand...

I see, but - essentially the same code could go into sbrk(), no?
I'm not sure how the different mk20dxXXX.ld files are being built, though.

pj
 
Last edited:
right, I have this now:

Code: 
void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
  while(!Serial);
  uint32_t n=0;
  void* m;
  while(m=malloc(1024)){
    Serial.printf("-- 0x%08x: %i MSP: 0x%08x: \n",m,n+=1024,STACK_POINTER());
    delay(10);
  }
  Serial.println("out of memory");
}

void loop() {
  // put your main code here, to run repeatedly
}

void* STACK_POINTER(){
  void* sp;
  asm volatile("mov %[result],r13" : [result] "=r" (sp) ::"r13" );
  return sp;
}
the output is
Code: 
-- 0x200000a0: 28672 MSP: 0x20007fd0: 
-- 0x200004a8: 29696 MSP: 0x20007fd0: 
-- 0x200008b0: 30720 MSP: 0x20007fd0: 
-- 0x20000cb8: 31744 MSP: 0x20007fd0: 
-- 0x200010c0: 32768 MSP: 0x20007fd0: 
-- 0x200014c8: 33792 MSP: 0x20007fd0: 
-- 0x200018d0: 34816 MSP: 0x20007fd0: 
-- 0x20001cd8: 35840 MSP: 0x20007fd0: 
-- 0x200020e0: 36864 MSP: 0x20007fd0: 
-- 0x200024e8: 37888 MSP: 0x20007fd0: 
-- 0x200028f0: 38912 MSP: 0x20007fd0: 
-- 0x20002cf8: 39936 MSP: 0x20007fd0: 
-- 0x20003100: 40960 MSP: 0x20007fd0: 
-- 0x20003508: 41984 MSP: 0x20007fd0: 
-- 0x20003910: 43008 MSP: 0x20007fd0: 
-- 0x20003d18: 44032 MSP: 0x20007fd0: 
-- 0x20004120: 45056 MSP: 0x20007fd0: 
-- 0x20004528: 46080 MSP: 0x20007fd0: 
-- 0x20004930: 47104 MSP: 0x20007fd0: 
-- 0x20004d38: 48128 MSP: 0x20007fd0: 
-- 0x20005140: 49152 MSP: 0x20007fd0: 
-- 0x20005548: 50176 MSP: 0x20007fd0: 
-- 0x20005950: 51200 MSP: 0x20007fd0: 
-- 0x20005d58: 52224 MSP: 0x20007fd0: 
-- 0x20006160: 53248 MSP: 0x20007fd0: 
-- 0x20006568: 54272 MSP: 0x20007fd0: 
-- 0x20006970: 55296 MSP: 0x20007fd0: 
-- 0x20006d78: 56320 MSP: 0x20007fd0: 
-- 0x20007180: 57344 MSP: 0x20007fd0: 
-- 0x20007588: 58368 MSP: 0x20007fd0: 
-- 0x20007990: 59392 MSP: 0x20007fd0:
and as you said, the crash happens as soon as the heap grows into the stack. Quite to be expected.
It also shows, that my little assembly code works (I had mv rather than mov before, that's fixed). Now the only question is: where to go with it to be able to test it and submit a patch?

pj
 
tni said:
I have a fix. I'll file a pull request later.
Click to expand...


thanks, I have just verified that r13 is indeed the correct Stack Pointer (it was, of course, invariant in my test, but when building a recursion loop, every level in added 0x08 bytes to the stack).

But just for my understanding: this core file won't be recompiled by the Arduino IDE, right? the different .ld versions are different configs for the same file based on different memory layouts and CPUs? Is there a readme on how to build them?

regards

pj
 
'arduino/hardware/teensy/avr/cores/teensy3/mk20dx128.c' (which has sbrk()) is automatically recompiled. It's used for all the 3.x Teensys. The .ld files are the linker scripts for the different Teensy versions (3.0: mk20dx128, 3.1/3.2: mk20dx256, 3.5: mk64fx512, 3.6: mk66fx1m0, LC: mkl26z64). You can change them and they are automatically used.

For malloc(), you would have to build your own version of newlib.
 
ah, got it, well, I'll drop my code into sbrk() then. I actually didn't look into the ld files, thinking they were object files. stupid me!

pj
 
Code: 
void* STACK_POINTER(){
  void* sp;
  asm volatile("mov %[result],r13" : [result] "=r" (sp) ::"r13" );
  return sp;
}

[...]

void * _sbrk(int incr)
{
        char *prev = __brkval;
        if((char*)prev+incr+256<=STACK_POINTER()){
                __brkval += incr;
                return prev;
        }else{
                return NULL;
        }
}

result:

Code: 
-- 0x200008b0: 30720 bytes MSP: 0x20007fd0: 
-- 0x20000cb8: 31744 bytes MSP: 0x20007fd0: 
-- 0x200010c0: 32768 bytes MSP: 0x20007fd0: 
-- 0x200014c8: 33792 bytes MSP: 0x20007fd0: 
-- 0x200018d0: 34816 bytes MSP: 0x20007fd0: 
-- 0x20001cd8: 35840 bytes MSP: 0x20007fd0: 
-- 0x200020e0: 36864 bytes MSP: 0x20007fd0: 
-- 0x200024e8: 37888 bytes MSP: 0x20007fd0: 
-- 0x200028f0: 38912 bytes MSP: 0x20007fd0: 
-- 0x20002cf8: 39936 bytes MSP: 0x20007fd0: 
-- 0x20003100: 40960 bytes MSP: 0x20007fd0: 
-- 0x20003508: 41984 bytes MSP: 0x20007fd0: 
-- 0x20003910: 43008 bytes MSP: 0x20007fd0: 
-- 0x20003d18: 44032 bytes MSP: 0x20007fd0: 
-- 0x20004120: 45056 bytes MSP: 0x20007fd0: 
-- 0x20004528: 46080 bytes MSP: 0x20007fd0: 
-- 0x20004930: 47104 bytes MSP: 0x20007fd0: 
-- 0x20004d38: 48128 bytes MSP: 0x20007fd0: 
-- 0x20005140: 49152 bytes MSP: 0x20007fd0: 
-- 0x20005548: 50176 bytes MSP: 0x20007fd0: 
-- 0x20005950: 51200 bytes MSP: 0x20007fd0: 
-- 0x20005d58: 52224 bytes MSP: 0x20007fd0: 
-- 0x20006160: 53248 bytes MSP: 0x20007fd0: 
-- 0x20006568: 54272 bytes MSP: 0x20007fd0: 
-- 0x20006970: 55296 bytes MSP: 0x20007fd0: 
out of memory

I'm certain that there's a better way to place the definition for STACK_POINTER()...

Thanks for your help

pj
 
Status
Not open for further replies.
Back
Top