Re: errbacktrace
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_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 Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: errbacktrace
Alvaro Herrera 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_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
Re: errbacktrace
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 Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: errbacktrace
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
Re: errbacktrace
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
Re: errbacktrace
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 Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: errbacktrace
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 Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 7341310cd75722fbfe7cb7996516b0b50d83bb1f Mon Sep 17 00:00:00 2001
From: Alvaro Herrera
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-3bf2cd015...@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
Re: errbacktrace
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
Re: errbacktrace
Peter Eisentraut 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
Re: errbacktrace
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 Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: errbacktrace
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
From f9d440ec41f49464661960c580c29ce0558a1642 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
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-3bf2cd015...@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"
Re: errbacktrace
po 12. 8. 2019 v 19:06 odesílatel Peter Eisentraut < peter.eisentr...@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 > > -- > Peter Eisentraut http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >
Re: errbacktrace
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
Re: errbacktrace
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 > we should test for backtrace_function[0] before edata->funcname, since > it seems more likely to be unset. > > -- > Álvaro Herrerahttps://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services > > > > >
Re: errbacktrace
On Tue, Jul 23, 2019 at 6:19 AM 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. 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
Re: errbacktrace
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
Re: errbacktrace
Peter Eisentraut 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
Re: errbacktrace
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. https://stackoverflow.com/questions/26398064/counterpart-to-glibcs-backtrace-and-backtrace-symbols-on-windows -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: errbacktrace
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
From 779231dfe56cf8f138a94ba0106ca72171b63350 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
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-3bf2cd015...@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
Re: errbacktrace
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
Re: errbacktrace
On 2019-Jul-08, Dmitry Dolgov wrote: > > On Sat, Jun 29, 2019 at 7:41 AM Jaime Casanova > > 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 > #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 . 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 Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: errbacktrace
> On Sat, Jun 29, 2019 at 7:41 AM Jaime Casanova > 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 #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 [0x55fa389bcbbe] #1 int4in+0xd [0x55fa38976d7b] #2 InputFunctionCall+0x6f [0x55fa38a488e9] #3 OidInputFunctionCall+0x44 [0x55fa38a48b0d] #4 stringTypeDatum+0x33 [0x55fa386e222e] #5 coerce_type+0x26d [0x55fa386ca14d] #6 coerce_to_target_type+0x79 [0x55fa386c9494] #7 transformTypeCast+0xaa [0x55fa386d0042] #8 transformExprRecurse+0x22f [0x55fa386cf650] #9 transformExpr+0x1a [0x55fa386cf30a] #10 transformTargetEntry+0x79 [0x55fa386e1131] #11 transformTargetList+0x86 [0x55fa386e11ce] #12 transformSelectStmt+0xa1 [0x55fa386a29c9] #13 transformStmt+0x9d [0x55fa386a345a] #14 transformOptionalSelectInto+0x94 [0x55fa386a3f49] #15 transformTopLevelStmt+0x15 [0x55fa386a3f88] #16 parse_analyze+0x4e [0x55fa386a3fef] #17 pg_analyze_and_rewrite+0x3e [0x55fa3890cfa5] #18 exec_simple_query+0x35b [0x55fa3890d5b5] #19 PostgresMain+0x91f [0x55fa3890f7a8] #20 BackendRun+0x1ac [0x55fa3887ed17] #21 BackendStartup+0x15c [0x55fa38881ea1] #22 ServerLoop+0x1e6 [0x55fa388821bb] #23 PostmasterMain+0x1101 [0x55fa388835a1] #24 main+0x21a [0x55fa387db1a9] #25 __libc_start_main+0xe7 [0x7f3d1a607fa7] #26 _start+0x2a [0x55fa3858e4ea]
Re: errbacktrace
On Tue, Jun 25, 2019 at 11:08 PM Peter Eisentraut 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
Re: errbacktrace
On Tue, 25 Jun 2019 at 06:08, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > New thread continuing from > < > https://www.postgresql.org/message-id/d4903af2-e7b7-b551-71f8-3e4a6bdc2...@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
Re: errbacktrace
On Tue, Jun 25, 2019 at 4:08 AM Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > New thread continuing from > < > https://www.postgresql.org/message-id/d4903af2-e7b7-b551-71f8-3e4a6bdc2...@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.
Re: errbacktrace
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 Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
