contrib: Make dg-extract-results.py tolerant of unparseable files

  This commit is for the benefit of GDB, but as the binutils-gdb
repository shares the contrib/ directory with GCC, this commit
must first be applied to GCC and then copied back to binutils-gdb.

When running GDB tests in parallel (make check -j$(nproc)), the
consolidated gdb.sum and gdb.log files are produced by
contrib/dg-extract-results.py, which merges per-test output files.

If any single per-test output file is malformed (e.g., due to a
DejaGnu EILSEQ crash, which is how I encountered this problem), the
script aborts via self.fatal().  Because this script is invoked via a
Makefile command using shell redirection, this causes the top-level
output files to be left as empty, zero-byte files, discarding valid
results from all other tests.

Fix by making the script tolerant of unparseable input files.  Wrap
each file's parsing in a try/except block.  When a file cannot be
parsed, emit a warning to stderr and continue processing remaining
files.  This ensures that crashing tests do not destroy the
consolidated output for the entire parallel build.

Tested on Fedora 44 using the GCC testsuite (make check-gcc
-j$(nproc)). The consolidated results are produced correctly with
no regressions.

This commit fixes this GDB bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=34147

contrib/ChangeLog:

	* dg-extract-results.py: Show warnings instead of erroring out
	when encountering an unparseable file.
---
 contrib/dg-extract-results.py | 44 +++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 12 deletions(-)
  

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

Kevin> This commit fixes this GDB bug:
Kevin> https://sourceware.org/bugzilla/show_bug.cgi?id=34147

FWIW I think this should go in.

It would be fine to land it in gdb and then merge it back.
I think we agreed that common files could be treated this way now.

Kevin> +class ParseError (Exception):

I guess gcc doesn't use 'black' for Python formatting.

thanks,
Tom
  

On Thu, May 28, 2026 at 12:03 PM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:
>
> Kevin> This commit fixes this GDB bug:
> Kevin> https://sourceware.org/bugzilla/show_bug.cgi?id=34147
>
> FWIW I think this should go in.

This is on my list of patches to review this week. I hope to get to it
today or tomorrow.

>
> It would be fine to land it in gdb and then merge it back.
> I think we agreed that common files could be treated this way now.
>
> Kevin> +class ParseError (Exception):
>
> I guess gcc doesn't use 'black' for Python formatting.

GCC coding style has a section on python formatting:
https://gcc.gnu.org/codingconventions.html#python
But I don't know much about python formatting to say much there.

Thanks,
Andrea


>
> thanks,
> Tom
  

On Thu, May 14, 2026 at 11:18 AM Kevin Buettner <kevinb@redhat.com> wrote:
>
> This commit is for the benefit of GDB, but as the binutils-gdb
> repository shares the contrib/ directory with GCC, this commit
> must first be applied to GCC and then copied back to binutils-gdb.
>
> When running GDB tests in parallel (make check -j$(nproc)), the
> consolidated gdb.sum and gdb.log files are produced by
> contrib/dg-extract-results.py, which merges per-test output files.
>
> If any single per-test output file is malformed (e.g., due to a
> DejaGnu EILSEQ crash, which is how I encountered this problem), the
> script aborts via self.fatal().  Because this script is invoked via a
> Makefile command using shell redirection, this causes the top-level
> output files to be left as empty, zero-byte files, discarding valid
> results from all other tests.
>
> Fix by making the script tolerant of unparseable input files.  Wrap
> each file's parsing in a try/except block.  When a file cannot be
> parsed, emit a warning to stderr and continue processing remaining
> files.  This ensures that crashing tests do not destroy the
> consolidated output for the entire parallel build.
>
> Tested on Fedora 44 using the GCC testsuite (make check-gcc
> -j$(nproc)). The consolidated results are produced correctly with
> no regressions.
>
> This commit fixes this GDB bug:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=34147
>
> contrib/ChangeLog:
>
>         * dg-extract-results.py: Show warnings instead of erroring out
>         when encountering an unparseable file.

