Message ID | 20240712140327.709430-1-qing.zhao@oracle.com |
---|---|
State | New |
Headers | show |
Series | [v2] Provide more contexts for -Warray-bounds warning messages | expand |
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.zhao@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.zhao@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.zhao@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.zhao@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.zhao@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 >
The 2nd ping for the following patch: https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657150.html thanks. Qing > On Jul 22, 2024, at 09:01, Qing Zhao <qing.zhao@oracle.com> wrote: > > 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.zhao@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.zhao@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.zhao@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.zhao@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.zhao@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 >> >
Hi, Richard, Do we still need such improvement into GCC? Could you please take a look at the patch and let me know Any comment or suggestions? thanks. Qing The 3rd ping for the following patch: https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657150.html > On Jul 29, 2024, at 11:32, Qing Zhao <qing.zhao@oracle.com> wrote: > > The 2nd ping for the following patch: > > https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657150.html > > thanks. > > Qing > >> On Jul 22, 2024, at 09:01, Qing Zhao <qing.zhao@oracle.com> wrote: >> >> 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.zhao@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.zhao@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.zhao@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.zhao@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.zhao@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 >>> >> >
Hi, Richard, I’d like to ping this patch again. For the convenience, the original 2nd version of the patch is at: https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657150.html The diagnostic part has been reviewed by David. Could you please take a look at the middle end implementation and let me know whether it’s okay for committing? It has been waiting for the middle-end review for 2 months already. The implementation is based on what you suggested during the discussion of this problem. Thanks. Qing > On Aug 12, 2024, at 09:50, Qing Zhao <qing.zhao@oracle.com> wrote: > > Hi, Richard, > > Do we still need such improvement into GCC? Could you please take a look at the patch and let me know > Any comment or suggestions? > > thanks. > > Qing > > The 3rd ping for the following patch: > > https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657150.html > >> On Jul 29, 2024, at 11:32, Qing Zhao <qing.zhao@oracle.com> wrote: >> >> The 2nd ping for the following patch: >> >> https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657150.html >> >> thanks. >> >> Qing >> >>> On Jul 22, 2024, at 09:01, Qing Zhao <qing.zhao@oracle.com> wrote: >>> >>> 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.zhao@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.zhao@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.zhao@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.zhao@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.zhao@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 >>>> >>> >> >
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.zhao@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.zhao@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.zhao@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.zhao@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