Re: [fortran, patch] Allow displaying backtraces from user code

[Janus Weil <janus at gcc dot gnu dot org>]

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

Attachment: pr36044_v2.diff
Description: Binary data