Ok.

> ---
>  contrib/dg-extract-results.py | 44 +++++++++++++++++++++++++----------
>  1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/contrib/dg-extract-results.py b/contrib/dg-extract-results.py
> index c7060753500..98b0f4989c9 100644
> --- a/contrib/dg-extract-results.py
> +++ b/contrib/dg-extract-results.py
> @@ -34,6 +34,16 @@ if sys.version_info >= (3, 0):
>      sys.stdout = io.TextIOWrapper (sys.stdout.buffer,
>                                     errors = 'surrogateescape')
>
> +# Exception raised to skip a file that cannot be parsed.  Used when
> +# a summary or log file is malformed (e.g. due to a DejaGnu EILSEQ
> +# crash).  We will warn about the file and continue processing the
> +# rest.
> +class ParseError (Exception):
> +    def __init__ (self, filename, message):
> +        Exception.__init__ (self, filename + ': ' + message)
> +        self.filename = filename
> +        self.message = message
> +
>  class Named:
>      def __init__ (self, name):
>          self.name = name
> @@ -205,7 +215,7 @@ class Prog:
>          try:
>              return int (value)
>          except ValueError:
> -            self.fatal (filename, 'expected an integer, got: ' + value)
> +            raise ParseError (filename, 'expected an integer, got: ' + value)
>
>      # Return a list that represents no test results.
>      def zero_counts (self):
> @@ -229,7 +239,7 @@ class Prog:
>          while True:
>              line = file.readline()
>              if line == '':
> -                self.fatal (filename, 'could not parse variation list')
> +                raise ParseError (filename, 'could not parse variation list')
>              if line == '\n':
>                  break
>              self.known_variations.add (line.strip())
> @@ -264,7 +274,7 @@ class Prog:
>          while True:
>              line = file.readline()
>              if line == '':
> -                self.fatal (filename, 'no recognised summary line')
> +                raise ParseError (filename, 'no recognised summary line')
>              if line == end:
>                  break
>
> @@ -292,7 +302,7 @@ class Prog:
>              match = self.result_re.match (line)
>              if match and (harness or not line.startswith ('WARNING:')):
>                  if not harness:
> -                    self.fatal (filename, 'saw test result before harness name')
> +                    raise ParseError (filename, 'saw test result before harness name')
>                  name = match.group (2)
>                  # Ugly hack to get the right order for gfortran.
>                  if name.startswith ('gfortran.dg/g77/'):
> @@ -354,7 +364,7 @@ class Prog:
>                      found = True
>                      break
>              if not found:
> -                self.fatal (filename, 'unknown test result: ' + line[:-1])
> +                raise ParseError (filename, 'unknown test result: ' + line[:-1])
>
>      # Parse an acats run, which uses a different format from dejagnu.
>      # We have just skipped over '=== acats configuration ==='.
> @@ -367,7 +377,7 @@ class Prog:
>          while True:
>              line = file.readline()
>              if line == '':
> -                self.fatal (filename, 'could not parse acats preamble')
> +                raise ParseError (filename, 'could not parse acats preamble')
>              if line == '\t\t=== acats tests ===\n':
>                  break
>              if record:
> @@ -423,9 +433,9 @@ class Prog:
>              if line.startswith ('Running target '):
>                  name = line[len ('Running target '):-1]
>                  if not tool:
> -                    self.fatal (filename, 'could not parse tool name')
> +                    raise ParseError (filename, 'could not parse tool name')
>                  if name not in self.known_variations:
> -                    self.fatal (filename, 'unknown target: ' + name)
> +                    raise ParseError (filename, 'unknown target: ' + name)
>                  self.parse_run (filename, file, tool,
>                                  tool.get_variation (name),
>                                  num_variations)
> @@ -474,7 +484,7 @@ class Prog:
>              # individual runs) and parse the version output.
>              if tool and line == '\t\t=== ' + tool.name + ' Summary ===\n':
>                  if file.readline() != '\n':
> -                    self.fatal (filename, 'expected blank line after summary')
> +                    raise ParseError (filename, 'expected blank line after summary')
>                  self.parse_final_summary (filename, file)
>                  continue
>
> @@ -490,7 +500,7 @@ class Prog:
>              # Sanity check to make sure that important text doesn't get
>              # dropped accidentally.
>              if strict and line.strip() != '':
> -                self.fatal (filename, 'unrecognised line: ' + line[:-1])
> +                raise ParseError (filename, 'unrecognised line: ' + line[:-1])
>
>      # Output a segment of text.
>      def output_segment (self, segment):
> @@ -569,8 +579,18 @@ class Prog:
>          try:
>              # Parse the input files.
>              for filename in self.files:
> -                with safe_open (filename) as file:
> -                    self.parse_file (filename, file)
> +                try:
> +                    with safe_open (filename) as file:
> +                        self.parse_file (filename, file)
> +                except ParseError as e:
> +                    # Partial state from this file is intentionally retained.
> +                    # This preserves any valid results and diagnostic ERROR
> +                    # lines that were parsed before the error, which is
> +                    # important for diagnosing problems like DejaGnu crashes.
> +                    # The unprocessed remainder of the file is lost.
> +                    sys.stderr.write ('warning: skipping ' + e.filename + ': '
> +                                      + e.message
> +                                      + '; results may be incomplete\n')
>
>              # Decide what to output.
>              if len (self.variations) == 0:
> --
> 2.54.0
>
  

