(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
>From 9803714aa0e493c603e04e282a241cfa89507da3 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@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/configure
index 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.in
index 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.c
index 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.in
index 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
>From 853d4c901ce4b53285b43ddf44cb567f774a2dd8 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@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.m4
index 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/configure
index 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.in
index 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
