PR diagnostics/121876 tracks an issue inside our crash-handling, where
if an ICE happens when we're within a nested diagnostic, an assertion
fails inside diagnostic::context::set_diagnostic_buffer, leading to
a 2nd ICE.  Happily, this does not infinitely recurse, but it obscures
the original ICE and the useful part of the backtrace, and any SARIF or
HTML sinks we were writing to are left as empty files.

This patch tweaks the above so that the assertion doesn't fail, and adds
test coverage (via a plugin) to ensure that such ICEs/crashes are
gracefully handled and e.g. captured in SARIF/HTML output.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r16-3827-g38cb4289180d13.

gcc/ChangeLog:
        PR diagnostics/121876
        * diagnostics/buffering.cc (context::set_diagnostic_buffer): Add
        early reject of the no-op case.

gcc/testsuite/ChangeLog:
        PR diagnostics/121876
        * gcc.dg/plugin/crash-test-nested-ice-html.py: New test.
        * gcc.dg/plugin/crash-test-nested-ice-sarif.py: New test.
        * gcc.dg/plugin/crash-test-nested-ice.c: New test.
        * gcc.dg/plugin/crash-test-nested-write-through-null-html.py: New test.
        * gcc.dg/plugin/crash-test-nested-write-through-null-sarif.py: New test.
        * gcc.dg/plugin/crash-test-nested-write-through-null.c: New test.
        * gcc.dg/plugin/crash_test_plugin.cc: Add "nested" argument, and when
        set, inject the problem within a nested diagnostic.
        * gcc.dg/plugin/plugin.exp: Add crash-test-nested-ice.c and
        crash-test-nested-write-through-null.c.
---
 gcc/diagnostics/buffering.cc                  |  5 ++
 .../plugin/crash-test-nested-ice-html.py      | 42 +++++++++++++++++
 .../plugin/crash-test-nested-ice-sarif.py     | 47 +++++++++++++++++++
 .../gcc.dg/plugin/crash-test-nested-ice.c     | 25 ++++++++++
 ...ash-test-nested-write-through-null-html.py | 42 +++++++++++++++++
 ...sh-test-nested-write-through-null-sarif.py | 47 +++++++++++++++++++
 .../crash-test-nested-write-through-null.c    | 25 ++++++++++
 .../gcc.dg/plugin/crash_test_plugin.cc        | 44 +++++++++++++++--
 gcc/testsuite/gcc.dg/plugin/plugin.exp        |  2 +
 9 files changed, 276 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice-html.py
 create mode 100644 gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice-sarif.py
 create mode 100644 gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice.c
 create mode 100644 
gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null-html.py
 create mode 100644 
gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null-sarif.py
 create mode 100644 
gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null.c