On Thu, May 28, 2026 at 12:21 PM Andrew Pinski
<andrew.pinski@oss.qualcomm.com> wrote:
>
> On Thu, May 28, 2026 at 12:03 PM Tom Tromey <tom@tromey.com> wrote:
> >
> > >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:
> >
> > Kevin> This commit fixes this GDB bug:
> > Kevin> https://sourceware.org/bugzilla/show_bug.cgi?id=34147
> >
> > FWIW I think this should go in.
>
> This is on my list of patches to review this week. I hope to get to it
> today or tomorrow.
>
> >
> > It would be fine to land it in gdb and then merge it back.
> > I think we agreed that common files could be treated this way now.
> >
> > Kevin> +class ParseError (Exception):
> >
> > I guess gcc doesn't use 'black' for Python formatting.
>
> GCC coding style has a section on python formatting:
> https://gcc.gnu.org/codingconventions.html#python
> But I don't know much about python formatting to say much there.

So looking into this further I see nobody has been following that either.
I don't know about the history here either.
Now I do think we (GCC and GDB) should standardized on a format. In
this case the folks who knew python the most in the GCC community I
don't see around any more. So picking up the same formatting as gdb
would make sense. And we can get the forge to do the checking there
for us.

Thanks,
Andrea

>
> Thanks,
> Andrea
>
>
> >
> > thanks,
> > Tom
  

On 2026-05-28 19:38, Andrew Pinski wrote:
> On Thu, May 28, 2026 at 12:21 PM Andrew Pinski
> <andrew.pinski@oss.qualcomm.com> wrote:
>>
>> On Thu, May 28, 2026 at 12:03 PM Tom Tromey <tom@tromey.com> wrote:
>>>
>>>>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:
>>>
>>> Kevin> This commit fixes this GDB bug:
>>> Kevin> https://sourceware.org/bugzilla/show_bug.cgi?id=34147
>>>
>>> FWIW I think this should go in.
>>
>> This is on my list of patches to review this week. I hope to get to it
>> today or tomorrow.
>>
>>>
>>> It would be fine to land it in gdb and then merge it back.
>>> I think we agreed that common files could be treated this way now.
>>>
>>> Kevin> +class ParseError (Exception):
>>>
>>> I guess gcc doesn't use 'black' for Python formatting.
>>
>> GCC coding style has a section on python formatting:
>> https://gcc.gnu.org/codingconventions.html#python
>> But I don't know much about python formatting to say much there.
> 
> So looking into this further I see nobody has been following that either.
> I don't know about the history here either.
> Now I do think we (GCC and GDB) should standardized on a format. In
> this case the folks who knew python the most in the GCC community I
> don't see around any more. So picking up the same formatting as gdb
> would make sense. And we can get the forge to do the checking there
> for us.

