diff mbox series

[pushed] diagnostics: SARIF output: fix "executionSuccessful" §3.20.14 [PR116177]

Message ID 20240806223115.1320557-1-dmalcolm@redhat.com
State New
Headers show
Series [pushed] diagnostics: SARIF output: fix "executionSuccessful" §3.20.14 [PR116177] | expand

Commit Message

David Malcolm Aug. 6, 2024, 10:31 p.m. UTC
Previously the invocation's "executionSuccessful" property (§3.20.14)
was only false if there was an ICE.

Update it so that it will also be false if we will exit with a non-zero
exit code (due to errors, Werror, and "sorry").

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r15-2769-g77f36e8016e11c.

gcc/ChangeLog:
	PR other/116177
	* diagnostic-format-sarif.cc (sarif_invocation::prepare_to_flush):
	If the diagnostics would lead to us exiting with a failure code,
	then emit "executionSuccessful": False (SARIF v2.1.0 section
	§3.20.14).
	* diagnostic.cc (diagnostic_context::execution_failed_p): New.
	* diagnostic.h (diagnostic_context::execution_failed_p): New decl.
	* toplev.cc (toplev::main): Use it for determining returned value.

gcc/testsuite/ChangeLog:
	PR other/116177
	* gcc.dg/sarif-output/include-chain-2.c: Remove pruning of
	"exit status is 1", as we expect this to exit with 0.
	* gcc.dg/sarif-output/no-diagnostics.c: New test.
	* gcc.dg/sarif-output/test-include-chain-1.py
	(test_execution_unsuccessful): Add.
	* gcc.dg/sarif-output/test-include-chain-2.py
	(test_execution_successful): Add.
	* gcc.dg/sarif-output/test-missing-semicolon.py
	(test_execution_unsuccessful): Add.
	* gcc.dg/sarif-output/test-no-diagnostics.py: New test.
	* gcc.dg/sarif-output/test-werror.py: New test.
	* gcc.dg/sarif-output/werror.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/diagnostic-format-sarif.cc                |  2 +
 gcc/diagnostic.cc                             | 13 +++++++
 gcc/diagnostic.h                              |  2 +
 .../gcc.dg/sarif-output/include-chain-2.c     |  5 ---
 .../gcc.dg/sarif-output/no-diagnostics.c      | 13 +++++++
 .../sarif-output/test-include-chain-1.py      | 11 ++++++
 .../sarif-output/test-include-chain-2.py      | 11 ++++++
 .../sarif-output/test-missing-semicolon.py    | 11 ++++++
 .../sarif-output/test-no-diagnostics.py       | 31 +++++++++++++++
 .../gcc.dg/sarif-output/test-werror.py        | 39 +++++++++++++++++++
 gcc/testsuite/gcc.dg/sarif-output/werror.c    | 18 +++++++++
 gcc/toplev.cc                                 |  2 +-
 12 files changed, 152 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/sarif-output/no-diagnostics.c
 create mode 100644 gcc/testsuite/gcc.dg/sarif-output/test-no-diagnostics.py
 create mode 100644 gcc/testsuite/gcc.dg/sarif-output/test-werror.py
 create mode 100644 gcc/testsuite/gcc.dg/sarif-output/werror.c
diff mbox series

Patch

diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-format-sarif.cc
index 7c2e96f4f746..963a185f6ced 100644
--- a/gcc/diagnostic-format-sarif.cc
+++ b/gcc/diagnostic-format-sarif.cc
@@ -811,6 +811,8 @@  void
 sarif_invocation::prepare_to_flush (diagnostic_context &context)
 {
   /* "executionSuccessful" property (SARIF v2.1.0 section 3.20.14).  */
+  if (context.execution_failed_p ())
+    m_success = false;
   set_bool ("executionSuccessful", m_success);
 
   /* "toolExecutionNotifications" property (SARIF v2.1.0 section 3.20.21).  */
diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index 71d2f44e40c8..3fc81ad47f56 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -399,6 +399,19 @@  diagnostic_context::finish ()
   m_original_argv = nullptr;
 }
 
