Bug 88137 - BACKTRACE seems to have a memory leak
Summary: BACKTRACE seems to have a memory leak
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 8.2.0
: P3 normal
Target Milestone: ---
Assignee: Janne Blomqvist
URL: https://gcc.gnu.org/ml/gcc-patches/20...
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-21 17:16 UTC by Bill Long
Modified: 2018-12-06 15:43 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-11-22 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bill Long 2018-11-21 17:16:39 UTC
Simple test case from site:

> cat test.f90
program test

implicit none
integer :: i

do i= 1,10000
call backtrace()
enddo

end program test

Tracing memory usage with gfortran 4.9.3:

Process Memory Details
Virtual maximum (MB) 1.262
Resident maximum (MB) 0.930
Virtual at exit (MB) 1.258
Resident at exit (MB) 0.930

But with gfortran 8.2.0 

Process Memory Details
 Virtual  maximum   (MB)       48282.594
 Resident maximum   (MB)       31592.816
 Virtual  at exit   (MB)       48282.594
 Resident at exit   (MB)       31592.816


While the test case is artificial, it does illustrate that calls to backtrace seems to accumulate memory usage.
Comment 1 Janne Blomqvist 2018-11-22 08:33:09 UTC
Confirmed on latest trunk (9.0.0 20181122), x86_64-pc-linux-gnu

Changing the example to execute 10 iterations of the loop, and running via /usr/bin/time shows

(0avgtext+0avgdata 67124maxresident)k

and with 100 iterations

(0avgtext+0avgdata 644684maxresident)k
Comment 2 Janne Blomqvist 2018-11-22 08:39:23 UTC
In backtrace.h (the header for the libbacktrace library that we use to print backtraces) we have for the backtrace_create_state function:

   Calling this function allocates resources that can not be freed.
   There is no backtrace_free_state function.  The state is used to
   cache information that is expensive to recompute.  Programs are
   expected to call this function at most once and to save the return
   value for all later calls to backtrace functions.


In libgfortran we don't do this, but rather with call backtrace_create_state() again for every invocation of the GFortran backtrace intrinsic.

Assigning to myself.
Comment 3 Janne Blomqvist 2018-11-22 09:20:13 UTC
Patch here: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01901.html
Comment 4 Janne Blomqvist 2018-11-30 16:45:00 UTC
Author: jb
Date: Fri Nov 30 16:44:27 2018
New Revision: 266677

URL: https://gcc.gnu.org/viewcvs?rev=266677&root=gcc&view=rev
Log:
Initialize backtrace state once

From backtrace.h for backtrace_create_state:

   Calling this function allocates resources that can not be freed.
   There is no backtrace_free_state function.  The state is used to
   cache information that is expensive to recompute.  Programs are
   expected to call this function at most once and to save the return
   value for all later calls to backtrace functions.

So instead of calling backtrace_create_state every time we wish to
show a backtrace, do it once and store the result in a static
variable.  libbacktrace allows multiple threads to access the state,
so no need to use TLS.

Regtested on x86_64-pc-linux-gnu.

libgfortran/ChangeLog:

2018-11-30  Janne Blomqvist  <jb@gcc.gnu.org>

	PR libfortran/88137
	* runtime/backtrace.c (show_backtrace): Make lbstate a static
	variable, initialize once.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/runtime/backtrace.c
Comment 5 Janne Blomqvist 2018-11-30 16:47:27 UTC
Author: jb
Date: Fri Nov 30 16:46:55 2018
New Revision: 266678

URL: https://gcc.gnu.org/viewcvs?rev=266678&root=gcc&view=rev
Log:
Initialize backtrace state once

From backtrace.h for backtrace_create_state:

   Calling this function allocates resources that can not be freed.
   There is no backtrace_free_state function.  The state is used to
   cache information that is expensive to recompute.  Programs are
   expected to call this function at most once and to save the return
   value for all later calls to backtrace functions.

So instead of calling backtrace_create_state every time we wish to
show a backtrace, do it once and store the result in a static
variable.  libbacktrace allows multiple threads to access the state,
so no need to use TLS.

Regtested on x86_64-pc-linux-gnu.

Backport from trunk.

libgfortran/ChangeLog:

2018-11-30  Janne Blomqvist  <jb@gcc.gnu.org>

	PR libfortran/88137
	* runtime/backtrace.c (show_backtrace): Make lbstate a static
	variable, initialize once.

Modified:
    branches/gcc-8-branch/libgfortran/ChangeLog
    branches/gcc-8-branch/libgfortran/runtime/backtrace.c
Comment 6 Janne Blomqvist 2018-12-02 15:13:16 UTC
Author: jb
Date: Sun Dec  2 15:12:44 2018
New Revision: 266724

URL: https://gcc.gnu.org/viewcvs?rev=266724&root=gcc&view=rev
Log:
Use atomic load/store to access static backtrace state pointer

As the static backtrace state pointer can be accessed from multiple
threads, use atomics to access it.

Regtested on x86_64-pc-linux-gnu.

libgfortran/ChangeLog:

2018-12-02  Janne Blomqvist  <jb@gcc.gnu.org>

	PR libfortran/88137
	* runtime/backtrace.c (show_backtrace): Use atomic load/store to
	access the static lbstate pointer.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/runtime/backtrace.c
Comment 7 Janne Blomqvist 2018-12-02 15:15:25 UTC
Author: jb
Date: Sun Dec  2 15:14:51 2018
New Revision: 266725

URL: https://gcc.gnu.org/viewcvs?rev=266725&root=gcc&view=rev
Log:
Use atomic load/store to access static backtrace state pointer

As the static backtrace state pointer can be accessed from multiple
threads, use atomics to access it.

Regtested on x86_64-pc-linux-gnu.

libgfortran/ChangeLog:

2018-12-02  Janne Blomqvist  <jb@gcc.gnu.org>

        Backport from trunk
	PR libfortran/88137
	* runtime/backtrace.c (show_backtrace): Use atomic load/store to
	access the static lbstate pointer.

Modified:
    branches/gcc-8-branch/libgfortran/ChangeLog
    branches/gcc-8-branch/libgfortran/runtime/backtrace.c
Comment 8 Janne Blomqvist 2018-12-06 15:38:57 UTC
Author: jb
Date: Thu Dec  6 15:38:25 2018
New Revision: 266858

URL: https://gcc.gnu.org/viewcvs?rev=266858&root=gcc&view=rev
Log:
Initialize backtrace state once

From backtrace.h for backtrace_create_state:

   Calling this function allocates resources that can not be freed.
   There is no backtrace_free_state function.  The state is used to
   cache information that is expensive to recompute.  Programs are
   expected to call this function at most once and to save the return
   value for all later calls to backtrace functions.

So instead of calling backtrace_create_state every time we wish to
show a backtrace, do it once and store the result in a static
variable.  libbacktrace allows multiple threads to access the state,
so no need to use TLS, but use atomic load/store to access the static
variable.

Regtested on x86_64-pc-linux-gnu.

libgfortran/ChangeLog:

2018-12-06  Janne Blomqvist  <jb@gcc.gnu.org>

	Backport from trunk
	PR libfortran/88137
	* runtime/backtrace.c (show_backtrace): Store backtrace state in a
	static variable, initialize once.

Modified:
    branches/gcc-7-branch/libgfortran/ChangeLog
    branches/gcc-7-branch/libgfortran/runtime/backtrace.c
Comment 9 Janne Blomqvist 2018-12-06 15:43:45 UTC
Fixed on trunk/8/7 branches, closing.

The commit to the gcc-7 branch is a merge of the two separate commits to trunk.