public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [fortran, patch] Allow displaying backtraces from user code
@ 2012-03-03  7:45 FX
  2012-03-03 15:42 ` Steve Kargl
  2012-06-21 12:35 ` Tobias Burnus
  0 siblings, 2 replies; 21+ messages in thread
From: FX @ 2012-03-03  7:45 UTC (permalink / raw)
  To: fortran; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

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



[-- Attachment #2: traceback2.ChangeLog --]
[-- Type: application/octet-stream, Size: 311 bytes --]

2012-03-03  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>

	PR libfortran/36044
	* gfortran.map (GFORTRAN_1.5): Create new list.
	* libgfortran.h (show_backtrace): Export prototype.
	* invoke.texi (-fno-backtrace): Fix spelling in options list.
	Document the use of _gfortran_show_backtrace from user code.


[-- Attachment #3: traceback2.diff --]
[-- Type: application/octet-stream, Size: 2050 bytes --]

Index: libgfortran/gfortran.map
===================================================================
--- libgfortran/gfortran.map	(revision 184642)
+++ libgfortran/gfortran.map	(working copy)
@@ -1189,6 +1189,11 @@ GFORTRAN_1.4 {
     _gfortran_eoshift2_16_char4;
 } GFORTRAN_1.3; 
 
+GFORTRAN_1.5 {
+  global:
+    _gfortran_show_backtrace;
+} GFORTRAN_1.4;
+
 F2C_1.0 {
   global:
     _gfortran_f2c_specific__abs_c4;
Index: libgfortran/libgfortran.h
===================================================================
--- libgfortran/libgfortran.h	(revision 184642)
+++ libgfortran/libgfortran.h	(working copy)
@@ -671,7 +671,7 @@ internal_proto(find_addr2line);
 /* backtrace.c */
 
 extern void show_backtrace (void);
-internal_proto(show_backtrace);
+export_proto(show_backtrace);
 
 /* error.c */
 
Index: gcc/fortran/invoke.texi
===================================================================
--- gcc/fortran/invoke.texi	(revision 184642)
+++ gcc/fortran/invoke.texi	(working copy)
@@ -153,7 +153,7 @@ and warnings}.
 
 @item Debugging Options
 @xref{Debugging Options,,Options for debugging your program or GNU Fortran}.
-@gccoptlist{-fbacktrace -fdump-fortran-optimized -fdump-fortran-original @gol
+@gccoptlist{-fno-backtrace -fdump-fortran-optimized -fdump-fortran-original @gol
 -fdump-parse-tree -ffpe-trap=@var{list}
 }
 
@@ -991,6 +991,25 @@ backtrace of the error. @code{-fno-backt
 generation. This option only has influence for compilation of the
 Fortran main program.
 
+Whether or not @code{-fno-backtrace} is specified, a backtrace can
+always be displayed from user code without interrupting the program
+execution by calling the @code{_gfortran_show_backtrace()} subroutine:
+
+@smallexample
+program test
+
+  interface
+    subroutine show_backtrace() bind(c,name="_gfortran_show_backtrace")
+    end subroutine
+  end interface
+
+  print *, "42"
+  call show_backtrace()
+  print *, "42 again"
+
+end program test
+@end smallexample
+
 @end table
 
 @xref{Debugging Options,,Options for Debugging Your Program or GCC,

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-03-03  7:45 [fortran, patch] Allow displaying backtraces from user code FX
@ 2012-03-03 15:42 ` Steve Kargl
  2012-03-03 21:21   ` FX
  2012-06-21 12:35 ` Tobias Burnus
  1 sibling, 1 reply; 21+ messages in thread
From: Steve Kargl @ 2012-03-03 15:42 UTC (permalink / raw)
  To: FX; +Cc: fortran, gcc-patches

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-03-03 15:42 ` Steve Kargl
@ 2012-03-03 21:21   ` FX
  2012-04-24 13:04     ` Janus Weil
  0 siblings, 1 reply; 21+ messages in thread
From: FX @ 2012-03-03 21:21 UTC (permalink / raw)
  To: Steve Kargl; +Cc: fortran, gcc-patches

> 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-03-03 21:21   ` FX
@ 2012-04-24 13:04     ` Janus Weil
  0 siblings, 0 replies; 21+ messages in thread
From: Janus Weil @ 2012-04-24 13:04 UTC (permalink / raw)
  To: FX; +Cc: Steve Kargl, fortran, gcc-patches

Hi guys,

(coming back to an old patch proposed by FX some time ago ...)

2012/3/3 FX <fxcoudert@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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-03-03  7:45 [fortran, patch] Allow displaying backtraces from user code FX
  2012-03-03 15:42 ` Steve Kargl
@ 2012-06-21 12:35 ` Tobias Burnus
  2012-06-21 14:48   ` Manfred Schwarb
  1 sibling, 1 reply; 21+ messages in thread
From: Tobias Burnus @ 2012-06-21 12:35 UTC (permalink / raw)
  To: FX; +Cc: fortran, gcc-patches

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
>
>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-06-21 12:35 ` Tobias Burnus
@ 2012-06-21 14:48   ` Manfred Schwarb
  2012-06-21 17:01     ` Janus Weil
  0 siblings, 1 reply; 21+ messages in thread
From: Manfred Schwarb @ 2012-06-21 14:48 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: FX, fortran, gcc-patches

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-06-21 14:48   ` Manfred Schwarb
@ 2012-06-21 17:01     ` Janus Weil
  2012-12-14 14:31       ` Janus Weil
  0 siblings, 1 reply; 21+ messages in thread
From: Janus Weil @ 2012-06-21 17:01 UTC (permalink / raw)
  To: Manfred Schwarb; +Cc: Tobias Burnus, FX, fortran, gcc-patches

>> 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-06-21 17:01     ` Janus Weil
@ 2012-12-14 14:31       ` Janus Weil
  2012-12-14 19:17         ` Janus Weil
  2012-12-15 13:07         ` Janne Blomqvist
  0 siblings, 2 replies; 21+ messages in thread
From: Janus Weil @ 2012-12-14 14:31 UTC (permalink / raw)
  To: Manfred Schwarb; +Cc: Tobias Burnus, FX, gfortran, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2257 bytes --]

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  <janus@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  <janus@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 <janus@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

[-- Attachment #2: pr36044_backtrace_intrinsic.diff --]
[-- Type: application/octet-stream, Size: 3914 bytes --]

Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 194387)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -496,6 +496,7 @@ enum gfc_isym_id
   GFC_ISYM_SHIFTA,
   GFC_ISYM_SHIFTL,
   GFC_ISYM_SHIFTR,
+  GFC_ISYM_SHOW_BACKTRACE,
   GFC_ISYM_SIGN,
   GFC_ISYM_SIGNAL,
   GFC_ISYM_SI_KIND,
Index: gcc/fortran/intrinsic.c
===================================================================
--- gcc/fortran/intrinsic.c	(revision 194387)
+++ gcc/fortran/intrinsic.c	(working copy)
@@ -3134,6 +3134,8 @@ add_subroutines (void)
 	      p2, BT_CHARACTER, dc, REQUIRED, INTENT_IN,
 	      st, BT_INTEGER, di, OPTIONAL, INTENT_OUT);
 
+  add_sym_0s ("show_backtrace", GFC_ISYM_SHOW_BACKTRACE, GFC_STD_GNU, NULL);
+
   add_sym_1s ("sleep", GFC_ISYM_SLEEP, CLASS_IMPURE, BT_UNKNOWN, 0, GFC_STD_GNU,
 	      gfc_check_sleep_sub, NULL, gfc_resolve_sleep_sub,
 	      sec, BT_INTEGER, di, REQUIRED, INTENT_IN);
Index: gcc/fortran/intrinsic.texi
===================================================================
--- gcc/fortran/intrinsic.texi	(revision 194387)
+++ gcc/fortran/intrinsic.texi	(working copy)
@@ -259,6 +259,7 @@ Some basic guidelines for editing this document:
 * @code{SHIFTA}:        SHIFTA,    Right shift with fill
 * @code{SHIFTL}:        SHIFTL,    Left shift
 * @code{SHIFTR}:        SHIFTR,    Right shift
+* @code{SHOW_BACKTRACE}: SHOW_BACKTRACE, Show a backtrace
 * @code{SIGN}:          SIGN,      Sign copying function
 * @code{SIGNAL}:        SIGNAL,    Signal handling subroutine (or function)
 * @code{SIN}:           SIN,       Sine function
@@ -349,6 +350,7 @@ the applicable standard for each intrinsic procedu
 @item @emph{Description}:
 @code{ABORT} causes immediate termination of the program.  On operating
 systems that support a core dump, @code{ABORT} will produce a core dump.
+It will also print a backtrace, unless @code{-fno-backtrace} is given.
 
 @item @emph{Standard}:
 GNU extension
@@ -371,7 +373,7 @@ end program test_abort
 @end smallexample
 
 @item @emph{See also}:
-@ref{EXIT}, @ref{KILL}
+@ref{EXIT}, @ref{KILL}, @ref{SHOW_BACKTRACE}
 
 @end table
 
@@ -11180,6 +11182,34 @@ The return value is of type @code{INTEGER} and of
 
 
 
+@node SHOW_BACKTRACE
+@section @code{SHOW_BACKTRACE} --- Show a backtrace
+@fnindex SHOW_BACKTRACE
+@cindex backtrace
+
+@table @asis
+@item @emph{Description}:
+@code{SHOW_BACKTRACE} shows a backtrace at an arbitrary place in user code.
+Program execution continues normally afterwards.
+
+@item @emph{Standard}:
+GNU Extension
+
+@item @emph{Class}:
+Subroutine
+
+@item @emph{Syntax}:
+@code{CALL SHOW_BACKTRACE}
+
+@item @emph{Arguments}:
+None
+
+@item @emph{See also}:
+@ref{ABORT}
+@end table
+
+
+
 @node SIGN
 @section @code{SIGN} --- Sign copying function
 @fnindex SIGN
Index: libgfortran/gfortran.map
===================================================================
--- libgfortran/gfortran.map	(revision 194387)
+++ libgfortran/gfortran.map	(working copy)
@@ -1192,6 +1192,7 @@ GFORTRAN_1.4 {
 GFORTRAN_1.5 {
   global:
     _gfortran_ftell2;
+    _gfortran_show_backtrace;
 } GFORTRAN_1.4; 
 
 F2C_1.0 {
Index: libgfortran/libgfortran.h
===================================================================
--- libgfortran/libgfortran.h	(revision 194387)
+++ libgfortran/libgfortran.h	(working copy)
@@ -668,7 +668,7 @@ internal_proto(find_addr2line);
 /* backtrace.c */
 
 extern void show_backtrace (void);
-internal_proto(show_backtrace);
+iexport_proto(show_backtrace);
 
 /* error.c */
 
Index: libgfortran/runtime/backtrace.c
===================================================================
--- libgfortran/runtime/backtrace.c	(revision 194387)
+++ libgfortran/runtime/backtrace.c	(working copy)
@@ -277,3 +277,4 @@ fallback_noerr:
   state.direct_output = 1;
   _Unwind_Backtrace (trace_function, &state);
 }
+iexport(show_backtrace);

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-14 14:31       ` Janus Weil
@ 2012-12-14 19:17         ` Janus Weil
  2012-12-15 13:07         ` Janne Blomqvist
  1 sibling, 0 replies; 21+ messages in thread
From: Janus Weil @ 2012-12-14 19:17 UTC (permalink / raw)
  To: Manfred Schwarb; +Cc: Tobias Burnus, FX, gfortran, gcc-patches

Btw: Forgot to mention that it regtests cleanly and the docu parts
were tested with make pdf.

Cheers,
Janus



2012/12/14 Janus Weil <janus@gcc.gnu.org>:
> 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  <janus@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  <janus@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 <janus@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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-14 14:31       ` Janus Weil
  2012-12-14 19:17         ` Janus Weil
@ 2012-12-15 13:07         ` Janne Blomqvist
  2012-12-15 16:04           ` Janus Weil
  1 sibling, 1 reply; 21+ messages in thread
From: Janne Blomqvist @ 2012-12-15 13:07 UTC (permalink / raw)
  To: Janus Weil; +Cc: Manfred Schwarb, Tobias Burnus, FX, gfortran, gcc-patches

On Fri, Dec 14, 2012 at 4:31 PM, Janus Weil <janus@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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-15 13:07         ` Janne Blomqvist
@ 2012-12-15 16:04           ` Janus Weil
  2012-12-15 17:07             ` Janne Blomqvist
  0 siblings, 1 reply; 21+ messages in thread
From: Janus Weil @ 2012-12-15 16:04 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Manfred Schwarb, Tobias Burnus, FX, gfortran, gcc-patches

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-15 16:04           ` Janus Weil
@ 2012-12-15 17:07             ` Janne Blomqvist
  2012-12-15 22:50               ` Janus Weil
  0 siblings, 1 reply; 21+ messages in thread
From: Janne Blomqvist @ 2012-12-15 17:07 UTC (permalink / raw)
  To: Janus Weil; +Cc: Manfred Schwarb, Tobias Burnus, FX, gfortran, gcc-patches

On Sat, Dec 15, 2012 at 6:04 PM, Janus Weil <janus@gcc.gnu.org> wrote:
>> - 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.

So far show_backtrace() is just an internal implementation detail in
libgfortran, as long as it's not completely retarded anything goes.
But when we make it part of the documented public API we should make
sure it has a good, descriptive name.

> 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.

I think nowadays most people would hit google before explicitly
checking the manual.. :) Still, that's not an excuse for not
organizing things in a logical manner.

> 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.

>> - 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}).


-- 
Janne Blomqvist

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-15 17:07             ` Janne Blomqvist
@ 2012-12-15 22:50               ` Janus Weil
  2012-12-16 11:42                 ` Tobias Burnus
  2012-12-19  9:40                 ` Janne Blomqvist
  0 siblings, 2 replies; 21+ messages in thread
From: Janus Weil @ 2012-12-15 22:50 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Manfred Schwarb, Tobias Burnus, FX, gfortran, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1985 bytes --]

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 #2: pr36044_v2.diff --]
[-- Type: application/octet-stream, Size: 5595 bytes --]

Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 194517)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -496,6 +496,7 @@ enum gfc_isym_id
   GFC_ISYM_SHIFTA,
   GFC_ISYM_SHIFTL,
   GFC_ISYM_SHIFTR,
