From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: gcov coverage data not full with immediate stop
Date: Sun, 10 May 2020 23:42:43 -0400
Message-ID: <20200511034243.GA5671@alvherre.pgsql> (raw)
In-Reply-To: <816a4de5-3f15-4d66-1ec1-791cab2ca6df@gmail.com>
(Strangely, I was just thinking about these branches of mine as I
closed my week last Friday...)
On 2020-May-10, Alexander Lakhin wrote:
> So if we want to make the coverage reports more precise, I see the three
> ways:
> 1. Change the stop mode in teardown_node to fast (probably only when
> configured with --enable-coverage);
> 2. Explicitly stop nodes in TAP tests (where it's important) -- seems
> too tedious and troublesome;
> 3. Explicitly call __gcov_flush in SIGQUIT handler (quickdie)?
I tried your idea 3 a long time ago and my experiments didn't show an
increase in coverage [1]. But I like this idea the best, and maybe I
did something wrong. Attached is the patch I had (on top of
fc115d0f9fc6), but I don't know if it still applies.
(The second attachment is another branch I had on this, I don't remember
why; that one was on top of 438e51987dcc. The curious thing is that I
didn't add the __gcov_flush to quickdie in this one. Maybe what we need
is a mix of both.)
I think we should definitely get this fixed for pg13 ...
[1] https://postgr.es/m/20190531170503.GA24057@alvherre.pgsql
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:download all (2 files, .tar.gz)0001-add-gcov_flush-call-in-quickdie.patch (text/x-diff, 2.1K) download
From 9803714aa0e493c603e04e282a241cfa89507da3 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 31 May 2019 13:09:46 -0400
Subject: [PATCH] add gcov_flush call in quickdie
---
configure | 4 ++++
configure.in | 4 +++-
src/backend/tcop/postgres.c | 8 ++++++++
src/include/pg_config.h.in | 3 +++
4 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/configure b/configureindex fd61bf6472..c0cf19d662 100755--- a/configure+++ b/configure@@ -3516,6 +3516,10 @@ fi
if test -z "$GENHTML"; then
as_fn_error $? "genhtml not found" "$LINENO" 5
fi
++$as_echo "#define USE_GCOV_COVERAGE 1" >>confdefs.h++
;;
no)
:
diff --git a/configure.in b/configure.inindex 4586a1716c..21465bbaa6 100644--- a/configure.in+++ b/configure.in@@ -222,7 +222,9 @@ fi
PGAC_PATH_PROGS(GENHTML, genhtml)
if test -z "$GENHTML"; then
AC_MSG_ERROR([genhtml not found])
-fi])+fi+AC_DEFINE([USE_GCOV_COVERAGE], 1, [Define to use gcov coverage support stuff])+])
AC_SUBST(enable_coverage)
#
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.cindex 44a59e1d4f..a483eba454 100644--- a/src/backend/tcop/postgres.c+++ b/src/backend/tcop/postgres.c@@ -2729,6 +2729,14 @@ quickdie(SIGNAL_ARGS)
errhint("In a moment you should be able to reconnect to the"
" database and repeat your command.")));
+#ifdef USE_GCOV_COVERAGE+ /*+ * We still want to flush coverage data down to disk, which gcov's atexit+ * callback would do, but we're preventing that below.+ */+ __gcov_flush();+#endif+
/*
* We DO NOT want to run proc_exit() or atexit() callbacks -- we're here
* because shared memory may be corrupted, so we don't want to try to
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.inindex 6cd4cfed0a..5fb1a545ed 100644--- a/src/include/pg_config.h.in+++ b/src/include/pg_config.h.in@@ -916,6 +916,9 @@
(--enable-float8-byval) */
#undef USE_FLOAT8_BYVAL
+/* Define to use gcov coverage support stuff */+#undef USE_GCOV_COVERAGE+
/* Define to build with ICU support. (--with-icu) */
#undef USE_ICU
--
2.20.1
0001-gcov_flush-stuff.patch (text/x-diff, 4.8K) download
From 853d4c901ce4b53285b43ddf44cb567f774a2dd8 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 31 May 2019 11:20:23 -0400
Subject: [PATCH] gcov_flush stuff
---
config/c-compiler.m4 | 15 +++++++
configure | 102 +++++++++++++++++++++++++++++++++++++++++++
configure.in | 9 ++++
3 files changed, 126 insertions(+)
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4index 71b645839d..0a73fd4624 100644--- a/config/c-compiler.m4+++ b/config/c-compiler.m4@@ -394,6 +394,21 @@ AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE$1]), 1,
[Define to 1 if your compiler understands $1.])
fi])# PGAC_CHECK_BUILTIN_FUNC
+AC_DEFUN([PGAC_CHECK_BUILTIN_FUNC0],+[AC_CACHE_CHECK(for $1, pgac_cv$1,+[AC_LINK_IFELSE([AC_LANG_PROGRAM([+int+call$1()+{+ return $1();+}], [])],+[pgac_cv$1=yes],+[pgac_cv$1=no])])+if test x"${pgac_cv$1}" = xyes ; then+AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE$1]), 1,+ [Define to 1 if your compiler understands $1.])+fi])# PGAC_CHECK_BUILTIN_FUNC0+
# PGAC_PROG_VARCC_VARFLAGS_OPT
diff --git a/configure b/configureindex fd61bf6472..28ebab733f 100755--- a/configure+++ b/configure@@ -11915,6 +11915,69 @@ fi
fi
fi
+if test "$enable_coverage" = yes ; then+ if test "$PORTNAME" != "win32"; then+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing __gcov_flush" >&5+$as_echo_n "checking for library containing __gcov_flush... " >&6; }+if ${ac_cv_search___gcov_flush+:} 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 __gcov_flush ();+int+main ()+{+return __gcov_flush ();+ ;+ return 0;+}+_ACEOF+for ac_lib in '' gcov; 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___gcov_flush=$ac_res+fi+rm -f core conftest.err conftest.$ac_objext \+ conftest$ac_exeext+ if ${ac_cv_search___gcov_flush+:} false; then :+ break+fi+done+if ${ac_cv_search___gcov_flush+:} false; then :++else+ ac_cv_search___gcov_flush=no+fi+rm conftest.$ac_ext+LIBS=$ac_func_search_save_LIBS+fi+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search___gcov_flush" >&5+$as_echo "$ac_cv_search___gcov_flush" >&6; }+ac_res=$ac_cv_search___gcov_flush+if test "$ac_res" != no; then :+ test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"++else+ as_fn_error $? "could not find __gcov_flush" "$LINENO" 5+fi++ fi+fi+
if test "$with_openssl" = yes ; then
if test "$PORTNAME" != "win32"; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for CRYPTO_new_ex_data in -lcrypto" >&5
@@ -15420,6 +15483,45 @@ _ACEOF
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __gcov_flush" >&5+$as_echo_n "checking for __gcov_flush... " >&6; }+if ${pgac_cv__gcov_flush+:} false; then :+ $as_echo_n "(cached) " >&6+else+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext+/* end confdefs.h. */++int+call__gcov_flush()+{+ return __gcov_flush(x);+}+int+main ()+{++ ;+ return 0;+}+_ACEOF+if ac_fn_c_try_link "$LINENO"; then :+ pgac_cv__gcov_flush=yes+else+ pgac_cv__gcov_flush=no+fi+rm -f core conftest.err conftest.$ac_objext \+ conftest$ac_exeext conftest.$ac_ext+fi+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__gcov_flush" >&5+$as_echo "$pgac_cv__gcov_flush" >&6; }+if test x"${pgac_cv__gcov_flush}" = xyes ; then++cat >>confdefs.h <<_ACEOF+#define HAVE__GCOV_FLUSH 1+_ACEOF++fi+
ac_fn_c_check_func "$LINENO" "fseeko" "ac_cv_func_fseeko"
if test "x$ac_cv_func_fseeko" = xyes; then :
$as_echo "#define HAVE_FSEEKO 1" >>confdefs.h
diff --git a/configure.in b/configure.inindex 4586a1716c..bc97398bda 100644--- a/configure.in+++ b/configure.in@@ -1197,6 +1197,13 @@ if test "$with_gssapi" = yes ; then
fi
fi
+if test "$enable_coverage" = yes ; then+ if test "$PORTNAME" != "win32"; then+ AC_SEARCH_LIBS(__gcov_flush, gcov, [],+ [AC_MSG_ERROR([could not find __gcov_flush])])+ fi+fi+
if test "$with_openssl" = yes ; then
dnl Order matters!
if test "$PORTNAME" != "win32"; then
@@ -1650,6 +1657,8 @@ PGAC_CHECK_BUILTIN_FUNC([__builtin_clz], [unsigned int x])
PGAC_CHECK_BUILTIN_FUNC([__builtin_ctz], [unsigned int x])
PGAC_CHECK_BUILTIN_FUNC([__builtin_popcount], [unsigned int x])
+PGAC_CHECK_BUILTIN_FUNC0([__gcov_flush])+
AC_REPLACE_FUNCS(fseeko)
case $host_os in
# NetBSD uses a custom fseeko/ftello built on fsetpos/fgetpos
--
2.20.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@2ndquadrant.com, exclusion@gmail.com
Subject: Re: gcov coverage data not full with immediate stop
In-Reply-To: <20200511034243.GA5671@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox