from:"Aaron Conole"

Re: [PATCH v2] gcov: Runtime configurable destination output

2016-06-04 Thread Aaron Conole

> It breaks profiledbootstrap:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71400

I am including a patch that should fix the issues introduced by my
patch.  I have confirmed behavior on my system, and built with
profiledbootstrap as well as bootstrap.

libgcc/ChangeLog:
2016-06-04  Aaron Conole  

* libgcov-driver-system.c (gcov_error): Remove
  redundant assignment.
  (get_gcov_error_file): Invert the IN_GCOV_TOOL test
  (__gcov_error_file): Only use this when !IN_GCOV_TOOL

---
 libgcc/libgcov-driver-system.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
index ff8a521..6bfe6ba 100644
--- a/libgcc/libgcov-driver-system.c
+++ b/libgcc/libgcov-driver-system.c
@@ -27,7 +27,9 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If 
not, see
it will either be stderr, or a file of the user's choosing.
Non-static to prevent multiple gcov-aware shared objects from
instantiating their own copies. */
+#if !IN_GCOV_TOOL
 FILE *__gcov_error_file = NULL;
+#endif
 
 /* A utility function to populate the __gcov_error_file pointer.
This should NOT be called outside of the gcov system driver code. */
@@ -35,7 +37,7 @@ FILE *__gcov_error_file = NULL;
 static FILE *
 get_gcov_error_file(void)
 {
-#if !IN_GCOV_TOOL
+#if IN_GCOV_TOOL
   return stderr;
 #else
   char *gcov_error_filename = getenv ("GCOV_ERROR_FILE");
@@ -60,11 +62,8 @@ gcov_error (const char *fmt, ...)
   int ret;
   va_list argp;
 
-  if (!__gcov_error_file)
-__gcov_error_file = get_gcov_error_file ();
-
   va_start (argp, fmt);
-  ret = vfprintf (__gcov_error_file, fmt, argp);
+  ret = vfprintf (get_gcov_error_file (), fmt, argp);
   va_end (argp);
   return ret;
 }
-- 
2.5.5

Re: [PATCH v2] gcov: Runtime configurable destination output

2016-06-03 Thread Aaron Conole

"H.J. Lu"  writes:

> On Mon, May 23, 2016 at 11:16 AM, Aaron Conole  wrote:
>> Nathan Sidwell  writes:
>>
>>> On 05/19/16 14:40, Aaron Conole wrote:
>>>> Nathan Sidwell  writes:
>>>
>>>>>> +FILE *__gcov_error_file = NULL;
>>>>>
>>>>> Unless I'm missing something, isn't this only accessed from this file?
>>>>> (So could be static with a non-underbarred name)
>>>>
>>>> Ack.
>>>
>>> I have a vague memory that perhaps the __gcov_error_file is seen from
>>> other dynamic objects, and one of them gets to open/close it?  I think
>>> the closing function needs to reset it to NULL though?  (In case it's
>>> reactivated before the process exits)
>>
>> This is being introduced here, so the actual variable won't be seen,
>> however you're correct - the APIs could still be called.
>>
>> I think there does exist a possibility that it can get re-activated
>> before the process exits. So, I've changed it to have a proper block
>> cope and to reset gcov_error_file to NULL.
>>
>>>>> And this protection here, makes me wonder what happens if one is
>>>>> IN_GCOV_TOOL. Does it pay attention to GCOV_ERROR_FILE?  That would
>>>>> seem incorrect, and thus the above should be changed so that stderr is
>>>>> unconditionally used when IN_GCOV_TOOL?
>>>>
>>>> You are correct.  I will fix it.
>>>
>>> thanks.
>>>
>>>>>> +static void
>>>>>> +gcov_error_exit(void)
>>>>>> +{
>>>>>> +  if (__gcov_error_file && __gcov_error_file != stderr)
>>>>>> +{
>>>>>
>>>>> Braces are not needed here.
>>>
>>> Unless of course my speculation about setting it to NULL is right.
>>
>> It is - I've fixed it, and will post the v3 patch shortly.
>>
>> Thank you for your help, Nathan!
>>
>
> It breaks profiledbootstrap:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71400

d'oh!  Okay, baking a patch.

Re: [PATCH v4] gcov: Runtime configurable destination output

2016-05-31 Thread Aaron Conole

Nathan Sidwell  writes:

> On 05/26/16 13:08, Aaron Conole wrote:
>> The previous gcov behavior was to always output errors on the stderr channel.
>> This is fine for most uses, but some programs will require stderr to be
>> untouched by libgcov for certain tests. This change allows configuring
>> the gcov output via an environment variable which will be used to open
>> the appropriate file.
>
> this is ok, thanks.
>
> 1) Do you know how to write and format a ChangeLog entry?

I can copy the style used, I hope.

> 2) Are you able to commit the patch yourself (or have someone at RH
> walk you through the process)

I don't know how to do this for gcc, but I will ask.

Thanks for your all your time!

-Aaron