In GDB we use the most up to date black version:

https://sourceware.org/gdb/wiki/Internals%20GDB-Python-Coding-Standards

Thanks to this, there is no thinking or discussion about formatting, it
is lovely.

Simon
  

On Thu, 28 May 2026 16:35:31 -0700
Andrew Pinski <andrew.pinski@oss.qualcomm.com> wrote:

> On Thu, May 14, 2026 at 11:18 AM Kevin Buettner <kevinb@redhat.com>
> wrote:
> >
> > This commit is for the benefit of GDB, but as the binutils-gdb
> > repository shares the contrib/ directory with GCC, this commit
> > must first be applied to GCC and then copied back to binutils-gdb.
> >
> > When running GDB tests in parallel (make check -j$(nproc)), the
> > consolidated gdb.sum and gdb.log files are produced by
> > contrib/dg-extract-results.py, which merges per-test output files.
> >
> > If any single per-test output file is malformed (e.g., due to a
> > DejaGnu EILSEQ crash, which is how I encountered this problem), the
> > script aborts via self.fatal().  Because this script is invoked via a
> > Makefile command using shell redirection, this causes the top-level
> > output files to be left as empty, zero-byte files, discarding valid
> > results from all other tests.
> >
> > Fix by making the script tolerant of unparseable input files.  Wrap
> > each file's parsing in a try/except block.  When a file cannot be
> > parsed, emit a warning to stderr and continue processing remaining
> > files.  This ensures that crashing tests do not destroy the
> > consolidated output for the entire parallel build.
> >
> > Tested on Fedora 44 using the GCC testsuite (make check-gcc
> > -j$(nproc)). The consolidated results are produced correctly with
> > no regressions.
> >
> > This commit fixes this GDB bug:
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=34147
> >
> > contrib/ChangeLog:
> >
> >         * dg-extract-results.py: Show warnings instead of erroring out
> >         when encountering an unparseable file.  
> 
> Ok.

This change has been pushed to the GCC repo.

It is my understanding that ChangeLog entries are automatically added
on a daily basis.  If that is incorrect, please let me know and I'll
add it by hand.

For copying back to GDB (binutils-gdb), I plan to wait until the
ChangeLog entry is in place.  Once that is done, I'll do the copy and
push to GDB as well.

Finally, I noticed some discussion regarding Python formatting.  I'll
note that I simply tried to follow the conventions already in use in
dg-extract-results.py.  But I do think it makes sense to use modern
Python formatting standards.  I'll leave it to someone else to
implement that.

Kevin
  

On Fri, 29 May 2026 13:04:03 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> For copying back to GDB (binutils-gdb), I plan to wait until the
> ChangeLog entry is in place.  Once that is done, I'll do the copy and
> push to GDB as well.

I found that binutils-gdb has a minimal contrib/ChangeLog file noting
when files have been copied from GCC.  I added an entry of my own
along with copying over dg-extract-results.py.  The commit log
describing the change and why it is needed is the same as what went
into gcc though.

In any case, it's pushed.  That means that we'll no longer see empty
gdb.sum and gdb.log files when testing Fedora 44 and Rawhide.

In addition, I pushed another fix earlier today which fixes the
UTF-8 issues which were causing the zero-sized files:

[gdb/testsuite, Tcl 9] Fix EILSEQ problems for UTF8 related tests

Kevin