+  GFC_ISYM_BACKTRACE_SHOW,
   GFC_ISYM_SIGN,
   GFC_ISYM_SIGNAL,
   GFC_ISYM_SI_KIND,
Index: gcc/fortran/intrinsic.c
===================================================================
--- gcc/fortran/intrinsic.c	(revision 194517)
+++ gcc/fortran/intrinsic.c	(working copy)
@@ -2896,6 +2896,8 @@ add_subroutines (void)
 	      "value", BT_INTEGER, di, REQUIRED, INTENT_OUT,
 	      "atom", BT_INTEGER, di, REQUIRED, INTENT_IN);
 
+  add_sym_0s ("backtrace_show", GFC_ISYM_BACKTRACE_SHOW, GFC_STD_GNU, NULL);
+
   add_sym_1s ("cpu_time", GFC_ISYM_CPU_TIME, CLASS_IMPURE, BT_UNKNOWN, 0,
 	      GFC_STD_F95, gfc_check_cpu_time, NULL, gfc_resolve_cpu_time,
 	      tm, BT_REAL, dr, REQUIRED, INTENT_OUT);
Index: gcc/fortran/intrinsic.texi
===================================================================
--- gcc/fortran/intrinsic.texi	(revision 194517)
+++ gcc/fortran/intrinsic.texi	(working copy)
@@ -63,6 +63,7 @@ Some basic guidelines for editing this document:
 * @code{ATANH}:         ATANH,     Inverse hyperbolic tangent function
 * @code{ATOMIC_DEFINE}: ATOMIC_DEFINE, Setting a variable atomically
 * @code{ATOMIC_REF}:    ATOMIC_REF, Obtaining the value of a variable atomically
