[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 Jerry DeLisle changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #17 from Jerry DeLisle --- Closing.
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #16 from Jerry DeLisle --- Author: jvdelisle Date: Fri Jun 1 18:34:09 2018 New Revision: 261077 URL: https://gcc.gnu.org/viewcvs?rev=261077&root=gcc&view=rev Log: 2018-06-01 Jerry DeLisle Backport from trunk. PR libgfortran/85840 * io/write.c (write_float_0, write_real, write_real_g0, write_complex): Use separate local variables for the float string length. Modified: branches/gcc-7-branch/libgfortran/ChangeLog branches/gcc-7-branch/libgfortran/io/write.c
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #15 from Jerry DeLisle --- Author: jvdelisle Date: Fri Jun 1 02:14:53 2018 New Revision: 261054 URL: https://gcc.gnu.org/viewcvs?rev=261054&root=gcc&view=rev Log: 2018-05-31 Jerry DeLisle Backport from trunk. PR libgfortran/85840 * io/write.c (write_real, write_real_g0, write_complex): Use separate local variables for the float string length. Modified: branches/gcc-8-branch/libgfortran/ChangeLog branches/gcc-8-branch/libgfortran/io/write.c
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #14 from Jerry DeLisle --- Author: jvdelisle Date: Mon May 28 21:55:31 2018 New Revision: 260851 URL: https://gcc.gnu.org/viewcvs?rev=260851&root=gcc&view=rev Log: 2018-05-28 Jerry DeLisle PR libgfortran/85840 * io/write.c (write_real, write_real_g0, write_complex): Use separate local variables for the float string length. Modified: trunk/libgfortran/ChangeLog trunk/libgfortran/io/write.c
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #13 from Jerry DeLisle --- Author: jvdelisle Date: Sun May 27 03:22:11 2018 New Revision: 260802 URL: https://gcc.gnu.org/viewcvs?rev=260802&root=gcc&view=rev Log: 2018-05-26 Jerry DeLisle Backport from trunk. PR libgfortran/85906 * io/write.c (write_integer): Initialise the fnode format to FMT_NONE, used for list directed write. (BUF_STACK_SZ): Bump default buffer size up to avoid allocs on small stuff. 2018-05-26 Jerry DeLisle Backport from trunk. PR libgfortran/85840 * io/write.c (write_float_0): Use separate local variable for the float string length. Modified: branches/gcc-8-branch/libgfortran/ChangeLog branches/gcc-8-branch/libgfortran/io/write.c
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #12 from Jerry DeLisle --- Fixed on trunk. I think this should be backported as it is a regression I think on 7 and 8 branches.
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #11 from Jerry DeLisle --- Author: jvdelisle Date: Sat May 26 17:30:52 2018 New Revision: 260793 URL: https://gcc.gnu.org/viewcvs?rev=260793&root=gcc&view=rev Log: 2018-05-26 Jerry DeLisle PR libgfortran/85840 * io/write.c (write_float_0): Use separate local variable for the float string length. Modified: trunk/libgfortran/io/write.c
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #10 from Jerry DeLisle --- (In reply to Joshua Cogliati from comment #9) --- snip --- > I could look into either method of fixing this if you want. (And for what > it is worth, I do have copyright assignment paperwork from both myself and > my employer for GCC filed) First, thank you very much for the offer to help. Truthfully we do need more contributors and it does take time to get familiar with the code base, and it can be fun to work on. This bug is my doing for sure. I did a significant re-factoring of the code and lost track of that variable, res_len. The intent is that it does not get modified outside of those functions that are allocating the buffers. In fact it is getting modified by build_float_string in write_float.def. The build_float_string is being called within the macros of get_float_string. A variable needs to be used by thos macros, but I don't think it needs to be passed up the call chain. The simplest thing would be to give a different variable to get float string. @@ -1566,19 +1568,19 @@ write_float_0 (st_parameter_dt *dtp, const fnode *f, const char *source, int kin char buf_stack[BUF_STACK_SZ]; char str_buf[BUF_STACK_SZ]; char *buffer, *result; - size_t buf_size, res_len; + size_t buf_size, res_len, flt_str_len; /* Precision for snprintf call. */ int precision = get_precision (dtp, f, source, kind); /* String buffer to hold final result. */ result = select_string (dtp, f, str_buf, &res_len, kind); buffer = select_buffer (dtp, f, precision, buf_stack, &buf_size, kind); get_float_string (dtp, f, source , kind, 0, buffer, - precision, buf_size, result, &res_len); - write_float_string (dtp, result, res_len); + precision, buf_size, result, &flt_str_len); + write_float_string (dtp, result, flt_str_len); if (buf_size > BUF_STACK_SZ) free (buffer); I also noticed that the buffer size for this small float is 323 which seems crazy big, but there are other use cases where we need to hold much higher precision numbers of digits. It seems pointless to do an alloc for this case so I also suggest we do this: @@ -1465,7 +1466,7 @@ write_character (st_parameter_dt *dtp, const char *source, int kind, size_t leng /* Floating point helper functions. */ -#define BUF_STACK_SZ 256 +#define BUF_STACK_SZ 384 This eliminates 3 allocs in one I/O operation. The purpose of all this alloc logic is to eliminate them. The stack size change will mask the problem uncovered in this test case, so I have tested with and without it. Since I pretty much have this figured out, I will go ahead and do the commit. However, please dont hesitate to take on other bugs and join the gfortranners club. Much appreciated.
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #9 from Joshua Cogliati --- Looking at write.c, there are multiple places where things like the pattern: result = select_string (dtp, f, str_buf, &res_len, kind); ... get_float_string (dtp, f, source , kind, 0, buffer, precision, buf_size, result, &res_len); ... if (res_len > BUF_STACK_SZ) free (result); appear (such as write_complex, write_real etc). So either they all need to be fixed, or get_float_string needs to never change the length compared to select_string. I could look into either method of fixing this if you want. (And for what it is worth, I do have copyright assignment paperwork from both myself and my employer for GCC filed)
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #8 from Joshua Cogliati --- Created attachment 44186 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=44186&action=edit Patch by keeping original length This patches the problem by storing the allocated length in a separate variable. This also fixes the problem with the free not being called. Both these patches are against svn 260704.
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #7 from Joshua Cogliati --- Created attachment 44185 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=44185&action=edit Patch by checking against original buffer This is one possible patch for this problem, it just checks if the buffer is at a different address than the stack buffer. It does fix the problem in my test case.
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 Jerry DeLisle changed: What|Removed |Added Status|WAITING |ASSIGNED Assignee|unassigned at gcc dot gnu.org |jvdelisle at gcc dot gnu.org --- Comment #6 from Jerry DeLisle --- That did not take long. Thanks for simple test case. Confirmed.
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #5 from Jerry DeLisle --- I will see if I can reproduce this here with you example and if so will get on it.
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #4 from Joshua Cogliati --- And to replicate it with the simple program: gfortran -Wall -g -o simple simple.f90 valgrind --leak-check=full ./simple and you get things like: ==19171== 323 bytes in 1 blocks are definitely lost in loss record 4 of 4 ==19171==at 0x4C2CB6B: malloc (vg_replace_malloc.c:299) ==19171==by 0x4E53454: _gfortrani_xmalloc (memory.c:42) ==19171==by 0x4FD637A: write_float_0 (write.c:1593) ==19171==by 0x4FCD98C: formatted_transfer_scalar_write (transfer.c:2041) ==19171==by 0x4FCDF9C: formatted_transfer (transfer.c:2279) ==19171==by 0x4008A5: MAIN__ (simple.f90:10) ==19171==by 0x4008EB: main (simple.f90:11)
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 --- Comment #3 from Joshua Cogliati --- Created attachment 44150 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=44150&action=edit simple program to demonstrate problem
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 Joshua Cogliati changed: What|Removed |Added CC||jjcogliati-r1 at yahoo dot com --- Comment #2 from Joshua Cogliati --- I haven't found a short reproducer yet. Here is how I can reproduce it. Get marbles from: http://jjc.freeshell.org/marbles/marbles.tar.gz tar -xzf marbles.tar.gz cd marbles/src cat > short <
[Bug fortran/85840] Memory leak in write.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85840 Dominique d'Humieres changed: What|Removed |Added Status|UNCONFIRMED |WAITING Last reconfirmed||2018-05-19 CC||jvdelisle at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #1 from Dominique d'Humieres --- > where a,b,c,d,e,f are complicated real kinds. What is "complicated real kinds"? This should not matter. I cannot reproduce the problem with integer :: i real :: a,b,c,d,e,f a = 1.0 b = 2.0 c = 3.0 d = 4.0 e = 5.0 f = 6.0 i = 1000 print '(A16,I7,6E25.16E3)',"start line print",i,a,b,c,d,e,f end Could you please provide a reproducer?
