[google] libgcov workaround for weak reference issue

[tejohnson, 2012-06-01 20:27:48]

This patch works around a subtlety in the way weak references to
symbols defined in archives are handled by the linker. This is an
issue because google binaries are using weak references to detect
whether the libgcov.a version contains the new __gcov_reset and
__gcov_dump interfaces. Since they are defined in their own object
file within the archive, and no strong references are made to any
other symbols within the same object file, the linker does not
resolve the weak references. The workaround is to add dummy
strong references to the new routines from the main object file
(defined under L_gcov) that will always be referenced during
-fprofile-generate builds.

Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for google
branches?

Thanks,
Teresa

2012-06-01   Teresa Johnson  <tejohnson@google.com>

	* libgcov.c: Add references to gcov_reset and gcov_dump
        from L_gcov section.

Index: libgcov.c
===================================================================
--- libgcov.c	(revision 188119)
+++ libgcov.c	(working copy)
@@ -119,6 +119,21 @@ extern int gcov_dump_complete ATTRIBUTE_HIDDEN;
 #ifdef L_gcov
 #include "gcov-io.c"
 
+/* Create a strong reference to these symbols so that they are
+   unconditionally pulled into the instrumented binary, even when
+   the only reference is a weak reference. This is necessary because
+   we are using weak references to handle older compilers that
+   pre-date these new functions. A subtlety of the linker is that
+   it will only resolve weak references defined within archive libraries
+   when there is a string reference to something else defined within
+   the same object file. Since these two functions are defined within
+   their own object files (using L_gcov_reset and L_gcov_dump), they
+   would not get resolved. Since there are symbols within the main L_gcov
+   section that are strongly referenced during -fprofile-generate builds,
+   these symbols will always need to be resolved.  */
+void (*unused1)() = &__gcov_reset;
+void (*unused2)() = &__gcov_dump;
+
 /* Utility function for outputing errors.  */
 static int
 gcov_error (const char *fmt, ...)

--
This patch is available for review at http://codereview.appspot.com/6276043

[davidxl, 2012-06-01 21:18:07]

Ok with better naming for the dummy function such as '__gcov_dummy_ref1'.
Another choice is to let __gcov_flush calls __gcov_dump + __gcov_reset -- but
the dump_completed state needs to be saved and restored.

David

[tejohnson, 2012-06-01 21:26:19]

Renamed to __gcov_dummy_ref1 and __gcov_dummy_ref2. I'd prefer that
approach for now to keep the differences with trunk to a minimum.

Thanks,
Teresa

On Fri, Jun 1, 2012 at 2:18 PM,  <davidxl@google.com> wrote:
> Ok with better naming for the dummy function such as
> '__gcov_dummy_ref1'. Another choice is to let __gcov_flush calls
> __gcov_dump + __gcov_reset -- but the dump_completed state needs to be
> saved and restored.
>
> David
>
> http://codereview.appspot.com/6276043/



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

[davidxl, 2012-06-01 21:27:32]

ok.

thanks,

David

On Fri, Jun 1, 2012 at 2:26 PM, Teresa Johnson <tejohnson@google.com> wrote:
> Renamed to __gcov_dummy_ref1 and __gcov_dummy_ref2. I'd prefer that
> approach for now to keep the differences with trunk to a minimum.
>
> Thanks,
> Teresa
>
> On Fri, Jun 1, 2012 at 2:18 PM,  <davidxl@google.com> wrote:
>> Ok with better naming for the dummy function such as
>> '__gcov_dummy_ref1'. Another choice is to let __gcov_flush calls
>> __gcov_dump + __gcov_reset -- but the dump_completed state needs to be
>> saved and restored.
>>
>> David
>>
>> http://codereview.appspot.com/6276043/
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413