diff mbox

[PR,51113] Fix gcov & PIC

Message ID 4EC8C260.3080802@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Nov. 20, 2011, 9:03 a.m. UTC
I've committed this patch to fix part of PR 51113. there are (at least) 3 issues 
going on.

*) I'd forgotten to propagate symbol visibility from the fn-decl to the 
externally visible gcov objects.  I don't know if the PR actually exposed that 
problem, but this patch fixes it.

*) I'd presumed too much about linker capabilities & ELF.  For comdat functions, 
the associated counter arrays are also externally visible.  However, in 
well-formed program that counter array will not be overridden *without* the 
function itself also being overridden. Ergo, it is safe to use a PC-relative 
offset to access the array from the function, in PIC code.  Unfortunately, this 
is not expressible in ELF.  Because the array is externally visible and itself 
weak, it is *separately* overridable.  The fix is straight-forward.  Rather than 
take a short-cut, use make_decl_one_only, as it sets flags correctly so that PIC 
access works.  This fixes the testcase Markus attached to the PR.

*) There is another problem exposed by gold, but apparently not by ld.  Markus 
has managed to create a reproducible testcase that I can work from.  I have a 
handle on this, but not yet a fix.

I thought it best to commit a fix for the first 2 issues, as I think they're 
orthogonal to the 3rd (from what I can see at the moment).

built & tested on i686-pc-linux-gnu.  Many thanks to Markus in working with me 
to create testcases and try out initial patches.

nathan
2011-11-20  Nathan Sidwell  <nathan@acm.org>

	PR gcov-profile/51113
	* coverage.c (build_var): Propagate visibility for public decls.

	testsuite/
	* gcc.misc-tests/gcov-13.c: Check gcovpart-13b coverage
	* gcc.misc-tests/gcov-16.c: New.
	* gcc.misc-tests/gcov-17.c: New.
	* g++.dg/gcov/gcov-8.C: New.
	* g++.dg/gcov/gcov-9.C: New.
	* g++.dg/gcov/gcov-10.C: New.
diff mbox

Patch

Index: coverage.c
===================================================================
--- coverage.c	(revision 181523)
+++ coverage.c	(working copy)
@@ -657,9 +657,8 @@  coverage_end_function (unsigned lineno_c
 }
 
 /* Build a coverage variable of TYPE for function FN_DECL.  If COUNTER
-   >= 0 it is a counter array, and thus local.  Otherwise it is the
-   function structure and needs to be globalized.  All cases must be
-   in the same comdat group as FN_DECL.  */
+   >= 0 it is a counter array, otherwise it is the function structure.
+   Propagate appropriate linkage and visibility from the function decl.  */
 
 static tree
 build_var (tree fn_decl, tree type, int counter)
@@ -668,29 +667,29 @@  build_var (tree fn_decl, tree type, int
   tree fn_name = DECL_ASSEMBLER_NAME (fn_decl);
   char *buf = (char *)alloca (IDENTIFIER_LENGTH (fn_name) + 10);
 
-  if (counter >= 0)
-    TREE_STATIC (var) = 1;
-  else
-    {
-      TREE_PUBLIC (var) = TREE_PUBLIC (fn_decl);
-      TREE_STATIC (var) = TREE_STATIC (fn_decl);
-    }
-  TREE_ADDRESSABLE (var) = 1;
-  DECL_ALIGN (var) = TYPE_ALIGN (type);
-
   if (counter < 0)
     sprintf (buf, "__gcov__%s", IDENTIFIER_POINTER (fn_name));
   else
     sprintf (buf, "__gcov%u_%s", counter, IDENTIFIER_POINTER (fn_name));
   DECL_NAME (var) = get_identifier (buf);
-
-  /* Initialize assembler name so we can stream out. */
+  TREE_STATIC (var) = 1;
+  TREE_ADDRESSABLE (var) = 1;
+  DECL_ALIGN (var) = TYPE_ALIGN (type);
+  DECL_WEAK (var) = DECL_WEAK (fn_decl);
+  TREE_PUBLIC (var)
+    = TREE_PUBLIC (fn_decl) && (counter < 0 || DECL_WEAK (fn_decl));
+  if (DECL_ONE_ONLY (fn_decl))
+    make_decl_one_only (var, DECL_COMDAT_GROUP (fn_decl));
+  
   if (TREE_PUBLIC (var))
-    DECL_ASSEMBLER_NAME (var);    
+    {
+      DECL_VISIBILITY (var) = DECL_VISIBILITY (fn_decl);
+      DECL_VISIBILITY_SPECIFIED (var)
+	= DECL_VISIBILITY_SPECIFIED (fn_decl);
 
-  DECL_WEAK (var) = TREE_PUBLIC (var) && DECL_WEAK (fn_decl);
-  DECL_COMDAT (var) = DECL_COMDAT (fn_decl);
-  DECL_COMDAT_GROUP (var) = DECL_COMDAT_GROUP (fn_decl);
+      /* Initialize assembler name so we can stream out. */
+      DECL_ASSEMBLER_NAME (var);
+    }
 
   return var;
 }