[PATCH v4] gcov: Runtime configurable destination output

2016-05-26 Thread Aaron Conole

The previous gcov behavior was to always output errors on the stderr channel.
This is fine for most uses, but some programs will require stderr to be
untouched by libgcov for certain tests. This change allows configuring
the gcov output via an environment variable which will be used to open
the appropriate file.
---
 libgcc/libgcov-driver-system.c | 49 --
 libgcc/libgcov-driver.c|  8 ++-
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
index 4e3b244..ff8a521 100644
--- a/libgcc/libgcov-driver-system.c
+++ b/libgcc/libgcov-driver-system.c
@@ -23,19 +23,64 @@ a copy of the GCC Runtime Library Exception along with this 
program;
 see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 .  */
 
-/* A utility function for outputing errors.  */
+/* Configured via the GCOV_ERROR_FILE environment variable;
+   it will either be stderr, or a file of the user's choosing.
+   Non-static to prevent multiple gcov-aware shared objects from
+   instantiating their own copies. */
+FILE *__gcov_error_file = NULL;
+
+/* A utility function to populate the __gcov_error_file pointer.
+   This should NOT be called outside of the gcov system driver code. */
+
+static FILE *
+get_gcov_error_file(void)
+{
+#if !IN_GCOV_TOOL
+  return stderr;
+#else
+  char *gcov_error_filename = getenv ("GCOV_ERROR_FILE");
+
+  if (gcov_error_filename)
+{
+  FILE *openfile = fopen (gcov_error_filename, "a");
+  if (openfile)
+__gcov_error_file = openfile;
+}
+  if (!__gcov_error_file)
+__gcov_error_file = stderr;
+  return __gcov_error_file;
+#endif
+}
+
+/* A utility function for outputting errors.  */
 
 static int __attribute__((format(printf, 1, 2)))
 gcov_error (const char *fmt, ...)
 {
   int ret;
   va_list argp;
+
+  if (!__gcov_error_file)
+__gcov_error_file = get_gcov_error_file ();
+
   va_start (argp, fmt);
-  ret = vfprintf (stderr, fmt, argp);
+  ret = vfprintf (__gcov_error_file, fmt, argp);
   va_end (argp);
   return ret;
 }
 
+#if !IN_GCOV_TOOL
+static void
+gcov_error_exit (void)
+{
+  if (__gcov_error_file && __gcov_error_file != stderr)
+{
+  fclose (__gcov_error_file);
+  __gcov_error_file = NULL;
+}
+}
+#endif
+
 /* Make sure path component of the given FILENAME exists, create
missing directories. FILENAME must be writable.
Returns zero on success, or -1 if an error occurred.  */
diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index 9c4eeca..d51397e 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -43,9 +43,13 @@ void __gcov_init (struct gcov_info *p __attribute__ 
((unused))) {}
 
 #ifdef L_gcov
 
-/* A utility function for outputing errors.  */
+/* A utility function for outputting errors.  */
 static int gcov_error (const char *, ...);
 
+#if !IN_GCOV_TOOL
+static void gcov_error_exit (void);
+#endif
+
 #include "gcov-io.c"
 
 struct gcov_fn_buffer
@@ -878,6 +882,8 @@ gcov_exit (void)
 __gcov_root.prev->next = __gcov_root.next;
   else
 __gcov_master.root = __gcov_root.next;
+
+  gcov_error_exit ();
 }
 
 /* Add a new object file onto the bb chain.  Invoked automatically
-- 
2.5.5

[PATCH v3] gcov: Runtime configurable destination output

2016-05-23 Thread Aaron Conole

The previous gcov behavior was to always output errors on the stderr channel.
This is fine for most uses, but some programs will require stderr to be
untouched by libgcov for certain tests. This change allows configuring
the gcov output via an environment variable which will be used to open
the appropriate file.
---
 libgcc/libgcov-driver-system.c | 43 +-
 libgcc/libgcov-driver.c|  6 ++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
index 4e3b244..461715f 100644
--- a/libgcc/libgcov-driver-system.c
+++ b/libgcc/libgcov-driver-system.c
@@ -23,6 +23,31 @@ a copy of the GCC Runtime Library Exception along with this 
program;
 see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 .  */
 
+/* Configured via the GCOV_ERROR_FILE environment variable;
+   it will either be stderr, or a file of the user's choosing. */
+static FILE *gcov_error_file;
+
+/* A utility function to populate the gcov_error_file pointer */
+
+static FILE *
+get_gcov_error_file(void)
+{
+#if IN_GCOV_TOOL
+  return stderr;
+#endif
+  char *gcov_error_filename = getenv ("GCOV_ERROR_FILE");
+
+  if (gcov_error_filename)
+{
+  FILE *openfile = fopen (gcov_error_filename, "a");
+  if (openfile)
+gcov_error_file = openfile;
+}
+  if (!gcov_error_file)
+gcov_error_file = stderr;
+  return gcov_error_file;
+}
+
 /* A utility function for outputing errors.  */
 
 static int __attribute__((format(printf, 1, 2)))