+* @code{BACKTRACE_SHOW}: BACKTRACE_SHOW, Show a backtrace
 * @code{BESSEL_J0}:     BESSEL_J0, Bessel function of the first kind of order 0
 * @code{BESSEL_J1}:     BESSEL_J1, Bessel function of the first kind of order 1
 * @code{BESSEL_JN}:     BESSEL_JN, Bessel function of the first kind
@@ -349,6 +350,7 @@ the applicable standard for each intrinsic procedu
 @item @emph{Description}:
 @code{ABORT} causes immediate termination of the program.  On operating
 systems that support a core dump, @code{ABORT} will produce a core dump.
+It will also print a backtrace, unless @code{-fno-backtrace} is given.
 
 @item @emph{Standard}:
 GNU extension
@@ -371,7 +373,7 @@ end program test_abort
 @end smallexample
 
 @item @emph{See also}:
-@ref{EXIT}, @ref{KILL}
+@ref{EXIT}, @ref{KILL}, @ref{BACKTRACE_SHOW}
 
 @end table
 
@@ -1644,6 +1646,34 @@ end program atomic
 
 
 
+@node BACKTRACE_SHOW
+@section @code{BACKTRACE_SHOW} --- Show a backtrace
+@fnindex BACKTRACE_SHOW
+@cindex backtrace
+
+@table @asis
+@item @emph{Description}:
+@code{BACKTRACE_SHOW} shows a backtrace at an arbitrary place in user code.
+Program execution continues normally afterwards.
+
+@item @emph{Standard}:
+GNU Extension
+
+@item @emph{Class}:
+Subroutine
+
+@item @emph{Syntax}:
+@code{CALL BACKTRACE_SHOW}
+
+@item @emph{Arguments}:
+None
+
+@item @emph{See also}:
+@ref{ABORT}
+@end table
+
+
+
 @node BESSEL_J0
 @section @code{BESSEL_J0} --- Bessel function of the first kind of order 0
 @fnindex BESSEL_J0
