diagnostics: bulletproof opening of SARIF output [PR116978]

Introduce a new RAII class diagnostic_output_file to track ownership
of the FILE * for SARIF output.

In particular, the .sarif file is now opened immediately, rather
than at the end of the compile, and so will fail earlier if the
file can't be opened.

Doing so fixes a couple of ICEs in -fdiagnostics-format=sarif-file when
invoking, say, cc1 directly, rather than from the driver.

gcc/ChangeLog:
	PR other/116978
	* diagnostic-format-sarif.cc (sarif_builder::sarif_builder):
	Gracefully handle "main_input_filename_" being NULL.
	(sarif_output_format::sarif_output_format): Replace param
	"base_file_name" with "output_file" and assert that the file
	was opened successfully and has a non-NULL filename.
	(sarif_output_format::~sarif_file_output_format): Move
	responsibility for building the filename and opening the file from
	here to the creator of the instance.
	(sarif_output_format::m_base_file_name): Replace with...
	(sarif_output_format::m_output_file): ...this.
	(diagnostic_output_format_init_sarif_file): Make "line_maps" param
	non-const.  Gracefully handle "base_file_name" being NULL.
	Construct the filename and open the file here, rather than in
	~sarif_file_output_format, and handle failures immediately here,
	rather than at the end of the compile.
	* diagnostic-format-sarif.h: Include "diagnostic-output-file.h".
	(diagnostic_output_format_init_sarif_file): Make "line_maps" param
	non-const.
	* diagnostic-output-file.h: New file.
	* diagnostic.cc (diagnostic_context::emit_diagnostic): New.
	(diagnostic_context::emit_diagnostic_va): New.
	* diagnostic.h (diagnostic_context::emit_diagnostic): New decl.
	(diagnostic_context::emit_diagnostic_va): New decl.

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

5 files changed, 165 insertions, 24 deletions

diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-format-sarif.cc
index aeba9f9..f053dec 100644
--- a/gcc/diagnostic-format-sarif.cc
+++ b/gcc/diagnostic-format-sarif.cc
@@ -1510,9 +1510,10 @@ sarif_builder::sarif_builder (diagnostic_context &context,
      since otherwise the "no diagnostics" case would quote the main input
      file, and doing so noticeably bloated the output seen in analyzer
      integration testing (build directory went from 20G -> 21G).  */
-  get_or_create_artifact (main_input_filename_,
-			  diagnostic_artifact_role::analysis_target,
-			  false);
+  if (main_input_filename_)
+    get_or_create_artifact (main_input_filename_,
+			    diagnostic_artifact_role::analysis_target,
+			    false);
 }
 
 sarif_builder::~sarif_builder ()
@@ -3239,29 +3240,17 @@ public:
 			    const char *main_input_filename_,
 			    bool formatted,
 			    enum sarif_version version,
-			    const char *base_file_name)
+			    diagnostic_output_file output_file)
   : sarif_output_format (context, line_maps, main_input_filename_,
 			 formatted, version),
-    m_base_file_name (xstrdup (base_file_name))
+    m_output_file (std::move (output_file))
   {
+    gcc_assert (m_output_file.get_open_file ());
+    gcc_assert (m_output_file.get_filename ());
   }
   ~sarif_file_output_format ()
   {
-    char *filename = concat (m_base_file_name, ".sarif", nullptr);
-    free (m_base_file_name);
-    m_base_file_name = nullptr;
-    FILE *outf = fopen (filename, "w");
-    if (!outf)
-      {
-	const char *errstr = xstrerror (errno);
-	fnotice (stderr, "error: unable to open '%s' for writing: %s\n",
-		 filename, errstr);
-	free (filename);
-	return;
-      }
-    m_builder.flush_to_file (outf);
-    fclose (outf);
-    free (filename);
+    m_builder.flush_to_file (m_output_file.get_open_file ());
   }
   bool machine_readable_stderr_p () const final override
   {
@@ -3269,7 +3258,7 @@ public:
   }
 
 private:
-  char *m_base_file_name;
+  diagnostic_output_file m_output_file;
 };
 
 /* Print the start of an embedded link to PP, as per 3.11.6.  */
@@ -3435,13 +3424,35 @@ diagnostic_output_format_init_sarif_stderr (diagnostic_context &context,
 
 void
 diagnostic_output_format_init_sarif_file (diagnostic_context &context,
-					  const line_maps *line_maps,
+					  line_maps *line_maps,
 					  const char *main_input_filename_,
 					  bool formatted,
 					  enum sarif_version version,
 					  const char *base_file_name)
 {
   gcc_assert (line_maps);
+
+  if (!base_file_name)
+    {
+      rich_location richloc (line_maps, UNKNOWN_LOCATION);
+      context.emit_diagnostic (DK_ERROR, richloc, nullptr, 0,
+			       "unable to determine filename for SARIF output");
+      return;
+    }
+
+  label_text filename = label_text::take (concat (base_file_name,
+						  ".sarif",
+						  nullptr));
+  FILE *outf = fopen (filename.get (), "w");
+  if (!outf)
+    {
+      rich_location richloc (line_maps, UNKNOWN_LOCATION);
+      context.emit_diagnostic (DK_ERROR, richloc, nullptr, 0,
+			       "unable to open %qs for SARIF output: %m",
+			       filename.get ());
+      return;
+    }
+  diagnostic_output_file output_file (outf, true, std::move (filename));
   diagnostic_output_format_init_sarif
     (context,
      ::make_unique<sarif_file_output_format> (context,
@@ -3449,7 +3460,7 @@ diagnostic_output_format_init_sarif_file (diagnostic_context &context,
 					      main_input_filename_,
 					      formatted,
 					      version,
-					      base_file_name));
+					      std::move (output_file)));
 }
 
 /* Populate CONTEXT in preparation for SARIF output to STREAM.  */