+/* Return true if sufficiently severe diagnostics have been seen that
+   we ought to exit with a non-zero exit code.  */
+
+bool
+diagnostic_context::execution_failed_p () const
+{
+  /* Equivalent to (seen_error () || werrorcount), but on
+     this context, rather than global_dc.  */
+  return (m_diagnostic_count [DK_ERROR]
+	  || m_diagnostic_count [DK_SORRY]
+	  || m_diagnostic_count [DK_WERROR]);
+}
+
 void
 diagnostic_context::set_output_format (diagnostic_output_format *output_format)
 {
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 79386ccbf856..83180ded414d 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -392,6 +392,8 @@  public:
 
   void finish ();
 
+  bool execution_failed_p () const;
+
   void set_original_argv (unique_argv original_argv);
   const char * const *get_original_argv ()
   {
diff --git a/gcc/testsuite/gcc.dg/sarif-output/include-chain-2.c b/gcc/testsuite/gcc.dg/sarif-output/include-chain-2.c
index 3f984f48979b..a04b647d259e 100644
--- a/gcc/testsuite/gcc.dg/sarif-output/include-chain-2.c
+++ b/gcc/testsuite/gcc.dg/sarif-output/include-chain-2.c
@@ -27,11 +27,6 @@  PATH/include-chain-2.h:6:3: warning: double-'free' of 'ptr' [CWE-415] [-Wanalyze
 
 #include "include-chain-2.h"
 
-/* We expect a failing compile due to the errors, but the use of 
-   -fdiagnostics-format=sarif-file means there should be no output to stderr.
-   DejaGnu injects this message; ignore it:
-   { dg-prune-output "exit status is 1" } */
-
 /* Verify that some JSON was written to a file with the expected name:
    { dg-final { verify-sarif-file } } */
 
diff --git a/gcc/testsuite/gcc.dg/sarif-output/no-diagnostics.c b/gcc/testsuite/gcc.dg/sarif-output/no-diagnostics.c
new file mode 100644
index 000000000000..2536e2ec91b0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sarif-output/no-diagnostics.c
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fdiagnostics-format=sarif-file" } */
+
+/* Verify our SARIF output for a translation unit with no diagnostics.  */
+
+int nonempty;
+
+/* Verify that some JSON was written to a file with the expected name:
+   { dg-final { verify-sarif-file } } */
+
+/* Use a Python script to verify various properties about the generated
+   .sarif file:
+   { dg-final { run-sarif-pytest no-diagnostics.c "test-no-diagnostics.py" } } */
diff --git a/gcc/testsuite/gcc.dg/sarif-output/test-include-chain-1.py b/gcc/testsuite/gcc.dg/sarif-output/test-include-chain-1.py
index 16cd6a6ac4d9..4bb2ebf61473 100644
--- a/gcc/testsuite/gcc.dg/sarif-output/test-include-chain-1.py
+++ b/gcc/testsuite/gcc.dg/sarif-output/test-include-chain-1.py
@@ -13,6 +13,17 @@  def test_basics(sarif):
     version = sarif['version']
     assert version == "2.1.0"
 
+def test_execution_unsuccessful(sarif):
+    runs = sarif['runs']
+    run = runs[0]
+
+    invocations = run['invocations']
+    assert len(invocations) == 1
+    invocation = invocations[0]
+
+    # We expect the errors to make executionSuccessful be false
+    assert invocation['executionSuccessful'] == False
+
 def test_location_relationships(sarif):
     runs = sarif['runs']
     run = runs[0]
diff --git a/gcc/testsuite/gcc.dg/sarif-output/test-include-chain-2.py b/gcc/testsuite/gcc.dg/sarif-output/test-include-chain-2.py
index aea9aabb5ef5..761fe1b59a9c 100644
--- a/gcc/testsuite/gcc.dg/sarif-output/test-include-chain-2.py
+++ b/gcc/testsuite/gcc.dg/sarif-output/test-include-chain-2.py
@@ -31,6 +31,17 @@  def test_basics(sarif):
     version = sarif['version']
     assert version == "2.1.0"
 
+def test_execution_successful(sarif):
+    runs = sarif['runs']
+    run = runs[0]
+
+    invocations = run['invocations']
+    assert len(invocations) == 1
+    invocation = invocations[0]
+
+    # We expect a mere 'warning' to allow executionSuccessful be true
+    assert invocation['executionSuccessful'] == True
+
 def test_result(sarif):
     runs = sarif['runs']
     run = runs[0]
diff --git a/gcc/testsuite/gcc.dg/sarif-output/test-missing-semicolon.py b/gcc/testsuite/gcc.dg/sarif-output/test-missing-semicolon.py
index 795980d88cc4..17759d35a468 100644
--- a/gcc/testsuite/gcc.dg/sarif-output/test-missing-semicolon.py
+++ b/gcc/testsuite/gcc.dg/sarif-output/test-missing-semicolon.py
@@ -13,6 +13,17 @@  def test_basics(sarif):
     version = sarif['version']
     assert version == "2.1.0"
 
+def test_execution_unsuccessful(sarif):
+    runs = sarif['runs']
+    run = runs[0]
+
+    invocations = run['invocations']
+    assert len(invocations) == 1
+    invocation = invocations[0]
+
+    # We expect the 'error' to make executionSuccessful be false
+    assert invocation['executionSuccessful'] == False
+
 def test_location_relationships(sarif):
     runs = sarif['runs']
     run = runs[0]
diff --git a/gcc/testsuite/gcc.dg/sarif-output/test-no-diagnostics.py b/gcc/testsuite/gcc.dg/sarif-output/test-no-diagnostics.py
new file mode 100644
index 000000000000..f5812df17dc0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sarif-output/test-no-diagnostics.py
@@ -0,0 +1,31 @@ 
+from sarif import *
+
+import pytest
+
+@pytest.fixture(scope='function', autouse=True)
+def sarif():
+    return sarif_from_env()
+
+def test_basics(sarif):
+    schema = sarif['$schema']
+    assert schema == "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json"
+
+    version = sarif['version']
+    assert version == "2.1.0"
+
+def test_execution_successful(sarif):
+    runs = sarif['runs']
+    run = runs[0]
+
+    invocations = run['invocations']
+    assert len(invocations) == 1
+    invocation = invocations[0]
+
+    assert invocation['executionSuccessful'] == True
+    assert invocation['toolExecutionNotifications'] == []
+
+def test_empty_results(sarif):
+    runs = sarif['runs']
+    run = runs[0]
+    results = run['results']
+    assert len(results) == 0
diff --git a/gcc/testsuite/gcc.dg/sarif-output/test-werror.py b/gcc/testsuite/gcc.dg/sarif-output/test-werror.py
new file mode 100644
index 000000000000..99c2c2c97919
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sarif-output/test-werror.py
@@ -0,0 +1,39 @@ 
+from sarif import *
+
+import pytest
+
+@pytest.fixture(scope='function', autouse=True)
+def sarif():
+    return sarif_from_env()
+
+def test_basics(sarif):
+    schema = sarif['$schema']
+    assert schema == "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json"
+
+    version = sarif['version']
+    assert version == "2.1.0"
+
+def test_execution_unsuccessful(sarif):
+    runs = sarif['runs']
+    run = runs[0]
+
+    invocations = run['invocations']
+    assert len(invocations) == 1
+    invocation = invocations[0]
+
+    assert '-Werror=unused-variable' in invocation['arguments']
+
+    # We expect the 'Werror' to make executionSuccessful be false
+    assert invocation['executionSuccessful'] == False
+
+def test_result(sarif):
+    runs = sarif['runs']
+    run = runs[0]
+    results = run['results']
+
+    assert len(results) == 1
+    
+    result = results[0]
+    assert result['ruleId'] == '-Werror=unused-variable'
+    assert result['level'] == 'error'
+    assert result['message']['text'] == "'ununsed' defined but not used"
diff --git a/gcc/testsuite/gcc.dg/sarif-output/werror.c b/gcc/testsuite/gcc.dg/sarif-output/werror.c
new file mode 100644
index 000000000000..fa9eb9a1ba06
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sarif-output/werror.c
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Werror=unused-variable -fdiagnostics-format=sarif-file" } */
+
+/* Verify our SARIF output for a translation unit with -Werror.  */
+
+static int ununsed;
+
+/* We expect a failing compile due to the Werror, but the use of 
+   -fdiagnostics-format=sarif-file means there should be no output to stderr.
+   DejaGnu injects this message; ignore it:
+   { dg-prune-output "exit status is 1" } */
+
+/* Verify that some JSON was written to a file with the expected name:
+   { dg-final { verify-sarif-file } } */
+
+/* Use a Python script to verify various properties about the generated
+   .sarif file:
+   { dg-final { run-sarif-pytest werror.c "test-werror.py" } } */
diff --git a/gcc/toplev.cc b/gcc/toplev.cc
index 7f19d5c52bd9..eee4805b504a 100644
--- a/gcc/toplev.cc
+++ b/gcc/toplev.cc
@@ -2355,7 +2355,7 @@  toplev::main (int argc, char **argv)
 
   after_memory_report = true;
 
-  if (seen_error () || werrorcount)
+  if (global_dc->execution_failed_p ())
     return (FATAL_EXIT_CODE);
 
   return (SUCCESS_EXIT_CODE);