[pushed:,r15-3971] diagnostics: fix memory leak in SARIF selftests

Message ID 20240930160248.2857030-1-dmalcolm@redhat.com
State Committed
Commit ab6c7a329d4958aa1f2975196358f7f56a463b3b
Headers
Series [pushed:,r15-3971] diagnostics: fix memory leak in SARIF selftests |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 warning Patch is already merged
linaro-tcwg-bot/tcwg_gcc_build--master-arm warning Patch is already merged

Commit Message

David Malcolm Sept. 30, 2024, 4:02 p.m. UTC
  "make selftest-valgrind" was complaining about leaks of artifact objects
in SARIF's selftest::test_make_location_object:

-fself-test: 7638695 pass(es) in 89.999249 seconds
==3306525==
==3306525== HEAP SUMMARY:
==3306525==     in use at exit: 1,215,639 bytes in 2,808 blocks
==3306525==   total heap usage: 2,860,898 allocs, 2,858,090 frees, 1,336,446,579 bytes allocated
==3306525==
==3306525== 11,728 (1,536 direct, 10,192 indirect) bytes in 16 blocks are definitely lost in loss record 353 of 375
==3306525==    at 0x514FE7D: operator new(unsigned long) (vg_replace_malloc.c:342)
==3306525==    by 0x36E5FD2: sarif_builder::get_or_create_artifact(char const*, diagnostic_artifact_role, bool) (diagnostic-format-sarif.cc:2884)
==3306525==    by 0x36E3D57: sarif_builder::maybe_make_physical_location_object(unsigned int, diagnostic_artifact_role, int, content_renderer const*) (diagnostic-format-sarif.cc:2097)
==3306525==    by 0x36E34CE: sarif_builder::make_location_object(sarif_location_manager&, rich_location const&, logical_location const*, diagnostic_artifact_role) (diagnostic-format-sarif.cc:1922)
==3306525==    by 0x36E72C6: selftest::test_make_location_object(selftest::line_table_case const&) (diagnostic-format-sarif.cc:3500)
==3306525==    by 0x375609B: selftest::for_each_line_table_case(void (*)(selftest::line_table_case const&)) (input.cc:3898)
==3306525==    by 0x36E9668: selftest::diagnostic_format_sarif_cc_tests() (diagnostic-format-sarif.cc:3910)
==3306525==    by 0x3592A11: selftest::run_tests() (selftest-run-tests.cc:100)
==3306525==    by 0x17DBEF3: toplev::run_self_tests() (toplev.cc:2268)
==3306525==    by 0x17DC2BF: toplev::main(int, char**) (toplev.cc:2376)
==3306525==    by 0x36A1919: main (main.cc:39)
==3306525==
==3306525== 12,400 (1,536 direct, 10,864 indirect) bytes in 16 blocks are definitely lost in loss record 355 of 375
==3306525==    at 0x514FE7D: operator new(unsigned long) (vg_replace_malloc.c:342)
==3306525==    by 0x36E5FD2: sarif_builder::get_or_create_artifact(char const*, diagnostic_artifact_role, bool) (diagnostic-format-sarif.cc:2884)
==3306525==    by 0x36E2323: sarif_builder::sarif_builder(diagnostic_context&, line_maps const*, char const*, bool) (diagnostic-format-sarif.cc:1500)
==3306525==    by 0x36E70AA: selftest::test_make_location_object(selftest::line_table_case const&) (diagnostic-format-sarif.cc:3469)
==3306525==    by 0x375609B: selftest::for_each_line_table_case(void (*)(selftest::line_table_case const&)) (input.cc:3898)
==3306525==    by 0x36E9668: selftest::diagnostic_format_sarif_cc_tests() (diagnostic-format-sarif.cc:3910)
==3306525==    by 0x3592A11: selftest::run_tests() (selftest-run-tests.cc:100)
==3306525==    by 0x17DBEF3: toplev::run_self_tests() (toplev.cc:2268)
==3306525==    by 0x17DC2BF: toplev::main(int, char**) (toplev.cc:2376)
==3306525==    by 0x36A1919: main (main.cc:39)
==3306525==
==3306525== LEAK SUMMARY:
==3306525==    definitely lost: 3,072 bytes in 32 blocks
==3306525==    indirectly lost: 21,056 bytes in 368 blocks
==3306525==      possibly lost: 0 bytes in 0 blocks
==3306525==    still reachable: 1,191,511 bytes in 2,408 blocks
==3306525==         suppressed: 0 bytes in 0 blocks
==3306525== Reachable blocks (those to which a pointer was found) are not shown.
==3306525== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==3306525==
==3306525== For lists of detected and suppressed errors, rerun with: -s
==3306525== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

Fixed thusly.

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

gcc/ChangeLog:
	* diagnostic-format-sarif.cc (sarif_builder::~sarif_builder): New,
	deleting any remaining artifact objects.
	(sarif_builder::make_run_object): Empty the artifact map.
	* ordered-hash-map.h (ordered_hash_map::empty): New.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/diagnostic-format-sarif.cc | 14 ++++++++++++++
 gcc/ordered-hash-map.h         |  2 ++
 2 files changed, 16 insertions(+)
  

Patch

diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-format-sarif.cc
index 6cd18cef6c89..7b11dfd89a31 100644
--- a/gcc/diagnostic-format-sarif.cc
+++ b/gcc/diagnostic-format-sarif.cc
@@ -649,6 +649,7 @@  public:
 		 const line_maps *line_maps,
 		 const char *main_input_filename_,
 		 bool formatted);
+  ~sarif_builder ();
 
   void on_report_diagnostic (const diagnostic_info &diagnostic,
 			     diagnostic_t orig_diag_kind);
@@ -1500,6 +1501,18 @@  sarif_builder::sarif_builder (diagnostic_context &context,
 			  false);
 }
 
+sarif_builder::~sarif_builder ()
+{
+  /* Normally m_filename_to_artifact_map will have been emptied as part
+     of make_run_object, but this isn't run by all the selftests.
+     Ensure the artifact objects are cleaned up for such cases.  */
+  for (auto iter : m_filename_to_artifact_map)
+    {
+      sarif_artifact *artifact_obj = iter.second;
+      delete artifact_obj;
+    }
+}
+
 /* Implementation of "on_report_diagnostic" for SARIF output.  */
 
 void
@@ -2697,6 +2710,7 @@  make_run_object (std::unique_ptr<sarif_invocation> invocation_obj,
       artifacts_arr->append (artifact_obj);
     }
   run_obj->set<json::array> ("artifacts", std::move (artifacts_arr));
+  m_filename_to_artifact_map.empty ();
 
   /* "results" property (SARIF v2.1.0 section 3.14.23).  */
   run_obj->set<json::array> ("results", std::move (results));
diff --git a/gcc/ordered-hash-map.h b/gcc/ordered-hash-map.h
index aa9c86de3989..cb410f29b58b 100644
--- a/gcc/ordered-hash-map.h
+++ b/gcc/ordered-hash-map.h
@@ -112,6 +112,8 @@  public:
 
   size_t elements () const { return m_map.elements (); }
 
+  void empty () { m_map.empty(); }
+
   class iterator
   {
   public: