C backtrace tweaks.

Rename the environment variable that triggers the backtrace before
warns and croaks as PERL_USE_C_BACKTRACE_ON_ERROR (and correspondingly,
the define as USE_C_BACKTRACE_ON_ERROR).

Pod cleanups and updates, and move the needed #includes from perl.h
to util.c since that's the only place where they are needed.

4 files changed, 57 insertions, 35 deletions

diff --git a/perl.h b/perl.h
index d6d9407485..e55e8217f8 100644
--- a/perl.h
+++ b/perl.h
@@ -5860,16 +5860,6 @@ extern void moncontrol(int);
 #define PERL_PV_PRETTY_DUMP  PERL_PV_PRETTY_ELLIPSES|PERL_PV_PRETTY_QUOTE
 #define PERL_PV_PRETTY_REGPROP PERL_PV_PRETTY_ELLIPSES|PERL_PV_PRETTY_LTGT|PERL_PV_ESCAPE_RE|PERL_PV_ESCAPE_NONASCII
 
-#if defined(USE_C_BACKTRACE) && defined(I_BFD)
-#  define USE_BFD
-#  ifdef PERL_DARWIN
-#    undef USE_BFD /* BFD is useless in OS X. */
-#  endif
-#  ifdef USE_BFD
-#    include <bfd.h>
-#  endif
-#endif
-
 /*
 
    (KEEP THIS LAST IN perl.h!)
diff --git a/pod/perlhacktips.pod b/pod/perlhacktips.pod
index c93a80b413..5cd04e4463 100644
--- a/pod/perlhacktips.pod
+++ b/pod/perlhacktips.pod
@@ -1399,40 +1399,46 @@ Note: you can define up to 20 conversion shortcuts in the gdb section.
 
 =head2 C backtrace
 
-Starting from Perl 5.21.1, on some platforms Perl supports retrieving
-the C level backtrace (similar to what symbolic debuggers like gdb do).
+On some platforms Perl supports retrieving the C level backtrace
+(similar to what symbolic debuggers like gdb do).
 
 The backtrace returns the stack trace of the C call frames,
 with the symbol names (function names), the object names (like "perl"),
 and if it can, also the source code locations (file:line).
 
-The supported platforms are Linux and OS X (some *BSD might work at
-least partly, but they have not yet been tested).
+The supported platforms are Linux, and OS X (some *BSD might
+work at least partly, but they have not yet been tested).
+
+This feature hasn't been tested with multiple threads, but it will
+only show the backtrace of the thread doing the backtracing.
 
 The feature needs to be enabled with C<Configure -Dusecbacktrace>.
 
-The C<-Dusecbacktrace> also enables keeping the debug information
-when compiling.  Many compilers/linkers do support having both
-optimization and keeping the debug information.  The debug information
-is needed for the symbol names and the source locations.
+The C<-Dusecbacktrace> also enables keeping the debug information when
+compiling/linking (often: C<-g>).  Many compilers/linkers do support
+having both optimization and keeping the debug information.  The debug
+information is needed for the symbol names and the source locations.
+
+Static functions might not be visible for the backtrace.
 
 Source code locations, even if available, can often be missing or
-misleading if the compiler has e.g. inlined code.
+misleading if the compiler has e.g. inlined code.  Optimizer can
+make matching the source code and the object code quite challenging.
 
 =over 4
 
 =item Linux
 
 You B<must> have the BFD (-lbfd) library installed, otherwise C<perl> will
-fail to link.  The BFD is usually distributed as part of the binutils.
+fail to link.  The BFD is usually distributed as part of the GNU binutils.
 
 Summary: C<Configure ... -Dusecbacktrace>
 and you need C<-lbfd>.
 
 =item OS X
 
-The source code locations are supported only if you have both C<-g>
-and have the Developer Tools installed.
+The source code locations are supported B<only> if you have
+the Developer Tools installed.  (BFD is B<not> needed.)
 
 Summary: C<Configure ... -Dusecbacktrace>
 and installing the Developer Tools would be good.
@@ -1440,17 +1446,17 @@ and installing the Developer Tools would be good.
 =back
 
 Optionally, for trying out the feature, you may want to enable
-automatic dumping of the backtrace just before a warning message
-is emitted (this includes coincidentally croaking) by adding
-C<-Accflags=-DUSE_C_BACKTRACE_ON_WARN> for Configure.
+automatic dumping of the backtrace just before a warning or croak (die)
+message is emitted, by adding C<-Accflags=-DUSE_C_BACKTRACE_ON_ERROR>
+for Configure.
 
 Unless the above additional feature is enabled, nothing about the
 backtrace functionality is visible, except for the Perl/XS level.
 
 Furthermore, even if you have enabled this feature to be compiled,
 you need to enable it in runtime with an environment variable:
-C<PERL_C_BACKTRACE_ON_WARN=10>.  It must be an integer higher
-than zero, and it tells the desired frame count.
+C<PERL_C_BACKTRACE_ON_ERROR=10>.  It must be an integer higher
+than zero, telling the desired frame count.
 
 Retrieving the backtrace from Perl level (using for example an XS
 extension) would be much less exciting than one would hope: normally
@@ -1458,7 +1464,7 @@ you would see C<runops>, C<entersub>, and not much else.  This API is
 intended to be called B<from within> the Perl implementation, not from
 Perl level execution.
 
-The C API for the backtrace is as follows (see L<perlintern>) for details).
+The C API for the backtrace is as follows:
 
 =over 4
 
diff --git a/util.c b/util.c
index 6b09db499d..df8a02faab 100644
--- a/util.c
+++ b/util.c
@@ -51,8 +51,16 @@ int putenv(char *);
 # endif
 #endif
 
-/* <bfd.h> will have been included, if necessary, by "perl.h" */
 #ifdef USE_C_BACKTRACE