@@ -30,12 +55,28 @@ gcov_error (const char *fmt, ...)
 {
   int ret;
   va_list argp;
+
+  if (!gcov_error_file)
+gcov_error_file = get_gcov_error_file ();
+
   va_start (argp, fmt);
-  ret = vfprintf (stderr, fmt, argp);
+  ret = vfprintf (gcov_error_file, fmt, argp);
   va_end (argp);
   return ret;
 }
 
+#if !IN_GCOV_TOOL
+static void
+gcov_error_exit(void)
+{
+  if (gcov_error_file && gcov_error_file != stderr)
+{
+  fclose(gcov_error_file);
+  gcov_error_file = NULL;
+}
+}
+#endif
+
 /* Make sure path component of the given FILENAME exists, create
missing directories. FILENAME must be writable.
Returns zero on success, or -1 if an error occurred.  */
diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index 9c4eeca..92fb8ab 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -46,6 +46,10 @@ void __gcov_init (struct gcov_info *p __attribute__ 
((unused))) {}
 /* A utility function for outputing errors.  */
 static int gcov_error (const char *, ...);
 
+#if !IN_GCOV_TOOL
+static void gcov_error_exit(void);
+#endif
+
 #include "gcov-io.c"
 
 struct gcov_fn_buffer
@@ -878,6 +882,8 @@ gcov_exit (void)
 __gcov_root.prev->next = __gcov_root.next;
   else
 __gcov_master.root = __gcov_root.next;
+
+  gcov_error_exit ();
 }
 
 /* Add a new object file onto the bb chain.  Invoked automatically
-- 
2.5.5

Re: [PATCH v2] gcov: Runtime configurable destination output

2016-05-23 Thread Aaron Conole

Nathan Sidwell  writes:

> On 05/19/16 14:40, Aaron Conole wrote:
>> Nathan Sidwell  writes:
>
>>>> +FILE *__gcov_error_file = NULL;
>>>
>>> Unless I'm missing something, isn't this only accessed from this file?
>>> (So could be static with a non-underbarred name)
>>
>> Ack.
>
> I have a vague memory that perhaps the __gcov_error_file is seen from
> other dynamic objects, and one of them gets to open/close it?  I think
> the closing function needs to reset it to NULL though?  (In case it's
> reactivated before the process exits)

This is being introduced here, so the actual variable won't be seen,
however you're correct - the APIs could still be called.

I think there does exist a possibility that it can get re-activated
before the process exits. So, I've changed it to have a proper block
cope and to reset gcov_error_file to NULL.

>>> And this protection here, makes me wonder what happens if one is
>>> IN_GCOV_TOOL. Does it pay attention to GCOV_ERROR_FILE?  That would
>>> seem incorrect, and thus the above should be changed so that stderr is
>>> unconditionally used when IN_GCOV_TOOL?
>>
>> You are correct.  I will fix it.
>
> thanks.
>
>>>> +static void
>>>> +gcov_error_exit(void)
>>>> +{
>>>> +  if (__gcov_error_file && __gcov_error_file != stderr)
>>>> +{
>>>
>>> Braces are not needed here.
>
> Unless of course my speculation about setting it to NULL is right.

It is - I've fixed it, and will post the v3 patch shortly.

Thank you for your help, Nathan!

> nathan

Re: [PATCH v2] gcov: Runtime configurable destination output

2016-05-19 Thread Aaron Conole

Nathan Sidwell  writes:

> On 02/24/16 16:52, Aaron Conole wrote:
>> The previous gcov behavior was to always output errors on the stderr channel.
>> This is fine for most uses, but some programs will require stderr to be
>> untouched by libgcov for certain tests. This change allows configuring
>> the gcov output via an environment variable which will be used to open
>> the appropriate file.
>
> this is ok in principle.  I have a couple of questions & nits below though.

Thank you for the consideration.  I will be submitting a new patch that
I hope fully addresses your comments below, either tomorrow or Monday.

Thanks so much for the review.

> I don't see a previous commit from you -- do you have a copyright
> assignment with the FSF? (although this patch is simple, my guess is
> the idea it implements is sufficiently novel to need one).  We can
> handle that off list.

I'm happy to report that I did send in some FSF paperwork this week.
Hopefully it is on record now, but even if it isn't I live a train ride
away from the FSF headquarters so I'd be happy to take the time to make
sure it's all signed correctly.

>> diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
>> index 4e3b244..0eb9755 100644
>> --- a/libgcc/libgcov-driver-system.c
>> +++ b/libgcc/libgcov-driver-system.c
>> @@ -23,6 +23,24 @@ a copy of the GCC Runtime Library Exception along
>> with this program;
>>  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>>  <http://www.gnu.org/licenses/>.  */
>>
>> +FILE *__gcov_error_file = NULL;
>
> Unless I'm missing something, isn't this only accessed from this file?
> (So could be static with a non-underbarred name)

