[Bug analyzer/114348] New: Corrupt SARIF output on stderr

[Bug analyzer/114348] New: Corrupt SARIF output on stderr

[specht.tobias at gmx dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114348

            Bug ID: 114348
           Summary: Corrupt SARIF output on stderr
           Product: gcc
           Version: 13.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: specht.tobias at gmx dot de
  Target Milestone: ---

I noticed a situation, where the SARIF output on stderr of the `-fanalyzer` is
corrupted by a compiler error message, also written to stderr:

```
$ cat test.c
#include "test.h"
$ gcc -fanalyzer -fdiagnostics-format=sarif-stderr -c test.c
{"$schema":
"https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
"version": "2.1.0", "runs": [{"tool": {"driver": {"name": "GNU C17",
"fullName": "GNU C17 (GCC) version 13.2.1 20231205 (Red Hat 13.2.1-6)
(x86_64-redhat-linux)", "version": "13.2.1 20231205 (Red Hat 13.2.1-6)",
"informationUri": "https://gcc.gnu.org/gcc-13/", "rules": []}}, "invocations":
[{"executionSuccessful": true, "toolExecutionNotifications": []}],
"originalUriBaseIds": {"PWD": {"uri": "file:///tmp"}}, "artifacts":
[{"location": {"uri": "test.c", "uriBaseId": "PWD"}, "contents": {"text":
"#include \"test.h\"\n"}, "sourceLanguage": "c"}], "results": [{"ruleId":
"fatal error", "message": {"text": "test.h: No such file or directory"},
"locations": [{"physicalLocation": {"artifactLocation": {"uri": "test.c",
"uriBaseId": "PWD"}, "region": {"startLine": 1, "startColumn": 10, "endColumn":
18}, "contextRegion": {"startLine": 1, "snippet": {"text": "#include
\"test.h\"\n"}}}}]}]}]}
compilation terminated.
```

The message `compilation terminated.` is written to stderr.

This happens when a header file is not found (also noted in the SARIF file).
Other compiler errors (e.g. missing symbol, syntax error) do not show this
problem and only generate SARIF output on stderr.
Maybe this bug is related to the preprocessor?

[Bug analyzer/114348] Corrupt SARIF output on stderr

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114348

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
It comes from emitting a fatal error:

    case DK_FATAL:
      if (m_abort_on_error)
        real_abort ();
      finish ();
      fnotice (stderr, "compilation terminated.\n");
      exit (FATAL_EXIT_CODE);

Which a missing header file is.

[Bug middle-end/114348] Corrupt SARIF output on stderr

[specht.tobias at gmx dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114348

--- Comment #2 from Tobias Specht <specht.tobias at gmx dot de> ---
From my understanding, the idea of the `-fdiagnostics-format=sarif-stderr`
option is, that the SARIF file will be printed to stderr and only the SARIF
file, so that one can take the stderr output and parse it as json for further
processing.
Due to the additional plain text output, the output on stderr is corrupted and
cannot be parsed as json.
A workaround could be, to only parse the first line as json, but this also
seems racy.

[Bug middle-end/114348] Corrupt SARIF output on stderr

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114348

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-03-18

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for reporting this.

Note that -fanalyzer isn't needed to reproduce this problem, e.g. on trunk
with:

$ (./xgcc -B. -fdiagnostics-format=sarif-stderr -c test.c 2>&1) | python -m
json.tool
Extra data: line 24 column 1 (char 1839)

Also affects -fdiagnostics-format=json-stderr.

fnotice (stderr, ...) is used in ~150 places in trunk.

I'm looking at ways of fixing this (perhaps by having fnotice bail out early on
these machine-readable stderr formats when outputting to stderr).

[Bug middle-end/114348] Corrupt SARIF output on stderr

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114348

--- Comment #4 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:0bf99b1b7eda2f4c34b9f56b895980ea1c261765

commit r14-9554-g0bf99b1b7eda2f4c34b9f56b895980ea1c261765
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Tue Mar 19 13:57:35 2024 -0400

    diagnostics: fix corrupt json/SARIF on stderr [PR114348]

    Various values of -fdiagnostics-format= request machine-readable output
    on stderr, using JSON, but in various places we use fnotice to write
    free-form text to stderr, such as "compilation terminated", leading to
    corrupt JSON.

    Fix by having fnotice skip the output for such cases.

    gcc/ChangeLog:
            PR middle-end/114348
            * diagnostic-format-json.cc
            (json_stderr_output_format::machine_readable_stderr_p): New.
            (json_file_output_format::machine_readable_stderr_p): New.
            * diagnostic-format-sarif.cc
            (sarif_stream_output_format::machine_readable_stderr_p): New.
            (sarif_file_output_format::machine_readable_stderr_p): New.
            * diagnostic.cc (diagnostic_context::action_after_output): Move
            "fnotice" to before "finish" call, so that we still have the
            diagnostic_context.
            (fnotice): Bail out if the user requested one of the
            machine-readable diagnostic output formats on stderr.
            * diagnostic.h
            (diagnostic_output_format::machine_readable_stderr_p): New pure
            virtual function.
            (diagnostic_text_output_format::machine_readable_stderr_p): New.
            (diagnostic_context::get_output_format): New accessor.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug middle-end/114348] Corrupt SARIF output on stderr

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114348

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed on trunk for GCC 14 by the above patch.  Keeping open to
backport.

(In reply to Tobias Specht from comment #2)
[...snip...]
> A workaround could be, to only parse the first line as json, but this also
> seems racy.

Note that although in earlier releases the JSON was all on one line, for GCC 14
I've added newlines and formatting to the output:
https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639625.html  (which
I've found *very* useful in my own usage of SARIF output).

[Bug middle-end/114348] Corrupt SARIF output on stderr

[specht.tobias at gmx dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114348

--- Comment #6 from Tobias Specht <specht.tobias at gmx dot de> ---
Thanks for fixing this!
Waiting for the gcc 13 backport.

Having formatted json output sounds like a nice feature for me too.

[Bug middle-end/114348] Corrupt SARIF output on stderr

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114348

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by David Malcolm
<dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:b7a2697733d19a093cbdd0e200ffce069a4bc812

commit r13-8761-gb7a2697733d19a093cbdd0e200ffce069a4bc812
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu May 9 13:09:33 2024 -0400

    diagnostics: fix corrupt json/SARIF on stderr [PR114348]

    Various values of -fdiagnostics-format= request machine-readable output
    on stderr, using JSON, but in various places we use fnotice to write
    free-form text to stderr, such as "compilation terminated", leading to
    corrupt JSON.

    Fix by having fnotice skip the output for such cases.

    Backported from r14-9554-g0bf99b1b7eda2f (using a variable rather
    than a vfunc of class diagnostic_output_format, since the latter
    was added in gcc 14)

    gcc/ChangeLog:
            PR middle-end/114348
            * diagnostic.cc (output_format): New variable.
            (fnotice): Bail out if the user requested one of the
            machine-readable diagnostic output formats on stderr.
            (diagnostic_output_format_init): Set output_format.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug middle-end/114348] Corrupt SARIF output on stderr

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114348

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed on GCC 13 for the upcoming GCC 13.3 by the above patch. 

I'm not planning to backport this further; closing.