sarif output: Fix ICE due to overzealous caching [PR124014]

PR diagnostics/124014 identifies an ICE in sarif output of
diagnostics that occur after free_lang_data has called
tree_diagnostics_defaults, which happens e.g. with lto.

The issue is that in r16-413-g8ab6899dce92e6 I introduced to sarif_sink
a cached pointer to the logical_locations::manager, which for tree-using
clients is part of the compiler_data_hooks.  Hence for the case above, the
pointer is freed from under the sarif_sink, and any diagnostic
issued after that point with a current_function_decl will
trigger a use-after-free.

Fix by removing the cached pointer.

gcc/ChangeLog:
	PR diagnostics/124014
	* diagnostics/sarif-sink.cc
	(sarif_builder::get_logical_location_manager): Reimplement, to
	eliminate m_logical_loc_mgr.
	(sarif_builder::m_logical_loc_mgr): Drop field.
	(sarif_builder::sarif_builder): Update for removed field.
	(sarif_builder::set_any_logical_locs_arr): Likewise.
	(sarif_builder::ensure_sarif_logical_location_for): Likewise.
	(sarif_builder::make_minimal_sarif_logical_location): Likewise.

gcc/testsuite/ChangeLog:
	PR diagnostics/124014
	* gcc.dg/sarif-output/ice-pr124014.c: New test.

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

diff --git a/gcc/diagnostics/sarif-sink.cc b/gcc/diagnostics/sarif-sink.cc
index 8e8947aa026a18a3098233a3d6dfae5fee85cfdc..2cd64d65112e9afd485b1b03f8098cbec8248118 100644 (file)
--- a/gcc/diagnostics/sarif-sink.cc
+++ b/gcc/diagnostics/sarif-sink.cc
@@ -801,7 +801,9 @@ public:
   const logical_locations::manager *
   get_logical_location_manager () const
   {
-    return m_logical_loc_mgr;
+    if (auto client_data_hooks = m_context.get_client_data_hooks ())
+      return client_data_hooks->get_logical_location_manager ();
+    return nullptr;
   }
 
   void
@@ -994,8 +996,6 @@ private:
   const line_maps *m_line_maps;
   sarif_token_printer m_token_printer;
 
-  const logical_locations::manager *m_logical_loc_mgr;
-
   /* The JSON object for the invocation object.  */
   std::unique_ptr<sarif_invocation> m_invocation_obj;
 
@@ -1700,7 +1700,6 @@ sarif_builder::sarif_builder (diagnostics::context &dc,
   m_printer (&printer),
   m_line_maps (line_maps),
   m_token_printer (*this),
-  m_logical_loc_mgr (nullptr),
   m_invocation_obj
     (std::make_unique<sarif_invocation> (*this,
                                         dc.get_original_argv ())),
@@ -1721,9 +1720,6 @@ sarif_builder::sarif_builder (diagnostics::context &dc,
 {
   gcc_assert (m_line_maps);
   gcc_assert (m_serialization_format);
-
-  if (auto client_data_hooks = dc.get_client_data_hooks ())
-    m_logical_loc_mgr = client_data_hooks->get_logical_location_manager ();
 }
 
 sarif_builder::~sarif_builder ()
@@ -2307,7 +2303,8 @@ set_any_logical_locs_arr (sarif_location &location_obj,
 {
   if (!logical_loc)
     return;
-  gcc_assert (m_logical_loc_mgr);
+  auto logical_loc_mgr = get_logical_location_manager ();
+  gcc_assert (logical_loc_mgr);
   auto location_locs_arr = std::make_unique<json::array> ();
 
   auto logical_loc_obj = make_minimal_sarif_logical_location (logical_loc);
@@ -3061,28 +3058,29 @@ int
 sarif_builder::
 ensure_sarif_logical_location_for (logical_locations::key k)
 {
-  gcc_assert (m_logical_loc_mgr);
+  auto logical_loc_mgr = get_logical_location_manager ();
+  gcc_assert (logical_loc_mgr);
 
   auto sarif_logical_loc = std::make_unique<sarif_logical_location> ();
 
-  if (const char *short_name = m_logical_loc_mgr->get_short_name (k))
+  if (const char *short_name = logical_loc_mgr->get_short_name (k))
     sarif_logical_loc->set_string ("name", short_name);
 
   /* "fullyQualifiedName" property (SARIF v2.1.0 section 3.33.5).  */
-  if (const char *name_with_scope = m_logical_loc_mgr->get_name_with_scope (k))
+  if (const char *name_with_scope = logical_loc_mgr->get_name_with_scope (k))
     sarif_logical_loc->set_string ("fullyQualifiedName", name_with_scope);
 
   /* "decoratedName" property (SARIF v2.1.0 section 3.33.6).  */
-  if (const char *internal_name = m_logical_loc_mgr->get_internal_name (k))
+  if (const char *internal_name = logical_loc_mgr->get_internal_name (k))
     sarif_logical_loc->set_string ("decoratedName", internal_name);
 
   /* "kind" property (SARIF v2.1.0 section 3.33.7).  */
-  enum logical_locations::kind kind = m_logical_loc_mgr->get_kind (k);
+  enum logical_locations::kind kind = logical_loc_mgr->get_kind (k);
   if (const char *sarif_kind_str = maybe_get_sarif_kind (kind))
     sarif_logical_loc->set_string ("kind", sarif_kind_str);
 
   /* "parentIndex" property (SARIF v2.1.0 section 3.33.8).  */
-  if (auto parent_key = m_logical_loc_mgr->get_parent (k))
+  if (auto parent_key = logical_loc_mgr->get_parent (k))
     {
       /* Recurse upwards.  */
       int parent_index = ensure_sarif_logical_location_for (parent_key);
@@ -3105,7 +3103,8 @@ std::unique_ptr<sarif_logical_location>
 sarif_builder::
 make_minimal_sarif_logical_location (logical_locations::key logical_loc)
 {
-  gcc_assert (m_logical_loc_mgr);
+  auto logical_loc_mgr = get_logical_location_manager ();
+  gcc_assert (logical_loc_mgr);
 
   /* Ensure that m_cached_logical_locs has a "logicalLocation" object
      (SARIF v2.1.0 section 3.33) for LOGICAL_LOC, and return its index within
@@ -3120,7 +3119,7 @@ make_minimal_sarif_logical_location (logical_locations::key logical_loc)
 
   /* "fullyQualifiedName" property (SARIF v2.1.0 section 3.33.5).  */
   if (const char *name_with_scope
-        = m_logical_loc_mgr->get_name_with_scope (logical_loc))
+       = logical_loc_mgr->get_name_with_scope (logical_loc))
     sarif_logical_loc->set_string ("fullyQualifiedName", name_with_scope);
 
   return sarif_logical_loc;

diff --git a/gcc/testsuite/gcc.dg/sarif-output/ice-pr124014.c b/gcc/testsuite/gcc.dg/sarif-output/ice-pr124014.c
new file mode 100644 (file)
index 0000000..6a7bc5d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sarif-output/ice-pr124014.c
@@ -0,0 +1,4 @@
+/* { dg-options "-fdiagnostics-add-output=sarif -fbranch-probabilities" } */
+/* { dg-additional-options "-flto" { target lto } }  */
+
+void main() {} /* { dg-warning "missing-profile" } */