Ack.

>> @@ -30,12 +48,27 @@ gcov_error (const char *fmt, ...)
>>  {
>>int ret;
>>va_list argp;
>> +
>> +  if (!__gcov_error_file)
>> +__gcov_error_file = get_gcov_error_file();
>
> Needs space before ()

Ack.

>> +
>>va_start (argp, fmt);
>> -  ret = vfprintf (stderr, fmt, argp);
>> +  ret = vfprintf (__gcov_error_file, fmt, argp);
>>va_end (argp);
>>return ret;
>>  }
>>
>> +#if !IN_GCOV_TOOL
>
> And this protection here, makes me wonder what happens if one is
> IN_GCOV_TOOL. Does it pay attention to GCOV_ERROR_FILE?  That would
> seem incorrect, and thus the above should be changed so that stderr is
> unconditionally used when IN_GCOV_TOOL?

You are correct.  I will fix it.

>> +static void
>> +gcov_error_exit(void)
>> +{
>> +  if (__gcov_error_file && __gcov_error_file != stderr)
>> +{
>
> Braces are not needed here.

Ack.

>> --- a/libgcc/libgcov-driver.c
>> +++ b/libgcc/libgcov-driver.c
>> @@ -46,6 +46,10 @@ void __gcov_init (struct gcov_info *p
>> __attribute__ ((unused))) {}
>
>> +  gcov_error_exit();
>
> Needs space before ().

Ack.

> nathan

Re: [PATCH v2] gcov: Runtime configurable destination output

2016-04-29 Thread Aaron Conole

Nathan Sidwell  writes:

> On 04/27/16 16:59, Aaron Conole wrote:
>> Apologies for the top post. Pinging on this again. It still applies
>> cleanly, so no need to resubmit, I think. Is there anything else missing
>> or required before this can go in?
>
> I'm not convinced this is a desirable feature.  IIRC your rationale
> for it was that that you're somehow building the target program with
> inconsistent coverage data, and the messages about that are
> interfering with your program's output.
>
> That's kind of the point of error messages -- to get in your face.

Perhaps I've poorly explained what I want. I want to be able to pipe
gcov error messages to a different file for post-processing / reporting
elsewhere. I don't want them mixed with the application's messages. Do
you think this kind of generic flexibility is not a good thing, when it
comes at such little cost?

The whole point of this is to provide a way to keep the error messages
around. After all, if I really didn't want to see them I could do at
least the following things (untested, just for example):
  1. `./myapp 2>/dev/null` (which I don't want to do)
  2. { ...; fclose(stderr); stderr = fopen("gcoverrfile", "w"); exit(0); }
  3. mkfifo something; ./myapp 2>something; sed -e s,gcov_error_msg,,g something

But, this appeared to me like a generic way of providing what I want
in a way that could apply to any other application, without relying on
workarounds. If that's not a convincing argument, then I guess NAK it
and I'll be done with it - apologies for the noise.

Much thanks for your time,
-Aaron

> nathan

Re: [PATCH v2] gcov: Runtime configurable destination output

2016-04-27 Thread Aaron Conole

Apologies for the top post. Pinging on this again. It still applies
cleanly, so no need to resubmit, I think. Is there anything else missing
or required before this can go in?

Thanks,
-Aaron

Aaron Conole  writes:

> The previous gcov behavior was to always output errors on the stderr channel.
> This is fine for most uses, but some programs will require stderr to be
> untouched by libgcov for certain tests. This change allows configuring
> the gcov output via an environment variable which will be used to open
> the appropriate file.
> ---
> v2:
> * Retitled subject
> * Cleaned up whitespace in libgcov-driver-system.c diff
> * Lazy error file opening
> * non-static error file
> * No warnings during compilation
>
>  libgcc/libgcov-driver-system.c | 35 ++-
>  libgcc/libgcov-driver.c|  6 ++
>  2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
> index 4e3b244..0eb9755 100644
> --- a/libgcc/libgcov-driver-system.c
> +++ b/libgcc/libgcov-driver-system.c
> @@ -23,6 +23,24 @@ a copy of the GCC Runtime Library Exception along with 
> this program;
>  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>  <http://www.gnu.org/licenses/>.  */
>  
> +FILE *__gcov_error_file = NULL;
> +
> +static FILE *
> +get_gcov_error_file(void)
> +{
> +  char *gcov_error_filename = getenv("GCOV_ERROR_FILE");
> +  FILE *gcov_error_file = NULL;
> +  if (gcov_error_filename)
> +{
> +  FILE *openfile = fopen(gcov_error_filename, "a");
> +  if (openfile)
> +gcov_error_file = openfile;
> +}
> +  if (!gcov_error_file)
> +gcov_error_file = stderr;
> +  return gcov_error_file;
> +}
> +
>  /* A utility function for outputing errors.  */
>  
>  static int __attribute__((format(printf, 1, 2)))
> @@ -30,12 +48,27 @@ gcov_error (const char *fmt, ...)
>  {
>int ret;
>va_list argp;
> +
> +  if (!__gcov_error_file)
> +__gcov_error_file = get_gcov_error_file();
> +
>va_start (argp, fmt);
> -  ret = vfprintf (stderr, fmt, argp);
> +  ret = vfprintf (__gcov_error_file, fmt, argp);
>va_end (argp);
>return ret;
>  }
>  
> +#if !IN_GCOV_TOOL
> +static void
> +gcov_error_exit(void)
> +{
> +  if (__gcov_error_file && __gcov_error_file != stderr)
> +{
> +  fclose(__gcov_error_file);
> +}
> +}
> +#endif
> +
>  /* Make sure path component of the given FILENAME exists, create
> missing directories. FILENAME must be writable.
> Returns zero on success, or -1 if an error occurred.  */
> diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
> index 9c4eeca..83d84c5c 100644
> --- a/libgcc/libgcov-driver.c
> +++ b/libgcc/libgcov-driver.c
> @@ -46,6 +46,10 @@ void __gcov_init (struct gcov_info *p __attribute__ 
> ((unused))) {}
>  /* A utility function for outputing errors.  */
>  static int gcov_error (const char *, ...);
>  
> +#if !IN_GCOV_TOOL
> +static void gcov_error_exit(void);
> +#endif
> +
>  #include "gcov-io.c"
>  
>  struct gcov_fn_buffer
> @@ -878,6 +882,8 @@ gcov_exit (void)
>  __gcov_root.prev->next = __gcov_root.next;
>else
>  __gcov_master.root = __gcov_root.next;
> +
> +  gcov_error_exit();
>  }
>  
>  /* Add a new object file onto the bb chain.  Invoked automatically

Re: [PATCH v2] gcov: Runtime configurable destination output

2016-04-15 Thread Aaron Conole

Ping on this; what are the next steps?

Thanks

Aaron Conole  writes:

> The previous gcov behavior was to always output errors on the stderr channel.
> This is fine for most uses, but some programs will require stderr to be
> untouched by libgcov for certain tests. This change allows configuring
> the gcov output via an environment variable which will be used to open
> the appropriate file.
> ---
> v2:
> * Retitled subject
> * Cleaned up whitespace in libgcov-driver-system.c diff
> * Lazy error file opening
> * non-static error file
> * No warnings during compilation
>
>  libgcc/libgcov-driver-system.c | 35 ++-
>  libgcc/libgcov-driver.c|  6 ++
>  2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
> index 4e3b244..0eb9755 100644
> --- a/libgcc/libgcov-driver-system.c
> +++ b/libgcc/libgcov-driver-system.c
> @@ -23,6 +23,24 @@ a copy of the GCC Runtime Library Exception along with 
> this program;
>  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>  <http://www.gnu.org/licenses/>.  */
>  
> +FILE *__gcov_error_file = NULL;
> +
> +static FILE *
> +get_gcov_error_file(void)
> +{
> +  char *gcov_error_filename = getenv("GCOV_ERROR_FILE");
> +  FILE *gcov_error_file = NULL;
> +  if (gcov_error_filename)
> +{
> +  FILE *openfile = fopen(gcov_error_filename, "a");
> +  if (openfile)
> +gcov_error_file = openfile;
> +}
> +  if (!gcov_error_file)
> +gcov_error_file = stderr;
> +  return gcov_error_file;
> +}
> +
>  /* A utility function for outputing errors.  */
>  
>  static int __attribute__((format(printf, 1, 2)))
> @@ -30,12 +48,27 @@ gcov_error (const char *fmt, ...)
>  {
>int ret;
>va_list argp;
> +
> +  if (!__gcov_error_file)
> +__gcov_error_file = get_gcov_error_file();
> +
>va_start (argp, fmt);
> -  ret = vfprintf (stderr, fmt, argp);
> +  ret = vfprintf (__gcov_error_file, fmt, argp);
>va_end (argp);
>return ret;
>  }
>  
> +#if !IN_GCOV_TOOL
> +static void
> +gcov_error_exit(void)
> +{
> +  if (__gcov_error_file && __gcov_error_file != stderr)
> +{
> +  fclose(__gcov_error_file);
> +}
> +}
> +#endif
> +
>  /* Make sure path component of the given FILENAME exists, create
> missing directories. FILENAME must be writable.
> Returns zero on success, or -1 if an error occurred.  */
> diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
> index 9c4eeca..83d84c5c 100644
> --- a/libgcc/libgcov-driver.c
> +++ b/libgcc/libgcov-driver.c
> @@ -46,6 +46,10 @@ void __gcov_init (struct gcov_info *p __attribute__ 
> ((unused))) {}
>  /* A utility function for outputing errors.  */
>  static int gcov_error (const char *, ...);
>  
> +#if !IN_GCOV_TOOL
> +static void gcov_error_exit(void);
> +#endif
> +
>  #include "gcov-io.c"
>  
>  struct gcov_fn_buffer
> @@ -878,6 +882,8 @@ gcov_exit (void)
>  __gcov_root.prev->next = __gcov_root.next;
>else
>  __gcov_master.root = __gcov_root.next;
> +
> +  gcov_error_exit();
>  }
>  
>  /* Add a new object file onto the bb chain.  Invoked automatically

[PATCH v2] gcov: Runtime configurable destination output

2016-02-24 Thread Aaron Conole

The previous gcov behavior was to always output errors on the stderr channel.
This is fine for most uses, but some programs will require stderr to be
untouched by libgcov for certain tests. This change allows configuring
the gcov output via an environment variable which will be used to open
the appropriate file.
---
v2:
* Retitled subject
* Cleaned up whitespace in libgcov-driver-system.c diff
* Lazy error file opening
* non-static error file
* No warnings during compilation

 libgcc/libgcov-driver-system.c | 35 ++-
 libgcc/libgcov-driver.c|  6 ++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
index 4e3b244..0eb9755 100644
--- a/libgcc/libgcov-driver-system.c
+++ b/libgcc/libgcov-driver-system.c
@@ -23,6 +23,24 @@ a copy of the GCC Runtime Library Exception along with this 
program;
 see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 .  */
 
+FILE *__gcov_error_file = NULL;
+
+static FILE *
+get_gcov_error_file(void)
+{
+  char *gcov_error_filename = getenv("GCOV_ERROR_FILE");
+  FILE *gcov_error_file = NULL;
+  if (gcov_error_filename)
+{
+  FILE *openfile = fopen(gcov_error_filename, "a");
+  if (openfile)
+gcov_error_file = openfile;
+}
+  if (!gcov_error_file)
+gcov_error_file = stderr;
+  return gcov_error_file;
+}
+
 /* A utility function for outputing errors.  */
 
 static int __attribute__((format(printf, 1, 2)))
@@ -30,12 +48,27 @@ gcov_error (const char *fmt, ...)
 {
   int ret;
   va_list argp;
+
+  if (!__gcov_error_file)
+__gcov_error_file = get_gcov_error_file();
+
   va_start (argp, fmt);
-  ret = vfprintf (stderr, fmt, argp);
+  ret = vfprintf (__gcov_error_file, fmt, argp);
   va_end (argp);
   return ret;
 }
 
+#if !IN_GCOV_TOOL
+static void
+gcov_error_exit(void)
+{
+  if (__gcov_error_file && __gcov_error_file != stderr)
+{
+  fclose(__gcov_error_file);
+}
+}
+#endif
+
 /* Make sure path component of the given FILENAME exists, create
missing directories. FILENAME must be writable.
Returns zero on success, or -1 if an error occurred.  */
diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index 9c4eeca..83d84c5c 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -46,6 +46,10 @@ void __gcov_init (struct gcov_info *p __attribute__ 
((unused))) {}
 /* A utility function for outputing errors.  */
 static int gcov_error (const char *, ...);
 
+#if !IN_GCOV_TOOL
+static void gcov_error_exit(void);
+#endif
+
 #include "gcov-io.c"
 
 struct gcov_fn_buffer
@@ -878,6 +882,8 @@ gcov_exit (void)
 __gcov_root.prev->next = __gcov_root.next;
   else
 __gcov_master.root = __gcov_root.next;
+
+  gcov_error_exit();
 }
 
 /* Add a new object file onto the bb chain.  Invoked automatically
-- 
2.5.0

Re: [PATCH] gcov: Configurable destination for error output

2016-02-23 Thread Aaron Conole

Nathan Sidwell  writes:

> On 02/22/16 14:35, Aaron Conole wrote:
>
>> D'oh, you're probably right. In my excitement to contribute, I forgot
>> this was shared. I think 'w' should be correct, since this isn't
>> intended to be read at all, but I could be convinced otherwise.
>
> sorry, I misremembered the encoding of write append, which is "a" --
> don't clobber the existing contents.  I think that's the usual
> behaviour for error logging (.xsession-error and the like).

Done.

>> By lazy load, do you mean only after the first gcov_error call? That's
>> probably a better approach, and I can recook, test, and resubmit with
>> these corrected.
>
> Lazy opening -- open on first error output.   Something like
>
> if (!gcov_error_file)
>   gcov_error_file = gcov_open_error_file ();
>
> in gcov_error?

Before I start cooking up this change, is it possible I need to worry about
gcov_error being invoked from multiple threads? If so, I'll need some
kind of mutex which I think is not needed with the current design.

> FWIW, I think this has missed the boat for gcc 6.1, as we're now in stage  4.

No worries.

-Aaron

> nathan

Re: [PATCH] gcov: Configurable destination for error output

2016-02-22 Thread Aaron Conole

Aaron Conole  writes:

> Nathan Sidwell  writes:
>
>> On 02/22/16 13:11, Aaron Conole wrote:
>>> Nathan Sidwell  writes:
>>>
>>> Hi Nathan, thanks so much for looking at this!
>>>
>>>> On 02/22/16 12:03, Aaron Conole wrote:
>>>>> The previous gcov behavior was to always output errors on the stderr 
>>>>> channel.
>>>>> This is fine for most uses, but some programs will require stderr to be
>>>>> silent for certain tests. This change allows configuring the gcov output 
>>>>> by
>>>>> an environment variable which will be used to open the appropriate file.
>>>>
>>>> Why is the invoker unable to direct fd2 before execing gcov?
>>>
>>> I want to make sure I understand your question. You are asking why
>>> something like: ` ./program_executed 2>/path/to/file `` is not preferred?
>>>
>>> If this is the question, the problem is program errors will be intermingled
>>> with gcov error messages. Let's suppose that I've got a unit test
>>> program (since that's what I have as specifically happening). I expect
>>> certain tests from that program to spit out errors on stderr. So, I
>>> filter out that text in stderr, so normal case stderr results will be
>>> clear. Now, I build with gcov enabled. In this case, gcov writes
>>> 'profiling:...' to stderr. Now, the test fails, even though nothing
>>> changed apart from using gcov.
>>
>> ah, gotcha!  I'd not understood this was in the gcov runtime (as
>> opposed to the gcov analysis programs).
>
> I'll update $subject to try and make it clearer. Thanks!
>
>> Doesn't your use of statics result in multiple fopens in different
>> shared objects, and subsequent tangling buffering? It would seem to
>> me that gcov_error_file needs the same linkage as __gcov_master?
>> Would lazy opening and "w+" be better behaviour?
>
> D'oh, you're probably right. In my excitement to contribute, I forgot
> this was shared. I think 'w' should be correct, since this isn't
> intended to be read at all, but I could be convinced otherwise.
>
> By lazy load, do you mean only after the first gcov_error call? That's
> probably a better approach, and I can recook, test, and resubmit with
> these corrected.
>
>> BTW, the code formatting needs correcting.
>
> Okay. Emacs is autoformatting it to gnu style for me (I thought), is
> there one I should prefer for gcc contributions?

Ignore this part, just realized what was going on and it will be correct
in the next version.

>> nathan

Re: [PATCH] gcov: Configurable destination for error output

2016-02-22 Thread Aaron Conole

Nathan Sidwell  writes:

> On 02/22/16 13:11, Aaron Conole wrote:
>> Nathan Sidwell  writes:
>>
>> Hi Nathan, thanks so much for looking at this!
>>
>>> On 02/22/16 12:03, Aaron Conole wrote:
>>>> The previous gcov behavior was to always output errors on the stderr 
>>>> channel.
>>>> This is fine for most uses, but some programs will require stderr to be
>>>> silent for certain tests. This change allows configuring the gcov output by
>>>> an environment variable which will be used to open the appropriate file.
>>>
>>> Why is the invoker unable to direct fd2 before execing gcov?
>>
>> I want to make sure I understand your question. You are asking why
>> something like: ` ./program_executed 2>/path/to/file `` is not preferred?
>>
>> If this is the question, the problem is program errors will be intermingled
>> with gcov error messages. Let's suppose that I've got a unit test
>> program (since that's what I have as specifically happening). I expect
>> certain tests from that program to spit out errors on stderr. So, I
>> filter out that text in stderr, so normal case stderr results will be
>> clear. Now, I build with gcov enabled. In this case, gcov writes
>> 'profiling:...' to stderr. Now, the test fails, even though nothing
>> changed apart from using gcov.
>
> ah, gotcha!  I'd not understood this was in the gcov runtime (as
> opposed to the gcov analysis programs).

I'll update $subject to try and make it clearer. Thanks!

> Doesn't your use of statics result in multiple fopens in different
> shared objects, and subsequent tangling buffering? It would seem to
> me that gcov_error_file needs the same linkage as __gcov_master?
> Would lazy opening and "w+" be better behaviour?

D'oh, you're probably right. In my excitement to contribute, I forgot
this was shared. I think 'w' should be correct, since this isn't
intended to be read at all, but I could be convinced otherwise.

By lazy load, do you mean only after the first gcov_error call? That's
probably a better approach, and I can recook, test, and resubmit with
these corrected.

> BTW, the code formatting needs correcting.

Okay. Emacs is autoformatting it to gnu style for me (I thought), is
there one I should prefer for gcc contributions?

> nathan

Re: [PATCH] gcov: Configurable destination for error output

2016-02-22 Thread Aaron Conole

Nathan Sidwell  writes:

Hi Nathan, thanks so much for looking at this!

> On 02/22/16 12:03, Aaron Conole wrote:
>> The previous gcov behavior was to always output errors on the stderr channel.
>> This is fine for most uses, but some programs will require stderr to be
>> silent for certain tests. This change allows configuring the gcov output by
>> an environment variable which will be used to open the appropriate file.
>
> Why is the invoker unable to direct fd2 before execing gcov?

I want to make sure I understand your question. You are asking why
something like: ` ./program_executed 2>/path/to/file `` is not preferred?

If this is the question, the problem is program errors will be intermingled
with gcov error messages. Let's suppose that I've got a unit test
program (since that's what I have as specifically happening). I expect
certain tests from that program to spit out errors on stderr. So, I
filter out that text in stderr, so normal case stderr results will be
clear. Now, I build with gcov enabled. In this case, gcov writes
'profiling:...' to stderr. Now, the test fails, even though nothing
changed apart from using gcov.

Sometimes this is a real user error (clean .gcda and .gcno files, do
clean build and then rerun), but other times, for instance if I use
openvswitch's 'make check', there is no rebuild which happens, but gcov
has merge mismatch errors anyway. I want to not squelch those errors,
but I don't want them intermingled with the program's error output.

I hope I explained myself and answered your question. Please let me know
if I didn't.

Thanks again,
-Aaron

> nathan

[PATCH] gcov: Configurable destination for error output

2016-02-22 Thread Aaron Conole

The previous gcov behavior was to always output errors on the stderr channel.
This is fine for most uses, but some programs will require stderr to be
silent for certain tests. This change allows configuring the gcov output by
an environment variable which will be used to open the appropriate file.
---
v0: Note, I do not have a signed FSF agreement at present. I do work at Red
Hat, but gcc is not my primary role. If this change requires paperwork, I can
sign and return asap.

 libgcc/libgcov-driver-system.c | 28 +++-
 libgcc/libgcov-driver.c|  5 +
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
index 4e3b244..09bb167 100644
--- a/libgcc/libgcov-driver-system.c
+++ b/libgcc/libgcov-driver-system.c
@@ -23,6 +23,8 @@ a copy of the GCC Runtime Library Exception along with this 
program;
 see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 .  */
 