diff --git a/gcc/diagnostic-format-sarif.h b/gcc/diagnostic-format-sarif.h
index 555ea60..5f8751a 100644
--- a/gcc/diagnostic-format-sarif.h
+++ b/gcc/diagnostic-format-sarif.h
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 
 #include "json.h"
 #include "diagnostic-format.h"
+#include "diagnostic-output-file.h"
 
 class logical_location;
 
@@ -42,7 +43,7 @@ diagnostic_output_format_init_sarif_stderr (diagnostic_context &context,
 					    enum sarif_version version);
 extern void
 diagnostic_output_format_init_sarif_file (diagnostic_context &context,
-					  const line_maps *line_maps,
+					  line_maps *line_maps,
 					  const char *main_input_filename_,
 					  bool formatted,
 					  enum sarif_version version,
diff --git a/gcc/diagnostic-output-file.h b/gcc/diagnostic-output-file.h
new file mode 100644
index 0000000..f0ae5e1
--- /dev/null
+++ b/gcc/diagnostic-output-file.h
@@ -0,0 +1,75 @@
+/* RAII class for managing FILE * for diagnostic formats.
+   Copyright (C) 2024 Free Software Foundation, Inc.
+   Contributed by David Malcolm <dmalcolm@redhat.com>.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_DIAGNOSTIC_OUTPUT_FILE_H
+#define GCC_DIAGNOSTIC_OUTPUT_FILE_H
+
+/* RAII class for wrapping a FILE * that could be borrowed or owned,
+   along with the underlying filename.  */
+
+class diagnostic_output_file
+{
+public:
+  diagnostic_output_file (FILE *outf, bool owned, label_text filename)
+  : m_outf (outf),
+    m_owned (owned),
+    m_filename (std::move (filename))
+  {
+    gcc_assert (m_filename.get ());
+    if (m_owned)
+      gcc_assert (m_outf);
+  }
+  ~diagnostic_output_file ()
+  {
+    if (m_owned)
+      {
+	gcc_assert (m_outf);
+	fclose (m_outf);
+      }
+  }
+  diagnostic_output_file (const diagnostic_output_file &other) = delete;
+  diagnostic_output_file (diagnostic_output_file &&other)
+  : m_outf (other.m_outf),
+    m_owned (other.m_owned),
+    m_filename (std::move (other.m_filename))
+  {
+    other.m_outf = nullptr;
+    other.m_owned = false;
+
+    gcc_assert (m_filename.get ());
+    if (m_owned)
+      gcc_assert (m_outf);
+  }
+  diagnostic_output_file &
+  operator= (const diagnostic_output_file &other) = delete;
+  diagnostic_output_file &
+  operator= (diagnostic_output_file &&other) = delete;
+
+  operator bool () const { return m_outf != nullptr; }
+  FILE *get_open_file () const { return m_outf; }
+  const char *get_filename () const { return m_filename.get (); }
+
+private:
+  FILE *m_outf;
+  bool m_owned;
+  label_text m_filename;
+};
+
+#endif /* ! GCC_DIAGNOSTIC_OUTPUT_FILE_H */
diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index 27ac2bd..7f741a0 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -1210,6 +1210,47 @@ diagnostic_context::warning_enabled_at (location_t loc,
   return diagnostic_enabled (&diagnostic);
 }
 
+/* Emit a diagnostic within a diagnostic group on this context.  */
+
+bool
+diagnostic_context::emit_diagnostic (diagnostic_t kind,
+				     rich_location &richloc,
+				     const diagnostic_metadata *metadata,
+				     diagnostic_option_id option_id,
+				     const char *gmsgid, ...)
+{
+  begin_group ();
+
+  va_list ap;
+  va_start (ap, gmsgid);
+  bool ret = emit_diagnostic_va (kind, richloc, metadata, option_id,
+				 gmsgid, &ap);
+  va_end (ap);
+
+  end_group ();
+
+  return ret;
+}
+
+/* As above, but taking a va_list *.  */
+
+bool
+diagnostic_context::emit_diagnostic_va (diagnostic_t kind,
+					rich_location &richloc,
+					const diagnostic_metadata *metadata,
+					diagnostic_option_id option_id,
+					const char *gmsgid, va_list *ap)
+{
+  begin_group ();
+
+  bool ret = diagnostic_impl (&richloc, metadata, option_id,
+			      gmsgid, ap, kind);
+
+  end_group ();
+
+  return ret;
+}
+
 /* Report a diagnostic message (an error or a warning) as specified by
    this diagnostic_context.
    front-end independent format specifiers are exactly those described
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 6cda3db..7ca9607 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -505,6 +505,19 @@ public:
     return m_option_classifier.option_unspecified_p (option_id);
   }
 
+  bool emit_diagnostic (diagnostic_t kind,
+			rich_location &richloc,
+			const diagnostic_metadata *metadata,
+			diagnostic_option_id option_id,
+			const char *gmsgid, ...)
+    ATTRIBUTE_GCC_DIAG(6,7);
+  bool emit_diagnostic_va (diagnostic_t kind,
+			   rich_location &richloc,
+			   const diagnostic_metadata *metadata,
+			   diagnostic_option_id option_id,
+			   const char *gmsgid, va_list *ap)
+    ATTRIBUTE_GCC_DIAG(6,0);
+
   bool report_diagnostic (diagnostic_info *);
 
   void check_max_errors (bool flush);