On 08/12/2016 04:08 PM, Martin Liška wrote:
> On 08/10/2016 02:53 PM, Nathan Sidwell wrote:
>> On 08/10/16 06:43, Martin Liška wrote:
>>> Hello.
>>>
>>> There are multiple PRs (mentioned in ChangeLog) which suffer from missing
>>> capability of gcov
>>> to save counters for functions with constructor/destructor attributes. I
>>> done that by simply
>>> replacing atexit handler (gcov_exit) with a new static destructor
>>> (__gcov_exit), which has
>>> priority 99 (I did the same for __gcov_init). However, I'm not sure whether
>>> it's possible
>>> that a ctor defined in a source file can be potentially executed before
>>> __gcov_init (w/ the default
>>> priority)?
>>>
>>> Patch survives:
>>> make check -k -j10 RUNTESTFLAGS="gcov.exp"
>>> make check -k -j10 RUNTESTFLAGS="tree-prof.exp"
>>>
>>> I've just also verified that a DSO gcov dump works as before. I'm attaching
>>> a test-case which
>>> tests both static ctors/dtors, as well as C++ ctors/dtors.
>>
>> Does a coverage bootstrap (--enable-coverage) still succeed?
>
> Well, looks results are more unstable than I thought.
> Even running 'make -j1' in objdir/x86_64-pc-linux-gnu/libgcc repeatedly
> generates different results.
> I'll dig in after weekend.
>
> Martin
Hi.
So the I found reason of inconsistencies, running multiple times -fselftest is
enough to
find that memory allocation related functions can be executed different times.
Small example:
--- /tmp/r1/ggc-page.c.gcov 2016-09-26 16:54:53.060921496 +0200
+++ /tmp/r2/ggc-page.c.gcov 2016-09-26 16:55:52.470058958 +0200
@@ -636,8 +636,8 @@
-: 631:#else
307718: 632: page_table table = G.lookup;
307718: 633: uintptr_t high_bits = (uintptr_t) p & ~ (uintptr_t)
0xffffffff;
- 307718: 634: while (table->high_bits != high_bits)
- #####: 635: table = table->next;
+ 307794: 634: while (table->high_bits != high_bits)
+ 38: 635: table = table->next;
307718: 636: base = &table->table[0];
-: 637:#endif
-: 638:
@@ -661,15 +661,15 @@
-: 656:#else
-: 657: page_table table;
2134: 658: uintptr_t high_bits = (uintptr_t) p & ~ (uintptr_t)
0xffffffff;
- 2134: 659: for (table = G.lookup; table; table = table->next)
- 2133: 660: if (table->high_bits == high_bits)
- 2133: 661: goto found;
+ 2322: 659: for (table = G.lookup; table; table = table->next)
+ 2320: 660: if (table->high_bits == high_bits)
+ 2132: 661: goto found;
-: 662:
-: 663: /* Not found -- allocate a new table. */
- 1: 664: table = XCNEW (struct page_table_chain);
- 1: 665: table->next = G.lookup;
- 1: 666: table->high_bits = high_bits;
- 1: 667: G.lookup = table;
+ 2: 664: table = XCNEW (struct page_table_chain);
+ 2: 665: table->next = G.lookup;
+ 2: 666: table->high_bits = high_bits;
+ 2: 667: G.lookup = table;
2134: 668:found:
2134: 669: base = &table->table[0];
-: 670:#endif
--- /tmp/r1/ggc-page.c.gcov 2016-09-26 16:54:53.060921496 +0200
+++ /tmp/r2/ggc-page.c.gcov 2016-09-26 16:58:22.440931009 +0200
@@ -679,7 +679,7 @@
2134: 674: L2 = LOOKUP_L2 (p);
-: 675:
2134: 676: if (base[L1] == NULL)
- 3: 677: base[L1] = XCNEWVEC (page_entry *, PAGE_L2_SIZE);
+ 2: 677: base[L1] = XCNEWVEC (page_entry *, PAGE_L2_SIZE);
-: 678:
2134: 679: base[L1][L2] = entry;
2134: 680:}
It's reasonable to me that it can change. However, the patch I would like to
install
does not cause any new differences.
Martin
>
>>
>> I think this is a good idea, but we should document the changed behavior. (I
>> don't think the current behaviour's documented).
I'm adding a new hunk that documents the behavior.
Is the patch ready to be installed?
Thanks,
Martin
>>
>>
>> nathan
>
>From 686e65a923288c2c5055a9edb61e6f0648d6a2a3 Mon Sep 17 00:00:00 2001
From: marxin <mli...@suse.cz>
Date: Wed, 10 Aug 2016 12:18:45 +0200
Subject: [PATCH] gcov: dump in a static dtor instead of in an atexit handler
gcc/testsuite/ChangeLog:
2016-08-10 Martin Liska <mli...@suse.cz>
PR gcov-profile/7970
PR gcov-profile/16855
PR gcov-profile/44779
* g++.dg/gcov/pr16855.C: New test.
gcc/ChangeLog:
2016-08-10 Martin Liska <mli...@suse.cz>
PR gcov-profile/7970
PR gcov-profile/16855
PR gcov-profile/44779
* coverage.c (build_gcov_exit_decl): New function.
(coverage_obj_init): Call the function and generate __gcov_exit
destructor.
* doc/gcov.texi: Document when __gcov_exit function is called.
libgcc/ChangeLog:
2016-08-10 Martin Liska <mli...@suse.cz>
PR gcov-profile/7970
PR gcov-profile/16855
PR gcov-profile/44779
* libgcov-driver.c (__gcov_init): Do not register a atexit
handler.
(__gcov_exit): Rename from gcov_exit.
* libgcov.h (__gcov_exit): Declare.
---
gcc/coverage.c | 27 +++++++++++++++++++--
gcc/doc/gcov.texi | 4 ++++
gcc/testsuite/g++.dg/gcov/pr16855.C | 47 +++++++++++++++++++++++++++++++++++++
libgcc/libgcov-driver.c | 5 ++--
libgcc/libgcov.h | 3 +++
5 files changed, 81 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/gcov/pr16855.C
diff --git a/gcc/coverage.c b/gcc/coverage.c
index 30cdc69..0b8c0b3 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -1055,8 +1055,30 @@ build_init_ctor (tree gcov_info_type)
stmt = build_call_expr (init_fn, 1, stmt);
append_to_statement_list (stmt, &ctor);
- /* Generate a constructor to run it. */
- cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY);
+ /* Generate a constructor to run it (with priority 99). */
+ cgraph_build_static_cdtor ('I', ctor, DEFAULT_INIT_PRIORITY - 1);
+}
+
+/* Generate the destructor function to call __gcov_exit. */
+
+static void
+build_gcov_exit_decl (void)
+{
+ tree init_fn = build_function_type_list (void_type_node, void_type_node,
+ NULL);
+ init_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL,
+ get_identifier ("__gcov_exit"), init_fn);
+ TREE_PUBLIC (init_fn) = 1;
+ DECL_EXTERNAL (init_fn) = 1;
+ DECL_ASSEMBLER_NAME (init_fn);
+
+ /* Generate a call to __gcov_exit (). */
+ tree dtor = NULL;
+ tree stmt = build_call_expr (init_fn, 0);
+ append_to_statement_list (stmt, &dtor);
+
+ /* Generate a destructor to run it (with priority 99). */
+ cgraph_build_static_cdtor ('D', dtor, DEFAULT_INIT_PRIORITY - 1);
}
/* Create the gcov_info types and object. Generate the constructor
@@ -1114,6 +1136,7 @@ coverage_obj_init (void)
DECL_NAME (gcov_info_var) = get_identifier (name_buf);
build_init_ctor (gcov_info_type);
+ build_gcov_exit_decl ();
return true;
}
diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index a0a7af7..626d441 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -598,6 +598,10 @@ facilities to restrict profile collection to the program region of
interest. Calling @code{__gcov_reset(void)} will clear all profile counters
to zero, and calling @code{__gcov_dump(void)} will cause the profile information
collected at that point to be dumped to @file{.gcda} output files.
+By default, every instrumented application calls __gcov_dump function
+via a static destructor with priority equal to 99. That would guarantee
+that all user defined destructors, as well as function handlers registered
+by atexit, would be executed before gcov dump function is executed.
@c man end
diff --git a/gcc/testsuite/g++.dg/gcov/pr16855.C b/gcc/testsuite/g++.dg/gcov/pr16855.C
new file mode 100644
index 0000000..91801d4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/pr16855.C
@@ -0,0 +1,47 @@
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+#include <stdlib.h>
+
+int a;
+
+void foo()
+{
+ a = 123; /* count(1) */
+}
+
+#include <iostream>
+using namespace std;
+class Test {
+public:
+ Test(void){
+ cout<< "In Test ctor" << endl; /* count(1) */
+ }
+ ~Test(void){
+ cout<< "In Test dtor" << endl; /* count(1) */
+ }
+}T1;
+
+void uncalled(void){
+ cout<< "In uncalled" << endl; /* count(#####) */
+}
+int main(void){
+atexit (&foo);
+// Test T2;
+cout<< "In main" << endl; /* count(1) */
+return 0;
+}
+
+#include <stdio.h>
+
+__attribute__((constructor))
+static void construct_navigationBarImages() {
+ fprintf (stderr, "((construct_navigationBarImages))"); /* count(1) */
+}
+
+__attribute__((destructor))
+static void destroy_navigationBarImages() {
+ fprintf (stderr, "((destroy_navigationBarImages))"); /* count(1) */
+}
+
+/* { dg-final { run-gcov branches { -b pr16855.C } } } */
diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index d51397e..84471bd 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -872,8 +872,8 @@ struct gcov_root __gcov_root;
struct gcov_master __gcov_master =
{GCOV_VERSION, 0};
-static void
-gcov_exit (void)
+void
+__gcov_exit (void)
{
__gcov_dump_one (&__gcov_root);
if (__gcov_root.next)
@@ -906,7 +906,6 @@ __gcov_init (struct gcov_info *info)
__gcov_master.root->prev = &__gcov_root;
__gcov_master.root = &__gcov_root;
}
- atexit (gcov_exit);
}
info->next = __gcov_root.list;
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index 25147de..c5f0732 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -235,6 +235,9 @@ extern void __gcov_dump_one (struct gcov_root *) ATTRIBUTE_HIDDEN;
/* Register a new object file module. */
extern void __gcov_init (struct gcov_info *) ATTRIBUTE_HIDDEN;
+/* GCOV exit function registered via a static destructor. */
+extern void __gcov_exit (void) ATTRIBUTE_HIDDEN;
+
/* Called before fork, to avoid double counting. */
extern void __gcov_flush (void) ATTRIBUTE_HIDDEN;
--
2.9.2
