Hi, Richard,
Could you please take a look at the patch and let me know any comment you have
(especially on the middle-end part)?
David, let me know if you have further comment and suggestions.
Thanks a lot.
Qing
> On Jul 12, 2024, at 10:03, Qing Zhao <qing.z...@oracle.com> wrote:
>
> due to code duplication from jump threading [PR109071]
> Control this with a new option -fdiagnostic-explain-harder.
>
> Compared to V1, the major difference are: (address David's comments)
>
> 1. Change the name of the option from:
>
> -fdiagnostic-try-to-explain-harder
> To
> -fdiagnostic-explain-harder
>
> 2. Sync the commit comments with the real output of the compilation message.
>
> 3. Add one more event in the end of the path to repeat the out-of-bound
> issue.
>
> 4. Fixed the unnecessary changes in Makefile.in.
>
> 5. Add new copy_history_diagnostic_path.[cc|h] to implement a new
> class copy_history_diagnostic_path : public diagnostic_path
>
> for copy_history_t.
>
> 6. Only building the rich locaiton and populating the path when warning_at
> is called.
>
> There are two comments from David that I didn't addressed in this version:
>
> 1. Make regenerate-opt-urls.
> will do this in a later version.
>
> 2. Add a ⚠️ emoji for the last event.
> I didn't add this yet since I think the current message is clear enough.
> might not worth the effort to add this emoji (it's not that straightforward
> to add on).
>
> With this new version, the message emitted by GCC:
>
> $gcc -O2 -Wall -fdiagnostics-explain-harder -c -o t.o t.c
> t.c: In function ‘sparx5_set’:
> t.c:12:23: warning: array subscript 4 is above array bounds of ‘int[4]’
> [-Warray-bounds=]
> 12 | int *val = &sg->vals[index];
> | ~~~~~~~~^~~~~~~
> ‘sparx5_set’: events 1-2
> 4 | if (*index >= 4)
> | ^
> | |
> | (1) when the condition is evaluated to true
> ......
> 12 | int *val = &sg->vals[index];
> | ~~~~~~~~~~~~~~~
> | |
> | (2) out of array bounds here
> t.c:8:18: note: while referencing ‘vals’
> 8 | struct nums {int vals[4];};
> | ^~~~
>
> Bootstrapped and regression tested on both aarch64 and x86. no issues.
>
> Let me know any further comments and suggestions.
>
> thanks.
>
> Qing
>
> ==================================================
> $ cat t.c
> extern void warn(void);
> static inline void assign(int val, int *regs, int *index)
> {
> if (*index >= 4)
> warn();
> *regs = val;
> }
> struct nums {int vals[4];};
>
> void sparx5_set (int *ptr, struct nums *sg, int index)
> {
> int *val = &sg->vals[index];
>
> assign(0, ptr, &index);
> assign(*val, ptr, &index);
> }
>
> $ gcc -Wall -O2 -c -o t.o t.c
> t.c: In function ‘sparx5_set’:
> t.c:12:23: warning: array subscript 4 is above array bounds of ‘int[4]’
> [-Warray-bounds=]
> 12 | int *val = &sg->vals[index];
> | ~~~~~~~~^~~~~~~
> t.c:8:18: note: while referencing ‘vals’
> 8 | struct nums {int vals[4];};
> | ^~~~
>
> In the above, Although the warning is correct in theory, the warning message
> itself is confusing to the end-user since there is information that cannot
> be connected to the source code directly.
>
> It will be a nice improvement to add more information in the warning message
> to report where such index value come from.
>
> In order to achieve this, we add a new data structure copy_history to record
> the condition and the transformation that triggered the code duplication.
> Whenever there is a code duplication due to some specific transformations,
> such as jump threading, loop switching, etc, a copy_history structure is
> created and attached to the duplicated gimple statement.
>
> During array out-of-bound checking or other warning checking, the copy_history
> that was attached to the gimple statement is used to form a sequence of
> diagnostic events that are added to the corresponding rich location to be used
> to report the warning message.
>
> This behavior is controled by the new option -fdiagnostic-explain-harder
> which is off by default.
>
> With this change, by adding -fdiagnostic-explain-harder,
> the warning message for the above testing case is now:
>
> $ gcc -Wall -O2 -fdiagnostics-explain-harder -c -o t.o t.c
> t.c: In function ‘sparx5_set’:
> t.c:12:23: warning: array subscript 4 is above array bounds of ‘int[4]’
> [-Warray-bounds=]
> 12 | int *val = &sg->vals[index];
> | ~~~~~~~~^~~~~~~
> ‘sparx5_set’: events 1-2
> 4 | if (*index >= 4)
> | ^
> | |
> | (1) when the condition is evaluated to true
> ......
> 12 | int *val = &sg->vals[index];
> | ~~~~~~~~~~~~~~~
> | |
> | (2) out of array bounds here
> t.c:8:18: note: while referencing ‘vals’
> 8 | struct nums {int vals[4];};
> | ^~~~
>
> gcc/ChangeLog:
>
> PR tree-optimization/109071
>
> * Makefile.in (OBJS): Add diagnostic-copy-history.o
> and copy-history-diagnostic-path.o.
> * gcc/common.opt (fdiagnostics-explain-harder): New option.
> * gcc/doc/invoke.texi (fdiagnostics-explain-harder): Add
> documentation for the new option.
> * gimple-array-bounds.cc (build_rich_location_with_diagnostic_path):
> New function.
> (check_out_of_bounds_and_warn): Add one new parameter. Use rich
> location with copy_history_diagnostic_path for warning_at.
> (array_bounds_checker::check_array_ref): Use rich location with
> copy_history_diagnostic_path for warning_at.
> (array_bounds_checker::check_mem_ref): Add one new parameter.
> Use rich location with copy_history_diagnostic_path for warning_at.
> (array_bounds_checker::check_addr_expr): Use rich location with
> copy_history_diagnostic_path for warning_at.
> (array_bounds_checker::check_array_bounds): Call check_mem_ref with
> one more parameter.
> * gimple-array-bounds.h: Update prototype for check_mem_ref.
> * gimple-iterator.cc (gsi_remove): (gsi_remove): Remove the copy
> history when removing the gimple.
> * toplev.cc (toplev::finalize): Call copy_history_finalize.
> * tree-ssa-threadupdate.cc (set_copy_history_to_stmts_in_bb): New
> function.
> (back_jt_path_registry::duplicate_thread_path): Call the new function
> to set copy history for every duplicated gimple.
> * copy-history-diagnostic-path.cc: New file.
> * copy-history-diagnostic-path.h: New file.
> * diagnostic-copy-history.cc: New file.
> * diagnostic-copy-history.h: New file.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/109071
>
> * gcc.dg/pr109071.c: New test.
> ---
> gcc/Makefile.in | 2 +
> gcc/common.opt | 4 +
> gcc/copy-history-diagnostic-path.cc | 86 +++++++++++++++
> gcc/copy-history-diagnostic-path.h | 87 +++++++++++++++
> gcc/diagnostic-copy-history.cc | 163 ++++++++++++++++++++++++++++
> gcc/diagnostic-copy-history.h | 80 ++++++++++++++
> gcc/doc/invoke.texi | 7 ++
> gcc/gimple-array-bounds.cc | 94 +++++++++++++---
> gcc/gimple-array-bounds.h | 2 +-
> gcc/gimple-iterator.cc | 3 +
> gcc/testsuite/gcc.dg/pr109071.c | 43 ++++++++
> gcc/toplev.cc | 3 +
> gcc/tree-ssa-threadupdate.cc | 42 +++++++
> 13 files changed, 598 insertions(+), 18 deletions(-)
> create mode 100644 gcc/copy-history-diagnostic-path.cc
> create mode 100644 gcc/copy-history-diagnostic-path.h
> create mode 100644 gcc/diagnostic-copy-history.cc
> create mode 100644 gcc/diagnostic-copy-history.h
> create mode 100644 gcc/testsuite/gcc.dg/pr109071.c
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 7d3ea27a6ab..f8bbfda845f 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1436,6 +1436,8 @@ OBJS = \
> df-problems.o \
> df-scan.o \
> dfp.o \
> + diagnostic-copy-history.o \
> + copy-history-diagnostic-path.o \
> digraph.o \
> dojump.o \
> dominance.o \
> diff --git a/gcc/common.opt b/gcc/common.opt
> index a300470bbc5..714280390d9 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1558,6 +1558,10 @@ fdiagnostics-minimum-margin-width=
> Common Joined UInteger Var(diagnostics_minimum_margin_width) Init(6)
> Set minimum width of left margin of source code when showing source.
>
> +fdiagnostics-explain-harder
> +Common Var(flag_diagnostics_explain_harder)
> +Collect and print more context information for diagnostics.
> +
> fdisable-
> Common Joined RejectNegative Var(common_deferred_options) Defer
> -fdisable-[tree|rtl|ipa]-<pass>=range1+range2 Disable an optimization pass.
> diff --git a/gcc/copy-history-diagnostic-path.cc
> b/gcc/copy-history-diagnostic-path.cc
> new file mode 100644
> index 00000000000..65e5c056165
> --- /dev/null
> +++ b/gcc/copy-history-diagnostic-path.cc
> @@ -0,0 +1,86 @@
> +/* Classes for implementing diagnostic paths for copy_history_t.
> + Copyright (C) 2024-2024 Free Software Foundation, Inc.
> + Contributed by Qing Zhao<qing.z...@oracle.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/>. */
> +
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "function.h"
> +#include "copy-history-diagnostic-path.h"
> +
> +bool
> +copy_history_diagnostic_path::same_function_p (int event_idx_a,
> + int event_idx_b) const
> +{
> + return (m_events[event_idx_a]->get_fndecl ()
> + == m_events[event_idx_b]->get_fndecl ());
> +}
> +
> +/* Add an event to this path at LOC within function FNDECL at
> + stack depth DEPTH.
> +
> + Use m_context's printer to format FMT, as the text of the new
> + event. */
> +
> +void
> +copy_history_diagnostic_path::add_event (location_t loc, tree fndecl, int
> depth,
> + const char *fmt, ...)
> +{
> + pretty_printer *pp = m_event_pp;
> + pp_clear_output_area (pp);
> +
> + rich_location rich_loc (line_table, UNKNOWN_LOCATION);
> +
> + va_list ap;
> +
> + va_start (ap, fmt);
> +
> + text_info ti (fmt, &ap, 0, nullptr, &rich_loc);
> + pp_format (pp, &ti);
> + pp_output_formatted_text (pp);
> +
> + va_end (ap);
> +
> + simple_diagnostic_event *new_event
> + = new simple_diagnostic_event (loc, fndecl, depth, pp_formatted_text
> (pp));
> + m_events.safe_push (new_event);
> +
> + pp_clear_output_area (pp);
> +
> + return;
> +}
> +
> +/* Populate the diagnostic_path from the copy_history. */
> +void
> +copy_history_diagnostic_path::populate_path_from_copy_history ()
> +{
> + if (!m_copy_history)
> + return;
> +
> + for (copy_history_t cur_ch = m_copy_history; cur_ch;
> + cur_ch = cur_ch->prev_copy)
> + add_event (cur_ch->condition, cfun->decl, 1,
> + "when the condition is evaluated to %s",
> + cur_ch->is_true_path ? "true" : "false");
> +
> + /* Add an end of path warning event in the end of the path. */
> + add_event (m_location, cfun->decl, 1,
> + "out of array bounds here");
> + return;
> +}
> diff --git a/gcc/copy-history-diagnostic-path.h
> b/gcc/copy-history-diagnostic-path.h
> new file mode 100644
> index 00000000000..377e9136070
> --- /dev/null
> +++ b/gcc/copy-history-diagnostic-path.h
> @@ -0,0 +1,87 @@
> +/* Classes for implementing diagnostic paths for copy_history_t.
> + Copyright (C) 2024-2024 Free Software Foundation, Inc.
> + Contributed by Qing Zhao<qing.z...@oracle.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_COPY_HISTORY_DIAGNOSTIC_PATH_H
> +#define GCC_COPY_HISTORY_DIAGNOSTIC_PATH_H
> +
> +#include "diagnostic-path.h"
> +#include "simple-diagnostic-path.h"
> +#include "diagnostic-copy-history.h"
> +
> +/* An implementation of diagnostic_path for copy_history_t, as a
> + vector of simple_diagnostic_event instances. */
> +
> +class copy_history_diagnostic_path : public diagnostic_path
> +{
> + public:
> + copy_history_diagnostic_path (pretty_printer *event_pp,
> + copy_history_t cp_history,
> + location_t loc)
> + : diagnostic_path (),
> + m_thread ("main"),
> + m_event_pp (event_pp),
> + m_copy_history (cp_history),
> + m_location (loc)
> + {}
> +
> + unsigned num_events () const final override
> + {
> + return m_events.length ();
> + }
> + const diagnostic_event & get_event (int idx) const final override
> + {
> + return *m_events[idx];
> + }
> + unsigned num_threads () const final override
> + {
> + return 1;
> + }
> + const diagnostic_thread &
> + get_thread (diagnostic_thread_id_t) const final override
> + {
> + return m_thread;
> + }
> + bool
> + same_function_p (int event_idx_a,
> + int event_idx_b) const final override;
> +
> + void add_event (location_t loc, tree fndecl, int depth,
> + const char *fmt, ...);
> +
> + void populate_path_from_copy_history ();
> +
> + private:
> + simple_diagnostic_thread m_thread;
> +
> + /* The events that have occurred along this path. */
> + auto_delete_vec<simple_diagnostic_event> m_events;
> +
> + pretty_printer *m_event_pp;
> +
> + /* The copy_history associated with this path. */
> + copy_history_t m_copy_history;
> +
> + /* The location for the gimple statement where the
> + diagnostic message emitted. */
> + location_t m_location;
> +
> +};
> +
> +#endif /* ! GCC_COPY_HISTORY_DIAGNOSTIC_PATH_H */
> diff --git a/gcc/diagnostic-copy-history.cc b/gcc/diagnostic-copy-history.cc
> new file mode 100644
> index 00000000000..2733d137622
> --- /dev/null
> +++ b/gcc/diagnostic-copy-history.cc
> @@ -0,0 +1,163 @@
> +/* Functions to handle copy history.
> + Copyright (C) 2024-2024 Free Software Foundation, Inc.
> + Contributed by Qing Zhao <qing.z...@oracle.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/>. */
> +
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "backend.h"
> +#include "tree.h"
> +#include "diagnostic-copy-history.h"
> +
> +/* A mapping from a gimple to a pointer to the copy history of it. */
> +static copy_history_map_t *copy_history_map;
> +
> +/* Obstack for copy history. */
> +static struct obstack copy_history_obstack;
> +
> +/* Create a new copy history. */
> +
> +copy_history_t
> +create_copy_history (location_t condition,
> + bool is_true_path,
> + enum copy_reason reason,
> + copy_history_t prev_copy)
> +{
> + static bool initialized = false;
> +
> + if (!initialized)
> + {
> + gcc_obstack_init (©_history_obstack);
> + initialized = true;
> + }
> +
> + copy_history_t copy_history
> + = (copy_history_t) obstack_alloc (©_history_obstack,
> + sizeof (struct copy_history));
> + copy_history->condition = condition;
> + copy_history->is_true_path = is_true_path;
> + copy_history->reason = reason;
> + copy_history->prev_copy = prev_copy;
> + return copy_history;
> +}
> +
> +/* Insert the copy history for the gimple STMT assuming the linked list
> + of CP_HISTORY does not have duplications. It's the caller's
> + responsibility to make sure that the linked list of CP_HISTORY does
> + not have duplications. */
> +
> +void
> +insert_copy_history (gimple *stmt, copy_history_t cp_history)
> +{
> + if (!copy_history_map)
> + copy_history_map = new copy_history_map_t;
> +
> + copy_history_map->put (stmt, cp_history);
> + return;
> +}
> +
> +/* Get the copy history for the gimple STMT, return NULL when there is
> + no associated copy history. */
> +
> +copy_history_t
> +get_copy_history (gimple *stmt)
> +{
> + if (!copy_history_map)
> + return NULL;
> +
> + if (const copy_history_t *cp_history_p = copy_history_map->get (stmt))
> + return *cp_history_p;
> +
> + return NULL;
> +}
> +
> +/* Remove the copy history for STMT. */
> +
> +void
> +remove_copy_history (gimple *stmt)
> +{
> + if (!copy_history_map)
> + return;
> + copy_history_map->remove (stmt);
> + return;
> +}
> +
> +/* Check whether the cond_location, is_true_path and reason existed
> + * in the OLD_COPY_HISTORY. */
> +
> +static bool
> +is_copy_history_existed (location_t cond_location, bool is_true_path,
> + enum copy_reason reason,
> + copy_history_t old_copy_history)
> +{
> + for (copy_history_t cur_ch = old_copy_history; cur_ch;
> + cur_ch = cur_ch->prev_copy)
> + if ((cur_ch->condition == cond_location)
> + && (cur_ch->is_true_path == is_true_path)
> + && (cur_ch->reason == reason))
> + return true;
> +
> + return false;
> +}
> +
> +/* Set copy history for the gimple STMT. Return TRUE when a new copy
> + * history is created and inserted. Otherwise return FALSE. */
> +
> +bool
> +set_copy_history (gimple *stmt, location_t cond_location,
> + bool is_true_path, enum copy_reason reason)
> +{
> +
> + /* First, get the old copy history associated with this STMT. */
> + copy_history_t old_cp_history = get_copy_history (stmt);
> +
> + /* If the current copy history is not in the STMT's copy history linked
> + list yet, create the new copy history, put the old_copy_history as the
> + prev_copy of it. */
> + copy_history_t new_cp_history = NULL;
> + if (!is_copy_history_existed (cond_location, is_true_path,
> + reason, old_cp_history))
> + new_cp_history
> + = create_copy_history (cond_location, is_true_path,
> + reason, old_cp_history);
> +
> + /* Insert the copy history into the hash map. */
> + if (new_cp_history)
> + {
> + insert_copy_history (stmt, new_cp_history);
> + return true;
> + }
> +
> + return false;
> +}
> +
> +/* Reset all state for diagnostic-copy-history.cc so that we can rerun the
> + compiler within the same process. For use by toplev::finalize. */
> +
> +void
> +copy_history_finalize (void)
> +{
> + if (copy_history_map)
> + {
> + delete copy_history_map;
> + copy_history_map = NULL;
> + }
> + obstack_free (©_history_obstack, NULL);
> + return;
> +}
> diff --git a/gcc/diagnostic-copy-history.h b/gcc/diagnostic-copy-history.h
> new file mode 100644
> index 00000000000..0c2c55a2a81
> --- /dev/null
> +++ b/gcc/diagnostic-copy-history.h
> @@ -0,0 +1,80 @@
> +/* Copy history associated with a gimple statement to record its history
> + of duplications due to different transformations.
> + The copy history will be used to construct events for later diagnostic.
> +
> + Copyright (C) 2024-2024 Free Software Foundation, Inc.
> + Contributed by Qing Zhao <qing.z...@oracle.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 DIAGNOSTIC_COPY_HISTORY_H_INCLUDED
> +#define DIAGNOSTIC_COPY_HISTORY_H_INCLUDED
> +
> +#include "hash-map.h"
> +#include "line-map.h"
> +
> +/* An enum for the reason why this copy is made. Right now, there are
> + three reasons, we can add more if needed. */
> +enum copy_reason {
> + COPY_BY_THREAD_JUMP,
> + COPY_BY_LOOP_UNSWITCH,
> + COPY_BY_LOOP_UNROLL
> +};
> +
> +/* This data structure records the information when a statement is
> + duplicated during different transformations. Such information
> + will be used by the later diagnostic messages to report more
> + contexts of the warnings or errors. */
> +struct copy_history {
> + /* The location of the condition statement that triggered the code
> + duplication. */
> + location_t condition;
> +
> + /* Whether this copy is on the TRUE path of the condition. */
> + bool is_true_path;
> +
> + /* The reason for the code duplication. */
> + enum copy_reason reason;
> +
> + /* This statement itself might be a previous code duplication. */
> + struct copy_history *prev_copy;
> +};
> +
> +typedef struct copy_history *copy_history_t;
> +
> +/* Create a new copy history. */
> +extern copy_history_t create_copy_history (location_t, bool,
> + enum copy_reason, copy_history_t);
> +
> +typedef hash_map<gimple *, copy_history_t> copy_history_map_t;
> +
> +/* Get the copy history for the gimple STMT, return NULL when there is
> + no associated copy history. */
> +extern copy_history_t get_copy_history (gimple *);
> +
> +/* Remove the copy history for STMT from the copy_history_map. */
> +extern void remove_copy_history (gimple *);
> +
> +/* Set copy history for the gimple STMT. */
> +extern bool set_copy_history (gimple *, location_t,
> + bool, enum copy_reason);
> +
> +/* Reset all state for diagnostic-copy-history.cc so that we can rerun the
> + compiler within the same process. For use by toplev::finalize. */
> +extern void copy_history_finalize (void);
> +
> +#endif // DIAGNOSTIC_COPY_HISTORY_H_INCLUDED
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index b37c7af7a39..bb0c6a87523 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -321,6 +321,7 @@ Objective-C and Objective-C++ Dialects}.
> -fdiagnostics-column-origin=@var{origin}
> -fdiagnostics-escape-format=@r{[}unicode@r{|}bytes@r{]}
> -fdiagnostics-text-art-charset=@r{[}none@r{|}ascii@r{|}unicode@r{|}emoji@r{]}}
> +-fdiagnostics-explain-harder
>
> @item Warning Options
> @xref{Warning Options,,Options to Request or Suppress Warnings}.
> @@ -5518,6 +5519,12 @@ left margin.
> This option controls the minimum width of the left margin printed by
> @option{-fdiagnostics-show-line-numbers}. It defaults to 6.
>
> +@opindex fdiagnostics-explain-harder
> +@item -fdiagnostics-explain-harder
> +With this option, the compiler collects more context information for
> +diagnostics and emits them to the users to provide more hints on how
> +the diagnostics come from.
> +
> @opindex fdiagnostics-parseable-fixits
> @item -fdiagnostics-parseable-fixits
> Emit fix-it hints in a machine-parseable format, suitable for consumption
> diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
> index 1637a2fc4f4..ba98ab3b413 100644
> --- a/gcc/gimple-array-bounds.cc
> +++ b/gcc/gimple-array-bounds.cc
> @@ -31,6 +31,9 @@ along with GCC; see the file COPYING3. If not see
> #include "tree-dfa.h"
> #include "fold-const.h"
> #include "diagnostic-core.h"
> +#include "simple-diagnostic-path.h"
> +#include "diagnostic-copy-history.h"
> +#include "copy-history-diagnostic-path.h"
> #include "intl.h"
> #include "tree-vrp.h"
> #include "alloc-pool.h"
> @@ -254,6 +257,25 @@ get_up_bounds_for_array_ref (tree ref, tree *decl,
> return;
> }
>
> +/* Build a rich location for LOCATION, and populate the diagonistic path
> + for it. */
> +
> +static rich_location *
> +build_rich_location_with_diagnostic_path (location_t location, gimple *stmt)
> +{
> + /* Generate a rich location for this location. */
> + rich_location *richloc = new rich_location (line_table, location);
> +
> + copy_history_t cp_history = stmt ? get_copy_history (stmt) : NULL;
> + copy_history_diagnostic_path *path
> + = new copy_history_diagnostic_path (global_dc->printer,
> + cp_history, location);
> + path->populate_path_from_copy_history ();
> +
> + richloc->set_path (path);
> + return richloc;
> +}
> +
> /* Given the LOW_SUB_ORG, LOW_SUB and UP_SUB, and the computed UP_BOUND
> and UP_BOUND_P1, check whether the array reference REF is out of bound.
> When out of bounds, set OUT_OF_BOUND to true.
> @@ -262,6 +284,7 @@ get_up_bounds_for_array_ref (tree ref, tree *decl,
>
> static bool
> check_out_of_bounds_and_warn (location_t location, tree ref,
> + gimple *stmt,
> tree low_sub_org, tree low_sub, tree up_sub,
> tree up_bound, tree up_bound_p1,
> const irange *vr,
> @@ -280,9 +303,13 @@ check_out_of_bounds_and_warn (location_t location, tree
> ref,
> {
> *out_of_bound = true;
> if (for_array_bound)
> - warned = warning_at (location, OPT_Warray_bounds_,
> + {
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + warned = warning_at (richloc, OPT_Warray_bounds_,
> "array subscript %E is outside array"
> " bounds of %qT", low_sub_org, artype);
> + }
> }
>
> if (warned)
> @@ -299,10 +326,14 @@ check_out_of_bounds_and_warn (location_t location, tree
> ref,
> {
> *out_of_bound = true;
> if (for_array_bound)
> - warned = warning_at (location, OPT_Warray_bounds_,
> + {
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + warned = warning_at (richloc, OPT_Warray_bounds_,
> "array subscript [%E, %E] is outside "
> "array bounds of %qT",
> low_sub, up_sub, artype);
> + }
> }
> }
> else if (up_bound
> @@ -313,18 +344,26 @@ check_out_of_bounds_and_warn (location_t location, tree
> ref,
> {
> *out_of_bound = true;
> if (for_array_bound)
> - warned = warning_at (location, OPT_Warray_bounds_,
> + {
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + warned = warning_at (richloc, OPT_Warray_bounds_,
> "array subscript %E is above array bounds of %qT",
> up_sub, artype);
> + }
> }
> else if (TREE_CODE (low_sub) == INTEGER_CST
> && tree_int_cst_lt (low_sub, low_bound))
> {
> *out_of_bound = true;
> if (for_array_bound)
> - warned = warning_at (location, OPT_Warray_bounds_,
> + {
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + warned = warning_at (richloc, OPT_Warray_bounds_,
> "array subscript %E is below array bounds of %qT",
> low_sub, artype);
> + }
> }
> return warned;
> }
> @@ -388,21 +427,24 @@ array_bounds_checker::check_array_ref (location_t
> location, tree ref,
> }
> }
>
> - warned = check_out_of_bounds_and_warn (location, ref,
> + warned = check_out_of_bounds_and_warn (location, ref, stmt,
> low_sub_org, low_sub, up_sub,
> up_bound, up_bound_p1, &vr,
> ignore_off_by_one, warn_array_bounds,
> &out_of_bound);
>
> -
> if (!warned && sam == special_array_member::int_0)
> - warned = warning_at (location, OPT_Wzero_length_bounds,
> + {
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + warned = warning_at (richloc, OPT_Wzero_length_bounds,
> (TREE_CODE (low_sub) == INTEGER_CST
> ? G_("array subscript %E is outside the bounds "
> "of an interior zero-length array %qT")
> : G_("array subscript %qE is outside the bounds "
> "of an interior zero-length array %qT")),
> low_sub, artype);
> + }
>
> if (warned && dump_file && (dump_flags & TDF_DETAILS))
> {
> @@ -419,8 +461,10 @@ array_bounds_checker::check_array_ref (location_t
> location, tree ref,
> || sam == special_array_member::trail_n)
> && DECL_NOT_FLEXARRAY (afield_decl))
> {
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> bool warned1
> - = warning_at (location, OPT_Wstrict_flex_arrays,
> + = warning_at (richloc, OPT_Wstrict_flex_arrays,
> "trailing array %qT should not be used as "
> "a flexible array member",
> artype);
> @@ -478,6 +522,7 @@ array_bounds_checker::check_array_ref (location_t
> location, tree ref,
>
> bool
> array_bounds_checker::check_mem_ref (location_t location, tree ref,
> + gimple *stmt,
> bool ignore_off_by_one)
> {
> if (warning_suppressed_p (ref, OPT_Warray_bounds_))
> @@ -580,16 +625,24 @@ array_bounds_checker::check_mem_ref (location_t
> location, tree ref,
> if (lboob)
> {
> if (offrange[0] == offrange[1])
> - warned = warning_at (location, OPT_Warray_bounds_,
> + {
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + warned = warning_at (richloc, OPT_Warray_bounds_,
> "array subscript %wi is outside array bounds "
> "of %qT",
> offrange[0].to_shwi (), reftype);
> + }
> else
> - warned = warning_at (location, OPT_Warray_bounds_,
> + {
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + warned = warning_at (richloc, OPT_Warray_bounds_,
> "array subscript [%wi, %wi] is outside "
> "array bounds of %qT",
> offrange[0].to_shwi (),
> offrange[1].to_shwi (), reftype);
> + }
> }
> else if (uboob && !ignore_off_by_one)
> {
> @@ -599,8 +652,9 @@ array_bounds_checker::check_mem_ref (location_t location,
> tree ref,
> it were an untyped array of bytes. */
> backtype = build_array_type_nelts (unsigned_char_type_node,
> aref.sizrng[1].to_uhwi ());
> -
> - warned = warning_at (location, OPT_Warray_bounds_,
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + warned = warning_at (richloc, OPT_Warray_bounds_,
> "array subscript %<%T[%wi]%> is partly "
> "outside array bounds of %qT",
> axstype, offrange[0].to_shwi (), backtype);
> @@ -623,7 +677,9 @@ array_bounds_checker::check_mem_ref (location_t location,
> tree ref,
> {
> HOST_WIDE_INT tmpidx = (aref.offmax[i] / eltsize).to_shwi ();
>
> - if (warning_at (location, OPT_Warray_bounds_,
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + if (warning_at (richloc, OPT_Warray_bounds_,
> "intermediate array offset %wi is outside array bounds "
> "of %qT", tmpidx, reftype))
> {
> @@ -656,7 +712,7 @@ array_bounds_checker::check_addr_expr (location_t
> location, tree t,
> ignore_off_by_one = false;
> }
> else if (TREE_CODE (t) == MEM_REF)
> - warned = check_mem_ref (location, t, ignore_off_by_one);
> + warned = check_mem_ref (location, t, stmt, ignore_off_by_one);
>
> if (warned)
> suppress_warning (t, OPT_Warray_bounds_);
> @@ -702,7 +758,9 @@ array_bounds_checker::check_addr_expr (location_t
> location, tree t,
> dump_generic_expr (MSG_NOTE, TDF_SLIM, t);
> fprintf (dump_file, "\n");
> }
> - warned = warning_at (location, OPT_Warray_bounds_,
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + warned = warning_at (richloc, OPT_Warray_bounds_,
> "array subscript %wi is below "
> "array bounds of %qT",
> idx.to_shwi (), TREE_TYPE (tem));
> @@ -716,7 +774,9 @@ array_bounds_checker::check_addr_expr (location_t
> location, tree t,
> dump_generic_expr (MSG_NOTE, TDF_SLIM, t);
> fprintf (dump_file, "\n");
> }
> - warned = warning_at (location, OPT_Warray_bounds_,
> + rich_location *richloc
> + = build_rich_location_with_diagnostic_path (location, stmt);
> + warned = warning_at (richloc, OPT_Warray_bounds_,
> "array subscript %wu is above "
> "array bounds of %qT",
> idx.to_uhwi (), TREE_TYPE (tem));
> @@ -811,7 +871,7 @@ array_bounds_checker::check_array_bounds (tree *tp, int
> *walk_subtree,
> warned = checker->check_array_ref (location, t, wi->stmt,
> false/*ignore_off_by_one*/);
> else if (TREE_CODE (t) == MEM_REF)
> - warned = checker->check_mem_ref (location, t,
> + warned = checker->check_mem_ref (location, t, wi->stmt,
> false /*ignore_off_by_one*/);
> else if (TREE_CODE (t) == ADDR_EXPR)
> {
> diff --git a/gcc/gimple-array-bounds.h b/gcc/gimple-array-bounds.h
> index aa7ca8e9730..2d1d48d1e94 100644
> --- a/gcc/gimple-array-bounds.h
> +++ b/gcc/gimple-array-bounds.h
> @@ -33,7 +33,7 @@ public:
> private:
> static tree check_array_bounds (tree *tp, int *walk_subtree, void *data);
> bool check_array_ref (location_t, tree, gimple *, bool ignore_off_by_one);
> - bool check_mem_ref (location_t, tree, bool ignore_off_by_one);
> + bool check_mem_ref (location_t, tree, gimple *, bool ignore_off_by_one);
> void check_addr_expr (location_t, tree, gimple *);
> void get_value_range (irange &r, const_tree op, gimple *);
>
> diff --git a/gcc/gimple-iterator.cc b/gcc/gimple-iterator.cc
> index 93646262eac..472a79360d7 100644
> --- a/gcc/gimple-iterator.cc
> +++ b/gcc/gimple-iterator.cc
> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3. If not see
> #include "tree-ssa.h"
> #include "value-prof.h"
> #include "gimplify.h"
> +#include "diagnostic-copy-history.h"
>
>
> /* Mark the statement STMT as modified, and update it. */
> @@ -581,6 +582,8 @@ gsi_remove (gimple_stmt_iterator *i, bool
> remove_permanently)
> cfun->debug_marker_count--;
> require_eh_edge_purge = remove_stmt_from_eh_lp (stmt);
> gimple_remove_stmt_histograms (cfun, stmt);
> + if (get_copy_history (stmt) != NULL)
> + remove_copy_history (stmt);
> }
>
> /* Update the iterator and re-wire the links in I->SEQ. */
> diff --git a/gcc/testsuite/gcc.dg/pr109071.c b/gcc/testsuite/gcc.dg/pr109071.c
> new file mode 100644
> index 00000000000..95ae576451e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr109071.c
> @@ -0,0 +1,43 @@
> +/* PR tree-optimization/109071 need more context for -Warray-bounds warnings
> + due to code duplication from jump threading. */
> +/* { dg-options "-O2 -Wall -fdiagnostics-explain-harder" } */
> +/* { dg-additional-options "-fdiagnostics-show-line-numbers
> -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
> +/* { dg-enable-nn-line-numbers "" } */
> +
> +extern void warn(void);
> +static inline void assign(int val, int *regs, int index)
> +{
> + if (index >= 4)
> + warn();
> + *regs = val;
> +}
> +struct nums {int vals[4];};
> +
> +void sparx5_set (int *ptr, struct nums *sg, int index)
> +{
> + int *val = &sg->vals[index]; /* { dg-warning "is above array bounds" } */
> +
> + assign(0, ptr, index);
> + assign(*val, ptr, index);
> +}
> +/* { dg-begin-multiline-output "" }
> + NN | int *val = &sg->vals[index];
> + | ~~~~~~~~^~~~~~~
> + 'sparx5_set': events 1-2
> + NN | if (index >= 4)
> + | ^
> + | |
> + | (1) when the condition is evaluated to true
> +......
> + { dg-end-multiline-output "" } */
> +
> +/* { dg-begin-multiline-output "" }
> + | ~~~~~~~~~~~~~~~
> + | |
> + | (2) out of array bounds here
> + { dg-end-multiline-output "" } */
> +
> +/* { dg-begin-multiline-output "" }
> + NN | struct nums {int vals[4];};
> + | ^~~~
> + { dg-end-multiline-output "" } */
> diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> index f715f977b72..e9ae55f64be 100644
> --- a/gcc/toplev.cc
> +++ b/gcc/toplev.cc
> @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3. If not see
> #include "cgraph.h"
> #include "coverage.h"
> #include "diagnostic.h"
> +#include "diagnostic-copy-history.h"
> #include "varasm.h"
> #include "tree-inline.h"
> #include "realmpfr.h" /* For GMP/MPFR/MPC versions, in print_version. */
> @@ -2381,6 +2382,8 @@ toplev::finalize (void)
> tree_cc_finalize ();
> reginfo_cc_finalize ();
>
> + copy_history_finalize ();
> +
> /* save_decoded_options uses opts_obstack, so these must
> be cleaned up together. */
> obstack_free (&opts_obstack, NULL);
> diff --git a/gcc/tree-ssa-threadupdate.cc b/gcc/tree-ssa-threadupdate.cc
> index fa61ba9512b..9292f0c2aa1 100644
> --- a/gcc/tree-ssa-threadupdate.cc
> +++ b/gcc/tree-ssa-threadupdate.cc
> @@ -36,6 +36,7 @@ along with GCC; see the file COPYING3. If not see
> #include "tree-cfg.h"
> #include "tree-vectorizer.h"
> #include "tree-pass.h"
> +#include "diagnostic-copy-history.h"
>
> /* Given a block B, update the CFG and SSA graph to reflect redirecting
> one or more in-edges to B to instead reach the destination of an
> @@ -2371,6 +2372,37 @@ back_jt_path_registry::adjust_paths_after_duplication
> (unsigned curr_path_num)
> }
> }
>
> +/* Set copy history to all the stmts in the basic block BB based on
> + the entry edge ENTRY and whether this basic block is on the true
> + path of the condition in the destination of the edge ENTRY.
> + Return TRUE when copy history are set, otherwise return FALSE. */
> +
> +static bool
> +set_copy_history_to_stmts_in_bb (basic_block bb, edge entry, bool
> is_true_path)
> +{
> + /* First, get the condition statement in the destination of the
> + edge ENTRY. */
> + basic_block cond_block = entry->dest;
> + gimple *cond_stmt = NULL;
> + gimple_stmt_iterator gsi;
> + gsi = gsi_last_bb (cond_block);
> + /* if the cond_block ends with a conditional statement, get it. */
> + if (!gsi_end_p (gsi)
> + && gsi_stmt (gsi)
> + && (gimple_code (gsi_stmt (gsi)) == GIMPLE_COND))
> + cond_stmt = gsi_stmt (gsi);
> +
> + if (!cond_stmt)
> + return false;
> +
> + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> + set_copy_history (gsi_stmt (gsi),
> + gimple_location (cond_stmt),
> + is_true_path,
> + COPY_BY_THREAD_JUMP);
> + return true;
> +}
> +
> /* Duplicates a jump-thread path of N_REGION basic blocks.
> The ENTRY edge is redirected to the duplicate of the region.
>
> @@ -2419,6 +2451,16 @@ back_jt_path_registry::duplicate_thread_path (edge
> entry,
> copy_bbs (region, n_region, region_copy, &exit, 1, &exit_copy, loop,
> split_edge_bb_loc (entry), false);
>
> + /* Set the copy history for all the stmts in both original and copied
> + basic blocks. The copied regions are in the true path. */
> + if (flag_diagnostics_explain_harder)
> + for (i = 0; i < n_region; i++)
> + {
> + set_copy_history_to_stmts_in_bb (region[i], entry, false);
> + set_copy_history_to_stmts_in_bb (region_copy[i], entry, true);
> + }
> +
> +
> /* Fix up: copy_bbs redirects all edges pointing to copied blocks. The
> following code ensures that all the edges exiting the jump-thread path
> are
> redirected back to the original code: these edges are exceptions
> --
> 2.31.1
>