Index: libgfortran/gfortran.map
===================================================================
--- libgfortran/gfortran.map	(revision 194517)
+++ libgfortran/gfortran.map	(working copy)
@@ -1192,6 +1192,7 @@ GFORTRAN_1.4 {
 GFORTRAN_1.5 {
   global:
     _gfortran_ftell2;
+    _gfortran_backtrace_show;
 } GFORTRAN_1.4; 
 
 F2C_1.0 {
Index: libgfortran/libgfortran.h
===================================================================
--- libgfortran/libgfortran.h	(revision 194517)
+++ libgfortran/libgfortran.h	(working copy)
@@ -667,8 +667,8 @@ internal_proto(find_addr2line);
 
 /* backtrace.c */
 
-extern void show_backtrace (void);
-internal_proto(show_backtrace);
+extern void backtrace_show (void);
+iexport_proto(backtrace_show);
 
 /* error.c */
 
Index: libgfortran/runtime/backtrace.c
===================================================================
--- libgfortran/runtime/backtrace.c	(revision 194517)
+++ libgfortran/runtime/backtrace.c	(working copy)
@@ -190,14 +190,12 @@ trace_function (struct _Unwind_Context *context, v
 /* Display the backtrace.  */
 
 void
-show_backtrace (void)
+backtrace_show (void)
 {
   bt_state state;
   state.frame_number = 0;
   state.error = 0;
 
-  estr_write ("\nBacktrace for this error:\n");
-
 #if CAN_PIPE
 
   if (addr2line_path == NULL)
@@ -261,6 +259,7 @@ void
     if (state.error)
       goto fallback;
     close (inp[1]);
+    close (f[0]);
     wait (NULL);
     return;
 
@@ -277,3 +276,4 @@ fallback_noerr:
   state.direct_output = 1;
   _Unwind_Backtrace (trace_function, &state);
 }
+iexport(backtrace_show);
Index: libgfortran/runtime/compile_options.c
===================================================================
--- libgfortran/runtime/compile_options.c	(revision 194517)
+++ libgfortran/runtime/compile_options.c	(working copy)
@@ -126,7 +126,8 @@ backtrace_handler (int signum)
   fatal_error_in_progress = 1;
 
   show_signal (signum);
-  show_backtrace();
+  estr_write ("\nBacktrace for this error:\n");
+  backtrace_show();
 
   /* Now reraise the signal.  We reactivate the signal's
      default handling, which is to terminate the process.
Index: libgfortran/runtime/error.c
===================================================================
--- libgfortran/runtime/error.c	(revision 194517)
+++ libgfortran/runtime/error.c	(working copy)
@@ -166,7 +166,8 @@ sys_abort (void)
   if (options.backtrace == 1
       || (options.backtrace == -1 && compile_options.backtrace == 1))
     {
-      show_backtrace ();
+      estr_write ("\nProgram aborted. Backtrace:\n");
+      backtrace_show ();
       signal (SIGABRT, SIG_DFL);
     }
 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-15 22:50               ` Janus Weil
@ 2012-12-16 11:42                 ` Tobias Burnus
  2012-12-16 13:18                   ` Janus Weil
  2012-12-19  9:40                 ` Janne Blomqvist
  1 sibling, 1 reply; 21+ messages in thread
From: Tobias Burnus @ 2012-12-16 11:42 UTC (permalink / raw)
  To: Janus Weil; +Cc: Janne Blomqvist, Manfred Schwarb, FX, gfortran, gcc-patches

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-16 11:42                 ` Tobias Burnus
@ 2012-12-16 13:18                   ` Janus Weil
  0 siblings, 0 replies; 21+ messages in thread
From: Janus Weil @ 2012-12-16 13:18 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: Janne Blomqvist, Manfred Schwarb, FX, gfortran, gcc-patches

>>>> >>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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-15 22:50               ` Janus Weil
  2012-12-16 11:42                 ` Tobias Burnus
@ 2012-12-19  9:40                 ` Janne Blomqvist
  2012-12-19 13:10                   ` Janus Weil
  1 sibling, 1 reply; 21+ messages in thread
From: Janne Blomqvist @ 2012-12-19  9:40 UTC (permalink / raw)
  To: Janus Weil; +Cc: Manfred Schwarb, Tobias Burnus, FX, gfortran, gcc-patches

On Sun, Dec 16, 2012 at 12:50 AM, Janus Weil <janus@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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-19  9:40                 ` Janne Blomqvist
@ 2012-12-19 13:10                   ` Janus Weil
  2012-12-19 14:31                     ` Tobias Burnus
  0 siblings, 1 reply; 21+ messages in thread
From: Janus Weil @ 2012-12-19 13:10 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Manfred Schwarb, Tobias Burnus, FX, gfortran, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

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

[-- Attachment #2: pr36044_v3.diff --]
[-- Type: application/octet-stream, Size: 5622 bytes --]

Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 194532)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -496,6 +496,7 @@ enum gfc_isym_id
   GFC_ISYM_SHIFTA,
   GFC_ISYM_SHIFTL,
   GFC_ISYM_SHIFTR,
+  GFC_ISYM_BACKTRACE,
   GFC_ISYM_SIGN,
   GFC_ISYM_SIGNAL,
   GFC_ISYM_SI_KIND,
Index: gcc/fortran/intrinsic.c
===================================================================
--- gcc/fortran/intrinsic.c	(revision 194532)
+++ gcc/fortran/intrinsic.c	(working copy)
@@ -2896,6 +2896,8 @@ add_subroutines (void)
 	      "value", BT_INTEGER, di, REQUIRED, INTENT_OUT,
 	      "atom", BT_INTEGER, di, REQUIRED, INTENT_IN);
 
+  add_sym_0s ("backtrace", GFC_ISYM_BACKTRACE, GFC_STD_GNU, NULL);
+
   add_sym_1s ("cpu_time", GFC_ISYM_CPU_TIME, CLASS_IMPURE, BT_UNKNOWN, 0,
 	      GFC_STD_F95, gfc_check_cpu_time, NULL, gfc_resolve_cpu_time,
 	      tm, BT_REAL, dr, REQUIRED, INTENT_OUT);
Index: gcc/fortran/intrinsic.texi
===================================================================
--- gcc/fortran/intrinsic.texi	(revision 194532)
+++ gcc/fortran/intrinsic.texi	(working copy)
@@ -63,6 +63,7 @@ Some basic guidelines for editing this document:
 * @code{ATANH}:         ATANH,     Inverse hyperbolic tangent function
 * @code{ATOMIC_DEFINE}: ATOMIC_DEFINE, Setting a variable atomically
 * @code{ATOMIC_REF}:    ATOMIC_REF, Obtaining the value of a variable atomically
+* @code{BACKTRACE}:     BACKTRACE, Show a backtrace
 * @code{BESSEL_J0}:     BESSEL_J0, Bessel function of the first kind of order 0
 * @code{BESSEL_J1}:     BESSEL_J1, Bessel function of the first kind of order 1
 * @code{BESSEL_JN}:     BESSEL_JN, Bessel function of the first kind
@@ -349,6 +350,7 @@ the applicable standard for each intrinsic procedu
 @item @emph{Description}:
 @code{ABORT} causes immediate termination of the program.  On operating
 systems that support a core dump, @code{ABORT} will produce a core dump.
+It will also print a backtrace, unless @code{-fno-backtrace} is given.
 
 @item @emph{Standard}:
 GNU extension
@@ -371,7 +373,7 @@ end program test_abort
 @end smallexample
 
 @item @emph{See also}:
-@ref{EXIT}, @ref{KILL}
+@ref{EXIT}, @ref{KILL}, @ref{BACKTRACE}
 
 @end table
 
@@ -1644,6 +1646,35 @@ end program atomic
 
 
 
+@node BACKTRACE
+@section @code{BACKTRACE} --- Show a backtrace
+@fnindex BACKTRACE
+@cindex backtrace
+
+@table @asis
+@item @emph{Description}:
+@code{BACKTRACE} shows a backtrace at an arbitrary place in user code. Program
+execution continues normally afterwards. The backtrace information is printed
+to the unit corresponding to @code{ERROR_UNIT} in @code{ISO_FORTRAN_ENV}.
+
+@item @emph{Standard}:
+GNU Extension
+
+@item @emph{Class}:
+Subroutine
+
+@item @emph{Syntax}:
+@code{CALL BACKTRACE}
+
+@item @emph{Arguments}:
+None
+
+@item @emph{See also}:
+@ref{ABORT}
+@end table
+
+
+
 @node BESSEL_J0
 @section @code{BESSEL_J0} --- Bessel function of the first kind of order 0
 @fnindex BESSEL_J0
Index: libgfortran/gfortran.map
===================================================================
--- libgfortran/gfortran.map	(revision 194532)
+++ libgfortran/gfortran.map	(working copy)
@@ -1192,6 +1192,7 @@ GFORTRAN_1.4 {
 GFORTRAN_1.5 {
   global:
     _gfortran_ftell2;
+    _gfortran_backtrace;
 } GFORTRAN_1.4; 
 
 F2C_1.0 {
Index: libgfortran/libgfortran.h
===================================================================
--- libgfortran/libgfortran.h	(revision 194532)
+++ libgfortran/libgfortran.h	(working copy)
@@ -667,8 +667,8 @@ internal_proto(find_addr2line);
 
 /* backtrace.c */
 
-extern void show_backtrace (void);
-internal_proto(show_backtrace);
+extern void backtrace (void);
+iexport_proto(backtrace);
 
 /* error.c */
 
Index: libgfortran/runtime/backtrace.c
===================================================================
--- libgfortran/runtime/backtrace.c	(revision 194532)
+++ libgfortran/runtime/backtrace.c	(working copy)
@@ -190,14 +190,12 @@ trace_function (struct _Unwind_Context *context, v
 /* Display the backtrace.  */
 
 void
-show_backtrace (void)
+backtrace (void)
 {
   bt_state state;
   state.frame_number = 0;
   state.error = 0;
 
-  estr_write ("\nBacktrace for this error:\n");
-
 #if CAN_PIPE
 
   if (addr2line_path == NULL)
@@ -261,6 +259,7 @@ void
     if (state.error)
       goto fallback;
     close (inp[1]);
+    close (f[0]);
     wait (NULL);
     return;
 
@@ -277,3 +276,4 @@ fallback_noerr:
   state.direct_output = 1;
   _Unwind_Backtrace (trace_function, &state);
 }
+iexport(backtrace);
Index: libgfortran/runtime/compile_options.c
===================================================================
--- libgfortran/runtime/compile_options.c	(revision 194532)
+++ libgfortran/runtime/compile_options.c	(working copy)
@@ -126,7 +126,8 @@ backtrace_handler (int signum)
   fatal_error_in_progress = 1;
 
   show_signal (signum);
-  show_backtrace();
+  estr_write ("\nBacktrace for this error:\n");
+  backtrace ();
 
   /* Now reraise the signal.  We reactivate the signal's
      default handling, which is to terminate the process.
Index: libgfortran/runtime/error.c
===================================================================
--- libgfortran/runtime/error.c	(revision 194532)
+++ libgfortran/runtime/error.c	(working copy)
@@ -166,7 +166,8 @@ sys_abort (void)
   if (options.backtrace == 1
       || (options.backtrace == -1 && compile_options.backtrace == 1))
     {
-      show_backtrace ();
+      estr_write ("\nProgram aborted. Backtrace:\n");
+      backtrace ();
       signal (SIGABRT, SIG_DFL);
     }
 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-19 13:10                   ` Janus Weil
@ 2012-12-19 14:31                     ` Tobias Burnus
  2012-12-19 15:23                       ` Janus Weil
  0 siblings, 1 reply; 21+ messages in thread
From: Tobias Burnus @ 2012-12-19 14:31 UTC (permalink / raw)
  To: Janus Weil; +Cc: Janne Blomqvist, Manfred Schwarb, FX, gfortran, gcc-patches

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-19 14:31                     ` Tobias Burnus
@ 2012-12-19 15:23                       ` Janus Weil
  2012-12-19 22:01                         ` Janne Blomqvist
  0 siblings, 1 reply; 21+ messages in thread
From: Janus Weil @ 2012-12-19 15:23 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: Janne Blomqvist, Manfred Schwarb, FX, gfortran, gcc-patches

>> 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-19 15:23                       ` Janus Weil
@ 2012-12-19 22:01                         ` Janne Blomqvist
  2012-12-20 19:33                           ` Janus Weil
  0 siblings, 1 reply; 21+ messages in thread
From: Janne Blomqvist @ 2012-12-19 22:01 UTC (permalink / raw)
  To: Janus Weil; +Cc: Tobias Burnus, Manfred Schwarb, FX, gfortran, gcc-patches

On Wed, Dec 19, 2012 at 5:23 PM, Janus Weil <janus@gcc.gnu.org> 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.
>
> Great, thanks. Janne?

Yes, Ok for trunk.

>
>
>> 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [fortran, patch] Allow displaying backtraces from user code
  2012-12-19 22:01                         ` Janne Blomqvist
@ 2012-12-20 19:33                           ` Janus Weil
  0 siblings, 0 replies; 21+ messages in thread
From: Janus Weil @ 2012-12-20 19:33 UTC (permalink / raw)
  To: Janne Blomqvist; +Cc: Tobias Burnus, Manfred Schwarb, FX, gfortran, gcc-patches

>>>> 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2012-12-20 19:33 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-03  7:45 [fortran, patch] Allow displaying backtraces from user code FX
2012-03-03 15:42 ` Steve Kargl
2012-03-03 21:21   ` FX
2012-04-24 13:04     ` Janus Weil
2012-06-21 12:35 ` Tobias Burnus
2012-06-21 14:48   ` Manfred Schwarb
2012-06-21 17:01     ` Janus Weil
2012-12-14 14:31       ` Janus Weil
2012-12-14 19:17         ` Janus Weil
2012-12-15 13:07         ` Janne Blomqvist
2012-12-15 16:04           ` Janus Weil
2012-12-15 17:07             ` Janne Blomqvist
2012-12-15 22:50               ` Janus Weil
2012-12-16 11:42                 ` Tobias Burnus
2012-12-16 13:18                   ` Janus Weil
2012-12-19  9:40                 ` Janne Blomqvist
2012-12-19 13:10                   ` Janus Weil
2012-12-19 14:31                     ` Tobias Burnus
2012-12-19 15:23                       ` Janus Weil
2012-12-19 22:01                         ` Janne Blomqvist
2012-12-20 19:33                           ` Janus Weil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).