Memory leaks in nested procedures

[tueda]

The first bug of #250 can be reproduced by

#procedure foo()
  #procedure bar()
  #endprocedure
#endprocedure

#call foo()
#call foo()
.end

Then valgrind --leak-check=full vorm test.frm gives

==25298== Memcheck, a memory error detector
==25298== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==25298== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==25298== Command: vorm test.frm
==25298==
FORM 4.2.0 (Dec 13 2017, v4.2.0-28-g4493f01) 64-bits  Run: Wed Dec 13 18:25:58 2017
    #procedure foo()
      #procedure bar()
      #endprocedure
    #endprocedure

    #call foo()
    #call foo()
    .end
  0.02 sec out of 0.02 sec
==25298==
==25298== HEAP SUMMARY:
==25298==     in use at exit: 387,475,840 bytes in 125 blocks
==25298==   total heap usage: 646 allocs, 521 frees, 387,529,621 bytes allocated
==25298==
==25298== 8 bytes in 1 blocks are definitely lost in loss record 9 of 120
==25298==    at 0x4C2AB96: malloc (vg_replace_malloc.c:299)
==25298==    by 0x4FFA16: Malloc1 (tools.c:2230)
==25298==    by 0x4FFA5A: strDup1 (tools.c:1816)
==25298==    by 0x4AC6D5: DoProcedure (pre.c:3880)
==25298==    by 0x4AF3D2: PreProInstruction (pre.c:1156)
==25298==    by 0x4AFF3A: PreProcessor (pre.c:931)
==25298==    by 0x4E8F2D: main (startup.c:1607)
==25298==
==25298== 136 bytes in 1 blocks are definitely lost in loss record 59 of 120
==25298==    at 0x4C2AB96: malloc (vg_replace_malloc.c:299)
==25298==    by 0x4FFA16: Malloc1 (tools.c:2230)
==25298==    by 0x4AC0FD: PreLoad (pre.c:4132)
==25298==    by 0x4AC64A: DoProcedure (pre.c:3867)
==25298==    by 0x4AF3D2: PreProInstruction (pre.c:1156)
==25298==    by 0x4AFF3A: PreProcessor (pre.c:931)
==25298==    by 0x4E8F2D: main (startup.c:1607)
==25298==
==25298== LEAK SUMMARY:
==25298==    definitely lost: 144 bytes in 2 blocks
==25298==    indirectly lost: 0 bytes in 0 blocks
==25298==      possibly lost: 0 bytes in 0 blocks
==25298==    still reachable: 387,475,696 bytes in 123 blocks
==25298==         suppressed: 0 bytes in 0 blocks

[tueda]

To me, it seems that
https://github.com/vermaseren/form/blob/4493f01b3732abd5124309ac05618c2689e07f46/sources/pre.c#L3870
should be set to 0 for procedures allocated in DoProcedure() (regardless whether or not nested), to make sure that they are freed at DoEndprocedure().

[jodavies]

Just changing this to zero is not quite the whole story. Actually the whole test suite passes apart from forcer tests. There I get: SchemeZno2 Line 26 ==> Multiple occurrence of procedure name z2y

[jodavies]

@vermaseren let me check that I understand how things should work:

• "locally defined" procedures (directly in the frm file, or via #include) all go onto the procedures stack, and are given a loadmode of "2". They stay there permanently and are never deallocated.
• #calling a procedure which is not locally defined results in a file lookup for the procedure, and the loadmode is 0. This means it is freed in DoEndprocedure when the procedure is popped and subsequent calls load it from file again.
• #calling a local procedure places it on the procedure stack a second time, with loadmode 1. In DoEndprocedure the procedure is popped and nothing is freed: the "copy" anyway only points to the permanent procedure lower in the stack.

To fix this I think the nesting needs to be detected in DoProcedure and introduce more logic in DoEndprocedure to decide when to free allocated memory.

[jodavies]

Now I think I have fixed this one:
jodavies@05ff339

Actually Forcer is still not valgrind clean, for seemingly unrelated reasons. I suppose this one was "hiding" behind the other reported errors...

==1530011== Invalid read of size 4
==1530011==    at 0x1914F1: UpdatePositions (execute.c:2435)
==1530011==    by 0x27F4E4: Processor (proces.c:104)
==1530011==    by 0x18C225: DoExecute (execute.c:859)
==1530011==    by 0x270C9F: PreProcessor (pre.c:1045)
==1530011==    by 0x139B9B: main (startup.c:1749)
==1530011==  Address 0x4ff06d40 is 0 bytes after a block of size 80 alloc'd
==1530011==    at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==1530011==    by 0x2F55B0: Malloc1 (tools.c:2241)
==1530011==    by 0x18B65E: PutInVflags (execute.c:584)
==1530011==    by 0x15207D: DoExpr (comexpr.c:251)
==1530011==    by 0x16E468: CompileStatement (compiler.c:657)
==1530011==    by 0x2713E0: PreProcessor (pre.c:1127)
==1530011==    by 0x139B9B: main (startup.c:1749)
==1530011==