Re: [fortran, patch] Allow displaying backtraces from user code
Attached is a new patch, which expands the documentation according to your proposal, and uses the name BACKTRACE. I hope that both Janne and Tobias can agree with this naming decision ... Looks fine from my side. Great, thanks. Janne? Yes, Ok for trunk. Thanks again to both of you. Committed as r194648. Cheers, Janus Can you also add a quip to http://gcc.gnu.org/wiki/GFortran#GCC4.8 ? Sure, as soon as the patch is committed ... Cheers, Janus -- Janne Blomqvist
Re: [fortran, patch] Allow displaying backtraces from user code
On Sun, Dec 16, 2012 at 12:50 AM, Janus Weil ja...@gcc.gnu.org wrote:
Hi,
So, in principle I'm fine with all your BACKTRACE_* variants (except
for _splurge, maybe ;)
Or, why not just (plain and simple) BACKTRACE?
The name is the same as backtrace() in glibc, but otherwise, sure why
not. _show/_print might be preferable in the sense that they convey
that stuff will be directly printed on the screen, rather than, say,
the procedure returning an array of strings with the stack trace info.
Agreed. Let's go with BACKTRACE_SHOW.
Attached is a new patch which uses this name. Moreover, it follow your
previous advice to move the message Backtrace for this error out of
backtrace_show into backtrace_handler. I also added Program aborted.
Backtrace: in sys_abort.
- 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.
Good point. Do you have any particular suggestions about what would be
needed in this direction? (You're probably much more familiar with the
libgfortran code than I am.)
As a simple test, something like the following (untested) code might do:
program b
integer :: i
do i = 1, 100
call backtrace_show
end do
read(*, *)
end program b
When the programs waits on user input, check with ps -eFH that your
a.out process (or whatever you call the binary) doesn't have any child
processes, then ls /proc/[PID]/fd and check that the process has
only 3 fd's (std{in,out,err}).
Ok, I tried this and indeed there seem to be no child processes left.
However, I do see open fd's (one for each backtrace invocation).
Looking at the code, it seems a close (f[0]) was missing (which I
added now).
Great, thanks for fixing this!
Do you have any further comments or do you think the patch is ok for trunk
now?
Ok for trunk. A minor addition, if you care, would be to mention in
the documentation for backtrace_show() that the error message is
printed to the unit corresponding to ERROR_UNIT in ISO_FORTRAN_ENV.
Thanks for the patch!
--
Janne Blomqvist
Re: [fortran, patch] Allow displaying backtraces from user code
Hi, first off: Some more words on the naming issue. I actually still prefer the most simple and straightforward variant (i.e. BACKTRACE, which can easily be found and does not sound 'clumsy') ... Or, why not just (plain and simple) BACKTRACE? The name is the same as backtrace() in glibc, but otherwise, sure why not. Is that actually an issue at all? The intrinsic procedure would be accessible as BACKTRACE from Fortran programs, but internally it receives the usual libgfortran name mangling, which makes it _gfortran_backtrace. So there should be no naming collision issues with glibc's backtrace(), right? _show/_print might be preferable in the sense that they convey that stuff will be directly printed on the screen, rather than, say, the procedure returning an array of strings with the stack trace info. I don't see that as a problem either, since we probably do not want to add another intrinsic which returns the backtrace as a string or something (if anything, we might add an optional integer argument, in order to print to a different unit, but I'm not proposing to do that right now). In any case, the procedure's behavior is described in the documentation. Do you have any further comments or do you think the patch is ok for trunk now? Ok for trunk. A minor addition, if you care, would be to mention in the documentation for backtrace_show() that the error message is printed to the unit corresponding to ERROR_UNIT in ISO_FORTRAN_ENV. Done. Thanks for the patch! Thanks for reviewing. Attached is a new patch, which expands the documentation according to your proposal, and uses the name BACKTRACE. I hope that both Janne and Tobias can agree with this naming decision ... Cheers, Janus pr36044_v3.diff Description: Binary data
Re: [fortran, patch] Allow displaying backtraces from user code
Janus Weil wrote: Attached is a new patch, which expands the documentation according to your proposal, and uses the name BACKTRACE. I hope that both Janne and Tobias can agree with this naming decision ... Looks fine from my side. Can you also add a quip to http://gcc.gnu.org/wiki/GFortran#GCC4.8 ? Tobias
Re: [fortran, patch] Allow displaying backtraces from user code
Attached is a new patch, which expands the documentation according to your proposal, and uses the name BACKTRACE. I hope that both Janne and Tobias can agree with this naming decision ... Looks fine from my side. Great, thanks. Janne? Can you also add a quip to http://gcc.gnu.org/wiki/GFortran#GCC4.8 ? Sure, as soon as the patch is committed ... Cheers, Janus
Re: [fortran, patch] Allow displaying backtraces from user code
Janus Weil wrote: So, in principle I'm fine with all your BACKTRACE_* variants (except for _splurge, maybe;) Or, why not just (plain and simple) BACKTRACE? The name is the same as backtrace() in glibc, but otherwise, sure why not. _show/_print might be preferable in the sense that they convey that stuff will be directly printed on the screen, rather than, say, the procedure returning an array of strings with the stack trace info. Agreed. Let's go with BACKTRACE_SHOW. I have to admit that I prefer show_backtrace to backtrace_show, which sounds a bit clumsy. I also don't think that finding show_backtrace is more difficult than finding backtrace_show. backtrace is in the index, looking at the documentation, one can still search for backtrace and search engines should find backtrace in either way. (A name which comes just into my mind is: backtrace_now(); I am not claiming that it is better than any of the others.) Enough of bikeshadding: I think having the possibility to simply print a backtrace is very useful! Hence, I leave the name to Janus and Janne. Tobias
Re: [fortran, patch] Allow displaying backtraces from user code
So, in principle I'm fine with all your BACKTRACE_* variants (except for _splurge, maybe;) Or, why not just (plain and simple) BACKTRACE? The name is the same as backtrace() in glibc, but otherwise, sure why not. _show/_print might be preferable in the sense that they convey that stuff will be directly printed on the screen, rather than, say, the procedure returning an array of strings with the stack trace info. Agreed. Let's go with BACKTRACE_SHOW. I have to admit that I prefer show_backtrace to backtrace_show, which sounds a bit clumsy. I also don't think that finding show_backtrace is more difficult than finding backtrace_show. backtrace is in the index, looking at the documentation, one can still search for backtrace and search engines should find backtrace in either way. (A name which comes just into my mind is: backtrace_now(); I am not claiming that it is better than any of the others.) Look, I really don't care if we call it APOCALYPSE_NOW or SERGEANT_FUZZY_BOOTS, as long as it prints a proper backtrace! If we can not agree on any name here, we could just open a doodle poll to decide this by democratic vote ...?!? Cheers, Janus
Re: [fortran, patch] Allow displaying backtraces from user code
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
Re: [fortran, patch] Allow displaying backtraces from user code
Hi Janne,
here is another attempt to make gfortran support user-requested backtraces.
[...]
Ok for trunk?
Some comments.
thanks for your 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.
well, yeah. There was discussion about the naming before. I don't have
a very strong opinion on that, and I just used what is there in
libgfortran, namely SHOW_BACKTRACE.
IMHO any reasonably intelligent person would be capable of using the
search-feature of his browser of pdf viewer to find any *BACKTRACE*
name in the list of intrinsics, but I guess there is no harm in
starting the name with BACKTRACE.
So, in principle I'm fine with all your BACKTRACE_* variants (except
for _splurge, maybe ;)
Or, why not just (plain and simple) BACKTRACE?
- 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.
Good point. Do you have any particular suggestions about what would be
needed in this direction? (You're probably much more familiar with the
libgfortran code than I am.)
- 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).
Yes, I also noticed this. I think your suggestion makes sense.
Otherwise the general idea is Ok, IMHO.
Thanks again for the review!
Cheers,
Janus
Re: [fortran, patch] Allow displaying backtraces from user code
Hi,
So, in principle I'm fine with all your BACKTRACE_* variants (except
for _splurge, maybe ;)
Or, why not just (plain and simple) BACKTRACE?
The name is the same as backtrace() in glibc, but otherwise, sure why
not. _show/_print might be preferable in the sense that they convey
that stuff will be directly printed on the screen, rather than, say,
the procedure returning an array of strings with the stack trace info.
Agreed. Let's go with BACKTRACE_SHOW.
Attached is a new patch which uses this name. Moreover, it follow your
previous advice to move the message Backtrace for this error out of
backtrace_show into backtrace_handler. I also added Program aborted.
Backtrace: in sys_abort.
- 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.
Good point. Do you have any particular suggestions about what would be
needed in this direction? (You're probably much more familiar with the
libgfortran code than I am.)
As a simple test, something like the following (untested) code might do:
program b
integer :: i
do i = 1, 100
call backtrace_show
end do
read(*, *)
end program b
When the programs waits on user input, check with ps -eFH that your
a.out process (or whatever you call the binary) doesn't have any child
processes, then ls /proc/[PID]/fd and check that the process has
only 3 fd's (std{in,out,err}).
Ok, I tried this and indeed there seem to be no child processes left.
However, I do see open fd's (one for each backtrace invocation).
Looking at the code, it seems a close (f[0]) was missing (which I
added now).
Do you have any further comments or do you think the patch is ok for trunk now?
Cheers,
Janus
pr36044_v2.diff
Description: Binary data
Re: [fortran, patch] Allow displaying backtraces from user code
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? Cheers, Janus 2012-12-14 Janus Weil ja...@gcc.gnu.org PR fortran/36044 * gfortran.h (gfc_isym_id): Add GFC_ISYM_SHOW_BACKTRACE. * intrinsic.c (add_subroutines): Add show_backtrace. * intrinsic.texi (SHOW_BACKTRACE): Document SHOW_BACKTRACE intrinsic. 2012-12-14 Janus Weil ja...@gcc.gnu.org PR fortran/36044 * gfortran.map: Add _gfortran_show_backtrace. * libgfortran.h: Export show_backtrace. * runtime/backtrace.c: Ditto. 2012/6/21 Janus Weil ja...@gcc.gnu.org: There are two possibilities: a) Making _gfortran_show_backtrace accessible from the outside (via manual C binding from Fortran) b) Adding a new intrinsic I would vote for b), as it gets documented then. It is enough useful for a wide range of programmers to deserve an intrinsic of its own, IMHO. And always directly available, no need of module convolutions. As noted before, I also prefer b). Name: simply show_backtrace ? This would be a self-explaining name, the odd QQ in tracebackqq is just this, odd. And why call it traceback when it is actually a backtrace ;-) Adopting the name from Intel would have the advantage of compatibility between ifort and gfortran. However, since other vendors have different names, compatibility between several compilers in this non-standard function will not be realized. Moreover I agree that the 'QQ' part is odd (I never understood what it is supposed to mean). Therefore I would also vote for something like show_backtrace (or simply backtrace?). Cheers, Janus pr36044_backtrace_intrinsic.diff Description: Binary data
Re: [fortran, patch] Allow displaying backtraces from user code
On 03/03/2012 08:44 AM, FX wrote [1]: PR 36044 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36044) is an enhancement request for a way to display backtraces from user code. I wanted to come back to that patch for some while. I think it makes sense to offer this feature in some why and as the PR but also a question on #gfortran shows, there is a need to do so. There are two possibilities: a) Making _gfortran_show_backtrace accessible from the outside (via manual C binding from Fortran) b) Adding a new intrinsic The latter is done by other vendors: - Intel via DEC/Digital has in the module IFCORE the subroutine TRACEBACKQQ [3] - Lahey has call ERRTRA() - SGI has call Trace_Back_Stack_and_Print() - IBM has call xl__trbk() FX suggest to do (a). Steve [4] is for (b) Janus [3] seems to be fine with (a) but favours (b) While I have to admit that I am happy either choice. It seems as if there is a small majority for adding another intrinsic. The big question is the name and whether it should be available by default or - as with ifort - via an intrinsic module. One possibility would be to take the name of and the arguments from Intel's version [3]. Tobias [1] http://gcc.gnu.org/ml/fortran/2012-03/msg00015.html [2] http://gcc.gnu.org/ml/fortran/2012-04/msg00131.html [3] http://software.intel.com/sites/products/documentation/hpc/compilerpro/en-us/fortran/lin/compiler_f/lref_for/source_files/rftrace.htm [4] http://gcc.gnu.org/ml/fortran/2012-03/msg00022.html I'm against adding yet another nonstandard intrinsic for this purpose (which is how Intel Fortran does it), but I would like to offer the following solution to the issue, as I think it can be useful in some cases (and the way I suggest should not be a maintainance burden for us): -- export _gfortran_show_backtrace() from libgfortran (instead of it being an internal function) -- add documentation on how to call this function from user-code using BIND(C) Patch was bootstrapped and regtested on x86_64-apple-darwin11, also tested with make info html pdf. OK for trunk? FX
Re: [fortran, patch] Allow displaying backtraces from user code
Am 21.06.2012 14:15, schrieb Tobias Burnus: On 03/03/2012 08:44 AM, FX wrote [1]: PR 36044 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36044) is an enhancement request for a way to display backtraces from user code. I wanted to come back to that patch for some while. I think it makes sense to offer this feature in some why and as the PR but also a question on #gfortran shows, there is a need to do so. It is definitely needed, IMHO. In a modular program the calling path is often needed to interpret some hiccups, and a debugger is not always suited. Program runs are not always reproducible, and with this you can add debugging statements into operational code. I ended up writing a wrapper around the c-functions backtrace() and backtrace_symbols_fd(), but then you have to interpret it externally with addr2line. There are two possibilities: a) Making _gfortran_show_backtrace accessible from the outside (via manual C binding from Fortran) b) Adding a new intrinsic I would vote for b), as it gets documented then. It is enough useful for a wide range of programmers to deserve an intrinsic of its own, IMHO. And always directly available, no need of module convolutions. Name: simply show_backtrace ? This would be a self-explaining name, the odd QQ in tracebackqq is just this, odd. And why call it traceback when it is actually a backtrace ;-) Cheers, Manfred
Re: [fortran, patch] Allow displaying backtraces from user code
There are two possibilities: a) Making _gfortran_show_backtrace accessible from the outside (via manual C binding from Fortran) b) Adding a new intrinsic I would vote for b), as it gets documented then. It is enough useful for a wide range of programmers to deserve an intrinsic of its own, IMHO. And always directly available, no need of module convolutions. As noted before, I also prefer b). Name: simply show_backtrace ? This would be a self-explaining name, the odd QQ in tracebackqq is just this, odd. And why call it traceback when it is actually a backtrace ;-) Adopting the name from Intel would have the advantage of compatibility between ifort and gfortran. However, since other vendors have different names, compatibility between several compilers in this non-standard function will not be realized. Moreover I agree that the 'QQ' part is odd (I never understood what it is supposed to mean). Therefore I would also vote for something like show_backtrace (or simply backtrace?). Cheers, Janus
Re: [fortran, patch] Allow displaying backtraces from user code
Hi guys, (coming back to an old patch proposed by FX some time ago ...) 2012/3/3 FX fxcoud...@gmail.com: I think that this approach is a mistake. The patch starts us down a slippery slope. Why not export all the nonstandard intrinsics? In additions, the _gfortran_ prefix was used to separate the libgfortran namespace from userspace. Providing a means to circumvent this separation seems to asking for more PR. Well, the mean exists. All _gfortran_* functions can already be called, they're part of libgfortran's public (and versioned) API. I'm just saying adding a simple backtrace function to that list is useful, and documenting it too. Yes, I agree that this is useful, and in that sense the patch is ok from my side ... I would rather see a new intrinsic procedure add to gfortran. The standard does not prevent a vendor from offer additional intrinsic procedures not found in the standard. I just think multiplicating vendor intrinsics makes our life harder. I'd like to hear other's opinions on this issue, but I'll implement the new intrinsic if that's the consensus. ... but I also think that having an intrinsic function for it would be useful, so that one can just call it without the detour via ISO_C_BINDING. Note that ifort also has a vendor intrinsic for this, called TRACEBACKQQ, so we're in good company. Cheers, Janus
Re: [fortran, patch] Allow displaying backtraces from user code
On Sat, Mar 03, 2012 at 08:44:56AM +0100, FX wrote: PR 36044 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36044) is an enhancement request for a way to display backtraces from user code. I'm against adding yet another nonstandard intrinsic for this purpose (which is how Intel Fortran does it), but I would like to offer the following solution to the issue, as I think it can be useful in some cases (and the way I suggest should not be a maintainance burden for us): -- export _gfortran_show_backtrace() from libgfortran (instead of it being an internal function) -- add documentation on how to call this function from user-code using BIND(C) Patch was bootstrapped and regtested on x86_64-apple-darwin11, also tested with make info html pdf. OK for trunk? FX I think that this approach is a mistake. The patch starts us down a slippery slope. Why not export all the nonstandard intrinsics? In additions, the _gfortran_ prefix was used to separate the libgfortran namespace from userspace. Providing a means to circumvent this separation seems to asking for more PR. I would rather see a new intrinsic procedure add to gfortran. The standard does not prevent a vendor from offer additional intrinsic procedures not found in the standard. -- Steve
Re: [fortran, patch] Allow displaying backtraces from user code
I think that this approach is a mistake. The patch starts us down a slippery slope. Why not export all the nonstandard intrinsics? In additions, the _gfortran_ prefix was used to separate the libgfortran namespace from userspace. Providing a means to circumvent this separation seems to asking for more PR. Well, the mean exists. All _gfortran_* functions can already be called, they're part of libgfortran's public (and versioned) API. I'm just saying adding a simple backtrace function to that list is useful, and documenting it too. I would rather see a new intrinsic procedure add to gfortran. The standard does not prevent a vendor from offer additional intrinsic procedures not found in the standard. I just think multiplicating vendor intrinsics makes our life harder. I'd like to hear other's opinions on this issue, but I'll implement the new intrinsic if that's the consensus. Thanks, FX
[fortran, patch] Allow displaying backtraces from user code
PR 36044 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36044) is an enhancement request for a way to display backtraces from user code. I'm against adding yet another nonstandard intrinsic for this purpose (which is how Intel Fortran does it), but I would like to offer the following solution to the issue, as I think it can be useful in some cases (and the way I suggest should not be a maintainance burden for us): -- export _gfortran_show_backtrace() from libgfortran (instead of it being an internal function) -- add documentation on how to call this function from user-code using BIND(C) Patch was bootstrapped and regtested on x86_64-apple-darwin11, also tested with make info html pdf. OK for trunk? FX traceback2.ChangeLog Description: Binary data traceback2.diff Description: Binary data
