Re: fix gfcov regression

Re: fix gfcov regression

[Dominique Dhumieres]

> I've committed the patch now.

It (r214840) breaks bootstrap on darwin:

...
/opt/gcc/build_w/./gcc/xgcc -B/opt/gcc/build_w/./gcc/ -B/opt/gcc/gcc4.10w/x86_64-apple-darwin13.3.0/bin/ -B/opt/gcc/gcc4.10w/x86_64-apple-darwin13.3.0/lib/ -isystem /opt/gcc/gcc4.10w/x86_64-apple-darwin13.3.0/include -isystem /opt/gcc/gcc4.10w/x86_64-apple-darwin13.3.0/sys-include    -g -O2 -m32 -O2  -g -O2 -DIN_GCC    -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wno-format -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition  -isystem ./include   -pipe -fno-common -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector   -pipe -fno-common -I. -I. -I../../.././gcc -I../../../../work/libgcc -I../../../../work/libgcc/. -I../../../../work/libgcc/../gcc -I../../../../work/libgcc/../include  -DHAVE_CC_TLS -DUSE_EMUTLS -o _gcov_reset.o -MT _gcov_reset.o -MD -MP -MF _gcov_reset.dep -DL_gcov_reset -c ../../../../work/libgcc/libgcov-interface.c
../../../../work/libgcc/libgcov-interface.c:136:33: error: only weak aliases are supported in this configuration
 STRONG_ALIAS (__gcov_reset_int, __gcov_reset);
                                 ^
