Re: gcov coverage data not full with immediate stop

From: Alexander Lakhin <exclusion(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: gcov coverage data not full with immediate stop
Date: 2020-05-11 09:00:00
Message-ID: 445c1dd9-bb8a-3153-04b2-40ba97be87b1@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello Alvaro,
11.05.2020 06:42, Alvaro Herrera wrote:
> (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.
Thanks for the reference to that discussion and your patch.
As I see the issue with that patch is that quickdie() is not the only
SIGQUIT handler. When a backend is interrupted with SIGQUIT, it's
exiting in SignalHandlerForCrashExit().
In fact if I only add __gcov_flush() in SignalHandlerForCrashExit(), it
raises test coverage for `make check -C src/test/recovery/` from
106198 lines/6319 functions
to
106420 lines/6328 functions

It's not yet clear to me what happens when __gcov_flush() called inside
__gcov_flush().
The test coverage changes to:
108432 lines/5417 functions
(number of function calls decreased)
And for example in coverage/src/backend/utils/cache/catcache.c.gcov.html
I see
     147           8 : int2eqfast(Datum a, Datum b)
...
     153           0 : int2hashfast(Datum datum)
but without __gcov_flush in quickdie() we have:
     147       78038 : int2eqfast(Datum a, Datum b)
...
     153      255470 : int2hashfast(Datum datum)
So it needs more investigation.

But I can confirm that calling __gcov_flush() in
SignalHandlerForCrashExit() really improves a code coverage report.
I tried to develop a test to elevate a coverage for gist:
https://coverage.postgresql.org/src/backend/access/gist/gistxlog.c.gcov.html
(Please look at the attached test if it could be interesting.)
and came to this issue with a coverage. I tried to play with
GCOV_PREFIX, but without luck.
Yesterday I found the more recent discussion:
https://www.postgresql.org/message-id/flat/44ecae53-9861-71b7-1d43-4658acc52519%402ndquadrant.com#d02e2e61212831fbceadf290637913a0
(where probably the same problem came out).

Finally I've managed to get an expected coverage when I performed
$node_standby->stop() (but __gcov_flush() in SignalHandlerForCrashExit()
helps too).

Best regards,
Alexander

Attachment Content-Type Size
021_indexes-test.patch text/x-patch 3.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2020-05-11 09:45:44 Re: 2020-05-14 Press Release Draft
Previous Message Antonin Houska 2020-05-11 08:44:14 Re: 2pc leaks fds