Memory leaks with static variables

[lwalewski]

Hi,

Thanks for the great job developing PicoC in the first place!

When playing around with the interpreter I noticed that using static variables triggers a memory leak. To reproduce the bug run the following minimal working example:

void main(void) {
  static int i;
}

(saved e. g. in static.c) under valgrind (picoc compiled with -O0 -g):

$ valgrind --leak-check=yes --track-origins=yes ./picoc static.c

On my Linux 3.16.0 with gcc 4.8.4 and valgrind 3.10.0.SVN it outputs:

==10829== HEAP SUMMARY:
==10829==     in use at exit: 40 bytes in 1 blocks
==10829==   total heap usage: 130 allocs, 129 frees, 136,416 bytes allocated
==10829== 
==10829== 40 bytes in 1 blocks are definitely lost in loss record 1 of 1
==10829==    at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10829==    by 0x40F0DC: HeapAllocMem (heap.c:138)
==10829==    by 0x410B10: VariableAlloc (variable.c:73)
==10829==    by 0x410B87: VariableAllocValueAndData (variable.c:91)
==10829==    by 0x4119A1: VariableDefinePlatformVar (variable.c:377)
==10829==    by 0x4116D0: VariableDefineButIgnoreIdentical (variable.c:332)
==10829==    by 0x408709: ParseDeclaration (parse.c:345)
==10829==    by 0x409852: ParseStatement (parse.c:772)
==10829==    by 0x408FD9: ParseBlock (parse.c:536)
==10829==    by 0x409375: ParseStatement (parse.c:659)
==10829==    by 0x40EBA5: ExpressionParseFunctionCall (expression.c:1545)
==10829==    by 0x40DDF8: ExpressionParse (expression.c:1251)
==10829== 
==10829== LEAK SUMMARY:
==10829==    definitely lost: 40 bytes in 1 blocks
==10829==    indirectly lost: 0 bytes in 0 blocks
==10829==      possibly lost: 0 bytes in 0 blocks
==10829==    still reachable: 0 bytes in 0 blocks
==10829==         suppressed: 0 bytes in 0 blocks

The problem vanishes when static qualifier is removed. The issue becomes much more serious when static variables are used within a function called periodically, e. g.:

void my_func(void) {
  static int i;
}

void main(void) {
  while (1) {
    my_func();
  }
}

The code above acquires heap memory pretty quickly (which you can see e. g. with top under Linux) and gets killed by the kernel as soon as it exhausts the physical memory, e. g. on my embedded device:

Out of memory: Kill process 918 (picoc) score 865 or sacrifice child
Killed process 918 (picoc) total-vm:227516kB, anon-rss:224908kB, file-rss:408kB

My quick and dirty fix would be to put all global variables allocated with VariableDefinePlatformVar on the stack instead of the heap, i. e. replace line 377 of variable.c:

struct Value *SomeValue = VariableAllocValueAndData(pc, NULL, 0, IsWritable, NULL, TRUE);

with:

struct Value *SomeValue = VariableAllocValueAndData(pc, NULL, 0, IsWritable, NULL, FALSE);

This way valgrind reports no memory leaks on my static.c above and the infinite loop calling my_func runs at constant memory. Moreover, all the regression tests keep passing with no errors.

Could you advise if this thinking makes any sense? Or maybe the call to VariableDefinePlatformVar should be context sensitive, such that local-scope copies of static variables only should be allocated on the stack?

Best regards,
Lukasz

[galacticstudios]

I would advise against putting variables on the stack. Stack space is typically much smaller than heap space. One of the fixes I recently made to PicoC was to move data off the stack and into the heap because putting it on the stack was causing "out of Memory" errors.

[lwalewski]

Thanks for the advice. My quick&dirty fix is not a real solution indeed. I tried to locate all calls to [c,m]alloc/free at runtime, so preceded each instance found in the code with printf("%s calling free\n", __func__); etc, which gave me the following:

$ ./picoc static.c | sort | uniq -c
    127 HeapAllocMem calling calloc
      1 HeapCleanup calling free
    127 HeapFreeMem calling free
      1 HeapInit calling malloc

This makes 128 allocs and 128 frees, whereas valgrind reports 130 allocs and 129 frees, so obviously I am missing 2 allocs and 1 free. Any idea, where could they hide?

On the other hand, when I activate DEBUG_HEAP at compile time, I get:

$ ./picoc mytests/static.c | grep -e popping -e pushing
pushing 40 at 0x7f9efff53010
pushing 24 at 0x7f9efff53038
pushing 40 at 0x7f9efff53138
popping 40 at 0x7f9efff53010

As far as I understand, 40+24+40 bytes were allocated and only 40 deallocated, which would suggest that 64 bytes were lost, as opposed to 40 reported by valgrind.

Any help to understand better what is going on here is welcome!