diagnostics: respect dumpdir when outputting SARIF and HTML file [PR110522]

PR diagnostics/110522 notes that when generating the output path for
diagnostics in SARIF or HTML form, the compiler ignores the path for the
compiler output, leading to race conditions and overwritten SARIF output
when compiling the same source file multiple times in a build.

Given e.g.

  gcc \
    -o build-dir/foo.o \
    -fdiagnostics-add-output=sarif
    foo.c

my intent was for the sarif sink to write to "build-dir/foo.c.sarif",
but in GCC 13 - GCC 15 it writes to "foo.c.sarif" instead.

The driver adds the option "-dumpdir build-dir" when invoking cc1, and
opts.cc's finish_options has the logic to prepend the dump_dir_name to
the dump_base_name, which in theory ought to have affected the name for
SARIF output.  The root cause is that finish_options is called *after*
processing the options for creating the diagnostic sinks, and so the
sarif_sink is created when dump_base_name is "foo.c", and thus
writes to "foo.c.sarif", rather than "build-dir/foo.c.sarif".

This patch introduces a new helper function for accessing the base name
for diagnostic sinks and refactors the prepending logic
in finish_options so that it is also used when creating diagnostic
sinks, so that in the above example the SARIF is written to
"build-dir/foo.c.sarif".

The patch took the number of .sarif files generated by the analyzer
integration tests from 5094 to 11309.  The most extreme example I could
find within them was where we previously got a single .sarif file:
  qemu-7.2.0/build/fpu_softfloat.c.c.sarif
and *with* the patch we get 66 .sarif files:
  qemu-7.2.0/build/libqemu-aarch64-linux-user.fa.p/fpu_softfloat.c.c.sarif
  qemu-7.2.0/build/libqemu-aarch64-softmmu.fa.p/fpu_softfloat.c.c.sarif
  [...snip lots of different architectures...]
  qemu-7.2.0/build/libqemu-xtensaeb-linux-user.fa.p/fpu_softfloat.c.c.sarif
  qemu-7.2.0/build/libqemu-xtensaeb-softmmu.fa.p/fpu_softfloat.c.c.sarif

which presumably were previously all being written to the same place
during the build.  Indeed, the old file is invalid JSON, and appears to
have been truncated.

gcc/ChangeLog:
	PR diagnostics/110522
	* gcc.cc (driver_handle_option): Use
	get_diagnostic_file_output_basename for
	OPT_fdiagnostics_format_.
	* opts-diagnostic.cc (get_base_filename): Likewise.
	(get_diagnostic_file_output_basename): New.
	* opts-diagnostic.h (get_diagnostic_file_output_basename): New
	decl.
	* opts.cc (maybe_prepend_dump_dir_name): New, based on code in
	finish_options.
	(finish_options): Move code for determining prepended
	dump_base_name to maybe_prepend_dump_dir_name and call it.
	(common_handle_option): Use get_diagnostic_file_output_basename
	for OPT_fdiagnostics_format_.
	* opts.h (maybe_prepend_dump_dir_name): New decl.

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

diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index 6b6f6f87c5209560ccf46fd22bccecfd6c9af157..2fc743af3d90ac37d0817d8006205dd146ebc5be 100644 (file)
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -4387,8 +4387,7 @@ driver_handle_option (struct gcc_options *opts,
 
     case OPT_fdiagnostics_format_:
        {
-         const char *basename = (opts->x_dump_base_name ? opts->x_dump_base_name
-                                 : opts->x_main_input_basename);
+         const char *basename = get_diagnostic_file_output_basename (*opts);
          gcc_assert (dc);
          diagnostics::output_format_init (*dc,
                                           opts->x_main_input_filename, basename,

diff --git a/gcc/opts-diagnostic.cc b/gcc/opts-diagnostic.cc
index f60628244489103b9034ebb497b4f0b4d6af53c7..594f735a8c674e0c3fc3f4a0066796ab60490eee 100644 (file)
--- a/gcc/opts-diagnostic.cc
+++ b/gcc/opts-diagnostic.cc
@@ -94,9 +94,7 @@ public:
   const char *
   get_base_filename () const final override
   {
-    return (m_opts.x_dump_base_name
-           ? m_opts.x_dump_base_name
-           : m_opts.x_main_input_basename);
+    return get_diagnostic_file_output_basename (m_opts);
   }
 
   const gcc_options &m_opts;
@@ -170,3 +168,24 @@ handle_OPT_fdiagnostics_set_output_ (const gcc_options &opts,
   if (auto sink = try_to_make_sink (opts, dc, option_name, unparsed_spec, loc))
     dc.set_sink (std::move (sink));
 }
+
+/* Return the base name to use when choosing names for output file for
+   diagnostic sinks (e.g. BASENAME.sarif or BASENAME.html).  */
+
+const char *
+get_diagnostic_file_output_basename (const gcc_options &opts)
+{
+  /* This might have been called before finish_options, which prepends
+     the dump dir to the dump base name.  If so, make a prepended copy
+     now and use it.  */
+  if (opts.x_dump_base_name
+      && ! opts.x_dump_base_name_prefixed)
+    if (const char *prepended_dump_base_name
+         = maybe_prepend_dump_dir_name (opts))
+      /* Allocated in opts_obstack.  */
+      return prepended_dump_base_name;
+
+  return (opts.x_dump_base_name
+         ? opts.x_dump_base_name
+         : opts.x_main_input_basename);
+}

diff --git a/gcc/opts-diagnostic.h b/gcc/opts-diagnostic.h
index f496ef04912c150dc4185beb08fa5f65a9461999..d0a8beddd81a10f1192636ed17181191e4d72abd 100644 (file)
--- a/gcc/opts-diagnostic.h
+++ b/gcc/opts-diagnostic.h
@@ -86,4 +86,8 @@ handle_OPT_fdiagnostics_set_output_ (const gcc_options &opts,
                                     diagnostics::context &dc,
                                     const char *arg,
                                     location_t loc);
+
+extern const char *
+get_diagnostic_file_output_basename (const gcc_options &opts);
+
 #endif

diff --git a/gcc/opts.cc b/gcc/opts.cc
index 6323fd5880166e5de1c15eaa6e59a668f801e298..6658b6acd378d6ac03a28acf15a2e7bbe1eac2c7 100644 (file)
--- a/gcc/opts.cc
+++ b/gcc/opts.cc
@@ -1070,6 +1070,37 @@ validate_ipa_reorder_locality_lto_partition (struct gcc_options *opts,
   validated_p = true;
 }
 
+/* If OPTS.x_dump_base_name doesn't contain any directory separators
+   and has not had OPTS.x_dump_dir_name prepended to it, generate
+   a new string in opts_obstack that has the dump_dir_name prepended to
+   the dump_base_name.  */
+
+const char *
+maybe_prepend_dump_dir_name (const gcc_options &opts)
+{
+  const char *sep = opts.x_dump_base_name;
+
+  for (; *sep; sep++)
+    if (IS_DIR_SEPARATOR (*sep))
+      break;
+
+  if (*sep)
+    {
+      /* If dump_base_name contains subdirectories, don't prepend
+        anything.  */
+      return nullptr;
+    }
+
+  if (opts.x_dump_dir_name)
+    {
+      /* We have a DUMP_DIR_NAME, prepend that.  */
+      return opts_concat (opts.x_dump_dir_name,
+                         opts.x_dump_base_name, NULL);
+    }
+
+  return nullptr;
+}
+
 /* After all options at LOC have been read into OPTS and OPTS_SET,
    finalize settings of those options and diagnose incompatible
    combinations.  */
@@ -1080,19 +1111,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
   if (opts->x_dump_base_name
       && ! opts->x_dump_base_name_prefixed)
     {
-      const char *sep = opts->x_dump_base_name;
-
-      for (; *sep; sep++)
-       if (IS_DIR_SEPARATOR (*sep))
-         break;
-
-      if (*sep)
-       /* If dump_base_path contains subdirectories, don't prepend
-          anything.  */;
-      else if (opts->x_dump_dir_name)
-       /* We have a DUMP_DIR_NAME, prepend that.  */
-       opts->x_dump_base_name = opts_concat (opts->x_dump_dir_name,
-                                             opts->x_dump_base_name, NULL);
+      if (const char *prepended_dump_base_name
+         = maybe_prepend_dump_dir_name (*opts))
+       opts->x_dump_base_name = prepended_dump_base_name;
 
       /* It is definitely prefixed now.  */
       opts->x_dump_base_name_prefixed = true;
@@ -3023,8 +3044,7 @@ common_handle_option (struct gcc_options *opts,
 
     case OPT_fdiagnostics_format_:
        {
-         const char *basename = (opts->x_dump_base_name ? opts->x_dump_base_name
-                                 : opts->x_main_input_basename);
+         const char *basename = get_diagnostic_file_output_basename (*opts);
          gcc_assert (dc);
          diagnostics::output_format_init (*dc,
                                           opts->x_main_input_filename, basename,

diff --git a/gcc/opts.h b/gcc/opts.h
index a59475b1daf845c28a5142a13007e48a17f6977b..2d28620edab5885b67aebf28bc8f41cd2fa4173d 100644 (file)
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -577,4 +577,7 @@ struct switchstr
 extern label_text
 get_option_url_suffix (int option_index, unsigned lang_mask);
 
+extern const char *
+maybe_prepend_dump_dir_name (const gcc_options &opts);
+
 #endif