> Am 30.01.2026 um 20:34 schrieb David Malcolm <[email protected]>:
>
> Richi/Jakub: PR diagnostics/110522 isn't a regression per-se, but this
> is a high-value bug to fix.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> Successful run of analyzer integration tests on x86_64-pc-linux-gnu,
> albeit showing a big increase in the number of results (see notes
> below).
>
> OK for trunk?
I’d like Alex to review this since he was refactoring those dump/aux file
handling things.
Generally I’d say backwards compatibility should not be an issue. People can
always manually specify the directory. So please drop that support (or split
it out for separate consideration)
OK from a RM perspective, but this isn’t a review.
Richard
> Thanks
> Dave
>
>
> 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.
>
> Given that some build systems may be relying on the existing buggy
> behavior, I attempted to implement a compatibility option to restore the
> old behavior, but this ran into further issues with the ordering in
> which options are processed. Hence the patch also introduces a new
> environment variable GCC_DIAGNOSTICS_IGNORE_DUMPDIR which when set
> restores the old behavior.
>
> gcc/ChangeLog:
> PR diagnostics/110522
> * doc/invoke.texi (GCC_DIAGNOSTICS_IGNORE_DUMPDIR): New
> environment variable.
> * 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 <[email protected]>
> ---
> gcc/doc/invoke.texi | 30 ++++++++++++++++++++++---
> gcc/gcc.cc | 3 +--
> gcc/opts-diagnostic.cc | 29 +++++++++++++++++++++---
> gcc/opts-diagnostic.h | 4 ++++
> gcc/opts.cc | 50 +++++++++++++++++++++++++++++-------------
> gcc/opts.h | 3 +++
> 6 files changed, 96 insertions(+), 23 deletions(-)
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index cad9e993140b..1e4b4a89b04c 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -6288,7 +6288,8 @@ The default is @samp{text}.
> @cindex diagnostic output formats, sarif-file
> The @samp{sarif-stderr} and @samp{sarif-file} formats both emit
> diagnostics in SARIF Version 2.1.0 format, either to stderr, or to a file
> -named @file{@var{source}.sarif}, respectively.
> +named @file{@var{source}.sarif}, respectively. See also
> +@env{GCC_DIAGNOSTICS_IGNORE_DUMPDIR}.
>
> @opindex fdiagnostics-add-output
> @item -fdiagnostics-add-output=@var{DIAGNOSTICS-OUTPUT-SPEC}
> @@ -6354,7 +6355,7 @@ Supported keys for the @code{sarif} scheme are:
> @item file=@var{FILENAME}
> Specify the filename to write the SARIF output to, potentially with a
> leading absolute or relative path. If not specified, it defaults to
> -@file{@var{source}.sarif}.
> +@file{@var{source}.sarif} (see also @env{GCC_DIAGNOSTICS_IGNORE_DUMPDIR}).
>
> @item serialization=@r{[}json@r{]}
> Specify the serialization format to use when writing out the SARIF.
> @@ -6397,7 +6398,7 @@ Add an embedded <style> to the generated HTML.
> Defaults to yes.
> @item file=@var{FILENAME}
> Specify the filename to write the HTML output to, potentially with a
> leading absolute or relative path. If not specified, it defaults to
> -@file{@var{source}.html}.
> +@file{@var{source}.html} (see also @env{GCC_DIAGNOSTICS_IGNORE_DUMPDIR}).
>
> @item javascript=@r{[}yes@r{|}no@r{]}
> Add an embedded <script> to the generated HTML providing a barebones UI
> @@ -39315,6 +39316,29 @@ attempt to open that file and write the information
> there.
> The precise content and format of the information is subject to change;
> it is intended for use by GCC developers, rather than end-users.
>
> +@vindex GCC_DIAGNOSTICS_IGNORE_DUMPDIR
> +@item GCC_DIAGNOSTICS_IGNORE_DUMPDIR
> +When outputing files for diagnostics (such as with
> +@option{-fdiagnostics-add-output=sarif} or
> +@option{-fdiagnostics-format=sarif-file}), the compiler will by default
> +select an output path that respects the dump directory, either when
> +set implicitly by the driver to match that of the output file, or
> +provided explicitly via @option{-dumpdir}.
> +
> +For example, with
> +@smallexample
> +gcc \
> + -o build-dir/foo.o \
> + -fdiagnostics-add-output=sarif
> + foo.c
> +@end smallexample
> +the compiler will write the SARIF to @file{build-dir/foo.c.sarif}.
> +
> +However in GCC 13, 14, and 15, SARIF output did not respect the dump
> +directory, and instead wrote the SARIF to @file{foo.c.sarif}. Setting
> +@env{GCC_DIAGNOSTICS_IGNORE_DUMPDIR} restores the old behavior, for
> +compatibility with build systems that expect this.
> +
> @vindex GCC_COMPARE_DEBUG
> @item GCC_COMPARE_DEBUG
> Setting @env{GCC_COMPARE_DEBUG} is nearly equivalent to passing
> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> index 6b6f6f87c520..2fc743af3d90 100644
> --- 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 f60628244489..6694cf081c7f 100644
> --- 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,28 @@ 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
> + /* In GCC <= 15 we failed to prepend the dumpdir.
> + Support the old behavior if GCC_DIAGNOSTICS_IGNORE_DUMPDIR is set
> + in the environment. */
> + && !getenv ("GCC_DIAGNOSTICS_IGNORE_DUMPDIR"))
> + 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 f496ef04912c..d0a8beddd81a 100644
> --- 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 6323fd588016..6658b6acd378 100644
> --- 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 a59475b1daf8..2d28620edab5 100644
> --- 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
> --
> 2.26.3
>
