https://gcc.gnu.org/g:38cb4289180d13a71c2e1005223f442747bcf56e
commit r16-3827-g38cb4289180d13a71c2e1005223f442747bcf56e Author: David Malcolm <[email protected]> Date: Fri Sep 12 10:24:36 2025 -0400 diagnostics: fix crash-handling inside nested diagnostics [PR121876] 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. 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. Signed-off-by: David Malcolm <[email protected]> Diff: --- gcc/diagnostics/buffering.cc | 5 +++ .../gcc.dg/plugin/crash-test-nested-ice-html.py | 42 +++++++++++++++++++ .../gcc.dg/plugin/crash-test-nested-ice-sarif.py | 47 ++++++++++++++++++++++ .../gcc.dg/plugin/crash-test-nested-ice.c | 25 ++++++++++++ .../crash-test-nested-write-through-null-html.py | 42 +++++++++++++++++++ .../crash-test-nested-write-through-null-sarif.py | 47 ++++++++++++++++++++++ .../plugin/crash-test-nested-write-through-null.c | 25 ++++++++++++ gcc/testsuite/gcc.dg/plugin/crash_test_plugin.cc | 44 ++++++++++++++++++-- gcc/testsuite/gcc.dg/plugin/plugin.exp | 2 + 9 files changed, 276 insertions(+), 3 deletions(-) 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 \