../../../../work/libgcc/libgcov-interface.c:49:25: note: in definition of macro 'STRONG_ALIAS'
   extern __typeof (src) dst __attribute__((alias (#src)))
                         ^
make[5]: *** [_gcov_reset.o] Error 1
...

TIA

Dominique

Re: fix gfcov regression

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 151 bytes --]

On 09/03/14 04:06, Dominique Dhumieres wrote:
>> I've committed the patch now.
>
> It (r214840) breaks bootstrap on darwin:

does this fix it?

nathan

[-- Attachment #2: all.diff --]
[-- Type: text/plain, Size: 1135 bytes --]

2014-09-03  Nathan sidwell  <nathan@acm.org>

	* libgcov-interface.c (STRONG_ALIAS): Rename to ...
	(ALIAS): ... here. Make weak.  Adjust uses.

Index: libgcc/libgcov-interface.c
===================================================================
--- libgcc/libgcov-interface.c	(revision 214840)
+++ libgcc/libgcov-interface.c	(working copy)
@@ -42,11 +42,11 @@ void __gcov_dump (void) {}
 
 #else
 
-
 /* Some functions we want to bind in this dynamic object, but have an
-   overridable global alias.  */
-#define STRONG_ALIAS(src,dst) \
-  extern __typeof (src) dst __attribute__((alias (#src)))
+   overridable global alias.  Weak aliases are supported in more
+   places than non-weak, and is adequate for our needs.  */
+#define ALIAS(src,dst) \
+  extern __typeof (src) dst __attribute__((weak, alias (#src)))
 
 extern __gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN;
 extern __gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN;
@@ -133,7 +133,7 @@ __gcov_reset_int (void)
   __gcov_root.dumped = 0;
 }
 
-STRONG_ALIAS (__gcov_reset_int, __gcov_reset);
+ALIAS (__gcov_reset_int, __gcov_reset);
 
 #endif /* L_gcov_reset */
 

Re: fix gfcov regression

[Dominique Dhumieres]

> does this fix it?

The answer after a quick update is yes, further testing scheduled for
tonight.

Thanks,

Dominique

Re: fix gfcov regression

[Dominique d'Humières]

As said in PR63182, the attached patch fixes bootstrap without regression. Any reason why it has not yet been committed?

Dominique

Le 3 sept. 2014 à 15:15, Nathan Sidwell <nathan@acm.org> a écrit :

> On 09/03/14 04:06, Dominique Dhumieres wrote:
>>> I've committed the patch now.
>> 
>> It (r214840) breaks bootstrap on darwin:
> 
> does this fix it?
> 
> nathan
> <all.diff>

Re: fix gfcov regression

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

On 09/05/14 03:06, roger@nextmovesoftware.com wrote:

> ../../../../gcc/libgcc/libgcov-interface.c:137:1: warning: alias definitions not
> supported in Mach-O; ignored
>   ALIAS (__gcov_reset_int, __gcov_reset);

That doesn't sound like it's going to work then.  I've gone with the attached 
patch.  If someone wants to invest time using an alias on targets that support 
it, that'd be great!

nathan

[-- Attachment #2: all.diff --]
[-- Type: text/plain, Size: 1208 bytes --]

2014-09-05  Nathan sidwell  <nathan@acm.org>

	* libgcov-interface.c (STRONG_ALIAS): Rename to ...
	(ALIAS_weak): ... here. Use forwarding function.  Adjust uses.

Index: libgcc/libgcov-interface.c
===================================================================
--- libgcc/libgcov-interface.c	(revision 214972)
+++ libgcc/libgcov-interface.c	(working copy)
@@ -42,11 +42,14 @@ void __gcov_dump (void) {}
 
 #else
 
-
 /* Some functions we want to bind in this dynamic object, but have an
-   overridable global alias.  */
-#define STRONG_ALIAS(src,dst) \
-  extern __typeof (src) dst __attribute__((alias (#src)))
+   overridable global alias.  Unfortunately not all targets support
+   aliases, so we just have a forwarding function.  That'll be tail
+   called, so the cost is a single jump instruction.*/
+
+#define ALIAS_void(src,dst) \
+  void dst (void)	    \
+  { src (); }
 
 extern __gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN;
 extern __gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN;
@@ -133,7 +136,7 @@ __gcov_reset_int (void)
   __gcov_root.dumped = 0;
 }
 
-STRONG_ALIAS (__gcov_reset_int, __gcov_reset);
+ALIAS_void (__gcov_reset_int, __gcov_reset);
 
 #endif /* L_gcov_reset */
 

Re: fix gfcov regression

[Nathan Sidwell]

On 08/23/14 10:33, Nathan Sidwell wrote:
> Hi,
> this patch fixes a defect Jan found with firefox and its shared objects.  We
> were inadvertently calling an externally visible and overridable symbol, rather
> than the local shared object's instance.  This led to strangely sparse gcov
> results.
>
> I've taken the STRONG_ALIAS #define from glibc.  I'm not 100% sure it's valid
> for all supported targets.
>
> Tested in x86_64-linux
>
> I've not committed this patch because of that, and (b) I'm  about to emmigrate,
> so likely to be unable to respond to any potential fallout in a timely manner.

I've committed the patch now.

fix gfcov regression

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

Hi,
this patch fixes a defect Jan found with firefox and its shared objects.  We 
were inadvertently calling an externally visible and overridable symbol, rather 
than the local shared object's instance.  This led to strangely sparse gcov results.

I've taken the STRONG_ALIAS #define from glibc.  I'm not 100% sure it's valid 
for all supported targets.

Tested in x86_64-linux

I've not committed this patch because of that, and (b) I'm  about to emmigrate, 
so likely to be unable to respond to any potential fallout in a timely manner.

Jan, if you think this patch is sufficiently safe, please apply.

nathan

[-- Attachment #2: all.diff --]
[-- Type: text/plain, Size: 2097 bytes --]

2014-08-22  Nathan sidwell  <nathan@acm.org>

	* libgcov-interface.c (STRONG_ALIAS): New.
	(__gcov_flush): Call __gcov_reset_int.
	(__gcov_reset): Strong alias for ...
	(__gcov_reset_ing): ... this renamed hidden version.
	* libgcov.h (__gcov_reset_int): New declaration.

Index: libgcc/libgcov-interface.c
===================================================================
--- libgcc/libgcov-interface.c	(revision 214320)
+++ libgcc/libgcov-interface.c	(working copy)
@@ -42,6 +42,12 @@ void __gcov_dump (void) {}
 
 #else
 
+
+/* Some functions we want to bind in this dynamic object, but have an
+   overridable global alias.  */
+#define STRONG_ALIAS(src,dst) \
+  extern __typeof (src) dst __attribute__((alias (#src)))
+
 extern __gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN;
 extern __gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN;
 
@@ -77,7 +83,7 @@ __gcov_flush (void)
   __gthread_mutex_lock (&__gcov_flush_mx);
 
   __gcov_dump_one (&__gcov_root);
-  __gcov_reset ();
+  __gcov_reset_int ();
 
   __gthread_mutex_unlock (&__gcov_flush_mx);
 }
@@ -121,12 +127,14 @@ gcov_clear (const struct gcov_info *list
    in order to collect profile in region of interest.  */
 
 void
-__gcov_reset (void)
+__gcov_reset_int (void)
 {
   gcov_clear (__gcov_root.list);
   __gcov_root.dumped = 0;
 }
 
+STRONG_ALIAS (__gcov_reset_int, __gcov_reset);
+
 #endif /* L_gcov_reset */
 
 #ifdef L_gcov_dump
Index: libgcc/libgcov.h
===================================================================
--- libgcc/libgcov.h	(revision 214320)
+++ libgcc/libgcov.h	(working copy)
@@ -225,8 +225,10 @@ extern void __gcov_init (struct gcov_inf
 /* Called before fork, to avoid double counting.  */
 extern void __gcov_flush (void) ATTRIBUTE_HIDDEN;
 
-/* Function to reset all counters to 0.  */
+/* Function to reset all counters to 0.  Both externally visible (and
+   overridable) and internal version.  */
 extern void __gcov_reset (void);
+extern void __gcov_reset_int (void) ATTRIBUTE_HIDDEN;
 
 /* Function to enable early write of profile information so far.  */
 extern void __gcov_dump (void);