[v2] gcov: Runtime configurable destination output

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(-)

Ping on this; what are the next steps?

Thanks

Aaron Conole <aconole@bytheb.org> 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

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 <aconole@bytheb.org> 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

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.

nathan

Nathan Sidwell <nathan@acm.org> 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

On 04/29/16 11:08, Aaron Conole wrote:

> 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?

Thanks for clarifying your rationale.  I'm not convinced, but I'm not (yet) 
saying no.

Jan, do you have any thoughts?

nathan

> On 04/29/16 11:08, Aaron Conole wrote:
> 
> >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?
> 
> Thanks for clarifying your rationale.  I'm not convinced, but I'm
> not (yet) saying no.
> 
> Jan, do you have any thoughts?

I can imagine this to be useful - if your application is outputting stuff into
error output during its training run, it may be quite disturbing having gcov
diagnostics randomly mixed in. (in particular I run myself into cases missing
the diagnostics) So I am fine with the feature.

Honza
> 
> nathan

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.

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.


> 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)


> @@ -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 ()

> +
>    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?

> +static void
> +gcov_error_exit(void)
> +{
> +  if (__gcov_error_file && __gcov_error_file != stderr)
> +    {

Braces are not needed here.


> --- 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 ().

nathan

Nathan Sidwell <nathan@acm.org> 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

On 05/19/2016 12:40 PM, Aaron Conole wrote:
> Nathan Sidwell <nathan@acm.org> 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.
Also note that Aaron works for Red Hat and should be covered by our 
existing assignments.

jeff

On 05/19/16 15:25, Jeff Law wrote:
> On 05/19/2016 12:40 PM, Aaron Conole wrote:

>> 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.



> Also note that Aaron works for Red Hat and should be covered by our existing
> assignments.

Yeah, Aaron hit  me with that clue bat privately -- I'd just grepped his name 
and not noticed the email!  I thought we were all settled, sorry if that wasn't 
clear.

nathan

On 05/19/16 14:40, Aaron Conole wrote:
> Nathan Sidwell <nathan@acm.org> 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)



>> 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.

nathan

On 05/19/2016 05:14 PM, Nathan Sidwell wrote:
> On 05/19/16 15:25, Jeff Law wrote:
>> On 05/19/2016 12:40 PM, Aaron Conole wrote:
>
>>> 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.
>
>
>
>> Also note that Aaron works for Red Hat and should be covered by our
>> existing
>> assignments.
>
> Yeah, Aaron hit  me with that clue bat privately -- I'd just grepped his
> name and not noticed the email!  I thought we were all settled, sorry if
> that wasn't clear.
No worries.  I didn't realize it either at first :-)

jeff

Nathan Sidwell <nathan@acm.org> writes:

> On 05/19/16 14:40, Aaron Conole wrote:
>> Nathan Sidwell <nathan@acm.org> 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

On Mon, May 23, 2016 at 11:16 AM, Aaron Conole <aconole@redhat.com> wrote:
> Nathan Sidwell <nathan@acm.org> writes:
>
>> On 05/19/16 14:40, Aaron Conole wrote:
>>> Nathan Sidwell <nathan@acm.org> 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

"H.J. Lu" <hjl.tools@gmail.com> writes:

> On Mon, May 23, 2016 at 11:16 AM, Aaron Conole <aconole@redhat.com> wrote:
>> Nathan Sidwell <nathan@acm.org> writes:
>>
>>> On 05/19/16 14:40, Aaron Conole wrote:
>>>> Nathan Sidwell <nathan@acm.org> 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.