tkoenig1 / FMPFR

A Fortran binding for the GNU MPFR library.
Other
7 stars 0 forks source link

memory leaks due to buggy finalization in gfortran #6

Open TobiasLinn opened 2 years ago

TobiasLinn commented 2 years ago

The interface is essentially unusable when using gfortran (Version 12.1.1), because it does not handle finalization properly (compiler bug). So on each assignment, or for temporary results from operators in formulas, memory is leaked. You can check this easily with valgrind.

tkoenig1 commented 2 years ago

Do you have a quick test program? It's easier to reproduce (and check for different compiler versions which do or do not work).

TobiasLinn commented 2 years ago

mytest.f90.txt (rename to mytest.f90)

The commands I ran are:

gfortran mytest.f90 -o mytest libfmpfr.a -lmpfr -lgmp valgrind --leak-check=full ./mytest

tkoenig1 commented 2 years ago

I can confirm that. I get

 6.00000000000000000000000000000000000E0 
 6.00000000000000000000000000000000000E0 
 6.00000000000000000000000000000000000E0 
 6.00000000000000000000000000000000000E0 
 6.00000000000000000000000000000000000E0 
 6.00000000000000000000000000000000000E0 
 6.00000000000000000000000000000000000E0 
 6.00000000000000000000000000000000000E0 
 6.00000000000000000000000000000000000E0 
 6.00000000000000000000000000000000000E0 
==8786== 
==8786== HEAP SUMMARY:
==8786==     in use at exit: 1,680 bytes in 70 blocks
==8786==   total heap usage: 301 allocs, 231 frees, 184,404 bytes allocated
==8786== 
==8786== LEAK SUMMARY:
==8786==    definitely lost: 1,608 bytes in 67 blocks
==8786==    indirectly lost: 0 bytes in 0 blocks
==8786==      possibly lost: 0 bytes in 0 blocks
==8786==    still reachable: 72 bytes in 3 blocks
==8786==                       of which reachable via heuristic:
==8786==                         newarray           : 72 bytes in 3 blocks
==8786==         suppressed: 0 bytes in 0 blocks

as output.

By comparision, nagfor 7.1 yields

6.00000000000000000000000000000000000E0
 6.00000000000000000000000000000000000E0
 6.00000000000000000000000000000000000E0
 6.00000000000000000000000000000000000E0
 6.00000000000000000000000000000000000E0
 6.00000000000000000000000000000000000E0
 6.00000000000000000000000000000000000E0
 6.00000000000000000000000000000000000E0
 6.00000000000000000000000000000000000E0
 6.00000000000000000000000000000000000E0
==10029== 
==10029== HEAP SUMMARY:
==10029==     in use at exit: 480 bytes in 20 blocks
==10029==   total heap usage: 141 allocs, 121 frees, 4,274 bytes allocated
==10029== 
==10029== LEAK SUMMARY:
==10029==    definitely lost: 480 bytes in 20 blocks
==10029==    indirectly lost: 0 bytes in 0 blocks
==10029==      possibly lost: 0 bytes in 0 blocks
==10029==    still reachable: 0 bytes in 0 blocks
==10029==         suppressed: 0 bytes in 0 blocks

which is better. I'll have to see if there is still a but in the code, and what to do about gfortran.

tkoenig1 commented 2 years ago

Part of the leaks for gfortran is due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71798 . Might be something to look at...

TobiasLinn commented 2 years ago

Note how that bug is from 2016 and still not fixed, they just don't care :) And btw: The interface also does not work with the Intel compiler (ifort) in its current state, it mainly complains that arguments declared as value also need to be intent(in) (for pure procedures). When you make these modifications, it seems to work and also only leaks a little bit of memory like nagfor. However, I know for a fact that finalization is also buggy in ifort (even though not for this small example). As a rule of thumb: gfortran does not finalize enough and leaks memory, ifort over finalizes (sometimes) which leads to double free and random crashes. This is why I gave up completely on final procedures in my own code.

I actually have a proposal for improvement: Create some kind of internal stack where the mpfr_t handles are stored. In the final procedure push the handle to another stack (safety check for ifort: only if it is not already in there). When a new handle is needed, use the free handles from that stack first before new memory is allocated.

The user is then required to get the current stack size by calling a function at the beginning of his subroutine (or loop). At the end, he must call another subroutine which will then actually free the newly created handles to restore the stack to its original state. Maybe two different versions should exist, a soft free (push handles to the free stack) and a hard free (actually calls mpfr_clear). Of course, thread safety also needs to be considered (maybe each thread has its own stack).

gfortran would then potentially use more memory than necessary, but at least there would be no actual leaks. When done carefully, it should also work with ifort. I think it could also be more efficient, calling mpfr_init and mpfr_clear a lot of times is probably slow (I did not test this).

tkoenig1 commented 2 years ago

Well, "they" do care (I should know :-) but finalization is very tricky (even J3 members disagree on occasion on what should be finalized), and there are never enough people to maintain gfortran. I will dive a little more into this and see if I can at least isolate bug reports, and also maybe working towards fixing them for gfortran 13, at least.

I am not yet 100% sure that my code is indeed conforming with respect to finalization - just when a function result should be finalized with a user-defined assignment. A solution like the one you are suggesting may indeed be necessary. I'll have to work on a test case first.

tkoenig1 commented 2 years ago

I think I have fixed the memory leaks conerning standard-conforming code (at least nagfor seems to say so). What is left are probably gfortran bugs in deallocation.

Question: What would be easier, try to put in workarounds here or fix it on the gfortan side...

tkoenig1 commented 2 years ago

The gfortran bug is now https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106576 .

TobiasLinn commented 2 years ago

It would be really nice if finalization finally worked fine in gfortran ^^. So, if you have the expertise and skill to fix it (I certainly don't), imho it would be a lot better to invest the time there, than to put workarounds in the library (a simple warning about memory leaks in gfortran in the readme is enough I think).

mmal commented 2 years ago

Running the example code "program memain" gives on my box (macOS, gfortran 11.3.0)

 1.299999999999999999999999999999999999998E0  3.299999999999999999999999999999999999998E0 

Program received signal SIGSEGV: Segmentation fault - invalid memory reference

Is that a similar (finalisation) or a different issue?