diff --git a/gcc/diagnostics/buffering.cc b/gcc/diagnostics/buffering.cc
index a7747b5a8f04..b09d1c2cc157 100644
--- a/gcc/diagnostics/buffering.cc
+++ b/gcc/diagnostics/buffering.cc
@@ -39,6 +39,11 @@ namespace diagnostics {
 void
 context::set_diagnostic_buffer (buffer *buffer_)
 {
+  /* Early reject of no-op (to avoid recursively crashing when handling an
+     ICE when inside a nested diagnostics; see PR diagnostics/121876).  */
+  if (buffer_ == m_diagnostic_buffer)
+    return;
+
   /* We don't allow changing buffering within a diagnostic group
      (to simplify handling of buffered diagnostics within the
      diagnostic_format implementations).  */
diff --git a/gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice-html.py 
b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice-html.py
new file mode 100644
index 000000000000..d1de6c53bebc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice-html.py
@@ -0,0 +1,42 @@
+from htmltest import *
+
+import pytest
+
[email protected](scope='function', autouse=True)
+def html_tree():
+    return html_tree_from_env()
+
+def test_results(html_tree):
+    root = html_tree.getroot ()
+    assert root.tag == make_tag('html')
+
+    head = root.find('xhtml:head', ns)
+    assert head is not None
+
+    body = root.find('xhtml:body', ns)
+    assert body is not None
+
+    diag_list = body.find("./xhtml:div[@class='gcc-diagnostic-list']", ns)
+    assert len(diag_list)
+
+    # Currently the ICE appears nested within the parent error
+    diag = diag_list.find('xhtml:div', ns)
+    assert diag is not None
+    assert diag.attrib['class'] == 'alert alert-danger'
+
+    icon = diag.find('xhtml:span', ns)
+    assert icon.attrib['class'] == 'pficon pficon-error-circle-o'
+
+    # The message line for the parent error:
+    message = diag.find("./xhtml:div[@class='gcc-message']", ns)
+    assert message is not None
+    assert message[0].tag == make_tag('strong')
+    assert message[0].text == 'error: '
+    assert message[0].tail == " placeholder"
+
+    # The ICE
+    ice = diag.find('.//xhtml:div[@class="gcc-diagnostic"]', ns)
+    assert ice is not None
+    ice_msg = ice.find("./xhtml:div[@class='gcc-message']", ns)
+    assert ice_msg is not None
+    assert ice_msg.text == "I'm sorry Dave, I'm afraid I can't do that"
diff --git a/gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice-sarif.py 
b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice-sarif.py
new file mode 100644
index 000000000000..7e32eb0bb08e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice-sarif.py
@@ -0,0 +1,47 @@
+from sarif import *
+
+import pytest
+
[email protected](scope='function', autouse=True)
+def sarif():
+    return sarif_from_env()
+
+def test_execution_failed(sarif):
+    runs = sarif['runs']
+    run = runs[0]
+
+    invocations = run['invocations']
+    assert len(invocations) == 1
+    invocation = invocations[0]
+
+    assert invocation['executionSuccessful'] == False
+
+def test_notification(sarif):
+    # We expect an execution notification for the ICE in the invocation
+    runs = sarif['runs']
+    run = runs[0]
+
+    invocations = run['invocations']
+    assert len(invocations) == 1
+    invocation = invocations[0]
+
+    assert invocation['executionSuccessful'] == False
+
+    notifications = invocation['toolExecutionNotifications']
+    assert len(notifications) == 1
+
+    notification = notifications[0]
+
+    assert notification['message']['text'] == "I'm sorry Dave, I'm afraid I 
can't do that"
+    assert notification['level'] == 'error'
+
+    loc0 = notification['locations'][0]
+    assert get_location_artifact_uri(loc0).endswith('crash-test-nested-ice.c')
+    assert 'inject_ice ();' in get_location_snippet_text(loc0)
+
+    # We may have backtrace information
+    if 'properties' in notification:
+        backtrace = notification['properties']['gcc/backtrace']
+        assert 'frames' in backtrace
+        # Ideally we should have a frame for 
pass_crash_test::execute(function*)
+        # but we can't rely on this.
diff --git a/gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice.c 
b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice.c
new file mode 100644
index 000000000000..db8bc2aecdb0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-ice.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fno-report-bug" } */
+/* { dg-additional-options "-fplugin-arg-crash_test_plugin-nested" } */
+/* { dg-additional-options "-fdiagnostics-add-output=sarif" } */
+/* { dg-additional-options 
"-fdiagnostics-add-output=experimental-html:javascript=no" } */
+
+extern void inject_ice (void);
+
+void test_1 (void)
+{
+  inject_ice ();   /* { dg-ice "I'm sorry Dave, I'm afraid I can't do that" } 
*/
+  /* { dg-error "placeholder" "" { target *-*-* } .-1 } */
+  /* { dg-regexp "during GIMPLE pass: crash_test" } */
+}
+
+/* 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 crash-test-nested-ice.c 
"crash-test-nested-ice-sarif.py" } } */
+
+/* Use a Python script to verify various properties about the generated
+   .html file:
+   { dg-final { run-html-pytest crash-test-nested-ice.c 
"crash-test-nested-ice-html.py" } } */
diff --git 
a/gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null-html.py 
b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null-html.py
new file mode 100644
index 000000000000..cc76baaeab07
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null-html.py
@@ -0,0 +1,42 @@
+from htmltest import *
+
+import pytest
+
[email protected](scope='function', autouse=True)
+def html_tree():
+    return html_tree_from_env()
+
+def test_results(html_tree):
+    root = html_tree.getroot ()
+    assert root.tag == make_tag('html')
+
+    head = root.find('xhtml:head', ns)
+    assert head is not None
+
+    body = root.find('xhtml:body', ns)
+    assert body is not None
+
+    diag_list = body.find("./xhtml:div[@class='gcc-diagnostic-list']", ns)
+    assert len(diag_list)
+
+    # Currently the ICE appears nested within the parent error
+    diag = diag_list.find('xhtml:div', ns)
+    assert diag is not None
+    assert diag.attrib['class'] == 'alert alert-danger'
+
+    icon = diag.find('xhtml:span', ns)
+    assert icon.attrib['class'] == 'pficon pficon-error-circle-o'
+
+    # The message line for the parent error:
+    message = diag.find("./xhtml:div[@class='gcc-message']", ns)
+    assert message is not None
+    assert message[0].tag == make_tag('strong')
+    assert message[0].text == 'error: '
+    assert message[0].tail == " placeholder"
+
+    # The ICE
+    ice = diag.find('.//xhtml:div[@class="gcc-diagnostic"]', ns)
+    assert ice is not None
+    ice_msg = ice.find("./xhtml:div[@class='gcc-message']", ns)
+    assert ice_msg is not None
+    assert ice_msg.text == "Segmentation fault"
diff --git 
a/gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null-sarif.py 
b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null-sarif.py
new file mode 100644
index 000000000000..d97e4b94cea6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null-sarif.py
@@ -0,0 +1,47 @@
+from sarif import *
+
+import pytest
+
[email protected](scope='function', autouse=True)
+def sarif():
+    return sarif_from_env()
+
+def test_execution_failed(sarif):
+    runs = sarif['runs']
+    run = runs[0]
+
+    invocations = run['invocations']
+    assert len(invocations) == 1
+    invocation = invocations[0]
+
+    assert invocation['executionSuccessful'] == False
+
+def test_notification(sarif):
+    # We expect an execution notification for the ICE in the invocation
+    runs = sarif['runs']
+    run = runs[0]
+
+    invocations = run['invocations']
+    assert len(invocations) == 1
+    invocation = invocations[0]
+
+    assert invocation['executionSuccessful'] == False
+
+    notifications = invocation['toolExecutionNotifications']
+    assert len(notifications) == 1
+
+    notification = notifications[0]
+
+    assert notification['message']['text'] == "Segmentation fault"
+    assert notification['level'] == 'error'
+
+    loc0 = notification['locations'][0]
+    assert 
get_location_artifact_uri(loc0).endswith('crash-test-nested-write-through-null.c')
+    assert 'inject_write_through_null ();' in get_location_snippet_text(loc0)
+
+    # We may have backtrace information
+    if 'properties' in notification:
+        backtrace = notification['properties']['gcc/backtrace']
+        assert 'frames' in backtrace
+        # Ideally we should have a frame for 
pass_crash_test::execute(function*)
+        # but we can't rely on this.
diff --git a/gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null.c 
b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null.c
new file mode 100644
index 000000000000..bd6a21a83bfb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/crash-test-nested-write-through-null.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fno-report-bug" } */
+/* { dg-additional-options "-fplugin-arg-crash_test_plugin-nested" } */
+/* { dg-additional-options "-fdiagnostics-add-output=sarif" } */
+/* { dg-additional-options 
"-fdiagnostics-add-output=experimental-html:javascript=no" } */
+
+extern void inject_write_through_null (void);
+
+void test_inject_write_through_null (void)
+{
+  inject_write_through_null (); /* { dg-ice "Segmentation fault" } */ 
+  /* { dg-error "placeholder" "" { target *-*-* } .-1 } */
+  /* { dg-regexp "during GIMPLE pass: crash_test" } */
+}
+
+/* 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 crash-test-nested-write-through-null.c 
"crash-test-nested-write-through-null-sarif.py" } } */
+
+/* Use a Python script to verify various properties about the generated
+   .html file:
+   { dg-final { run-html-pytest crash-test-nested-write-through-null.c 
"crash-test-nested-write-through-null-html.py" } } */
diff --git a/gcc/testsuite/gcc.dg/plugin/crash_test_plugin.cc 
b/gcc/testsuite/gcc.dg/plugin/crash_test_plugin.cc
index 03ad096964b3..27c027da6a67 100644
--- a/gcc/testsuite/gcc.dg/plugin/crash_test_plugin.cc
+++ b/gcc/testsuite/gcc.dg/plugin/crash_test_plugin.cc
@@ -50,14 +50,20 @@ const pass_data pass_data_crash_test =
 class pass_crash_test : public gimple_opt_pass
 {
 public:
-  pass_crash_test(gcc::context *ctxt)
-    : gimple_opt_pass(pass_data_crash_test, ctxt)
+  pass_crash_test (gcc::context *ctxt, bool nested)
+  : gimple_opt_pass (pass_data_crash_test, ctxt),
+    m_nested (nested)
   {}
 
   /* opt_pass methods: */
   bool gate (function *) final override { return true; }
   unsigned int execute (function *) final override;
 
+private:
+  /* If true, inject the crash whilst within a nested diagnostic
+     (PR diagnostics/121876).  */
+  bool m_nested;
+
 }; // class pass_test_groups
 
 /* Determine if STMT is a call to a function named FUNCNAME.
@@ -94,15 +100,39 @@ pass_crash_test::execute (function *fun)
        gimple *stmt = gsi_stmt (gsi);
        if (gcall *call = check_for_named_call (stmt, "inject_ice"))
          {
+           auto_diagnostic_group d;
+           if (m_nested)
+             {
+               error_at (stmt->location, "placeholder");
+               global_dc->push_nesting_level ();
+             }
+
            input_location = stmt->location;
            internal_error ("I'm sorry Dave, I'm afraid I can't do that");
+
+           if (m_nested)
+             {
+               global_dc->pop_nesting_level ();
+             }
          }
        if (gcall *call = check_for_named_call (stmt,
                                                "inject_write_through_null"))
          {
+           auto_diagnostic_group d;
+           if (m_nested)
+             {
+               error_at (stmt->location, "placeholder");
+               global_dc->push_nesting_level ();
+             }
+
            input_location = stmt->location;
            int *p = NULL;
            *p = 42;
+
+           if (m_nested)
+             {
+               global_dc->pop_nesting_level ();
+             }
          }
       }
 
@@ -124,7 +154,15 @@ plugin_init (struct plugin_name_args *plugin_info,
   if (!plugin_default_version_check (version, &gcc_version))
     return 1;
 
-  pass_info.pass = new pass_crash_test (g);
+  bool nested = false;
+
+  for (int i = 0; i < argc; i++)
+    {
+      if (!strcmp (argv[i].key, "nested"))
+       nested = true;
+    }
+
+  pass_info.pass = new pass_crash_test (g, nested);
   pass_info.reference_pass_name = "*warn_function_noreturn";
   pass_info.ref_pass_instance_number = 1;
   pass_info.pos_op = PASS_POS_INSERT_AFTER;
diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp 
b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index 3d7ba3f38c3b..38991e8e6191 100644
--- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
+++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
@@ -74,6 +74,8 @@ set plugin_test_list [list \
          crash-test-ice-in-header-sarif-2.2.c \
          crash-test-ice-sarif.c \
          crash-test-ice-stderr.c \
+         crash-test-nested-ice.c \
+         crash-test-nested-write-through-null.c \
          crash-test-write-through-null-sarif.c \
          crash-test-write-through-null-stderr.c } \
     { diagnostic_group_plugin.cc \
-- 
2.26.3

Reply via email to