errbacktrace
New thread continuing from
</messages/by-id/d4903af2-e7b7-b551-71f8-3e4a6bdc2e73@2ndquadrant.com>.
Here is a extended version of Álvaro's patch that adds an errbacktrace()
function. You can do two things with this:
- Manually attach it to an ereport() call site that you want to debug.
- Set a configuration parameter like backtrace_function = 'int8in' to
debug ereport()/elog() calls in a specific function.
There was also mention of settings that would automatically produce
backtraces for PANICs etc. Those could surely be added if there is
enough interest.
For the implementation, I support both backtrace() provided by the OS as
well as using libunwind. The former seems to be supported by a number
of platforms, including glibc, macOS, and FreeBSD, so maybe we don't
need the libunwind suport. I haven't found any difference in quality in
the backtraces between the two approaches, but surely that is highly
dependent on the exact configuration.
I would welcome testing in all direction with this, to see how well it
works in different circumstances.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v2-0001-Add-errbacktrace-and-backtrace_function-GUC.patchtext/plain; charset=UTF-8; name=v2-0001-Add-errbacktrace-and-backtrace_function-GUC.patch; x-mac-creator=0; x-mac-type=0Download
From ea79d7d04e940e70fa5b63f1be5b5272d287677c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 18 Jun 2019 00:36:20 +0200
Subject: [PATCH v2] Add errbacktrace() and backtrace_function GUC
---
configure | 101 ++++++++++++++++++++++++++++-
configure.in | 17 +++++
src/Makefile.global.in | 1 +
src/backend/utils/error/elog.c | 115 +++++++++++++++++++++++++++++++++
src/backend/utils/misc/guc.c | 10 +++
src/include/pg_config.h.in | 9 +++
src/include/utils/elog.h | 3 +
src/include/utils/guc.h | 1 +
8 files changed, 255 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index 8d47071e4a..a8394ecfd9 100755
--- a/configure
+++ b/configure
@@ -710,6 +710,7 @@ with_uuid
with_systemd
with_selinux
with_openssl
+with_libunwind
with_ldap
with_krb_srvnam
krb_srvtab
@@ -853,6 +854,7 @@ with_pam
with_bsd_auth
with_ldap
with_bonjour
+with_libunwind
with_openssl
with_selinux
with_systemd
@@ -1557,6 +1559,7 @@ Optional Packages:
--with-bsd-auth build with BSD Authentication support
--with-ldap build with LDAP support
--with-bonjour build with Bonjour support
+ --with-libunwind build with libunwind support
--with-openssl build with OpenSSL support
--with-selinux build with SELinux support
--with-systemd build with systemd support
@@ -7861,6 +7864,39 @@ fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_bonjour" >&5
$as_echo "$with_bonjour" >&6; }
+#
+# libunwind
+#
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with libunwind" >&5
+$as_echo_n "checking whether to build with libunwind... " >&6; }
+
+
+
+# Check whether --with-libunwind was given.
+if test "${with_libunwind+set}" = set; then :
+ withval=$with_libunwind;
+ case $withval in
+ yes)
+
+$as_echo "#define USE_LIBUNWIND 1" >>confdefs.h
+
+ ;;
+ no)
+ :
+ ;;
+ *)
+ as_fn_error $? "no argument expected for --with-libunwind option" "$LINENO" 5
+ ;;
+ esac
+
+else
+ with_libunwind=no
+
+fi
+
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_libunwind" >&5
+$as_echo "$with_libunwind" >&6; }
#
# OpenSSL
@@ -12224,6 +12260,67 @@ fi
fi
+if test "$with_libunwind" = yes; then
+ # On macOS, the library does not exist, the functionality is built
+ # into the OS.
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing unw_getcontext" >&5
+$as_echo_n "checking for library containing unw_getcontext... " >&6; }
+if ${ac_cv_search_unw_getcontext+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_func_search_save_LIBS=$LIBS
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+/* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+#ifdef __cplusplus
+extern "C"
+#endif
+char unw_getcontext ();
+int
+main ()
+{
+return unw_getcontext ();
+ ;
+ return 0;
+}
+_ACEOF
+for ac_lib in '' unwind; do
+ if test -z "$ac_lib"; then
+ ac_res="none required"
+ else
+ ac_res=-l$ac_lib
+ LIBS="-l$ac_lib $ac_func_search_save_LIBS"
+ fi
+ if ac_fn_c_try_link "$LINENO"; then :
+ ac_cv_search_unw_getcontext=$ac_res
+fi
+rm -f core conftest.err conftest.$ac_objext \
+ conftest$ac_exeext
+ if ${ac_cv_search_unw_getcontext+:} false; then :
+ break
+fi
+done
+if ${ac_cv_search_unw_getcontext+:} false; then :
+
+else
+ ac_cv_search_unw_getcontext=no
+fi
+rm conftest.$ac_ext
+LIBS=$ac_func_search_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_unw_getcontext" >&5
+$as_echo "$ac_cv_search_unw_getcontext" >&6; }
+ac_res=$ac_cv_search_unw_getcontext
+if test "$ac_res" != no; then :
+ test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+
+fi
+
+fi
+
if test "$with_libxml" = yes ; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for xmlSaveToBuffer in -lxml2" >&5
$as_echo_n "checking for xmlSaveToBuffer in -lxml2... " >&6; }
@@ -12793,7 +12890,7 @@ $as_echo "#define HAVE_STDBOOL_H 1" >>confdefs.h
fi
-for ac_header in atomic.h copyfile.h crypt.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/ipc.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h
+for ac_header in atomic.h copyfile.h crypt.h execinfo.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/ipc.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h
do :
as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
@@ -15176,7 +15273,7 @@ fi
LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-for ac_func in cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range uselocale utime utimes wcstombs_l
+for ac_func in backtrace_symbols cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range uselocale utime utimes wcstombs_l
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.in b/configure.in
index 74938d4190..1349daf54a 100644
--- a/configure.in
+++ b/configure.in
@@ -831,6 +831,15 @@ PGAC_ARG_BOOL(with, bonjour, no,
[AC_DEFINE([USE_BONJOUR], 1, [Define to 1 to build with Bonjour support. (--with-bonjour)])])
AC_MSG_RESULT([$with_bonjour])
+#
+# libunwind
+#
+AC_MSG_CHECKING([whether to build with libunwind])
+PGAC_ARG_BOOL(with, libunwind, no,
+ [build with libunwind support],
+ [AC_DEFINE([USE_LIBUNWIND], 1, [Define to 1 to build with libunwind support. (--with-libunwind)])])
+AC_MSG_RESULT([$with_libunwind])
+AC_SUBST(with_libunwind)[]dnl
#
# OpenSSL
@@ -1223,6 +1232,12 @@ if test "$with_pam" = yes ; then
AC_CHECK_LIB(pam, pam_start, [], [AC_MSG_ERROR([library 'pam' is required for PAM])])
fi
+if test "$with_libunwind" = yes; then
+ # On macOS, the library does not exist, the functionality is built
+ # into the OS.
+ AC_SEARCH_LIBS(unw_getcontext, [unwind])
+fi
+
if test "$with_libxml" = yes ; then
AC_CHECK_LIB(xml2, xmlSaveToBuffer, [], [AC_MSG_ERROR([library 'xml2' (version >= 2.6.23) is required for XML support])])
fi
@@ -1298,6 +1313,7 @@ AC_CHECK_HEADERS(m4_normalize([
atomic.h
copyfile.h
crypt.h
+ execinfo.h
fp_class.h
getopt.h
ieeefp.h
@@ -1612,6 +1628,7 @@ LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
AC_CHECK_FUNCS(m4_normalize([
+ backtrace_symbols
cbrt
clock_gettime
copyfile
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index b9d86acaa9..6e14a4b217 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -190,6 +190,7 @@ with_systemd = @with_systemd@
with_gssapi = @with_gssapi@
with_krb_srvnam = @with_krb_srvnam@
with_ldap = @with_ldap@
+with_libunwind = @with_libunwind@
with_libxml = @with_libxml@
with_libxslt = @with_libxslt@
with_llvm = @with_llvm@
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 8b4720ef3a..901287c335 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -62,6 +62,12 @@
#ifdef HAVE_SYSLOG
#include <syslog.h>
#endif
+#ifdef HAVE_EXECINFO_H
+#include <execinfo.h>
+#endif
+#ifdef USE_LIBUNWIND
+#include <libunwind.h>
+#endif
#include "access/transam.h"
#include "access/xact.h"
@@ -166,6 +172,7 @@ static char formatted_log_time[FORMATTED_TS_LEN];
} while (0)
+static void set_backtrace(ErrorData *edata, int num_skip);
static const char *err_gettext(const char *str) pg_attribute_format_arg(1);
static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *str);
static void write_console(const char *line, int len);
@@ -424,6 +431,12 @@ errfinish(int dummy,...)
*/
oldcontext = MemoryContextSwitchTo(ErrorContext);
+ if (!edata->backtrace &&
+ edata->funcname &&
+ backtrace_function[0] &&
+ strcmp(backtrace_function, edata->funcname) == 0)
+ set_backtrace(edata, 2);
+
/*
* Call any context callback functions. Errors occurring in callback
* functions will be treated as recursive errors --- this ensures we will
@@ -488,6 +501,8 @@ errfinish(int dummy,...)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -798,6 +813,85 @@ errmsg(const char *fmt,...)
return 0; /* return value does not matter */
}
+int
+errbacktrace(void)
+{
+ ErrorData *edata = &errordata[errordata_stack_depth];
+ MemoryContext oldcontext;
+
+ recursion_depth++;
+ CHECK_STACK_DEPTH();
+ oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+
+ set_backtrace(edata, 1);
+
+ MemoryContextSwitchTo(oldcontext);
+ recursion_depth--;
+
+ return 0;
+}
+
+static void
+set_backtrace(ErrorData *edata, int num_skip)
+{
+ StringInfoData errtrace;
+
+ initStringInfo(&errtrace);
+
+#ifdef USE_LIBUNWIND
+ {
+ unw_context_t context;
+ unw_cursor_t cursor;
+ int n = 0;
+
+ unw_getcontext(&context);
+ unw_init_local(&cursor, &context);
+
+ while (unw_step(&cursor))
+ {
+ unw_word_t ip, off;
+ char symbol[256] = "<unknown>";
+
+ if (n < num_skip - 1)
+ {
+ n++;
+ continue;
+ }
+
+ unw_get_reg(&cursor, UNW_REG_IP, &ip);
+ unw_get_proc_name(&cursor, symbol, sizeof(symbol), &off);
+
+ appendStringInfo(&errtrace,
+ "#%-2d %s+0x%llx [0x%016llx]\n",
+ n - 1,
+ symbol,
+ (unsigned long long) off,
+ (unsigned long long) ip);
+
+ n++;
+ }
+ }
+#elif defined(HAVE_EXECINFO_H) && defined(HAVE_BACKTRACE_SYMBOLS)
+ {
+ void *buf[100];
+ int nframes;
+ char **strfrms;
+
+ nframes = backtrace(buf, sizeof(buf));
+ strfrms = backtrace_symbols(buf, nframes);
+ if (strfrms == NULL)
+ return;
+
+ for (int i = num_skip; i < nframes; i++)
+ appendStringInfo(&errtrace, "%s\n", strfrms[i]);
+ free(strfrms);
+ }
+#else
+ appendStringInfoString(&errtrace, "backtrace not supported");
+#endif
+
+ edata->backtrace = errtrace.data;
+}
/*
* errmsg_internal --- add a primary error message text to the current error
@@ -1353,6 +1447,12 @@ elog_finish(int elevel, const char *fmt,...)
recursion_depth++;
oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+ if (!edata->backtrace &&
+ edata->funcname &&
+ backtrace_function[0] &&
+ strcmp(backtrace_function, edata->funcname) == 0)
+ set_backtrace(edata, 2);
+
edata->message_id = fmt;
EVALUATE_MESSAGE(edata->domain, message, false, false);
@@ -1509,6 +1609,8 @@ CopyErrorData(void)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -1547,6 +1649,8 @@ FreeErrorData(ErrorData *edata)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -1622,6 +1726,8 @@ ThrowErrorData(ErrorData *edata)
newedata->hint = pstrdup(edata->hint);
if (edata->context)
newedata->context = pstrdup(edata->context);
+ if (edata->backtrace)
+ newedata->backtrace = pstrdup(edata->backtrace);
/* assume message_id is not available */
if (edata->schema_name)
newedata->schema_name = pstrdup(edata->schema_name);
@@ -1689,6 +1795,8 @@ ReThrowError(ErrorData *edata)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -2914,6 +3022,13 @@ send_message_to_server_log(ErrorData *edata)
append_with_tabs(&buf, edata->context);
appendStringInfoChar(&buf, '\n');
}
+ if (edata->backtrace)
+ {
+ log_line_prefix(&buf, edata);
+ appendStringInfoString(&buf, _("BACKTRACE: "));
+ append_with_tabs(&buf, edata->backtrace);
+ appendStringInfoChar(&buf, '\n');
+ }
if (Log_error_verbosity >= PGERROR_VERBOSE)
{
/* assume no newlines in funcname or filename... */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 1208eb9a68..328e5b6699 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -514,6 +514,7 @@ int log_temp_files = -1;
double log_statement_sample_rate = 1.0;
double log_xact_sample_rate = 0;
int trace_recovery_messages = LOG;
+char *backtrace_function;
int temp_file_limit = -1;
@@ -4213,6 +4214,15 @@ static struct config_string ConfigureNamesString[] =
NULL, NULL, NULL
},
+ {
+ {"backtrace_function", PGC_SUSET, DEVELOPER_OPTIONS,
+ gettext_noop("Log backtrace for errors in this function."),
+ },
+ &backtrace_function,
+ "",
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 6cd4cfed0a..6959ce6c61 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -96,6 +96,9 @@
/* Define to 1 if you have the <atomic.h> header file. */
#undef HAVE_ATOMIC_H
+/* Define to 1 if you have the `backtrace_symbols' function. */
+#undef HAVE_BACKTRACE_SYMBOLS
+
/* Define to 1 if you have the `BIO_get_data' function. */
#undef HAVE_BIO_GET_DATA
@@ -201,6 +204,9 @@
/* Define to 1 if you have the <editline/readline.h> header file. */
#undef HAVE_EDITLINE_READLINE_H
+/* Define to 1 if you have the <execinfo.h> header file. */
+#undef HAVE_EXECINFO_H
+
/* Define to 1 if you have the `fdatasync' function. */
#undef HAVE_FDATASYNC
@@ -922,6 +928,9 @@
/* Define to 1 to build with LDAP support. (--with-ldap) */
#undef USE_LDAP
+/* Define to 1 to build with libunwind support. (--with-libunwind) */
+#undef USE_LIBUNWIND
+
/* Define to 1 to build with XML support. (--with-libxml) */
#undef USE_LIBXML
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index dbfd8efd26..b58aee792a 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -189,6 +189,8 @@ extern int errcontext_msg(const char *fmt,...) pg_attribute_printf(1, 2);
extern int errhidestmt(bool hide_stmt);
extern int errhidecontext(bool hide_ctx);
+extern int errbacktrace(void);
+
extern int errfunction(const char *funcname);
extern int errposition(int cursorpos);
@@ -362,6 +364,7 @@ typedef struct ErrorData
char *detail_log; /* detail error message for server log only */
char *hint; /* hint message */
char *context; /* context message */
+ char *backtrace; /* backtrace */
const char *message_id; /* primary message's id (original string) */
char *schema_name; /* name of schema */
char *table_name; /* name of table */
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index e709177c37..032f4f16ab 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -255,6 +255,7 @@ extern int log_min_duration_statement;
extern int log_temp_files;
extern double log_statement_sample_rate;
extern double log_xact_sample_rate;
+extern char *backtrace_function;
extern int temp_file_limit;
--
2.22.0
On 2019-Jun-25, Peter Eisentraut wrote:
Here is a extended version of �lvaro's patch that adds an errbacktrace()
function.
Great stuff, thanks for working on it.
You can do two things with this:
- Manually attach it to an ereport() call site that you want to debug.
- Set a configuration parameter like backtrace_function = 'int8in' to
debug ereport()/elog() calls in a specific function.
WFM. I tried specifying int4in -- didn't work. Turns out the errors
are inside another function which is what I have to put in
backtrace_function:
$ PGOPTIONS="-c backtrace_function=pg_strtoint32" psql
alvherre=# select int 'foobar';
2019-06-25 10:03:51.034 -04 [11711] ERROR: invalid input syntax for type integer: "foobar" at character 12
2019-06-25 10:03:51.034 -04 [11711] BACKTRACE: postgres: alvherre alvherre [local] SELECT(pg_strtoint32+0xef) [0x55862737bdaf]
postgres: alvherre alvherre [local] SELECT(int4in+0xd) [0x558627336d7d]
postgres: alvherre alvherre [local] SELECT(InputFunctionCall+0x7b) [0x55862740b10b]
postgres: alvherre alvherre [local] SELECT(OidInputFunctionCall+0x48) [0x55862740b378]
postgres: alvherre alvherre [local] SELECT(coerce_type+0x297) [0x5586270b2f67]
postgres: alvherre alvherre [local] SELECT(coerce_to_target_type+0x9d) [0x5586270b37ad]
postgres: alvherre alvherre [local] SELECT(+0x1ed3d8) [0x5586270b83d8]
postgres: alvherre alvherre [local] SELECT(transformExpr+0x18) [0x5586270bbcc8]
postgres: alvherre alvherre [local] SELECT(transformTargetEntry+0xb2) [0x5586270c81c2]
postgres: alvherre alvherre [local] SELECT(transformTargetList+0x58) [0x5586270c9808]
postgres: alvherre alvherre [local] SELECT(transformStmt+0x9d1) [0x55862708caf1]
postgres: alvherre alvherre [local] SELECT(parse_analyze+0x57) [0x55862708f177]
postgres: alvherre alvherre [local] SELECT(pg_analyze_and_rewrite+0x12) [0x5586272d2f02]
postgres: alvherre alvherre [local] SELECT(+0x4085ca) [0x5586272d35ca]
postgres: alvherre alvherre [local] SELECT(PostgresMain+0x1a37) [0x5586272d52b7]
postgres: alvherre alvherre [local] SELECT(+0xbf635) [0x558626f8a635]
postgres: alvherre alvherre [local] SELECT(PostmasterMain+0xf3e) [0x55862724e27e]
postgres: alvherre alvherre [local] SELECT(main+0x723) [0x558626f8c603]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7) [0x7f99d1931b97]
postgres: alvherre alvherre [local] SELECT(_start+0x2a) [0x558626f8c6ca]
Didn't think too much about the libunwind format string (or even try to
compile it.)
Despite possible shortcomings in the produced backtraces, this is a
*much* more convenient interface than requesting users to attach gdb,
set breakpoint on errfinish, hey why does my SQL not run, "oh you forgot
'cont' in gdb", etc.
There was also mention of settings that would automatically produce
backtraces for PANICs etc. Those could surely be added if there is
enough interest.
Let's have the basics first, we can add niceties afterwards. (IMO yes,
we should have backtraces in PANICs and assertion failures).
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Jun 25, 2019 at 4:08 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
New thread continuing from
<
/messages/by-id/d4903af2-e7b7-b551-71f8-3e4a6bdc2e73@2ndquadrant.com.
Here is a extended version of Álvaro's patch that adds an errbacktrace()
function. You can do two things with this:- Manually attach it to an ereport() call site that you want to debug.
- Set a configuration parameter like backtrace_function = 'int8in' to
debug ereport()/elog() calls in a specific function.
Thank You. This is very helpful. Surprised is missing for so long time. We
have printing backtrace in Greenplum and its extremely helpful during
development and production.
There was also mention of settings that would automatically produce
backtraces for PANICs etc. Those could surely be added if there is
enough interest.
In Greenplum, we have backtrace enabled for PANICs, SEGV/BUS/ILL and
internal ERRORs, proves very helpful.
For the implementation, I support both backtrace() provided by the OS as
well as using libunwind. The former seems to be supported by a number
of platforms, including glibc, macOS, and FreeBSD, so maybe we don't
need the libunwind suport. I haven't found any difference in quality in
the backtraces between the two approaches, but surely that is highly
dependent on the exact configuration.
We have implemented it using backtrace(). Also, using addr2line() (or atos
for mac) can convert addresses to file and line numbers before printing if
available, to take it a step further.
On Tue, 25 Jun 2019 at 06:08, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
New thread continuing from
<
/messages/by-id/d4903af2-e7b7-b551-71f8-3e4a6bdc2e73@2ndquadrant.com.
Here is a extended version of Álvaro's patch that adds an errbacktrace()
function. You can do two things with this:- Manually attach it to an ereport() call site that you want to debug.
- Set a configuration parameter like backtrace_function = 'int8in' to
debug ereport()/elog() calls in a specific function.There was also mention of settings that would automatically produce
backtraces for PANICs etc. Those could surely be added if there is
enough interest.For the implementation, I support both backtrace() provided by the OS as
well as using libunwind. The former seems to be supported by a number
of platforms, including glibc, macOS, and FreeBSD, so maybe we don't
need the libunwind suport. I haven't found any difference in quality in
the backtraces between the two approaches, but surely that is highly
dependent on the exact configuration.I would welcome testing in all direction with this, to see how well it
works in different circumstances.
Hi Peter,
This is certainly a very useful thing. Sadly, it doesn't seem to compile
when trying to use libunwind.
I tried it in a Debian 9 machine with gcc 6.3.0 and debian says i installed
libunwind8 (1.1)
./configure --prefix=/home/jcasanov/Documentos/pgdg/pgbuild/pg13
--enable-debug --enable-profiling --enable-cassert --enable-depend
--with-libunwind
at make i get these errors:
"""
utils/error/elog.o: En la función `set_backtrace':
/home/jcasanov/Documentos/pgdg/projects/postgresql/src/backend/utils/error/elog.c:847:
referencia a `_Ux86_64_getcontext' sin definir
/home/jcasanov/Documentos/pgdg/projects/postgresql/src/backend/utils/error/elog.c:848:
referencia a `_Ux86_64_init_local' sin definir
/home/jcasanov/Documentos/pgdg/projects/postgresql/src/backend/utils/error/elog.c:850:
referencia a `_Ux86_64_step' sin definir
/home/jcasanov/Documentos/pgdg/projects/postgresql/src/backend/utils/error/elog.c:861:
referencia a `_Ux86_64_get_reg' sin definir
/home/jcasanov/Documentos/pgdg/projects/postgresql/src/backend/utils/error/elog.c:862:
referencia a `_Ux86_64_get_proc_name' sin definir
collect2: error: ld returned 1 exit status
make[2]: *** [postgres] Error 1
make[1]: *** [all-backend-recurse] Error 2
make: *** [all-src-recurse] Error 2
"""
--
Jaime Casanova www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Jun 25, 2019 at 11:08 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
For the implementation, I support both backtrace() provided by the OS as
well as using libunwind. The former seems to be supported by a number
of platforms, including glibc, macOS, and FreeBSD, so maybe we don't
need the libunwind suport. I haven't found any difference in quality in
the backtraces between the two approaches, but surely that is highly
dependent on the exact configuration.I would welcome testing in all direction with this, to see how well it
works in different circumstances.
I like it.
Works out of the box on my macOS machine, but for FreeBSD I had to add
-lexecinfo to LIBS.
--
Thomas Munro
https://enterprisedb.com
On Sat, Jun 29, 2019 at 7:41 AM Jaime Casanova <jaime.casanova@2ndquadrant.com> wrote:
This is certainly a very useful thing. Sadly, it doesn't seem to compile when
trying to use libunwind.
Yeah, the same for me. To make it works I've restricted libunwind to local
unwinding only:
#ifdef USE_LIBUNWIND
#define UNW_LOCAL_ONLY
#include <libunwind.h>
#endif
And result looks pretty nice:
2019-07-08 17:24:08.406 CEST [31828] ERROR: invalid input syntax for
type integer: "foobar" at character 12
2019-07-08 17:24:08.406 CEST [31828] BACKTRACE: #0
pg_strtoint32+0x1d1 [0x000055fa389bcbbe]
#1 int4in+0xd [0x000055fa38976d7b]
#2 InputFunctionCall+0x6f [0x000055fa38a488e9]
#3 OidInputFunctionCall+0x44 [0x000055fa38a48b0d]
#4 stringTypeDatum+0x33 [0x000055fa386e222e]
#5 coerce_type+0x26d [0x000055fa386ca14d]
#6 coerce_to_target_type+0x79 [0x000055fa386c9494]
#7 transformTypeCast+0xaa [0x000055fa386d0042]
#8 transformExprRecurse+0x22f [0x000055fa386cf650]
#9 transformExpr+0x1a [0x000055fa386cf30a]
#10 transformTargetEntry+0x79 [0x000055fa386e1131]
#11 transformTargetList+0x86 [0x000055fa386e11ce]
#12 transformSelectStmt+0xa1 [0x000055fa386a29c9]
#13 transformStmt+0x9d [0x000055fa386a345a]
#14 transformOptionalSelectInto+0x94 [0x000055fa386a3f49]
#15 transformTopLevelStmt+0x15 [0x000055fa386a3f88]
#16 parse_analyze+0x4e [0x000055fa386a3fef]
#17 pg_analyze_and_rewrite+0x3e [0x000055fa3890cfa5]
#18 exec_simple_query+0x35b [0x000055fa3890d5b5]
#19 PostgresMain+0x91f [0x000055fa3890f7a8]
#20 BackendRun+0x1ac [0x000055fa3887ed17]
#21 BackendStartup+0x15c [0x000055fa38881ea1]
#22 ServerLoop+0x1e6 [0x000055fa388821bb]
#23 PostmasterMain+0x1101 [0x000055fa388835a1]
#24 main+0x21a [0x000055fa387db1a9]
#25 __libc_start_main+0xe7 [0x00007f3d1a607fa7]
#26 _start+0x2a [0x000055fa3858e4ea]
On 2019-Jul-08, Dmitry Dolgov wrote:
On Sat, Jun 29, 2019 at 7:41 AM Jaime Casanova <jaime.casanova@2ndquadrant.com> wrote:
This is certainly a very useful thing. Sadly, it doesn't seem to compile when
trying to use libunwind.Yeah, the same for me. To make it works I've restricted libunwind to local
unwinding only:#ifdef USE_LIBUNWIND
#define UNW_LOCAL_ONLY
#include <libunwind.h>
#endif
Ah, yes. unwind's manpage says:
Normally, libunwind supports both local and remote unwinding (the latter will
be explained in the next section). However, if you tell libunwind that your
program only needs local unwinding, then a special implementation can be
selected which may run much faster than the generic implementation which
supports both kinds of unwinding. To select this optimized version, simply
define the macro UNW_LOCAL_ONLY before including the headerfile <libunwind.h>.
so I agree with unconditionally defining that symbol.
Nitpicking dept: I think in these tests:
+ if (!edata->backtrace &&
+ edata->funcname &&
+ backtrace_function[0] &&
+ strcmp(backtrace_function, edata->funcname) == 0)
+ set_backtrace(edata, 2);
we should test for backtrace_function[0] before edata->funcname, since
it seems more likely to be unset.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
After further research I'm thinking about dropping the libunwind
support. The backtrace()/backtrace_symbols() API is more widely
available: darwin, freebsd, linux, netbsd, openbsd (via port), solaris,
and of course it's built-in, whereas libunwind is only available for
linux, freebsd, hpux, solaris, and requires an external dependency.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-07-09 11:43, Peter Eisentraut wrote:
After further research I'm thinking about dropping the libunwind
support. The backtrace()/backtrace_symbols() API is more widely
available: darwin, freebsd, linux, netbsd, openbsd (via port), solaris,
and of course it's built-in, whereas libunwind is only available for
linux, freebsd, hpux, solaris, and requires an external dependency.
Here is an updated patch without the libunwind support, some minor
cleanups, documentation, and automatic back traces from assertion failures.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v3-0001-Add-backtrace-support.patchtext/plain; charset=UTF-8; name=v3-0001-Add-backtrace-support.patch; x-mac-creator=0; x-mac-type=0Download
From 779231dfe56cf8f138a94ba0106ca72171b63350 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 22 Jul 2019 20:08:37 +0200
Subject: [PATCH v3] Add backtrace support
Add some support for automatically showing backtraces in certain error
situations in the server. Backtraces are shown on assertion failure.
A new setting backtrace_function can be set to the name of a C
function, and all ereport()s and elog()s from the function will have
backtraces generated. Finally, the function errbacktrace() can be
manually added to an ereport() call to generate a backtrace for that
call.
Discussion: https://www.postgresql.org/message-id/flat/5f48cb47-bf1e-05b6-7aae-3bf2cd01586d@2ndquadrant.com
---
configure | 61 +++++++++++++++++++++-
configure.in | 4 ++
doc/src/sgml/config.sgml | 25 +++++++++
src/backend/utils/error/assert.c | 13 +++++
src/backend/utils/error/elog.c | 90 ++++++++++++++++++++++++++++++++
src/backend/utils/misc/guc.c | 12 +++++
src/include/pg_config.h.in | 6 +++
src/include/utils/elog.h | 3 ++
src/include/utils/guc.h | 1 +
9 files changed, 213 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index 7a6bfc2339..fbd8224f50 100755
--- a/configure
+++ b/configure
@@ -11662,6 +11662,63 @@ if test "$ac_res" != no; then :
fi
+# *BSD:
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing backtrace_symbols" >&5
+$as_echo_n "checking for library containing backtrace_symbols... " >&6; }
+if ${ac_cv_search_backtrace_symbols+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_func_search_save_LIBS=$LIBS
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+/* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+#ifdef __cplusplus
+extern "C"
+#endif
+char backtrace_symbols ();
+int
+main ()
+{
+return backtrace_symbols ();
+ ;
+ return 0;
+}
+_ACEOF
+for ac_lib in '' execinfo; do
+ if test -z "$ac_lib"; then
+ ac_res="none required"
+ else
+ ac_res=-l$ac_lib
+ LIBS="-l$ac_lib $ac_func_search_save_LIBS"
+ fi
+ if ac_fn_c_try_link "$LINENO"; then :
+ ac_cv_search_backtrace_symbols=$ac_res
+fi
+rm -f core conftest.err conftest.$ac_objext \
+ conftest$ac_exeext
+ if ${ac_cv_search_backtrace_symbols+:} false; then :
+ break
+fi
+done
+if ${ac_cv_search_backtrace_symbols+:} false; then :
+
+else
+ ac_cv_search_backtrace_symbols=no
+fi
+rm conftest.$ac_ext
+LIBS=$ac_func_search_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_backtrace_symbols" >&5
+$as_echo "$ac_cv_search_backtrace_symbols" >&6; }
+ac_res=$ac_cv_search_backtrace_symbols
+if test "$ac_res" != no; then :
+ test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+
+fi
+
if test "$with_readline" = yes; then
@@ -12760,7 +12817,7 @@ $as_echo "#define HAVE_STDBOOL_H 1" >>confdefs.h
fi
-for ac_header in atomic.h copyfile.h crypt.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/ipc.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h
+for ac_header in atomic.h copyfile.h crypt.h execinfo.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/ipc.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h
do :
as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
@@ -15143,7 +15200,7 @@ fi
LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-for ac_func in cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range uselocale utime utimes wcstombs_l
+for ac_func in backtrace_symbols cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range uselocale utime utimes wcstombs_l
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.in b/configure.in
index dde3eec89f..f0c5ec9f47 100644
--- a/configure.in
+++ b/configure.in
@@ -1131,6 +1131,8 @@ AC_SEARCH_LIBS(sched_yield, rt)
AC_SEARCH_LIBS(gethostbyname_r, nsl)
# Cygwin:
AC_SEARCH_LIBS(shmget, cygipc)
+# *BSD:
+AC_SEARCH_LIBS(backtrace_symbols, execinfo)
if test "$with_readline" = yes; then
PGAC_CHECK_READLINE
@@ -1274,6 +1276,7 @@ AC_CHECK_HEADERS(m4_normalize([
atomic.h
copyfile.h
crypt.h
+ execinfo.h
fp_class.h
getopt.h
ieeefp.h
@@ -1588,6 +1591,7 @@ LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
AC_CHECK_FUNCS(m4_normalize([
+ backtrace_symbols
cbrt
clock_gettime
copyfile
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c91e3e1550..7bc5bfe733 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9303,6 +9303,31 @@ <title>Developer Options</title>
</listitem>
</varlistentry>
+ <varlistentry id="guc-backtrace-function" xreflabel="backtrace_function">
+ <term><varname>backtrace_function</varname> (<type>string</type>)
+ <indexterm>
+ <primary><varname>backtrace_function</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ If an error is raised and the name of the internal C function where
+ the error happens matches the value of this setting, then a backtrace
+ is written to the server log together with the error message. This
+ can be used to debug specific areas of the source code.
+ </para>
+
+ <para>
+ Backtrace support is not available on all platforms, and the quality
+ of the backtraces depends on compilation options.
+ </para>
+
+ <para>
+ This parameter can only be set by superusers.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-ignore-system-indexes" xreflabel="ignore_system_indexes">
<term><varname>ignore_system_indexes</varname> (<type>boolean</type>)
<indexterm>
diff --git a/src/backend/utils/error/assert.c b/src/backend/utils/error/assert.c
index 2050b4355d..14707fc296 100644
--- a/src/backend/utils/error/assert.c
+++ b/src/backend/utils/error/assert.c
@@ -18,6 +18,9 @@
#include "postgres.h"
#include <unistd.h>
+#ifdef HAVE_EXECINFO_H
+#include <execinfo.h>
+#endif
/*
* ExceptionalCondition - Handles the failure of an Assert()
@@ -42,6 +45,16 @@ ExceptionalCondition(const char *conditionName,
/* Usually this shouldn't be needed, but make sure the msg went out */
fflush(stderr);
+#ifdef HAVE_BACKTRACE_SYMBOLS
+ {
+ void *buf[100];
+ int nframes;
+
+ nframes = backtrace(buf, sizeof(buf));
+ backtrace_symbols_fd(buf, nframes, fileno(stderr));
+ }
+#endif
+
#ifdef SLEEP_ON_ASSERT
/*
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 8b4720ef3a..7f5c248992 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -62,6 +62,9 @@
#ifdef HAVE_SYSLOG
#include <syslog.h>
#endif
+#ifdef HAVE_EXECINFO_H
+#include <execinfo.h>
+#endif
#include "access/transam.h"
#include "access/xact.h"
@@ -166,6 +169,7 @@ static char formatted_log_time[FORMATTED_TS_LEN];
} while (0)
+static pg_noinline void set_backtrace(ErrorData *edata, int num_skip);
static const char *err_gettext(const char *str) pg_attribute_format_arg(1);
static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *str);
static void write_console(const char *line, int len);
@@ -424,6 +428,13 @@ errfinish(int dummy,...)
*/
oldcontext = MemoryContextSwitchTo(ErrorContext);
+ if (!edata->backtrace &&
+ edata->funcname &&
+ backtrace_function &&
+ backtrace_function[0] &&
+ strcmp(backtrace_function, edata->funcname) == 0)
+ set_backtrace(edata, 2);
+
/*
* Call any context callback functions. Errors occurring in callback
* functions will be treated as recursive errors --- this ensures we will
@@ -488,6 +499,8 @@ errfinish(int dummy,...)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -798,6 +811,62 @@ errmsg(const char *fmt,...)
return 0; /* return value does not matter */
}
+/*
+ * Add a backtrace to the containing ereport() call. This is intended to be
+ * added temporarily during debugging.
+ */
+int
+errbacktrace(void)
+{
+ ErrorData *edata = &errordata[errordata_stack_depth];
+ MemoryContext oldcontext;
+
+ recursion_depth++;
+ CHECK_STACK_DEPTH();
+ oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+
+ set_backtrace(edata, 1);
+
+ MemoryContextSwitchTo(oldcontext);
+ recursion_depth--;
+
+ return 0;
+}
+
+/*
+ * Compute backtrace data and add it to the supplied ErrorData. num_skip
+ * specifies how many inner frames to skip. Use this to avoid showing the
+ * internal backtrace support functions in the backtrace. This requires that
+ * this and related functions are not inlined.
+ */
+static void
+set_backtrace(ErrorData *edata, int num_skip)
+{
+ StringInfoData errtrace;
+
+ initStringInfo(&errtrace);
+
+#ifdef HAVE_BACKTRACE_SYMBOLS
+ {
+ void *buf[100];
+ int nframes;
+ char **strfrms;
+
+ nframes = backtrace(buf, sizeof(buf));
+ strfrms = backtrace_symbols(buf, nframes);
+ if (strfrms == NULL)
+ return;
+
+ for (int i = num_skip; i < nframes; i++)
+ appendStringInfo(&errtrace, "\n%s", strfrms[i]);
+ free(strfrms);
+ }
+#else
+ appendStringInfoString(&errtrace, "backtrace not supported");
+#endif
+
+ edata->backtrace = errtrace.data;
+}
/*
* errmsg_internal --- add a primary error message text to the current error
@@ -1353,6 +1422,12 @@ elog_finish(int elevel, const char *fmt,...)
recursion_depth++;
oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+ if (!edata->backtrace &&
+ edata->funcname &&
+ backtrace_function[0] &&
+ strcmp(backtrace_function, edata->funcname) == 0)
+ set_backtrace(edata, 2);
+
edata->message_id = fmt;
EVALUATE_MESSAGE(edata->domain, message, false, false);
@@ -1509,6 +1584,8 @@ CopyErrorData(void)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -1547,6 +1624,8 @@ FreeErrorData(ErrorData *edata)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -1622,6 +1701,8 @@ ThrowErrorData(ErrorData *edata)
newedata->hint = pstrdup(edata->hint);
if (edata->context)
newedata->context = pstrdup(edata->context);
+ if (edata->backtrace)
+ newedata->backtrace = pstrdup(edata->backtrace);
/* assume message_id is not available */
if (edata->schema_name)
newedata->schema_name = pstrdup(edata->schema_name);
@@ -1689,6 +1770,8 @@ ReThrowError(ErrorData *edata)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -2914,6 +2997,13 @@ send_message_to_server_log(ErrorData *edata)
append_with_tabs(&buf, edata->context);
appendStringInfoChar(&buf, '\n');
}
+ if (edata->backtrace)
+ {
+ log_line_prefix(&buf, edata);
+ appendStringInfoString(&buf, _("BACKTRACE: "));
+ append_with_tabs(&buf, edata->backtrace);
+ appendStringInfoChar(&buf, '\n');
+ }
if (Log_error_verbosity >= PGERROR_VERBOSE)
{
/* assume no newlines in funcname or filename... */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index fc463601ff..15eb45785f 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -514,6 +514,7 @@ int log_temp_files = -1;
double log_statement_sample_rate = 1.0;
double log_xact_sample_rate = 0;
int trace_recovery_messages = LOG;
+char *backtrace_function;
int temp_file_limit = -1;
@@ -4212,6 +4213,17 @@ static struct config_string ConfigureNamesString[] =
NULL, NULL, NULL
},
+ {
+ {"backtrace_function", PGC_SUSET, DEVELOPER_OPTIONS,
+ gettext_noop("Log backtrace for errors in this function."),
+ NULL,
+ GUC_NOT_IN_SAMPLE
+ },
+ &backtrace_function,
+ "",
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 512213aa32..21cd01a66c 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -96,6 +96,9 @@
/* Define to 1 if you have the <atomic.h> header file. */
#undef HAVE_ATOMIC_H
+/* Define to 1 if you have the `backtrace_symbols' function. */
+#undef HAVE_BACKTRACE_SYMBOLS
+
/* Define to 1 if you have the `BIO_get_data' function. */
#undef HAVE_BIO_GET_DATA
@@ -201,6 +204,9 @@
/* Define to 1 if you have the <editline/readline.h> header file. */
#undef HAVE_EDITLINE_READLINE_H
+/* Define to 1 if you have the <execinfo.h> header file. */
+#undef HAVE_EXECINFO_H
+
/* Define to 1 if you have the `fdatasync' function. */
#undef HAVE_FDATASYNC
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index dbfd8efd26..b58aee792a 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -189,6 +189,8 @@ extern int errcontext_msg(const char *fmt,...) pg_attribute_printf(1, 2);
extern int errhidestmt(bool hide_stmt);
extern int errhidecontext(bool hide_ctx);
+extern int errbacktrace(void);
+
extern int errfunction(const char *funcname);
extern int errposition(int cursorpos);
@@ -362,6 +364,7 @@ typedef struct ErrorData
char *detail_log; /* detail error message for server log only */
char *hint; /* hint message */
char *context; /* context message */
+ char *backtrace; /* backtrace */
const char *message_id; /* primary message's id (original string) */
char *schema_name; /* name of schema */
char *table_name; /* name of table */
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index e709177c37..032f4f16ab 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -255,6 +255,7 @@ extern int log_min_duration_statement;
extern int log_temp_files;
extern double log_statement_sample_rate;
extern double log_xact_sample_rate;
+extern char *backtrace_function;
extern int temp_file_limit;
base-commit: 23bccc823d434d9dcf3c12622fe260d9235baae2
--
2.22.0
On 2019-Jul-22, Peter Eisentraut wrote:
On 2019-07-09 11:43, Peter Eisentraut wrote:
After further research I'm thinking about dropping the libunwind
support. The backtrace()/backtrace_symbols() API is more widely
available: darwin, freebsd, linux, netbsd, openbsd (via port), solaris,
and of course it's built-in, whereas libunwind is only available for
linux, freebsd, hpux, solaris, and requires an external dependency.Here is an updated patch without the libunwind support, some minor
cleanups, documentation, and automatic back traces from assertion failures.
The only possibly complaint I see is that the backtrace support in
ExceptionalCondition does not work for Windows eventlog/console ... but
that seems moot since Windows does not have backtrace support anyway.
+1 to get this patch in at this stage; we can further refine (esp. add
Windows support) later, if need be.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
Here is an updated patch without the libunwind support, some minor
cleanups, documentation, and automatic back traces from assertion failures.
Just noticing that ExceptionalCondition has an "fflush(stderr);"
in front of what you added --- perhaps you should also add one
after the backtrace_symbols_fd call? It's not clear to me that
that function guarantees to fflush, nor do I want to assume that
abort() does.
regards, tom lane
I wrote:
Just noticing that ExceptionalCondition has an "fflush(stderr);"
in front of what you added --- perhaps you should also add one
after the backtrace_symbols_fd call? It's not clear to me that
that function guarantees to fflush, nor do I want to assume that
abort() does.
Oh, wait, it's writing to fileno(stderr) so it couldn't be
buffering anything. Disregard ...
regards, tom lane
On Tue, Jul 23, 2019 at 6:19 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 2019-07-09 11:43, Peter Eisentraut wrote:
After further research I'm thinking about dropping the libunwind
support. The backtrace()/backtrace_symbols() API is more widely
available: darwin, freebsd, linux, netbsd, openbsd (via port), solaris,
and of course it's built-in, whereas libunwind is only available for
linux, freebsd, hpux, solaris, and requires an external dependency.Here is an updated patch without the libunwind support, some minor
cleanups, documentation, and automatic back traces from assertion failures.
Now works out of the box on FreeBSD. The assertion thing is a nice touch.
I wonder if it'd make sense to have a log_min_backtrace GUC that you
could set to error/fatal/panicwhatever (perhaps in a later patch).
--
Thomas Munro
https://enterprisedb.com
Hi
so I agree with unconditionally defining that symbol.
Nitpicking dept: I think in these tests:
+ if (!edata->backtrace && + edata->funcname && + backtrace_function[0] && + strcmp(backtrace_function, edata->funcname) == 0) + set_backtrace(edata, 2);
If I understand well, backtrace is displayed only when edata->funcname is
same like backtrace_function GUC. Isn't it too strong limit?
For example, I want to see backtrace for all PANIC level errors on
production, and I would not to limit the source function?
Regards
Pavel
Show quoted text
we should test for backtrace_function[0] before edata->funcname, since
it seems more likely to be unset.--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-08-12 13:19, Pavel Stehule wrote:
If I understand well, backtrace is displayed only when edata->funcname
is same like backtrace_function GUC. Isn't it too strong limit?For example, I want to see backtrace for all PANIC level errors on
production, and I would not to limit the source function?
We can add additional ways to invoke this once we have the basic
functionality in.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
po 12. 8. 2019 v 19:06 odesílatel Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> napsal:
On 2019-08-12 13:19, Pavel Stehule wrote:
If I understand well, backtrace is displayed only when edata->funcname
is same like backtrace_function GUC. Isn't it too strong limit?For example, I want to see backtrace for all PANIC level errors on
production, and I would not to limit the source function?We can add additional ways to invoke this once we have the basic
functionality in.
ok
Pavel
Show quoted text
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-07-22 20:19, Peter Eisentraut wrote:
On 2019-07-09 11:43, Peter Eisentraut wrote:
After further research I'm thinking about dropping the libunwind
support. The backtrace()/backtrace_symbols() API is more widely
available: darwin, freebsd, linux, netbsd, openbsd (via port), solaris,
and of course it's built-in, whereas libunwind is only available for
linux, freebsd, hpux, solaris, and requires an external dependency.Here is an updated patch without the libunwind support, some minor
cleanups, documentation, and automatic back traces from assertion failures.
Another updated version.
I have changed the configuration setting to backtrace_functions plural,
so that you can debug more than one location at once. I had originally
wanted to do that but using existing functions like
SplitIdentifierString() resulted in lots of complications with error
handling (inside error handling!). So here I just hand-coded the list
splitting. Seems simple enough.
I think this patch is now good to go from my perspective.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v4-0001-Add-backtrace-support.patchtext/plain; charset=UTF-8; name=v4-0001-Add-backtrace-support.patch; x-mac-creator=0; x-mac-type=0Download
From f9d440ec41f49464661960c580c29ce0558a1642 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 13 Aug 2019 01:47:22 +0200
Subject: [PATCH v4] Add backtrace support
Add some support for automatically showing backtraces in certain error
situations in the server. Backtraces are shown on assertion failure.
A new setting backtrace_functions can be set to a list of C function
names, and all ereport()s and elog()s from the functions will have
backtraces generated. Finally, the function errbacktrace() can be
manually added to an ereport() call to generate a backtrace for that
call.
Discussion: https://www.postgresql.org/message-id/flat/5f48cb47-bf1e-05b6-7aae-3bf2cd01586d@2ndquadrant.com
---
configure | 61 +++++++++++++++-
configure.in | 4 ++
doc/src/sgml/config.sgml | 26 +++++++
src/backend/utils/error/assert.c | 13 ++++
src/backend/utils/error/elog.c | 115 +++++++++++++++++++++++++++++++
src/backend/utils/misc/guc.c | 12 ++++
src/include/pg_config.h.in | 6 ++
src/include/utils/elog.h | 3 +
src/include/utils/guc.h | 1 +
9 files changed, 239 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index 7a6bfc2339..fbd8224f50 100755
--- a/configure
+++ b/configure
@@ -11662,6 +11662,63 @@ if test "$ac_res" != no; then :
fi
+# *BSD:
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing backtrace_symbols" >&5
+$as_echo_n "checking for library containing backtrace_symbols... " >&6; }
+if ${ac_cv_search_backtrace_symbols+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_func_search_save_LIBS=$LIBS
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+/* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+#ifdef __cplusplus
+extern "C"
+#endif
+char backtrace_symbols ();
+int
+main ()
+{
+return backtrace_symbols ();
+ ;
+ return 0;
+}
+_ACEOF
+for ac_lib in '' execinfo; do
+ if test -z "$ac_lib"; then
+ ac_res="none required"
+ else
+ ac_res=-l$ac_lib
+ LIBS="-l$ac_lib $ac_func_search_save_LIBS"
+ fi
+ if ac_fn_c_try_link "$LINENO"; then :
+ ac_cv_search_backtrace_symbols=$ac_res
+fi
+rm -f core conftest.err conftest.$ac_objext \
+ conftest$ac_exeext
+ if ${ac_cv_search_backtrace_symbols+:} false; then :
+ break
+fi
+done
+if ${ac_cv_search_backtrace_symbols+:} false; then :
+
+else
+ ac_cv_search_backtrace_symbols=no
+fi
+rm conftest.$ac_ext
+LIBS=$ac_func_search_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_backtrace_symbols" >&5
+$as_echo "$ac_cv_search_backtrace_symbols" >&6; }
+ac_res=$ac_cv_search_backtrace_symbols
+if test "$ac_res" != no; then :
+ test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+
+fi
+
if test "$with_readline" = yes; then
@@ -12760,7 +12817,7 @@ $as_echo "#define HAVE_STDBOOL_H 1" >>confdefs.h
fi
-for ac_header in atomic.h copyfile.h crypt.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/ipc.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h
+for ac_header in atomic.h copyfile.h crypt.h execinfo.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/ipc.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h
do :
as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
@@ -15143,7 +15200,7 @@ fi
LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-for ac_func in cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range uselocale utime utimes wcstombs_l
+for ac_func in backtrace_symbols cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range uselocale utime utimes wcstombs_l
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.in b/configure.in
index dde3eec89f..f0c5ec9f47 100644
--- a/configure.in
+++ b/configure.in
@@ -1131,6 +1131,8 @@ AC_SEARCH_LIBS(sched_yield, rt)
AC_SEARCH_LIBS(gethostbyname_r, nsl)
# Cygwin:
AC_SEARCH_LIBS(shmget, cygipc)
+# *BSD:
+AC_SEARCH_LIBS(backtrace_symbols, execinfo)
if test "$with_readline" = yes; then
PGAC_CHECK_READLINE
@@ -1274,6 +1276,7 @@ AC_CHECK_HEADERS(m4_normalize([
atomic.h
copyfile.h
crypt.h
+ execinfo.h
fp_class.h
getopt.h
ieeefp.h
@@ -1588,6 +1591,7 @@ LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
AC_CHECK_FUNCS(m4_normalize([
+ backtrace_symbols
cbrt
clock_gettime
copyfile
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index cdc30fa5e3..10e6c80ac1 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9285,6 +9285,32 @@ <title>Developer Options</title>
</listitem>
</varlistentry>
+ <varlistentry id="guc-backtrace-functions" xreflabel="backtrace_functions">
+ <term><varname>backtrace_functions</varname> (<type>string</type>)
+ <indexterm>
+ <primary><varname>backtrace_functions</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ This parameter contains a comma-separated list of C function names.
+ If an error is raised and the name of the internal C function where
+ the error happens matches a value in the list, then a backtrace is
+ written to the server log together with the error message. This can
+ be used to debug specific areas of the source code.
+ </para>
+
+ <para>
+ Backtrace support is not available on all platforms, and the quality
+ of the backtraces depends on compilation options.
+ </para>
+
+ <para>
+ This parameter can only be set by superusers.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-ignore-system-indexes" xreflabel="ignore_system_indexes">
<term><varname>ignore_system_indexes</varname> (<type>boolean</type>)
<indexterm>
diff --git a/src/backend/utils/error/assert.c b/src/backend/utils/error/assert.c
index 2050b4355d..14707fc296 100644
--- a/src/backend/utils/error/assert.c
+++ b/src/backend/utils/error/assert.c
@@ -18,6 +18,9 @@
#include "postgres.h"
#include <unistd.h>
+#ifdef HAVE_EXECINFO_H
+#include <execinfo.h>
+#endif
/*
* ExceptionalCondition - Handles the failure of an Assert()
@@ -42,6 +45,16 @@ ExceptionalCondition(const char *conditionName,
/* Usually this shouldn't be needed, but make sure the msg went out */
fflush(stderr);
+#ifdef HAVE_BACKTRACE_SYMBOLS
+ {
+ void *buf[100];
+ int nframes;
+
+ nframes = backtrace(buf, sizeof(buf));
+ backtrace_symbols_fd(buf, nframes, fileno(stderr));
+ }
+#endif
+
#ifdef SLEEP_ON_ASSERT
/*
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 8b4720ef3a..12a37e4ac5 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -62,6 +62,9 @@
#ifdef HAVE_SYSLOG
#include <syslog.h>
#endif
+#ifdef HAVE_EXECINFO_H
+#include <execinfo.h>
+#endif
#include "access/transam.h"
#include "access/xact.h"
@@ -166,6 +169,7 @@ static char formatted_log_time[FORMATTED_TS_LEN];
} while (0)
+static pg_noinline void set_backtrace(ErrorData *edata, int num_skip);
static const char *err_gettext(const char *str) pg_attribute_format_arg(1);
static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *str);
static void write_console(const char *line, int len);
@@ -398,6 +402,33 @@ errstart(int elevel, const char *filename, int lineno,
return true;
}
+/*
+ * Checks whether the given funcname matches the comma-separated list in backtrace_functions.
+ *
+ * This is coded separately here instead of using an existing list-splitting
+ * function so that we can control the error behavior carefully, since this is
+ * called during error processing.
+ */
+static bool
+matches_backtrace_functions(const char *funcname)
+{
+ char *setting = strdup(backtrace_functions);
+
+ if (!setting)
+ return false;
+
+ for (char *token = strtok(setting, ","); token; token = strtok(NULL, ","))
+ {
+ if (strcmp(token, funcname) == 0)
+ {
+ free(setting);
+ return true;
+ }
+ }
+ free(setting);
+ return false;
+}
+
/*
* errfinish --- end an error-reporting cycle
*
@@ -424,6 +455,12 @@ errfinish(int dummy,...)
*/
oldcontext = MemoryContextSwitchTo(ErrorContext);
+ if (!edata->backtrace &&
+ edata->funcname &&
+ backtrace_functions &&
+ matches_backtrace_functions(edata->funcname))
+ set_backtrace(edata, 2);
+
/*
* Call any context callback functions. Errors occurring in callback
* functions will be treated as recursive errors --- this ensures we will
@@ -488,6 +525,8 @@ errfinish(int dummy,...)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -798,6 +837,62 @@ errmsg(const char *fmt,...)
return 0; /* return value does not matter */
}
+/*
+ * Add a backtrace to the containing ereport() call. This is intended to be
+ * added temporarily during debugging.
+ */
+int
+errbacktrace(void)
+{
+ ErrorData *edata = &errordata[errordata_stack_depth];
+ MemoryContext oldcontext;
+
+ recursion_depth++;
+ CHECK_STACK_DEPTH();
+ oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+
+ set_backtrace(edata, 1);
+
+ MemoryContextSwitchTo(oldcontext);
+ recursion_depth--;
+
+ return 0;
+}
+
+/*
+ * Compute backtrace data and add it to the supplied ErrorData. num_skip
+ * specifies how many inner frames to skip. Use this to avoid showing the
+ * internal backtrace support functions in the backtrace. This requires that
+ * this and related functions are not inlined.
+ */
+static void
+set_backtrace(ErrorData *edata, int num_skip)
+{
+ StringInfoData errtrace;
+
+ initStringInfo(&errtrace);
+
+#ifdef HAVE_BACKTRACE_SYMBOLS
+ {
+ void *buf[100];
+ int nframes;
+ char **strfrms;
+
+ nframes = backtrace(buf, sizeof(buf));
+ strfrms = backtrace_symbols(buf, nframes);
+ if (strfrms == NULL)
+ return;
+
+ for (int i = num_skip; i < nframes; i++)
+ appendStringInfo(&errtrace, "\n%s", strfrms[i]);
+ free(strfrms);
+ }
+#else
+ appendStringInfoString(&errtrace, "backtrace not supported");
+#endif
+
+ edata->backtrace = errtrace.data;
+}
/*
* errmsg_internal --- add a primary error message text to the current error
@@ -1353,6 +1448,11 @@ elog_finish(int elevel, const char *fmt,...)
recursion_depth++;
oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+ if (!edata->backtrace &&
+ edata->funcname &&
+ matches_backtrace_functions(edata->funcname) == 0)
+ set_backtrace(edata, 2);
+
edata->message_id = fmt;
EVALUATE_MESSAGE(edata->domain, message, false, false);
@@ -1509,6 +1609,8 @@ CopyErrorData(void)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -1547,6 +1649,8 @@ FreeErrorData(ErrorData *edata)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -1622,6 +1726,8 @@ ThrowErrorData(ErrorData *edata)
newedata->hint = pstrdup(edata->hint);
if (edata->context)
newedata->context = pstrdup(edata->context);
+ if (edata->backtrace)
+ newedata->backtrace = pstrdup(edata->backtrace);
/* assume message_id is not available */
if (edata->schema_name)
newedata->schema_name = pstrdup(edata->schema_name);
@@ -1689,6 +1795,8 @@ ReThrowError(ErrorData *edata)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -2914,6 +3022,13 @@ send_message_to_server_log(ErrorData *edata)
append_with_tabs(&buf, edata->context);
appendStringInfoChar(&buf, '\n');
}
+ if (edata->backtrace)
+ {
+ log_line_prefix(&buf, edata);
+ appendStringInfoString(&buf, _("BACKTRACE: "));
+ append_with_tabs(&buf, edata->backtrace);
+ appendStringInfoChar(&buf, '\n');
+ }
if (Log_error_verbosity >= PGERROR_VERBOSE)
{
/* assume no newlines in funcname or filename... */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index eb78522053..2bb05414e1 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -513,6 +513,7 @@ int log_min_duration_statement = -1;
int log_temp_files = -1;
double log_xact_sample_rate = 0;
int trace_recovery_messages = LOG;
+char *backtrace_functions;
int temp_file_limit = -1;
@@ -4199,6 +4200,17 @@ static struct config_string ConfigureNamesString[] =
NULL, NULL, NULL
},
+ {
+ {"backtrace_functions", PGC_SUSET, DEVELOPER_OPTIONS,
+ gettext_noop("Log backtrace for errors in these functions."),
+ NULL,
+ GUC_NOT_IN_SAMPLE
+ },
+ &backtrace_functions,
+ "",
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 512213aa32..21cd01a66c 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -96,6 +96,9 @@
/* Define to 1 if you have the <atomic.h> header file. */
#undef HAVE_ATOMIC_H
+/* Define to 1 if you have the `backtrace_symbols' function. */
+#undef HAVE_BACKTRACE_SYMBOLS
+
/* Define to 1 if you have the `BIO_get_data' function. */
#undef HAVE_BIO_GET_DATA
@@ -201,6 +204,9 @@
/* Define to 1 if you have the <editline/readline.h> header file. */
#undef HAVE_EDITLINE_READLINE_H
+/* Define to 1 if you have the <execinfo.h> header file. */
+#undef HAVE_EXECINFO_H
+
/* Define to 1 if you have the `fdatasync' function. */
#undef HAVE_FDATASYNC
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index ba0b7f6f79..6a315414e2 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -189,6 +189,8 @@ extern int errcontext_msg(const char *fmt,...) pg_attribute_printf(1, 2);
extern int errhidestmt(bool hide_stmt);
extern int errhidecontext(bool hide_ctx);
+extern int errbacktrace(void);
+
extern int errfunction(const char *funcname);
extern int errposition(int cursorpos);
@@ -362,6 +364,7 @@ typedef struct ErrorData
char *detail_log; /* detail error message for server log only */
char *hint; /* hint message */
char *context; /* context message */
+ char *backtrace; /* backtrace */
const char *message_id; /* primary message's id (original string) */
char *schema_name; /* name of schema */
char *table_name; /* name of table */
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 6791e0cbc2..af8d5a6cdd 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -254,6 +254,7 @@ extern PGDLLIMPORT int client_min_messages;
extern int log_min_duration_statement;
extern int log_temp_files;
extern double log_xact_sample_rate;
+extern char *backtrace_functions;
extern int temp_file_limit;
base-commit: 66bde49d96a9ddacc49dcbdf1b47b5bd6e31ead5
--
2.22.0
On 2019-Aug-13, Peter Eisentraut wrote:
I have changed the configuration setting to backtrace_functions plural,
so that you can debug more than one location at once. I had originally
wanted to do that but using existing functions like
SplitIdentifierString() resulted in lots of complications with error
handling (inside error handling!). So here I just hand-coded the list
splitting. Seems simple enough.
Hmm ... but is that the natural way to write this? I would have thought
you'd split the list at config-read time (the assign hook for the GUC)
and turn it into a List of simple strings. Then you don't have to
loop strtok() on each errfinish().
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
I have changed the configuration setting to backtrace_functions plural,
so that you can debug more than one location at once. I had originally
wanted to do that but using existing functions like
SplitIdentifierString() resulted in lots of complications with error
handling (inside error handling!). So here I just hand-coded the list
splitting. Seems simple enough.
I think it's a pretty bad idea for anything invocable from elog to
trample on the process-wide strtok() state. Even if there's no
conflict today, there will be one eventually, unless you are going
to adopt the position that nobody else is allowed to use strtok().
regards, tom lane
On 2019-08-13 15:24, Alvaro Herrera wrote:
On 2019-Aug-13, Peter Eisentraut wrote:
I have changed the configuration setting to backtrace_functions plural,
so that you can debug more than one location at once. I had originally
wanted to do that but using existing functions like
SplitIdentifierString() resulted in lots of complications with error
handling (inside error handling!). So here I just hand-coded the list
splitting. Seems simple enough.Hmm ... but is that the natural way to write this? I would have thought
you'd split the list at config-read time (the assign hook for the GUC)
and turn it into a List of simple strings. Then you don't have to
loop strtok() on each errfinish().
The memory management of that seems too complicated. The "extra"
mechanism of the check/assign hooks only supports one level of malloc.
Using a List seems impossible. I don't know if you can safely do a
malloc-ed array of malloc-ed strings either.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-Aug-20, Peter Eisentraut wrote:
The memory management of that seems too complicated. The "extra"
mechanism of the check/assign hooks only supports one level of malloc.
Using a List seems impossible. I don't know if you can safely do a
malloc-ed array of malloc-ed strings either.
Here's an idea -- have the check/assign hooks create a different
representation, which is a single guc_malloc'ed chunk that is made up of
every function name listed in the GUC, separated by \0. That can be
scanned at error time comparing the function name with each piece.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v5-0001-Add-backtrace-support.patchtext/x-diff; charset=us-asciiDownload
From 7341310cd75722fbfe7cb7996516b0b50d83bb1f Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 13 Sep 2019 12:51:13 -0300
Subject: [PATCH v5] Add backtrace support
Add some support for automatically showing backtraces in certain error
situations in the server. Backtraces are shown on assertion failure.
A new setting backtrace_functions can be set to a list of C function
names, and all ereport()s and elog()s from the functions will have
backtraces generated. Finally, the function errbacktrace() can be
manually added to an ereport() call to generate a backtrace for that
call.
Discussion: https://www.postgresql.org/message-id/flat/5f48cb47-bf1e-05b6-7aae-3bf2cd01586d@2ndquadrant.com
---
configure | 61 +++++++++++++++-
configure.in | 4 ++
doc/src/sgml/config.sgml | 26 +++++++
src/backend/utils/error/assert.c | 13 ++++
src/backend/utils/error/elog.c | 117 +++++++++++++++++++++++++++++++
src/backend/utils/misc/guc.c | 67 ++++++++++++++++++
src/include/pg_config.h.in | 6 ++
src/include/utils/elog.h | 3 +
src/include/utils/guc.h | 2 +
9 files changed, 297 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index b3c92764be..900b83dc4e 100755
--- a/configure
+++ b/configure
@@ -11606,6 +11606,63 @@ if test "$ac_res" != no; then :
fi
+# *BSD:
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing backtrace_symbols" >&5
+$as_echo_n "checking for library containing backtrace_symbols... " >&6; }
+if ${ac_cv_search_backtrace_symbols+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_func_search_save_LIBS=$LIBS
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+/* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+#ifdef __cplusplus
+extern "C"
+#endif
+char backtrace_symbols ();
+int
+main ()
+{
+return backtrace_symbols ();
+ ;
+ return 0;
+}
+_ACEOF
+for ac_lib in '' execinfo; do
+ if test -z "$ac_lib"; then
+ ac_res="none required"
+ else
+ ac_res=-l$ac_lib
+ LIBS="-l$ac_lib $ac_func_search_save_LIBS"
+ fi
+ if ac_fn_c_try_link "$LINENO"; then :
+ ac_cv_search_backtrace_symbols=$ac_res
+fi
+rm -f core conftest.err conftest.$ac_objext \
+ conftest$ac_exeext
+ if ${ac_cv_search_backtrace_symbols+:} false; then :
+ break
+fi
+done
+if ${ac_cv_search_backtrace_symbols+:} false; then :
+
+else
+ ac_cv_search_backtrace_symbols=no
+fi
+rm conftest.$ac_ext
+LIBS=$ac_func_search_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_backtrace_symbols" >&5
+$as_echo "$ac_cv_search_backtrace_symbols" >&6; }
+ac_res=$ac_cv_search_backtrace_symbols
+if test "$ac_res" != no; then :
+ test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+
+fi
+
if test "$with_readline" = yes; then
@@ -12704,7 +12761,7 @@ $as_echo "#define HAVE_STDBOOL_H 1" >>confdefs.h
fi
-for ac_header in atomic.h copyfile.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/ipc.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h
+for ac_header in atomic.h copyfile.h execinfo.h fp_class.h getopt.h ieeefp.h ifaddrs.h langinfo.h mbarrier.h poll.h sys/epoll.h sys/ipc.h sys/prctl.h sys/procctl.h sys/pstat.h sys/resource.h sys/select.h sys/sem.h sys/shm.h sys/sockio.h sys/tas.h sys/un.h termios.h ucred.h utime.h wchar.h wctype.h
do :
as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default"
@@ -15087,7 +15144,7 @@ fi
LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
-for ac_func in cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memset_s memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range uselocale utime utimes wcstombs_l
+for ac_func in backtrace_symbols cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memset_s memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range uselocale utime utimes wcstombs_l
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.in b/configure.in
index 0d16c1a971..15f9fffacb 100644
--- a/configure.in
+++ b/configure.in
@@ -1130,6 +1130,8 @@ AC_SEARCH_LIBS(sched_yield, rt)
AC_SEARCH_LIBS(gethostbyname_r, nsl)
# Cygwin:
AC_SEARCH_LIBS(shmget, cygipc)
+# *BSD:
+AC_SEARCH_LIBS(backtrace_symbols, execinfo)
if test "$with_readline" = yes; then
PGAC_CHECK_READLINE
@@ -1272,6 +1274,7 @@ AC_HEADER_STDBOOL
AC_CHECK_HEADERS(m4_normalize([
atomic.h
copyfile.h
+ execinfo.h
fp_class.h
getopt.h
ieeefp.h
@@ -1586,6 +1589,7 @@ LIBS_including_readline="$LIBS"
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
AC_CHECK_FUNCS(m4_normalize([
+ backtrace_symbols
cbrt
clock_gettime
copyfile
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 7f9ce8fcba..78b0f79d79 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9299,6 +9299,32 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</listitem>
</varlistentry>
+ <varlistentry id="guc-backtrace-functions" xreflabel="backtrace_functions">
+ <term><varname>backtrace_functions</varname> (<type>string</type>)
+ <indexterm>
+ <primary><varname>backtrace_functions</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ This parameter contains a comma-separated list of C function names.
+ If an error is raised and the name of the internal C function where
+ the error happens matches a value in the list, then a backtrace is
+ written to the server log together with the error message. This can
+ be used to debug specific areas of the source code.
+ </para>
+
+ <para>
+ Backtrace support is not available on all platforms, and the quality
+ of the backtraces depends on compilation options.
+ </para>
+
+ <para>
+ This parameter can only be set by superusers.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-ignore-system-indexes" xreflabel="ignore_system_indexes">
<term><varname>ignore_system_indexes</varname> (<type>boolean</type>)
<indexterm>
diff --git a/src/backend/utils/error/assert.c b/src/backend/utils/error/assert.c
index 2050b4355d..14707fc296 100644
--- a/src/backend/utils/error/assert.c
+++ b/src/backend/utils/error/assert.c
@@ -18,6 +18,9 @@
#include "postgres.h"
#include <unistd.h>
+#ifdef HAVE_EXECINFO_H
+#include <execinfo.h>
+#endif
/*
* ExceptionalCondition - Handles the failure of an Assert()
@@ -42,6 +45,16 @@ ExceptionalCondition(const char *conditionName,
/* Usually this shouldn't be needed, but make sure the msg went out */
fflush(stderr);
+#ifdef HAVE_BACKTRACE_SYMBOLS
+ {
+ void *buf[100];
+ int nframes;
+
+ nframes = backtrace(buf, sizeof(buf));
+ backtrace_symbols_fd(buf, nframes, fileno(stderr));
+ }
+#endif
+
#ifdef SLEEP_ON_ASSERT
/*
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 8b4720ef3a..715b140ebb 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -62,6 +62,9 @@
#ifdef HAVE_SYSLOG
#include <syslog.h>
#endif
+#ifdef HAVE_EXECINFO_H
+#include <execinfo.h>
+#endif
#include "access/transam.h"
#include "access/xact.h"
@@ -166,6 +169,7 @@ static char formatted_log_time[FORMATTED_TS_LEN];
} while (0)
+static pg_noinline void set_backtrace(ErrorData *edata, int num_skip);
static const char *err_gettext(const char *str) pg_attribute_format_arg(1);
static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *str);
static void write_console(const char *line, int len);
@@ -398,6 +402,31 @@ errstart(int elevel, const char *filename, int lineno,
return true;
}
+/*
+ * Checks whether the given funcname matches backtrace_functions.
+ */
+static bool
+matches_backtrace_functions(const char *funcname)
+{
+ char *p;
+
+ if (!backtrace_symbol_list || funcname == NULL || funcname[0] == '\0')
+ return false;
+
+ p = backtrace_symbol_list;
+ for (;;)
+ {
+ if (*p == '\0')
+ break;
+
+ if (strcmp(funcname, p) == 0)
+ return true;
+ p += strlen(p) + 1;
+ }
+
+ return false;
+}
+
/*
* errfinish --- end an error-reporting cycle
*
@@ -424,6 +453,12 @@ errfinish(int dummy,...)
*/
oldcontext = MemoryContextSwitchTo(ErrorContext);
+ if (!edata->backtrace &&
+ edata->funcname &&
+ backtrace_functions &&
+ matches_backtrace_functions(edata->funcname))
+ set_backtrace(edata, 2);
+
/*
* Call any context callback functions. Errors occurring in callback
* functions will be treated as recursive errors --- this ensures we will
@@ -488,6 +523,8 @@ errfinish(int dummy,...)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -798,6 +835,66 @@ errmsg(const char *fmt,...)
return 0; /* return value does not matter */
}
+/*
+ * Add a backtrace to the containing ereport() call. This is intended to be
+ * added temporarily during debugging.
+ */
+int
+errbacktrace(void)
+{
+ ErrorData *edata = &errordata[errordata_stack_depth];
+ MemoryContext oldcontext;
+
+ Assert(false);
+
+ recursion_depth++;
+ CHECK_STACK_DEPTH();
+ oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+
+ set_backtrace(edata, 1);
+
+ MemoryContextSwitchTo(oldcontext);
+ recursion_depth--;
+
+ return 0;
+}
+
+/*
+ * Compute backtrace data and add it to the supplied ErrorData. num_skip
+ * specifies how many inner frames to skip. Use this to avoid showing the
+ * internal backtrace support functions in the backtrace. This requires that
+ * this and related functions are not inlined.
+ */
+static void
+set_backtrace(ErrorData *edata, int num_skip)
+{
+ StringInfoData errtrace;
+
+ fprintf(stderr, "set_backtrace called\n");
+
+ initStringInfo(&errtrace);
+
+#ifdef HAVE_BACKTRACE_SYMBOLS
+ {
+ void *buf[100];
+ int nframes;
+ char **strfrms;
+
+ nframes = backtrace(buf, sizeof(buf));
+ strfrms = backtrace_symbols(buf, nframes);
+ if (strfrms == NULL)
+ return;
+
+ for (int i = num_skip; i < nframes; i++)
+ appendStringInfo(&errtrace, "\n%s", strfrms[i]);
+ free(strfrms);
+ }
+#else
+ appendStringInfoString(&errtrace, "backtrace not supported");
+#endif
+
+ edata->backtrace = errtrace.data;
+}
/*
* errmsg_internal --- add a primary error message text to the current error
@@ -1353,6 +1450,11 @@ elog_finish(int elevel, const char *fmt,...)
recursion_depth++;
oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+ if (!edata->backtrace &&
+ edata->funcname &&
+ matches_backtrace_functions(edata->funcname))
+ set_backtrace(edata, 2);
+
edata->message_id = fmt;
EVALUATE_MESSAGE(edata->domain, message, false, false);
@@ -1509,6 +1611,8 @@ CopyErrorData(void)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -1547,6 +1651,8 @@ FreeErrorData(ErrorData *edata)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -1622,6 +1728,8 @@ ThrowErrorData(ErrorData *edata)
newedata->hint = pstrdup(edata->hint);
if (edata->context)
newedata->context = pstrdup(edata->context);
+ if (edata->backtrace)
+ newedata->backtrace = pstrdup(edata->backtrace);
/* assume message_id is not available */
if (edata->schema_name)
newedata->schema_name = pstrdup(edata->schema_name);
@@ -1689,6 +1797,8 @@ ReThrowError(ErrorData *edata)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -2914,6 +3024,13 @@ send_message_to_server_log(ErrorData *edata)
append_with_tabs(&buf, edata->context);
appendStringInfoChar(&buf, '\n');
}
+ if (edata->backtrace)
+ {
+ log_line_prefix(&buf, edata);
+ appendStringInfoString(&buf, _("BACKTRACE: "));
+ append_with_tabs(&buf, edata->backtrace);
+ appendStringInfoChar(&buf, '\n');
+ }
if (Log_error_verbosity >= PGERROR_VERBOSE)
{
/* assume no newlines in funcname or filename... */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 90ffd89339..687434b7fa 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -201,6 +201,8 @@ static bool check_cluster_name(char **newval, void **extra, GucSource source);
static const char *show_unix_socket_permissions(void);
static const char *show_log_file_mode(void);
static const char *show_data_directory_mode(void);
+static bool check_backtrace_functions(char **newval, void **extra, GucSource source);
+static void assign_backtrace_functions(const char *newval, void *extra);
static bool check_recovery_target_timeline(char **newval, void **extra, GucSource source);
static void assign_recovery_target_timeline(const char *newval, void *extra);
static bool check_recovery_target(char **newval, void **extra, GucSource source);
@@ -513,6 +515,8 @@ int log_min_duration_statement = -1;
int log_temp_files = -1;
double log_xact_sample_rate = 0;
int trace_recovery_messages = LOG;
+char *backtrace_functions;
+char *backtrace_symbol_list;
int temp_file_limit = -1;
@@ -4199,6 +4203,17 @@ static struct config_string ConfigureNamesString[] =
NULL, NULL, NULL
},
+ {
+ {"backtrace_functions", PGC_SUSET, DEVELOPER_OPTIONS,
+ gettext_noop("Log backtrace for errors in these functions."),
+ NULL,
+ GUC_NOT_IN_SAMPLE
+ },
+ &backtrace_functions,
+ "",
+ check_backtrace_functions, assign_backtrace_functions, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL
@@ -11433,6 +11448,58 @@ show_data_directory_mode(void)
return buf;
}
+static bool
+check_backtrace_functions(char **newval, void **extra, GucSource source)
+{
+ int newvallen = strlen(*newval);
+ char *someval;
+ int validlen;
+ int i;
+ int j;
+
+ newvallen = strlen(*newval);
+ validlen = strspn(*newval,
+ ",_ \n\t"
+ "abcdefghijklmnopqrstuvwxyz"
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+ "0123456789");
+ if (validlen != newvallen)
+ {
+ GUC_check_errdetail("invalid character");
+ return false;
+ }
+
+ if (*newval[0] == '\0')
+ {
+ *extra = NULL;
+ return true;
+ }
+
+ someval = guc_malloc(ERROR, newvallen + 1 + 1);
+ for (i = 0, j = 0; i < newvallen; i++)
+ {
+ if ((*newval)[i] == ',')
+ someval[j++] = '\0'; /* next item */
+ else if ((*newval)[i] == ' ' || (*newval)[i] == '\n')
+ ; /* ignore */
+ else
+ someval[j++] = (*newval)[i];
+ }
+
+ /* two \0s end the setting */
+ someval[j] = '\0';
+ someval[j + 1] = '\0';
+ *extra = someval;
+
+ return true;
+}
+
+static void
+assign_backtrace_functions(const char *newval, void *extra)
+{
+ backtrace_symbol_list = (char *) extra;
+}
+
static bool
check_recovery_target_timeline(char **newval, void **extra, GucSource source)
{
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index c6014e83fa..4d6a306849 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -96,6 +96,9 @@
/* Define to 1 if you have the <atomic.h> header file. */
#undef HAVE_ATOMIC_H
+/* Define to 1 if you have the `backtrace_symbols' function. */
+#undef HAVE_BACKTRACE_SYMBOLS
+
/* Define to 1 if you have the `BIO_get_data' function. */
#undef HAVE_BIO_GET_DATA
@@ -198,6 +201,9 @@
/* Define to 1 if you have the `explicit_bzero' function. */
#undef HAVE_EXPLICIT_BZERO
+/* Define to 1 if you have the <execinfo.h> header file. */
+#undef HAVE_EXECINFO_H
+
/* Define to 1 if you have the `fdatasync' function. */
#undef HAVE_FDATASYNC
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index ba0b7f6f79..6a315414e2 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -189,6 +189,8 @@ extern int errcontext_msg(const char *fmt,...) pg_attribute_printf(1, 2);
extern int errhidestmt(bool hide_stmt);
extern int errhidecontext(bool hide_ctx);
+extern int errbacktrace(void);
+
extern int errfunction(const char *funcname);
extern int errposition(int cursorpos);
@@ -362,6 +364,7 @@ typedef struct ErrorData
char *detail_log; /* detail error message for server log only */
char *hint; /* hint message */
char *context; /* context message */
+ char *backtrace; /* backtrace */
const char *message_id; /* primary message's id (original string) */
char *schema_name; /* name of schema */
char *table_name; /* name of table */
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 6791e0cbc2..1e1876d320 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -254,6 +254,8 @@ extern PGDLLIMPORT int client_min_messages;
extern int log_min_duration_statement;
extern int log_temp_files;
extern double log_xact_sample_rate;
+extern char *backtrace_functions;
+extern char *backtrace_symbol_list;
extern int temp_file_limit;
--
2.17.1
On 2019-Sep-13, Alvaro Herrera wrote:
On 2019-Aug-20, Peter Eisentraut wrote:
The memory management of that seems too complicated. The "extra"
mechanism of the check/assign hooks only supports one level of malloc.
Using a List seems impossible. I don't know if you can safely do a
malloc-ed array of malloc-ed strings either.Here's an idea -- have the check/assign hooks create a different
representation, which is a single guc_malloc'ed chunk that is made up of
every function name listed in the GUC, separated by \0. That can be
scanned at error time comparing the function name with each piece.
Peter, would you like me to clean this up for commit, or do you prefer
to keep authorship and get it done yourself?
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-09-27 17:50, Alvaro Herrera wrote:
On 2019-Sep-13, Alvaro Herrera wrote:
On 2019-Aug-20, Peter Eisentraut wrote:
The memory management of that seems too complicated. The "extra"
mechanism of the check/assign hooks only supports one level of malloc.
Using a List seems impossible. I don't know if you can safely do a
malloc-ed array of malloc-ed strings either.Here's an idea -- have the check/assign hooks create a different
representation, which is a single guc_malloc'ed chunk that is made up of
every function name listed in the GUC, separated by \0. That can be
scanned at error time comparing the function name with each piece.Peter, would you like me to clean this up for commit, or do you prefer
to keep authorship and get it done yourself?
If you want to finish it using the idea from your previous message,
please feel free. I won't get to it this week.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-09-30 20:16, Peter Eisentraut wrote:
On 2019-09-27 17:50, Alvaro Herrera wrote:
On 2019-Sep-13, Alvaro Herrera wrote:
On 2019-Aug-20, Peter Eisentraut wrote:
The memory management of that seems too complicated. The "extra"
mechanism of the check/assign hooks only supports one level of malloc.
Using a List seems impossible. I don't know if you can safely do a
malloc-ed array of malloc-ed strings either.Here's an idea -- have the check/assign hooks create a different
representation, which is a single guc_malloc'ed chunk that is made up of
every function name listed in the GUC, separated by \0. That can be
scanned at error time comparing the function name with each piece.Peter, would you like me to clean this up for commit, or do you prefer
to keep authorship and get it done yourself?If you want to finish it using the idea from your previous message,
please feel free. I won't get to it this week.
I hadn't realized that you had already attached a patch that implements
your idea. It looks good to me. Maybe a small comment near
check_backtrace_functions() why we're not using a regular list. Other
than that, please go ahead with this.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2019-Oct-26, Peter Eisentraut wrote:
I hadn't realized that you had already attached a patch that implements
your idea. It looks good to me. Maybe a small comment near
check_backtrace_functions() why we're not using a regular list. Other
than that, please go ahead with this.
Thanks, I added that comment and others, and pushed. Let's see what
happens now ...
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2019-Oct-26, Peter Eisentraut wrote:
I hadn't realized that you had already attached a patch that implements
your idea. It looks good to me. Maybe a small comment near
check_backtrace_functions() why we're not using a regular list. Other
than that, please go ahead with this.
Thanks, I added that comment and others, and pushed. Let's see what
happens now ...
I had occasion to try to use errbacktrace() just now, and it blew up
on me. Investigation finds this:
int
errbacktrace(void)
{
ErrorData *edata = &errordata[errordata_stack_depth];
MemoryContext oldcontext;
Assert(false);
I suppose that's a debugging leftover that shouldn't have been committed?
It did what I wanted after I took out the Assert.
regards, tom lane
On 2019-Nov-23, Tom Lane wrote:
I had occasion to try to use errbacktrace() just now, and it blew up
on me. Investigation finds this:int
errbacktrace(void)
{
ErrorData *edata = &errordata[errordata_stack_depth];
MemoryContext oldcontext;Assert(false);
I suppose that's a debugging leftover that shouldn't have been committed?
It did what I wanted after I took out the Assert.
Uhh ... facepalm. Yes, that's not intended. I don't remember why would
I want to put that there.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services