+static FILE *gcov_error_file;
+
 /* A utility function for outputing errors.  */
 
 static int __attribute__((format(printf, 1, 2)))
@@ -31,11 +33,35 @@ gcov_error (const char *fmt, ...)
   int ret;
   va_list argp;
   va_start (argp, fmt);
-  ret = vfprintf (stderr, fmt, argp);
+  ret = vfprintf (gcov_error_file, fmt, argp);
   va_end (argp);
   return ret;
 }
 
+static void
+gcov_error_exit(void)
+{
+  if (gcov_error_file != stderr)
+   {
+ fclose(gcov_error_file);
+   }
+}
+
+static void
+gcov_error_init(void)
+{
+  char *gcov_error_filename = getenv("GCOV_ERROR_FILE");
+  gcov_error_file = NULL;
+  if (gcov_error_filename)
+   {
+ FILE *openfile = fopen(gcov_error_filename, "w");
+ if (openfile)
+   gcov_error_file = openfile;
+   }
+  if (!gcov_error_file)
+   gcov_error_file = stderr;
+}
+
 /* Make sure path component of the given FILENAME exists, create
missing directories. FILENAME must be writable.
Returns zero on success, or -1 if an error occurred.  */
diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index 9c4eeca..082755a 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -45,6 +45,8 @@ void __gcov_init (struct gcov_info *p __attribute__ 
((unused))) {}
 
 /* A utility function for outputing errors.  */
 static int gcov_error (const char *, ...);