+#  ifdef I_BFD
+#    define USE_BFD
+#    ifdef PERL_DARWIN
+#      undef USE_BFD /* BFD is useless in OS X. */
+#    endif
+#    ifdef USE_BFD
+#      include <bfd.h>
+#    endif
+#  endif
 #  ifdef I_DLFCN
 #    include <dlfcn.h>
 #  endif
@@ -1361,12 +1369,12 @@ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
     dVAR;
     SV *sv;
 
-#if defined(USE_C_BACKTRACE) && defined(USE_C_BACKTRACE_ON_WARN)
+#if defined(USE_C_BACKTRACE) && defined(USE_C_BACKTRACE_ON_ERROR)
     {
         char *ws;
         int wi;
         /* The PERL_C_BACKTRACE_ON_WARN must be an integer of one or more. */
-        if ((ws = PerlEnv_getenv("PERL_C_BACKTRACE_ON_WARN")) &&
+        if ((ws = PerlEnv_getenv("PERL_C_BACKTRACE_ON_ERROR")) &&
             (wi = atoi(ws)) > 0) {
             Perl_dump_c_backtrace(aTHX_ Perl_debug_log, wi, 1);
         }
@@ -5506,8 +5514,11 @@ Perl_drand48_r(perl_drand48_t *random_state)
 #ifdef USE_BFD
 
 typedef struct {
+    /* abfd is the BFD handle. */
     bfd* abfd;
+    /* bfd_syms is the BFD symbol table. */
     asymbol** bfd_syms;
+    /* bfd_text is handle to the the ".text" section of the object file. */
     asection* bfd_text;
     /* Since opening the executable and scanning its symbols is quite
      * heavy operation, we remember the filename we used the last time,
@@ -5611,10 +5622,20 @@ static void bfd_symbolize(bfd_context* ctx,
  * use high-level stuff.  Thanks, Apple. */
 
 typedef struct {
+    /* tool is set to the absolute pathname of the tool to use:
+     * xcrun or atos. */
     const char* tool;
+    /* format is set to a printf format string used for building
+     * the external command to run. */
     const char* format;
+    /* unavail is set if e.g. xcrun cannot be found, or something
+     * else happens that makes getting the backtrace dubious.  Note,
+     * however, that the context isn't persistent, the next call to
+     * get_c_backtrace() will start from scratch. */
     bool unavail;
+    /* fname is the current object file name. */
     const char* fname;
+    /* object_base_addr is the base address of the shared object. */
     void* object_base_addr;
 } atos_context;
 
@@ -5750,7 +5771,6 @@ static void atos_symbolize(atos_context* ctx,
          * We could play tricks with atos by batching the stack
          * addresses to be resolved: atos can either take multiple
          * addresses from the command line, or read addresses from
-         *
          * a file (though the mess of creating temporary files would
          * probably negate much of any possible speedup).
          *
@@ -5875,9 +5895,9 @@ Perl_get_c_backtrace(pTHX_ int depth, int skip)
     /* We use dladdr() instead of backtrace_symbols() because we want
      * the full details instead of opaque strings.  This is useful for
      * two reasons: () the details are needed for further symbolic
-     * digging (2) by having the details we fully control the output,
-     * which in turn is useful when more platforms are added:
-     * we can keep out output "portable". */
+     * digging, for example in OS X (2) by having the details we fully
+     * control the output, which in turn is useful when more platforms
+     * are added: we can keep out output "portable". */
 
     /* We want a single linear allocation, which can then be freed
      * with a single swoop.  We will do the usual trick of first
diff --git a/util.h b/util.h
index 57a3ad0b65..736f978edf 100644
--- a/util.h
+++ b/util.h
@@ -121,6 +121,12 @@ typedef struct {
      * output - quite disgusting.  And that won't work if the
      * Developer Tools isn't installed. */
 
+    /* FreeBSD notes: execinfo.h exists, but probably would need also
+     * the library -lexecinfo.  BFD exists if the pkg devel/binutils
+     * has been installed, but there seems to be a known problem that
+     * the "bfd.h" getting installed refers to "ansidecl.h", which
+     * doesn't get installed. */
+
     /* Win32 notes: as moral equivalents of backtrace() + dladdr(),
      * one could possibly first use GetCurrentProcess() +
      * SymInitialize(), and then CaptureStackBackTrace() +