Index: testsuite/gcc.misc-tests/gcov-13.c
===================================================================
--- testsuite/gcc.misc-tests/gcov-13.c	(revision 181507)
+++ testsuite/gcc.misc-tests/gcov-13.c	(working copy)
@@ -16,3 +16,4 @@  int main ()
 }
 
 /* { dg-final { run-gcov { -a gcov-13.c } } } */
+/* { dg-final { run-gcov { -a gcovpart-13b.c } } } */
Index: testsuite/gcc.misc-tests/gcov-16.c
===================================================================
--- testsuite/gcc.misc-tests/gcov-16.c	(revision 0)
+++ testsuite/gcc.misc-tests/gcov-16.c	(revision 0)
@@ -0,0 +1,11 @@ 
+/* Test visibility is copied */
+
+/* { dg-options "-fprofile-arcs -fvisibility=hidden" } */
+/* { dg-require-visibility "" } */
+/* { dg-require-weak "" } */
+
+void Foo ()
+{
+}
+
+ /* { dg-final { scan-assembler "\\.hidden\t__gcov__Foo" } } */
Index: testsuite/gcc.misc-tests/gcov-17.c
===================================================================
--- testsuite/gcc.misc-tests/gcov-17.c	(revision 0)
+++ testsuite/gcc.misc-tests/gcov-17.c	(revision 0)
@@ -0,0 +1,11 @@ 
+/* Test visibility is copied */
+
+/* { dg-options "-fprofile-arcs" } */
+/* { dg-require-visibility "" } */
+/* { dg-require-weak "" } */
+
+void __attribute__ ((visibility ("hidden"), weak)) Foo ()
+{
+}
+
+/* { dg-final { scan-assembler "\\.hidden\t__gcov__Foo" } } */
Index: testsuite/g++.dg/gcov/gcov-8.C
===================================================================
--- testsuite/g++.dg/gcov/gcov-8.C	(revision 0)
+++ testsuite/g++.dg/gcov/gcov-8.C	(revision 0)
@@ -0,0 +1,13 @@ 
+/* { dg-options "-fprofile-arcs -fvisibility=hidden" } */
+/* { dg-require-visibility "" } */
+
+struct __attribute__((visibility ("hidden"))) X
+{
+  void Fink ();
+};
+
+void X::Fink ()
+{
+}
+
+/* { dg-final { scan-assembler "\\.hidden\t__gcov___ZN1X4FinkEv" } } */
Index: testsuite/g++.dg/gcov/gcov-9.C
===================================================================
--- testsuite/g++.dg/gcov/gcov-9.C	(revision 0)
+++ testsuite/g++.dg/gcov/gcov-9.C	(revision 0)
@@ -0,0 +1,15 @@ 
+/* { dg-options "-fprofile-arcs -fvisibility-inlines-hidden" } */
+/* { dg-require-visibility "" } */
+
+inline void Boo ()
+{
+}
+
+extern "C" void (*Foo ()) ()
+{
+  return Boo;
+}
+
+/* { dg-final { scan-assembler "\\.hidden\t__gcov___Z3Boov" } } */
+/* { dg-final { scan-assembler "__gcov__Foo:" } } */
+/* { dg-final { scan-assembler-not "\\.hidden\t__gcov__Foo" } } */
Index: testsuite/g++.dg/gcov/gcov-10.C
===================================================================
--- testsuite/g++.dg/gcov/gcov-10.C	(revision 0)
+++ testsuite/g++.dg/gcov/gcov-10.C	(revision 0)
@@ -0,0 +1,20 @@ 
+/* Ensure PIC sequence used for comdat functions */
+
+/* { dg-options "-fprofile-arcs -ftest-coverage -fpic" } */
+/* { dg-do run { target native } } */
+/* { dg-require-effective-target fpic } */
+
+inline int __attribute__ ((noinline)) Foo ()
+{
+  static int x[1];
+
+  return x[0]++;  /* count (1) */
+}
+
+int main ()
+{
+  Foo ();  /* count (1) */
+  return 0;  /* count (1) */
+}
+
+/* { dg-final { run-gcov gcov-10.C } } */
Index: testsuite/g++.dg/gcov/gcov-11.C
===================================================================
--- testsuite/g++.dg/gcov/gcov-11.C	(revision 0)
+++ testsuite/g++.dg/gcov/gcov-11.C	(revision 0)
@@ -0,0 +1,42 @@ 
+/* Check that unexecuted exception processing regions are shown
+   distinct from  unexecuted normal regions.  */
+
+/* { dg-options "-fprofile-arcs -ftest-coverage" } */
+/* { dg-do run { target native } } */
+
+void Baz (int i)
+{
+  if (i)
+    throw 1;
+}
+
+void Boz () throw ()
+{
+}
+
+int main ()
+{
+  try
+    {
+      Baz (0);  /* count (1) */
+      Baz (0);  /* count (1) */
+    }
+  catch (...)
+    {
+      Boz ();  /* count (=====) */
+    }
+
+  try
+    {
+      Baz (1);  /* count (1) */
+      Baz (0);  /* count (#####) */
+    }
+  catch (...)
+    {
+      Boz ();  /* count (1) */
+    }
+
+  return 0;  /* count (1) */
+}
+
+/* { dg-final { run-gcov gcov-11.C } } */