+static void gcov_error_init(void);
+static void gcov_error_exit(void);
 
 #include "gcov-io.c"
 
@@ -878,6 +880,8 @@ gcov_exit (void)
 __gcov_root.prev->next = __gcov_root.next;
   else
 __gcov_master.root = __gcov_root.next;
+
+  gcov_error_exit();
 }
 
 /* Add a new object file onto the bb chain.  Invoked automatically
@@ -900,6 +904,7 @@ __gcov_init (struct gcov_info *info)
__gcov_master.root->prev = &__gcov_root;
  __gcov_master.root = &__gcov_root;
}
+ gcov_error_init();
  atexit (gcov_exit);
}
 
-- 
2.5.0

Re: [PATCH v2] gcov: Runtime configurable destination output

Re: [PATCH v2] gcov: Runtime configurable destination output

Re: [PATCH v4] gcov: Runtime configurable destination output

[PATCH v4] gcov: Runtime configurable destination output

[PATCH v3] gcov: Runtime configurable destination output

Re: [PATCH v2] gcov: Runtime configurable destination output

Re: [PATCH v2] gcov: Runtime configurable destination output

Re: [PATCH v2] gcov: Runtime configurable destination output

Re: [PATCH v2] gcov: Runtime configurable destination output

Re: [PATCH v2] gcov: Runtime configurable destination output

[PATCH v2] gcov: Runtime configurable destination output

Re: [PATCH] gcov: Configurable destination for error output

Re: [PATCH] gcov: Configurable destination for error output

Re: [PATCH] gcov: Configurable destination for error output

Re: [PATCH] gcov: Configurable destination for error output

[PATCH] gcov: Configurable destination for error output

16 matches

Site Navigation

Mail list logo

Footer information