diff mbox

[RFC] gcov: dump in a static dtor instead of in an atexit handler

Message ID dd23acd2-392b-e39f-1f42-b6d383308abf@suse.cz
State New
Headers show

Commit Message

Martin Liška Aug. 10, 2016, 10:43 a.m. UTC
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.

Thoughts?
Martin

Comments

Nathan Sidwell Aug. 10, 2016, 12:53 p.m. UTC | #1
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?

I think this is a good idea, but we should document the changed behavior. (I 
don't think the current behaviour's documented).


nathan
Nathan Sidwell Aug. 10, 2016, 12:55 p.m. UTC | #2
On 08/10/16 08:53, Nathan Sidwell wrote:

> I think this is a good idea, but we should document the changed behavior. (I
> don't think the current behaviour's documented).

oh, gcov_exit is a user callable routine.  You'll have to keep it available.
Martin Liška Aug. 12, 2016, 2:08 p.m. UTC | #3
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

> 
> I think this is a good idea, but we should document the changed behavior. (I don't think the current behaviour's documented).
> 
> 
> nathan
diff mbox

Patch

From 0e7f660b77628533679e6302a3f4b444166fc365 Mon Sep 17 00:00:00 2001
From: marxin <mliska@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  <mliska@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  <mliska@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.

libgcc/ChangeLog:

2016-08-10  Martin Liska  <mliska@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/testsuite/g++.dg/gcov/pr16855.C | 37 +++++++++++++++++++++++++++++++++++++
 libgcc/libgcov-driver.c             |  5 ++---
 libgcc/libgcov.h                    |  3 +++
 4 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/gcov/pr16855.C

diff --git a/gcc/coverage.c b/gcc/coverage.c
index d4d371e..da7f915 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -1054,8 +1054,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
@@ -1113,6 +1135,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/testsuite/g++.dg/gcov/pr16855.C b/gcc/testsuite/g++.dg/gcov/pr16855.C
new file mode 100644
index 0000000..ec72e99
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gcov/pr16855.C
@@ -0,0 +1,37 @@ 
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+#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){
+// 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 80f13e2..113f877 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