On Fri, Dec 14, 2012 at 4:31 PM, Janus Weil <ja...@gcc.gnu.org> wrote:
> Hi all,
>
> here is another attempt to make gfortran support user-requested backtraces.
>
> A patch in this direction was already proposed by FX in March, but did
> not make it in so far. It was last discussed in June, cf.
> http://gcc.gnu.org/ml/fortran/2012-06/msg00131.html and follow-ups,
> where the consensus seemed to be to add a new intrinsic subroutine for
> this (as GNU extension).
>
> This is just what the attached patch does: It exports
> _gfortran_show_backtrace from libgfortran and adds an intrinsic
> SHOW_BACKTRACE, together with some documentation in intrinsic.texi (it
> also documents the fact that ABORT gives a backtrace recently).
>
> Ok for trunk?

Some comments.

- I'd prefer to reverse the name, e.g. BACKTRACE_{SHOW,PRINT,SPLURGE},
to make it more "discoverable" when browsing the manual.
BACKTRACE_PRINT would have the advantage of matching backtrace_print()
from libbacktrace, but then again that function takes a couple of
arguments so perhaps it would cause more confusion than enlightenment.

- As previously show_backtrace() was always followed by program
termination, we now need to ensure that it properly cleans up after
itself in case the application continues execution. In particular,
make sure it doesn't leak file descriptors, and that the addr2line
child process terminates properly.

- In the beginning of show_backtrace(), we print a line "Backtrace for
this error:". Now that we allow the user to initiate a backtrace print
at any point, this line may not be what the user wants. I suggest
moving that line to runtime/compile_options.c (show_signal).

Otherwise the general idea is Ok, IMHO.


-- 
Janne Blomqvist

Reply via email to