[v3,1/2,RFC] Provide more contexts for -Warray-bounds, -Wstringop-* warning messages due to code movements from compiler transformation [PR109071]
Checks
Commit Message
Control this with a new option -fdiagnostics-details.
$ 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 "move_history" to record
1. the "condition" that triggers the code movement;
2. whether the code movement is on the true path of the "condition";
3. the "compiler transformation" that triggers the code movement.
Whenever there is a code movement along control flow graph due to some
specific transformations, such as jump threading, path isolation, tree
sinking, etc., a move_history structure is created and attached to the
moved gimple statement.
During array out-of-bound checking or -Wstringop-* warning checking, the
"move_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 -fdiagnostics-details
which is off by default.
With this change, by adding -fdiagnostics-details,
the warning message for the above testing case is now:
$ gcc -Wall -O2 -fdiagnostics-details -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];};
| ^~~~
PR tree-optimization/109071
gcc/ChangeLog:
* Makefile.in (OBJS): Add diagnostic-move-history.o
and move-history-diagnostic-path.o.
* gcc/common.opt (fdiagnostics-details): New option.
* gcc/doc/invoke.texi (fdiagnostics-details): 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 move_history_diagnostic_path for warning_at.
(array_bounds_checker::check_array_ref): Use rich location with
move_history_diagnostic_path for warning_at.
(array_bounds_checker::check_mem_ref): Add one new parameter.
Use rich location with move_history_diagnostic_path for warning_at.
(array_bounds_checker::check_addr_expr): Use rich location with
move_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 move
history when removing the gimple.
* gimple-pretty-print.cc (pp_gimple_stmt_1): Emit MV_H marking
if the gimple has a move_history.
* gimple-ssa-isolate-paths.cc (isolate_path): Set move history
for the gimples of the duplicated blocks.
* gimple-ssa-warn-restrict.cc (maybe_diag_access_bounds): Use
rich location with move_history_diagnostic_path for warning_at.
* gimple-ssa-warn-access.cc (warn_string_no_nul): Likewise.
(maybe_warn_nonstring_arg): Likewise.
(maybe_warn_for_bound): Likewise.
(warn_for_access): Likewise.
(check_access): Likewise.
(pass_waccess::check_strncat): Likewise.
(pass_waccess::maybe_check_access_sizes): Likewise.
* tree-ssa-sink.cc (sink_code_in_bb): Create move_history for
stmt when it is sinked.
* toplev.cc (toplev::finalize): Call move_history_finalize.
* tree-ssa-threadupdate.cc (ssa_redirect_edges): Create move_history
for stmts when they are duplicated.
(back_jt_path_registry::duplicate_thread_path): Likewise.
* move-history-diagnostic-path.cc: New file.
* move-history-diagnostic-path.h: New file.
* diagnostic-move-history.cc: New file.
* diagnostic-move-history.h: New file.
gcc/testsuite/ChangeLog:
PR tree-optimization/109071
* gcc.dg/pr109071.c: New test.
* gcc.dg/pr109071_1.c: New test.
* gcc.dg/pr109071_2.c: New test.
* gcc.dg/pr109071_3.c: New test.
* gcc.dg/pr109071_4.c: New test.
* gcc.dg/pr109071_5.c: New test.
* gcc.dg/pr109071_6.c: New test.
---
gcc/Makefile.in | 2 +
gcc/common.opt | 4 +
gcc/diagnostic-move-history.cc | 264 ++++++++++++++++++++++++++++
gcc/diagnostic-move-history.h | 92 ++++++++++
gcc/doc/invoke.texi | 7 +
gcc/gimple-array-bounds.cc | 75 ++++++--
gcc/gimple-array-bounds.h | 2 +-
gcc/gimple-iterator.cc | 3 +
gcc/gimple-pretty-print.cc | 4 +
gcc/gimple-ssa-isolate-paths.cc | 11 ++
gcc/gimple-ssa-warn-access.cc | 153 ++++++++++------
gcc/gimple-ssa-warn-restrict.cc | 27 +--
gcc/move-history-diagnostic-path.cc | 119 +++++++++++++
gcc/move-history-diagnostic-path.h | 96 ++++++++++
gcc/testsuite/gcc.dg/pr109071.c | 43 +++++
gcc/testsuite/gcc.dg/pr109071_1.c | 36 ++++
gcc/testsuite/gcc.dg/pr109071_2.c | 50 ++++++
gcc/testsuite/gcc.dg/pr109071_3.c | 42 +++++
gcc/testsuite/gcc.dg/pr109071_4.c | 41 +++++
gcc/testsuite/gcc.dg/pr109071_5.c | 33 ++++
gcc/testsuite/gcc.dg/pr109071_6.c | 49 ++++++
gcc/toplev.cc | 3 +
gcc/tree-ssa-sink.cc | 10 ++
gcc/tree-ssa-threadupdate.cc | 25 +++
24 files changed, 1105 insertions(+), 86 deletions(-)
create mode 100644 gcc/diagnostic-move-history.cc
create mode 100644 gcc/diagnostic-move-history.h
create mode 100644 gcc/move-history-diagnostic-path.cc
create mode 100644 gcc/move-history-diagnostic-path.h
create mode 100644 gcc/testsuite/gcc.dg/pr109071.c
create mode 100644 gcc/testsuite/gcc.dg/pr109071_1.c
create mode 100644 gcc/testsuite/gcc.dg/pr109071_2.c
create mode 100644 gcc/testsuite/gcc.dg/pr109071_3.c
create mode 100644 gcc/testsuite/gcc.dg/pr109071_4.c
create mode 100644 gcc/testsuite/gcc.dg/pr109071_5.c
create mode 100644 gcc/testsuite/gcc.dg/pr109071_6.c
Comments
Qing Zhao <qing.zhao@oracle.com> writes:
> Control this with a new option -fdiagnostics-details.
>
> [...]
The patch doesn't apply for me on very latest trunk -- I think David's
recent diag refactoring means it needs a slight rebase. Could you send
that?
On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
> Qing Zhao <qing.zhao@oracle.com> writes:
>
> > Control this with a new option -fdiagnostics-details.
> >
> > [...]
>
> The patch doesn't apply for me on very latest trunk -- I think
> David's
> recent diag refactoring means it needs a slight rebase. Could you
> send
> that?
If it's broken, it was probably by:
r15-4610 ("Use unique_ptr in more places in pretty_printer/diagnostics
[PR116613]")
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=bf43fe6aa966eaf397ea3b8ebd6408d3d124e285
and/or
r15-4617 ("analyzer: avoid implicit use of global_dc's pretty_printer
[PR116613]")
https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666390.html
which both made small changes to the internal interface for creating
events in a diagnostic path, and possibly by:
r15-4760 ("diagnostics: support multiple output formats simultaneously
[PR116613]")
https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666807.html
which made big changes to diagnostic_context internally.
Sorry about this; let me know if you want help debugging/fixing things.
Dave
> On Oct 30, 2024, at 10:34, Sam James <sam@gentoo.org> wrote:
>
> Qing Zhao <qing.zhao@oracle.com> writes:
>
>> Control this with a new option -fdiagnostics-details.
>>
>> [...]
>
> The patch doesn't apply for me on very latest trunk -- I think David's
> recent diag refactoring means it needs a slight rebase. Could you send
> that?
I rebased the patch sets against a later trunk, but looks like not late enough…
I will rebase it on the latest one and resend the patch.
Sorry about this.
Qing
>
> On Oct 30, 2024, at 10:48, David Malcolm <dmalcolm@redhat.com> wrote:
>
> On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
>> Qing Zhao <qing.zhao@oracle.com> writes:
>>
>>> Control this with a new option -fdiagnostics-details.
>>>
>>> [...]
>>
>> The patch doesn't apply for me on very latest trunk -- I think
>> David's
>> recent diag refactoring means it needs a slight rebase. Could you
>> send
>> that?
>
> If it's broken, it was probably by:
>
> r15-4610 ("Use unique_ptr in more places in pretty_printer/diagnostics
> [PR116613]")
> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=bf43fe6aa966eaf397ea3b8ebd6408d3d124e285
Yes, due to the following change in the above commit:
diff --git a/gcc/toplev.cc b/gcc/toplev.cc
index 62034c32b4aff32cdf2cb051bf9d0803b4730b3f..a12a2e1afba15ba16f6ade624cde3e60907ba5d2 100644 (file)
--- a/gcc/toplev.cc
+++ b/gcc/toplev.cc
@@ -42,6 +42,7 @@ along with GCC; see the file COPYING3. If not see
#include "cgraph.h"
#include "coverage.h"
#include "diagnostic.h"
+#include "pretty-print-urlifier.h"
#include "varasm.h"
#include "tree-inline.h"
#include "realmpfr.h" /* For GMP/MPFR/MPC versions, in print_version. */
>
> and/or
>
> r15-4617 ("analyzer: avoid implicit use of global_dc's pretty_printer
> [PR116613]")
> https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666390.html
>
> which both made small changes to the internal interface for creating
> events in a diagnostic path, and possibly by:
>
> r15-4760 ("diagnostics: support multiple output formats simultaneously
> [PR116613]")
> https://gcc.gnu.org/pipermail/gcc-patches/2024-October/666807.html
>
> which made big changes to diagnostic_context internally.
Just rebased the patch against the latest trunk today.
Redo the bootstrap and regression test now.
>
> Sorry about this; let me know if you want help debugging/fixing things.
I have a question on the changes to the “warning_at”: (there are a lot of such changes for -Warray-bounds and -Wstringop-**)
- 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_,
The above is the current change.
My concern with this change is:
even when -fdiagnostics_details is NOT on, the rich_location is created.
How much is the additional overhead when using “rich_location *” other than “location_t” as the 1st argument of warning_at?
Should I control the creation of “rich_location" with the flag “flag_diagnostics_details” (Similar as I control the creation of “move_history” data structure with the flag “flag_diagnostics_details”?
If so, how should I do it? Do you have a suggestion on a clean and simply coding here (Sorry for the stupid question on this)
Thanks a lot for the help.
Qing
> Dave
>
Qing Zhao <qing.zhao@oracle.com> writes:
> Control this with a new option -fdiagnostics-details.
It would be useful to be also able to print the inline call stack,
maybe with a separate option.
In some array bounds cases I looked at the problem was hidden in some inlines
and it wasn't trivial to figure it out.
I wrote this patch for it at some point.
Print inline stack for warn access warnings
The warnings reported by gimple-ssa-warn-access often depend on the
caller with inlining, and when there are a lot of callers it can be
difficult to figure out which caller triggered a warning.
Print the function context including inline stack for these
warnings.
gcc/ChangeLog:
* gimple-ssa-warn-access.cc (maybe_inform_function): New
function to report function context.
(warn_string_no_nul): Use maybe_inform_function.
(maybe_warn_nonstring_arg): Dito.
(maybe_warn_for_bound): Dito.
(warn_for_access): Dito.
(check_access): Dito.
(warn_dealloc_offset): Dito.
(maybe_warn_alloc_args_overflow): Dito.
(pass_waccess::check_strncat): Dito.
(pass_waccess::maybe_check_access_sizes): Dito.
(pass_waccess::maybe_check_dealloc_call): Dito.
(pass_waccess::warn_invalid_pointer): Dito.
(maybe_warn_mismatched_realloc): Dito.
(pass_waccess::check_dangling_stores): Dito.
(pass_waccess::execute): Reset last_function variable.
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 61f9f0f3d310..94c043531988 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -125,6 +125,21 @@ call_arg (tree expr, unsigned argno)
return CALL_EXPR_ARG (expr, argno);
}
+/* Already printed inform for the function. */
+static bool printed_function;
+
+/* Inform about the function stack unless warning is suppressed at LOC
+ with opt code OPT. */
+static void
+maybe_inform_function (location_t loc, int opt)
+{
+ if (printed_function)
+ return;
+ printed_function = true;
+ if (!warning_suppressed_at (loc, (opt_code)opt))
+ inform (DECL_SOURCE_LOCATION (cfun->decl), "in function %qD", cfun->decl);
+}
+
/* For a call EXPR at LOC to a function FNAME that expects a string
in the argument ARG, issue a diagnostic due to it being a called
with an argument that is a character array with no terminating
@@ -162,6 +177,8 @@ warn_string_no_nul (location_t loc, GimpleOrTree expr, const char *fname,
auto_diagnostic_group d;
+ maybe_inform_function (loc, opt);
+
const tree maxobjsize = max_object_size ();
const wide_int maxsiz = wi::to_wide (maxobjsize);
if (expr)
@@ -485,6 +502,7 @@ maybe_warn_nonstring_arg (tree fndecl, GimpleOrTree exp)
if (tree_int_cst_lt (maxobjsize, bndrng[0]))
{
bool warned = false;
+ maybe_inform_function (loc, OPT_Wstringop_overread);
if (tree_int_cst_equal (bndrng[0], bndrng[1]))
warned = warning_at (loc, OPT_Wstringop_overread,
"%qD specified bound %E "
@@ -638,6 +656,7 @@ maybe_warn_nonstring_arg (tree fndecl, GimpleOrTree exp)
auto_diagnostic_group d;
if (wi::ltu_p (asize, wibnd))
{
+ maybe_inform_function (loc, OPT_Wstringop_overread);
if (bndrng[0] == bndrng[1])
warned = warning_at (loc, OPT_Wstringop_overread,
"%qD argument %i declared attribute "
@@ -723,6 +742,7 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
auto_diagnostic_group d;
if (tree_int_cst_lt (maxobjsize, bndrng[0]))
{
+ maybe_inform_function (loc, opt);
if (bndrng[0] == bndrng[1])
warned = (func
? warning_at (loc, opt,
@@ -760,7 +780,9 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
else if (!size || tree_int_cst_le (bndrng[0], size))
return false;
else if (tree_int_cst_equal (bndrng[0], bndrng[1]))
- warned = (func
+ {
+ maybe_inform_function (loc, opt);
+ warned = (func
? warning_at (loc, opt,
(maybe
? G_("%qD specified bound %E may exceed "
@@ -775,8 +797,11 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
: G_("specified bound %E exceeds "
"source size %E")),
bndrng[0], size));
+ }
else
- warned = (func
+ {
+ maybe_inform_function (loc, opt);
+ warned = (func
? warning_at (loc, opt,
(maybe
? G_("%qD specified bound [%E, %E] may "
@@ -791,6 +816,7 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
: G_("specified bound [%E, %E] exceeds "
"source size %E")),
bndrng[0], bndrng[1], size));
+ }
if (warned)
{
if (pad && pad->src.ref
@@ -816,7 +842,9 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
if (tree_int_cst_lt (maxobjsize, bndrng[0]))
{
if (bndrng[0] == bndrng[1])
- warned = (func
+ {
+ maybe_inform_function (loc, opt);
+ warned = (func
? warning_at (loc, opt,
(maybe
? G_("%qD specified size %E may "
@@ -831,8 +859,11 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
: G_("specified size %E exceeds "
"maximum object size %E")),
bndrng[0], maxobjsize));
+ }
else
- warned = (func
+ {
+ maybe_inform_function (loc, opt);
+ warned = (func
? warning_at (loc, opt,
(maybe
? G_("%qD specified size between %E and %E "
@@ -847,11 +878,14 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
: G_("specified size between %E and %E "
"exceeds maximum object size %E")),
bndrng[0], bndrng[1], maxobjsize));
+ }
}
else if (!size || tree_int_cst_le (bndrng[0], size))
return false;
else if (tree_int_cst_equal (bndrng[0], bndrng[1]))
- warned = (func
+ {
+ maybe_inform_function (loc, opt);
+ warned = (func
? warning_at (loc, opt,
(maybe
? G_("%qD specified bound %E may exceed "
@@ -866,8 +900,11 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
: G_("specified bound %E exceeds "
"destination size %E")),
bndrng[0], size));
+ }
else
- warned = (func
+ {
+ maybe_inform_function (loc, opt);
+ warned = (func
? warning_at (loc, opt,
(maybe
? G_("%qD specified bound [%E, %E] may exceed "
@@ -882,6 +919,7 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
: G_("specified bound [%E, %E] exceeds "
"destination size %E")),
bndrng[0], bndrng[1], size));
+ }
if (warned)
{
@@ -928,6 +966,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
if (write && read)
{
+ maybe_inform_function (loc, opt);
if (tree_int_cst_equal (range[0], range[1]))
warned = (func
? warning_n (loc, opt, tree_to_uhwi (range[0]),
@@ -994,6 +1033,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
if (write)
{
+ maybe_inform_function (loc, opt);
if (tree_int_cst_equal (range[0], range[1]))
warned = (func
? warning_n (loc, opt, tree_to_uhwi (range[0]),
@@ -1064,6 +1104,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
if (read)
{
+ maybe_inform_function (loc, OPT_Wstringop_overread);
if (tree_int_cst_equal (range[0], range[1]))
warned = (func
? warning_n (loc, OPT_Wstringop_overread,
@@ -1134,6 +1175,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
return warned;
}
+ maybe_inform_function (loc, OPT_Wstringop_overread);
if (tree_int_cst_equal (range[0], range[1])
|| tree_int_cst_sign_bit (range[1]))
warned = (func
@@ -1400,6 +1442,7 @@ check_access (GimpleOrTree exp, tree dstwrite,
bool warned = false;
if (dstwrite == slen && at_least_one)
{
+ maybe_inform_function (loc, opt);
/* This is a call to strcpy with a destination of 0 size
and a source of unknown length. The call will write
at least one byte past the end of the destination. */
@@ -2046,6 +2089,7 @@ warn_dealloc_offset (location_t loc, gimple *call, const access_ref &aref)
}
auto_diagnostic_group d;
+ maybe_inform_function (loc, OPT_Wfree_nonheap_object);
if (!warning_at (loc, OPT_Wfree_nonheap_object,
"%qD called on pointer %qE with nonzero offset%s",
dealloc_decl, aref.ref, offstr))
@@ -2273,6 +2317,7 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
if (tree_int_cst_lt (args[i], integer_zero_node))
{
+ maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
warned = warning_at (loc, OPT_Walloc_size_larger_than_,
"argument %i value %qE is negative",
idx[i] + 1, args[i]);
@@ -2290,9 +2335,12 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
? IDENTIFIER_LENGTH (DECL_NAME (fn)) != 6
: !lookup_attribute ("returns_nonnull",
TYPE_ATTRIBUTES (fntype)))
- warned = warning_at (loc, OPT_Walloc_zero,
+ {
+ maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
+ warned = warning_at (loc, OPT_Walloc_zero,
"argument %i value is zero",
idx[i] + 1);
+ }
}
else if (tree_int_cst_lt (maxobjsize, args[i]))
{
@@ -2308,6 +2356,7 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
&& integer_all_onesp (args[i]))
continue;
+ maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
warned = warning_at (loc, OPT_Walloc_size_larger_than_,
"argument %i value %qE exceeds "
"maximum object size %E",
@@ -2322,6 +2371,7 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
if (tree_int_cst_lt (argrange[i][0], integer_zero_node)
&& tree_int_cst_le (argrange[i][1], integer_zero_node))
{
+ maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
warned = warning_at (loc, OPT_Walloc_size_larger_than_,
"argument %i range [%E, %E] is negative",
idx[i] + 1,
@@ -2329,6 +2379,7 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
}
else if (tree_int_cst_lt (maxobjsize, argrange[i][0]))
{
+ maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
warned = warning_at (loc, OPT_Walloc_size_larger_than_,
"argument %i range [%E, %E] exceeds "
"maximum object size %E",
@@ -2359,18 +2410,24 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
wide_int prod = wi::umul (x, y, &vflow);
if (vflow)
- warned = warning_at (loc, OPT_Walloc_size_larger_than_,
+ {
+ maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
+ warned = warning_at (loc, OPT_Walloc_size_larger_than_,
"product %<%E * %E%> of arguments %i and %i "
"exceeds %<SIZE_MAX%>",
argrange[0][0], argrange[1][0],
idx[0] + 1, idx[1] + 1);
+ }
else if (wi::ltu_p (wi::to_wide (maxobjsize, szprec), prod))
- warned = warning_at (loc, OPT_Walloc_size_larger_than_,
+ {
+ maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
+ warned = warning_at (loc, OPT_Walloc_size_larger_than_,
"product %<%E * %E%> of arguments %i and %i "
"exceeds maximum object size %E",
argrange[0][0], argrange[1][0],
idx[0] + 1, idx[1] + 1,
maxobjsize);
+ }
if (warned)
{
@@ -2563,6 +2620,7 @@ pass_waccess::check_strncat (gcall *stmt)
&& tree_int_cst_equal (destsize, maxread))
{
location_t loc = get_location (stmt);
+ maybe_inform_function (loc, OPT_Wstringop_overflow_);
warning_at (loc, OPT_Wstringop_overflow_,
"%qD specified bound %E equals destination size",
get_callee_fndecl (stmt), maxread);
@@ -3454,6 +3512,7 @@ pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, tree fndecl, tree fntype,
const std::string argtypestr
= access.second.array_as_string (ptrtype);
+ maybe_inform_function (loc, OPT_Wstringop_overflow_);
if (warning_at (loc, OPT_Wstringop_overflow_,
"bound argument %i value %s is "
"negative for a variable length array "
@@ -3508,6 +3567,7 @@ pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, tree fndecl, tree fntype,
different from also declaring the pointer argument with
attribute nonnull when the function accepts null pointers
only when the corresponding size is zero. */
+ maybe_inform_function (loc, OPT_Wnonnull);
if (warning_at (loc, OPT_Wnonnull,
"argument %i is null but "
"the corresponding size argument "
@@ -3706,6 +3766,7 @@ pass_waccess::maybe_check_dealloc_call (gcall *call)
if (aref.ref_declared ())
{
auto_diagnostic_group d;
+ maybe_inform_function (loc, OPT_Wfree_nonheap_object);
if (warning_at (loc, OPT_Wfree_nonheap_object,
"%qD called on unallocated object %qD",
dealloc_decl, ref))
@@ -3725,6 +3786,7 @@ pass_waccess::maybe_check_dealloc_call (gcall *call)
else if (CONSTANT_CLASS_P (ref))
{
auto_diagnostic_group d;
+ maybe_inform_function (loc, OPT_Wfree_nonheap_object);
if (warning_at (loc, OPT_Wfree_nonheap_object,
"%qD called on a pointer to an unallocated "
"object %qE", dealloc_decl, ref))
@@ -3767,6 +3829,7 @@ pass_waccess::maybe_check_dealloc_call (gcall *call)
|| DECL_IS_OPERATOR_DELETE_P (dealloc_decl)
? OPT_Wmismatched_new_delete
: OPT_Wmismatched_dealloc);
+ maybe_inform_function (loc, opt);
warned = warning_at (loc, opt,
"%qD called on pointer returned "
"from a mismatched allocation "
@@ -3776,10 +3839,13 @@ pass_waccess::maybe_check_dealloc_call (gcall *call)
else if (gimple_call_builtin_p (def_stmt, BUILT_IN_ALLOCA)
|| gimple_call_builtin_p (def_stmt,
BUILT_IN_ALLOCA_WITH_ALIGN))
- warned = warning_at (loc, OPT_Wfree_nonheap_object,
+ {
+ maybe_inform_function (loc, OPT_Wfree_nonheap_object);
+ warned = warning_at (loc, OPT_Wfree_nonheap_object,
"%qD called on pointer to "
"an unallocated object",
dealloc_decl);
+ }
else if (warn_dealloc_offset (loc, call, aref))
return;
@@ -3923,6 +3989,7 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple *use_stmt,
const tree inval_decl = gimple_call_fndecl (inval_stmt);
auto_diagnostic_group d;
+ maybe_inform_function (use_loc, OPT_Wuse_after_free);
if ((ref && warning_at (use_loc, OPT_Wuse_after_free,
(maybe
? G_("pointer %qE may be used after %qD")
@@ -3949,6 +4016,7 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple *use_stmt,
if (DECL_NAME (var))
{
auto_diagnostic_group d;
+ maybe_inform_function (use_loc, OPT_Wdangling_pointer_);
if ((ref
&& warning_at (use_loc, OPT_Wdangling_pointer_,
(maybe
@@ -3967,6 +4035,7 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple *use_stmt,
return;
}
+ maybe_inform_function (use_loc, OPT_Wdangling_pointer_);
if ((ref
&& warning_at (use_loc, OPT_Wdangling_pointer_,
(maybe
@@ -4073,6 +4142,7 @@ maybe_warn_mismatched_realloc (tree ptr, gimple *realloc_stmt, gimple *stmt)
location_t loc = gimple_location (stmt);
tree realloc_decl = gimple_call_fndecl (realloc_stmt);
tree dealloc_decl = gimple_call_fndecl (stmt);
+ maybe_inform_function (loc, OPT_Wmismatched_dealloc);
if (ptr && !warning_at (loc, OPT_Wmismatched_dealloc,
"%qD called on pointer %qE passed to mismatched "
"allocation function %qD",
@@ -4603,6 +4673,7 @@ pass_waccess::check_dangling_stores (basic_block bb,
auto_diagnostic_group d;
location_t loc = gimple_location (stmt);
+ maybe_inform_function (loc, OPT_Wdangling_pointer_);
if (warning_at (loc, OPT_Wdangling_pointer_,
"storing the address of local variable %qD in %qE",
rhs_ref.ref, lhs))
@@ -4750,6 +4821,7 @@ pass_waccess::check_call_dangling (gcall *call)
unsigned
pass_waccess::execute (function *fun)
{
+ printed_function = false;
calculate_dominance_info (CDI_DOMINATORS);
calculate_dominance_info (CDI_POST_DOMINATORS);
-Andi
Qing Zhao <qing.zhao@oracle.com> writes:
>> On Oct 30, 2024, at 10:48, David Malcolm <dmalcolm@redhat.com> wrote:
>>
>> On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
>>> Qing Zhao <qing.zhao@oracle.com> writes:
>>>
>>>> Control this with a new option -fdiagnostics-details.
>>>>
>>>> [...]
>>>
>>> The patch doesn't apply for me on very latest trunk -- I think
>>> David's
>>> recent diag refactoring means it needs a slight rebase. Could you
>>> send
>>> that?
>>
>> If it's broken, it was probably by:
>>
>> r15-4610 ("Use unique_ptr in more places in pretty_printer/diagnostics
>> [PR116613]")
>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=bf43fe6aa966eaf397ea3b8ebd6408d3d124e285
>
> Yes, due to the following change in the above commit:
>
> diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> index 62034c32b4aff32cdf2cb051bf9d0803b4730b3f..a12a2e1afba15ba16f6ade624cde3e60907ba5d2 100644 (file)
> --- a/gcc/toplev.cc
> +++ b/gcc/toplev.cc
> @@ -42,6 +42,7 @@ along with GCC; see the file COPYING3. If not see
> #include "cgraph.h"
> #include "coverage.h"
> #include "diagnostic.h"
> +#include "pretty-print-urlifier.h"
> #include "varasm.h"
> #include "tree-inline.h"
> #include "realmpfr.h" /* For GMP/MPFR/MPC versions, in print_version. */
>
>
> [...]
>>
To continue testing, I am using the attached hacked up patches (David,
please don't hurt me, it was minimal "just get it building" x "do more
than needed to reduce iterations" ;)).
When the rebase is ready, I'll switch to those. Thank you both!
Qing Zhao <qing.zhao@oracle.com> writes:
> Control this with a new option -fdiagnostics-details.
>
> $ 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 "move_history" to record
> 1. the "condition" that triggers the code movement;
> 2. whether the code movement is on the true path of the "condition";
> 3. the "compiler transformation" that triggers the code movement.
>
> Whenever there is a code movement along control flow graph due to some
> specific transformations, such as jump threading, path isolation, tree
> sinking, etc., a move_history structure is created and attached to the
> moved gimple statement.
>
> During array out-of-bound checking or -Wstringop-* warning checking, the
> "move_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 -fdiagnostics-details
> which is off by default.
>
> With this change, by adding -fdiagnostics-details,
> the warning message for the above testing case is now:
>
> $ gcc -Wall -O2 -fdiagnostics-details -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];};
> | ^~~~
>
> PR tree-optimization/109071
>
> gcc/ChangeLog:
>
> * Makefile.in (OBJS): Add diagnostic-move-history.o
> and move-history-diagnostic-path.o.
> * gcc/common.opt (fdiagnostics-details): New option.
> * gcc/doc/invoke.texi (fdiagnostics-details): 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 move_history_diagnostic_path for warning_at.
> (array_bounds_checker::check_array_ref): Use rich location with
> move_history_diagnostic_path for warning_at.
> (array_bounds_checker::check_mem_ref): Add one new parameter.
> Use rich location with move_history_diagnostic_path for warning_at.
> (array_bounds_checker::check_addr_expr): Use rich location with
> move_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 move
> history when removing the gimple.
> * gimple-pretty-print.cc (pp_gimple_stmt_1): Emit MV_H marking
> if the gimple has a move_history.
> * gimple-ssa-isolate-paths.cc (isolate_path): Set move history
> for the gimples of the duplicated blocks.
> * gimple-ssa-warn-restrict.cc (maybe_diag_access_bounds): Use
> rich location with move_history_diagnostic_path for warning_at.
> * gimple-ssa-warn-access.cc (warn_string_no_nul): Likewise.
> (maybe_warn_nonstring_arg): Likewise.
> (maybe_warn_for_bound): Likewise.
> (warn_for_access): Likewise.
> (check_access): Likewise.
> (pass_waccess::check_strncat): Likewise.
> (pass_waccess::maybe_check_access_sizes): Likewise.
> * tree-ssa-sink.cc (sink_code_in_bb): Create move_history for
> stmt when it is sinked.
> * toplev.cc (toplev::finalize): Call move_history_finalize.
> * tree-ssa-threadupdate.cc (ssa_redirect_edges): Create move_history
> for stmts when they are duplicated.
> (back_jt_path_registry::duplicate_thread_path): Likewise.
> * move-history-diagnostic-path.cc: New file.
> * move-history-diagnostic-path.h: New file.
> * diagnostic-move-history.cc: New file.
> * diagnostic-move-history.h: New file.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/109071
>
> * gcc.dg/pr109071.c: New test.
> * gcc.dg/pr109071_1.c: New test.
> * gcc.dg/pr109071_2.c: New test.
> * gcc.dg/pr109071_3.c: New test.
> * gcc.dg/pr109071_4.c: New test.
> * gcc.dg/pr109071_5.c: New test.
> * gcc.dg/pr109071_6.c: New test.
> ---
> gcc/Makefile.in | 2 +
> gcc/common.opt | 4 +
> gcc/diagnostic-move-history.cc | 264 ++++++++++++++++++++++++++++
> gcc/diagnostic-move-history.h | 92 ++++++++++
> gcc/doc/invoke.texi | 7 +
> gcc/gimple-array-bounds.cc | 75 ++++++--
> gcc/gimple-array-bounds.h | 2 +-
> gcc/gimple-iterator.cc | 3 +
> gcc/gimple-pretty-print.cc | 4 +
> gcc/gimple-ssa-isolate-paths.cc | 11 ++
> gcc/gimple-ssa-warn-access.cc | 153 ++++++++++------
> gcc/gimple-ssa-warn-restrict.cc | 27 +--
> gcc/move-history-diagnostic-path.cc | 119 +++++++++++++
> gcc/move-history-diagnostic-path.h | 96 ++++++++++
> gcc/testsuite/gcc.dg/pr109071.c | 43 +++++
> gcc/testsuite/gcc.dg/pr109071_1.c | 36 ++++
> gcc/testsuite/gcc.dg/pr109071_2.c | 50 ++++++
> gcc/testsuite/gcc.dg/pr109071_3.c | 42 +++++
> gcc/testsuite/gcc.dg/pr109071_4.c | 41 +++++
> gcc/testsuite/gcc.dg/pr109071_5.c | 33 ++++
> gcc/testsuite/gcc.dg/pr109071_6.c | 49 ++++++
> gcc/toplev.cc | 3 +
> gcc/tree-ssa-sink.cc | 10 ++
> gcc/tree-ssa-threadupdate.cc | 25 +++
> 24 files changed, 1105 insertions(+), 86 deletions(-)
> create mode 100644 gcc/diagnostic-move-history.cc
> create mode 100644 gcc/diagnostic-move-history.h
> create mode 100644 gcc/move-history-diagnostic-path.cc
> create mode 100644 gcc/move-history-diagnostic-path.h
> create mode 100644 gcc/testsuite/gcc.dg/pr109071.c
> create mode 100644 gcc/testsuite/gcc.dg/pr109071_1.c
> create mode 100644 gcc/testsuite/gcc.dg/pr109071_2.c
> create mode 100644 gcc/testsuite/gcc.dg/pr109071_3.c
> create mode 100644 gcc/testsuite/gcc.dg/pr109071_4.c
> create mode 100644 gcc/testsuite/gcc.dg/pr109071_5.c
> create mode 100644 gcc/testsuite/gcc.dg/pr109071_6.c
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 059cf2e8f79f..0d119ba46e1b 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1432,6 +1432,8 @@ OBJS = \
> df-problems.o \
> df-scan.o \
> dfp.o \
> + diagnostic-move-history.o \
> + move-history-diagnostic-path.o \
> digraph.o \
> dojump.o \
> dominance.o \
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 12b25ff486de..84ebef080143 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1566,6 +1566,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-details
> +Common Var(flag_diagnostics_details)
> +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/diagnostic-move-history.cc b/gcc/diagnostic-move-history.cc
> new file mode 100644
> index 000000000000..b0e8308dbf6b
> --- /dev/null
> +++ b/gcc/diagnostic-move-history.cc
> @@ -0,0 +1,264 @@
> +/* Functions to handle move history.
> + Copyright (C) 2024-2024 Free Software Foundation, Inc.
Just '2024'.
> + Contributed by Qing Zhao <qing.zhao@oracle.com>
> +
> [...]
> diff --git a/gcc/diagnostic-move-history.h b/gcc/diagnostic-move-history.h
> new file mode 100644
> index 000000000000..cac9cb1e2675
> --- /dev/null
> +++ b/gcc/diagnostic-move-history.h
> @@ -0,0 +1,92 @@
> +/* Move history associated with a gimple statement to record its history
> + of movement due to different transformations.
> + The move 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_MOVE_HISTORY_H_INCLUDED
> +#define DIAGNOSTIC_MOVE_HISTORY_H_INCLUDED
I think usually the guards lack _INCLUDED.
> +
> +#include "hash-map.h"
> +#include "line-map.h"
> +
> +/* An enum for the reason why this move is made. Right now, there are
> + three reasons, we can add more if needed. */
> +enum move_reason {
> + COPY_BY_THREAD_JUMP,
> + COPY_BY_ISOLATE_PATH,
> + MOVE_BY_SINK,
> + COPY_BY_MAX
> +};
> +
> +/* This data structure records the information when a statement is
> + moved along control flow graph during different transformations.
> + Such information will be used by the later diagnostic messages
> + to report more contexts of the warnings or errors. */
> +struct move_history {
> + /* The location of the condition statement that triggered the code
> + movement. */
> + location_t condition;
> +
> + /* Whether this move is on the TRUE path of the condition. */
> + bool is_true_path;
> +
> + /* The reason for the code movement. */
> + enum move_reason reason;
> +
> + /* This statement itself might be a previous code movement. */
> + struct move_history *prev_move;
> +};
> +
> +typedef struct move_history *move_history_t;
> +
> +/* Create a new move history. */
> +extern move_history_t create_move_history (location_t, bool,
> + enum move_reason, move_history_t);
> +
> +typedef hash_map<const gimple *, move_history_t> move_history_map_t;
> +
> +/* Get the move history for the gimple STMT, return NULL when there is
> + no associated move history. */
> +extern move_history_t get_move_history (const gimple *);
> +
> +/* Remove the move history for STMT from the move_history_map. */
> +extern void remove_move_history (gimple *);
> +
> +/* Set move history for the gimple STMT. */
> +extern bool set_move_history (gimple *, location_t,
> + bool, enum move_reason);
> +
> +/* Reset all state for diagnostic-move-history.cc so that we can rerun the
> + compiler within the same process. For use by toplev::finalize. */
> +extern void move_history_finalize (void);
> +
> +/* Set move history to the stmt based on the edge ENTRY and whether this stmt
> + will be in the destination of the ENTRY. */
> +extern bool set_move_history_to_stmt (gimple *, edge,
> + bool, enum move_reason);
> +
> +/* Set move history to all the stmts in the basic block based on
> + the entry edge and whether this basic block will be the destination
> + of the entry edge. */
> +extern bool set_move_history_to_stmts_in_bb (basic_block, edge,
> + bool, enum move_reason);
> +
> +#endif // DIAGNOSTIC_MOVE_HISTORY_H_INCLUDED
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 987b63601520..8bb7568d0e30 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -324,6 +324,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-details
>
> @item Warning Options
> @xref{Warning Options,,Options to Request or Suppress Warnings}.
> @@ -5609,6 +5610,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-details
> +@item -fdiagnostics-details
> +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.
> +
Two comments:
1) I don't know if we should note here that it might be a bit slower as it
requires that collection or if it's implied/obvious?
2) Should we mention a list of warnings we know are wired up to it?
(either here, or in the docs for the warnings themselves)
> @opindex fdiagnostics-parseable-fixits
> @item -fdiagnostics-parseable-fixits
> Emit fix-it hints in a machine-parseable format, suitable for consumption
> [...]
thanks,
sam
On Wed, 2024-10-30 at 15:53 +0000, Qing Zhao wrote:
>
>
> > On Oct 30, 2024, at 10:48, David Malcolm <dmalcolm@redhat.com>
> > wrote:
> >
> > On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
> > > Qing Zhao <qing.zhao@oracle.com> writes:
> > >
> > > > Control this with a new option -fdiagnostics-details.
> > > >
[...]
>
> I have a question on the changes to the “warning_at”: (there are a
> lot of such changes for -Warray-bounds and -Wstringop-**)
>
> - 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_,
>
> The above is the current change.
>
> My concern with this change is:
> even when -fdiagnostics_details is NOT on, the rich_location is
> created.
A rich_location instance is always constructed when emitting
diagnostics; warning_at with a location_t simply makes a rich_location
on the stack.
>
> How much is the additional overhead when using “rich_location *”
> other than “location_t” as the 1st argument of warning_at?
The warning_at overload taking a rich_location * takes a borrowed
pointer to a rich_location; it doesn't take ownership. Hence, as
written, the patch has a memory leak: every call to
build_rich_location_with_diagnostic_path is using "new" to make a new
rich_location instance on the heap, and they aren't being deleted.
>
> Should I control the creation of “rich_location" with the flag
> “flag_diagnostics_details” (Similar as I control the creation of
> “move_history” data structure with the flag
> “flag_diagnostics_details”?
>
> If so, how should I do it? Do you have a suggestion on a clean and
> simply coding here (Sorry for the stupid question on this)
You can probably do all of this on the stack; make a new rich_location
subclass, with something like:
class rich_location_with_details : public gcc_rich_location
{
public:
rich_location_with_details (location_t location, gimple *stmt);
private:
class deferred_move_history_path {
public:
deferred_move_history_path (location_t location, gimple *stmt)
: m_location (location), m_stmt (stmt)
{
}
std::unique_ptr<diagnostic_path>
make_path () const final override;
/* TODO: you'll need to implement this; it will be called on
demand if a diagnostic is acutally emitted for this
rich_location. */
location_t m_location;
gimple *m_stmt;
} m_deferred_move_history_path;
};
rich_location_with_details::
rich_location_with_details (location_t location, gimple *stmt)
: gcc_rich_location (location),
m_deferred_move_history_path (location, stmt)
{
set_path (&m_deferred_move_history_path);
}
using class deferred_diagnostic_path from the attached patch (caveat: I
haven't tried bootstrapping it yet).
With that support subclass, you should be able to do something like
this to make them on the stack:
rich_location_with_details richloc (location, stmt);
warned = warning_at (&richloc, OPT_Warray_bounds_,
"array subscript %E is outside array"
" bounds of %qT", low_sub_org, artype);
and no work will be done for path creation unless and until a
diagnostic is actually emitted for richloc - the richloc ctor will just
initialize the vtable and some location_t/gimple * fields, which ought
to be very cheap for the "warning is disabled" case .
I'll try bootstrapping the attached patch.
Hope this makes sense.
Dave
On Wed, 2024-10-30 at 17:33 +0000, Sam James wrote:
> Qing Zhao <qing.zhao@oracle.com> writes:
>
> > > On Oct 30, 2024, at 10:48, David Malcolm <dmalcolm@redhat.com>
> > > wrote:
> > >
> > > On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
> > > > Qing Zhao <qing.zhao@oracle.com> writes:
> > > >
> > > > > Control this with a new option -fdiagnostics-details.
> > > > >
> > > > > [...]
> > > >
> > > > The patch doesn't apply for me on very latest trunk -- I think
> > > > David's
> > > > recent diag refactoring means it needs a slight rebase. Could
> > > > you
> > > > send
> > > > that?
> > >
> > > If it's broken, it was probably by:
> > >
> > > r15-4610 ("Use unique_ptr in more places in
> > > pretty_printer/diagnostics
> > > [PR116613]")
> > > https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=bf43fe6aa966eaf397ea3b8ebd6408d3d124e285
> >
> > Yes, due to the following change in the above commit:
> >
> > diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> > index
> > 62034c32b4aff32cdf2cb051bf9d0803b4730b3f..a12a2e1afba15ba16f6ade624
> > cde3e60907ba5d2 100644 (file)
> > --- a/gcc/toplev.cc
> > +++ b/gcc/toplev.cc
> > @@ -42,6 +42,7 @@ along with GCC; see the file COPYING3. If not
> > see
> > #include "cgraph.h"
> > #include "coverage.h"
> > #include "diagnostic.h"
> > +#include "pretty-print-urlifier.h"
> > #include "varasm.h"
> > #include "tree-inline.h"
> > #include "realmpfr.h" /* For GMP/MPFR/MPC versions, in
> > print_version. */
> >
> >
> > [...]
> > >
>
> To continue testing, I am using the attached hacked up patches
Thanks; FWIW the fixes in those patches look correct to me.
Dave
Thanks, Sam.
Yes, the changes you made are exactly what I made in my local area for the rebase.
All the new testing cases passed.
I am doing the complete regression test on X86 and also bootstrap on aarch64 right now.
Qing
> On Oct 30, 2024, at 13:33, Sam James <sam@gentoo.org> wrote:
>
> Qing Zhao <qing.zhao@oracle.com> writes:
>
>>> On Oct 30, 2024, at 10:48, David Malcolm <dmalcolm@redhat.com> wrote:
>>>
>>> On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
>>>> Qing Zhao <qing.zhao@oracle.com> writes:
>>>>
>>>>> Control this with a new option -fdiagnostics-details.
>>>>>
>>>>> [...]
>>>>
>>>> The patch doesn't apply for me on very latest trunk -- I think
>>>> David's
>>>> recent diag refactoring means it needs a slight rebase. Could you
>>>> send
>>>> that?
>>>
>>> If it's broken, it was probably by:
>>>
>>> r15-4610 ("Use unique_ptr in more places in pretty_printer/diagnostics
>>> [PR116613]")
>>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=bf43fe6aa966eaf397ea3b8ebd6408d3d124e285
>>
>> Yes, due to the following change in the above commit:
>>
>> diff --git a/gcc/toplev.cc b/gcc/toplev.cc
>> index 62034c32b4aff32cdf2cb051bf9d0803b4730b3f..a12a2e1afba15ba16f6ade624cde3e60907ba5d2 100644 (file)
>> --- a/gcc/toplev.cc
>> +++ b/gcc/toplev.cc
>> @@ -42,6 +42,7 @@ along with GCC; see the file COPYING3. If not see
>> #include "cgraph.h"
>> #include "coverage.h"
>> #include "diagnostic.h"
>> +#include "pretty-print-urlifier.h"
>> #include "varasm.h"
>> #include "tree-inline.h"
>> #include "realmpfr.h" /* For GMP/MPFR/MPC versions, in print_version. */
>>
>>
>> [...]
>>>
>
> To continue testing, I am using the attached hacked up patches (David,
> please don't hurt me, it was minimal "just get it building" x "do more
> than needed to reduce iterations" ;)).
>
> From f0d521fb56035e71a2b7da3a6c524abab811b42b Mon Sep 17 00:00:00 2001
> Message-ID: <f0d521fb56035e71a2b7da3a6c524abab811b42b.1730309582.git.sam@gentoo.org>
> In-Reply-To: <cover.1730309582.git.sam@gentoo.org>
> References: <cover.1730309582.git.sam@gentoo.org>
> From: Sam James <sam@gentoo.org>
> Date: Wed, 30 Oct 2024 16:15:55 +0000
> Subject: [PATCH 1/2] gcc: add INCLUDE_MEMORY to diagnostic-move-history (and
> friends)
>
> gcc/ChangeLog:
>
> * diagnostic-move-history.cc (INCLUDE_MEMORY): Define INCLUDE_MEMORY.
> * move-history-diagnostic-path.cc (INCLUDE_MEMORY): Ditto.
> * move-history-diagnostic-path.h (INCLUDE_MEMORY): Ditto.
> ---
> gcc/diagnostic-move-history.cc | 1 +
> gcc/gimple-array-bounds.cc | 1 +
> gcc/move-history-diagnostic-path.cc | 1 +
> gcc/move-history-diagnostic-path.h | 1 +
> 4 files changed, 4 insertions(+)
>
> diff --git a/gcc/diagnostic-move-history.cc b/gcc/diagnostic-move-history.cc
> index e4c471ab50f..49adeac1094 100644
> --- a/gcc/diagnostic-move-history.cc
> +++ b/gcc/diagnostic-move-history.cc
> @@ -18,6 +18,7 @@
> along with GCC; see the file COPYING3. If not see
> <http://www.gnu.org/licenses/>. */
>
> +#define INCLUDE_MEMORY
> #include "config.h"
> #include "system.h"
> #include "coretypes.h"
> diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc
> index 464dafa6555..a0b04ed0bc5 100644
> --- a/gcc/gimple-array-bounds.cc
> +++ b/gcc/gimple-array-bounds.cc
> @@ -17,6 +17,7 @@ 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/>. */
>
> +#define INCLUDE_MEMORY
> #include "config.h"
> #include "system.h"
> #include "coretypes.h"
> diff --git a/gcc/move-history-diagnostic-path.cc b/gcc/move-history-diagnostic-path.cc
> index ab29893d1f6..15034616be7 100644
> --- a/gcc/move-history-diagnostic-path.cc
> +++ b/gcc/move-history-diagnostic-path.cc
> @@ -18,6 +18,7 @@ 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/>. */
>
> +#define INCLUDE_MEMORY
> #include "config.h"
> #include "system.h"
> #include "coretypes.h"
> diff --git a/gcc/move-history-diagnostic-path.h b/gcc/move-history-diagnostic-path.h
> index d04337ea377..dd27eeeecec 100644
> --- a/gcc/move-history-diagnostic-path.h
> +++ b/gcc/move-history-diagnostic-path.h
> @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see
> #ifndef GCC_MOVE_HISTORY_DIAGNOSTIC_PATH_H
> #define GCC_MOVE_HISTORY_DIAGNOSTIC_PATH_H
>
> +#define INCLUDE_MEMORY
> #include "diagnostic-path.h"
> #include "simple-diagnostic-path.h"
> #include "diagnostic-move-history.h"
> --
> 2.47.0
>
> From 4933baa95dd6994443e299606e4dbfc0bad67be0 Mon Sep 17 00:00:00 2001
> Message-ID: <4933baa95dd6994443e299606e4dbfc0bad67be0.1730309582.git.sam@gentoo.org>
> In-Reply-To: <cover.1730309582.git.sam@gentoo.org>
> References: <cover.1730309582.git.sam@gentoo.org>
> From: Sam James <sam@gentoo.org>
> Date: Wed, 30 Oct 2024 17:12:08 +0000
> Subject: [PATCH 2/2] gcc: adapt to m_printer change
>
> gcc/ChangeLog:
>
> * move-history-diagnostic-path.cc (build_rich_location_with_diagnostic_path): Use get_reference_printer.
> ---
> gcc/move-history-diagnostic-path.cc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/move-history-diagnostic-path.cc b/gcc/move-history-diagnostic-path.cc
> index 15034616be7..12de0050bb0 100644
> --- a/gcc/move-history-diagnostic-path.cc
> +++ b/gcc/move-history-diagnostic-path.cc
> @@ -97,7 +97,7 @@ build_rich_location_with_diagnostic_path (location_t location, gimple *stmt)
>
> move_history_t mv_history = stmt ? get_move_history (stmt) : NULL;
> move_history_diagnostic_path *path
> - = new move_history_diagnostic_path (global_dc->m_printer,
> + = new move_history_diagnostic_path (global_dc->get_reference_printer (),
> mv_history, location);
> path->populate_path_from_move_history ();
>
> --
> 2.47.0
>
>
> When the rebase is ready, I'll switch to those. Thank you both!
> On Oct 30, 2024, at 13:48, Sam James <sam@gentoo.org> wrote:
>
> Qing Zhao <qing.zhao@oracle.com> writes:
>
>> Control this with a new option -fdiagnostics-details.
>>
>> $ 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 "move_history" to record
>> 1. the "condition" that triggers the code movement;
>> 2. whether the code movement is on the true path of the "condition";
>> 3. the "compiler transformation" that triggers the code movement.
>>
>> Whenever there is a code movement along control flow graph due to some
>> specific transformations, such as jump threading, path isolation, tree
>> sinking, etc., a move_history structure is created and attached to the
>> moved gimple statement.
>>
>> During array out-of-bound checking or -Wstringop-* warning checking, the
>> "move_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 -fdiagnostics-details
>> which is off by default.
>>
>> With this change, by adding -fdiagnostics-details,
>> the warning message for the above testing case is now:
>>
>> $ gcc -Wall -O2 -fdiagnostics-details -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];};
>> | ^~~~
>>
>> PR tree-optimization/109071
>>
>> gcc/ChangeLog:
>>
>> * Makefile.in (OBJS): Add diagnostic-move-history.o
>> and move-history-diagnostic-path.o.
>> * gcc/common.opt (fdiagnostics-details): New option.
>> * gcc/doc/invoke.texi (fdiagnostics-details): 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 move_history_diagnostic_path for warning_at.
>> (array_bounds_checker::check_array_ref): Use rich location with
>> move_history_diagnostic_path for warning_at.
>> (array_bounds_checker::check_mem_ref): Add one new parameter.
>> Use rich location with move_history_diagnostic_path for warning_at.
>> (array_bounds_checker::check_addr_expr): Use rich location with
>> move_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 move
>> history when removing the gimple.
>> * gimple-pretty-print.cc (pp_gimple_stmt_1): Emit MV_H marking
>> if the gimple has a move_history.
>> * gimple-ssa-isolate-paths.cc (isolate_path): Set move history
>> for the gimples of the duplicated blocks.
>> * gimple-ssa-warn-restrict.cc (maybe_diag_access_bounds): Use
>> rich location with move_history_diagnostic_path for warning_at.
>> * gimple-ssa-warn-access.cc (warn_string_no_nul): Likewise.
>> (maybe_warn_nonstring_arg): Likewise.
>> (maybe_warn_for_bound): Likewise.
>> (warn_for_access): Likewise.
>> (check_access): Likewise.
>> (pass_waccess::check_strncat): Likewise.
>> (pass_waccess::maybe_check_access_sizes): Likewise.
>> * tree-ssa-sink.cc (sink_code_in_bb): Create move_history for
>> stmt when it is sinked.
>> * toplev.cc (toplev::finalize): Call move_history_finalize.
>> * tree-ssa-threadupdate.cc (ssa_redirect_edges): Create move_history
>> for stmts when they are duplicated.
>> (back_jt_path_registry::duplicate_thread_path): Likewise.
>> * move-history-diagnostic-path.cc: New file.
>> * move-history-diagnostic-path.h: New file.
>> * diagnostic-move-history.cc: New file.
>> * diagnostic-move-history.h: New file.
>>
>> gcc/testsuite/ChangeLog:
>>
>> PR tree-optimization/109071
>>
>> * gcc.dg/pr109071.c: New test.
>> * gcc.dg/pr109071_1.c: New test.
>> * gcc.dg/pr109071_2.c: New test.
>> * gcc.dg/pr109071_3.c: New test.
>> * gcc.dg/pr109071_4.c: New test.
>> * gcc.dg/pr109071_5.c: New test.
>> * gcc.dg/pr109071_6.c: New test.
>> ---
>> gcc/Makefile.in | 2 +
>> gcc/common.opt | 4 +
>> gcc/diagnostic-move-history.cc | 264 ++++++++++++++++++++++++++++
>> gcc/diagnostic-move-history.h | 92 ++++++++++
>> gcc/doc/invoke.texi | 7 +
>> gcc/gimple-array-bounds.cc | 75 ++++++--
>> gcc/gimple-array-bounds.h | 2 +-
>> gcc/gimple-iterator.cc | 3 +
>> gcc/gimple-pretty-print.cc | 4 +
>> gcc/gimple-ssa-isolate-paths.cc | 11 ++
>> gcc/gimple-ssa-warn-access.cc | 153 ++++++++++------
>> gcc/gimple-ssa-warn-restrict.cc | 27 +--
>> gcc/move-history-diagnostic-path.cc | 119 +++++++++++++
>> gcc/move-history-diagnostic-path.h | 96 ++++++++++
>> gcc/testsuite/gcc.dg/pr109071.c | 43 +++++
>> gcc/testsuite/gcc.dg/pr109071_1.c | 36 ++++
>> gcc/testsuite/gcc.dg/pr109071_2.c | 50 ++++++
>> gcc/testsuite/gcc.dg/pr109071_3.c | 42 +++++
>> gcc/testsuite/gcc.dg/pr109071_4.c | 41 +++++
>> gcc/testsuite/gcc.dg/pr109071_5.c | 33 ++++
>> gcc/testsuite/gcc.dg/pr109071_6.c | 49 ++++++
>> gcc/toplev.cc | 3 +
>> gcc/tree-ssa-sink.cc | 10 ++
>> gcc/tree-ssa-threadupdate.cc | 25 +++
>> 24 files changed, 1105 insertions(+), 86 deletions(-)
>> create mode 100644 gcc/diagnostic-move-history.cc
>> create mode 100644 gcc/diagnostic-move-history.h
>> create mode 100644 gcc/move-history-diagnostic-path.cc
>> create mode 100644 gcc/move-history-diagnostic-path.h
>> create mode 100644 gcc/testsuite/gcc.dg/pr109071.c
>> create mode 100644 gcc/testsuite/gcc.dg/pr109071_1.c
>> create mode 100644 gcc/testsuite/gcc.dg/pr109071_2.c
>> create mode 100644 gcc/testsuite/gcc.dg/pr109071_3.c
>> create mode 100644 gcc/testsuite/gcc.dg/pr109071_4.c
>> create mode 100644 gcc/testsuite/gcc.dg/pr109071_5.c
>> create mode 100644 gcc/testsuite/gcc.dg/pr109071_6.c
>>
>> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
>> index 059cf2e8f79f..0d119ba46e1b 100644
>> --- a/gcc/Makefile.in
>> +++ b/gcc/Makefile.in
>> @@ -1432,6 +1432,8 @@ OBJS = \
>> df-problems.o \
>> df-scan.o \
>> dfp.o \
>> + diagnostic-move-history.o \
>> + move-history-diagnostic-path.o \
>> digraph.o \
>> dojump.o \
>> dominance.o \
>> diff --git a/gcc/common.opt b/gcc/common.opt
>> index 12b25ff486de..84ebef080143 100644
>> --- a/gcc/common.opt
>> +++ b/gcc/common.opt
>> @@ -1566,6 +1566,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-details
>> +Common Var(flag_diagnostics_details)
>> +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/diagnostic-move-history.cc b/gcc/diagnostic-move-history.cc
>> new file mode 100644
>> index 000000000000..b0e8308dbf6b
>> --- /dev/null
>> +++ b/gcc/diagnostic-move-history.cc
>> @@ -0,0 +1,264 @@
>> +/* Functions to handle move history.
>> + Copyright (C) 2024-2024 Free Software Foundation, Inc.
>
> Just '2024'.
>
>> + Contributed by Qing Zhao <qing.zhao@oracle.com>
>> +
>> [...]
>> diff --git a/gcc/diagnostic-move-history.h b/gcc/diagnostic-move-history.h
>> new file mode 100644
>> index 000000000000..cac9cb1e2675
>> --- /dev/null
>> +++ b/gcc/diagnostic-move-history.h
>> @@ -0,0 +1,92 @@
>> +/* Move history associated with a gimple statement to record its history
>> + of movement due to different transformations.
>> + The move 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_MOVE_HISTORY_H_INCLUDED
>> +#define DIAGNOSTIC_MOVE_HISTORY_H_INCLUDED
>
> I think usually the guards lack _INCLUDED.
Just checked GCC’s source code, yes, only “diagnostic-spec.h”, “simple-predicate-analysis.h” use _INCLUDED to guard.
Other header files do not add _INCLUDED.
I can remove it in the next version.
>
>> +
>> +#include "hash-map.h"
>> +#include "line-map.h"
>> +
>> +/* An enum for the reason why this move is made. Right now, there are
>> + three reasons, we can add more if needed. */
>> +enum move_reason {
>> + COPY_BY_THREAD_JUMP,
>> + COPY_BY_ISOLATE_PATH,
>> + MOVE_BY_SINK,
>> + COPY_BY_MAX
>> +};
>> +
>> +/* This data structure records the information when a statement is
>> + moved along control flow graph during different transformations.
>> + Such information will be used by the later diagnostic messages
>> + to report more contexts of the warnings or errors. */
>> +struct move_history {
>> + /* The location of the condition statement that triggered the code
>> + movement. */
>> + location_t condition;
>> +
>> + /* Whether this move is on the TRUE path of the condition. */
>> + bool is_true_path;
>> +
>> + /* The reason for the code movement. */
>> + enum move_reason reason;
>> +
>> + /* This statement itself might be a previous code movement. */
>> + struct move_history *prev_move;
>> +};
>> +
>> +typedef struct move_history *move_history_t;
>> +
>> +/* Create a new move history. */
>> +extern move_history_t create_move_history (location_t, bool,
>> + enum move_reason, move_history_t);
>> +
>> +typedef hash_map<const gimple *, move_history_t> move_history_map_t;
>> +
>> +/* Get the move history for the gimple STMT, return NULL when there is
>> + no associated move history. */
>> +extern move_history_t get_move_history (const gimple *);
>> +
>> +/* Remove the move history for STMT from the move_history_map. */
>> +extern void remove_move_history (gimple *);
>> +
>> +/* Set move history for the gimple STMT. */
>> +extern bool set_move_history (gimple *, location_t,
>> + bool, enum move_reason);
>> +
>> +/* Reset all state for diagnostic-move-history.cc so that we can rerun the
>> + compiler within the same process. For use by toplev::finalize. */
>> +extern void move_history_finalize (void);
>> +
>> +/* Set move history to the stmt based on the edge ENTRY and whether this stmt
>> + will be in the destination of the ENTRY. */
>> +extern bool set_move_history_to_stmt (gimple *, edge,
>> + bool, enum move_reason);
>> +
>> +/* Set move history to all the stmts in the basic block based on
>> + the entry edge and whether this basic block will be the destination
>> + of the entry edge. */
>> +extern bool set_move_history_to_stmts_in_bb (basic_block, edge,
>> + bool, enum move_reason);
>> +
>> +#endif // DIAGNOSTIC_MOVE_HISTORY_H_INCLUDED
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index 987b63601520..8bb7568d0e30 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -324,6 +324,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-details
>>
>> @item Warning Options
>> @xref{Warning Options,,Options to Request or Suppress Warnings}.
>> @@ -5609,6 +5610,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-details
>> +@item -fdiagnostics-details
>> +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.
>> +
>
> Two comments:
> 1) I don't know if we should note here that it might be a bit slower as it
> requires that collection or if it's implied/obvious?
I think that adding such notes should not hurt anything.
>
> 2) Should we mention a list of warnings we know are wired up to it?
> (either here, or in the docs for the warnings themselves)
Yes, this is reasonable to clarify on both sides.
I will update this.
Thanks for the comments and suggestions.
Qing
>
>> @opindex fdiagnostics-parseable-fixits
>> @item -fdiagnostics-parseable-fixits
>> Emit fix-it hints in a machine-parseable format, suitable for consumption
>> [...]
>
> thanks,
> sam
Hi, Andi
> On Oct 30, 2024, at 12:15, Andi Kleen <ak@linux.intel.com> wrote:
>
> Qing Zhao <qing.zhao@oracle.com> writes:
>
>> Control this with a new option -fdiagnostics-details.
>
> It would be useful to be also able to print the inline call stack,
> maybe with a separate option.
Thank you for the suggestion.
Yes, inline call stack will also be very helpful to the users to understand the warning message better.
I noticed that currently, some inlining information has already been issued when reporting warning. For example,
For the testing case of PR115274, when it was compiled with -O2 -Wall, we got:
$ cat t_115274.c
#include <string.h>
char *c;
void a();
int b(char *d) { return strlen(d); }
void e() {
long f = 1;
f = b(c + f);
if (c == 0)
a(f);
}
$/home/opc/Install/latest-d/bin/gcc -O2 -Wall t_115274.c
In function ‘b’,
inlined from ‘e’ at t_115274.c:7:7:
t_115274.c:4:25: warning: ‘strlen’ reading 1 or more bytes from a region of size 0 [-Wstringop-overread]
4 | int b(char *d) { return strlen(d); }
| ^~~~~~~~~
In function ‘e’:
cc1: note: source object is likely at address zero.
I located that the following routine in gcc/langhooks.cc <http://langhooks.cc/> reports the inlining information when reporting an error.
/* The default function to print out name of current function that caused
an error. */
void
lhd_print_error_function (diagnostic_text_output_format &text_output,
const char *file,
const diagnostic_info *diagnostic)
So, I am wondering whether there already is some available utility routine we can use to report the inlining chain for one location?
Thanks.
Qing
>
> In some array bounds cases I looked at the problem was hidden in some inlines
> and it wasn't trivial to figure it out.
>
> I wrote this patch for it at some point.
>
>
> Print inline stack for warn access warnings
>
> The warnings reported by gimple-ssa-warn-access often depend on the
> caller with inlining, and when there are a lot of callers it can be
> difficult to figure out which caller triggered a warning.
>
> Print the function context including inline stack for these
> warnings.
>
> gcc/ChangeLog:
>
> * gimple-ssa-warn-access.cc (maybe_inform_function): New
> function to report function context.
> (warn_string_no_nul): Use maybe_inform_function.
> (maybe_warn_nonstring_arg): Dito.
> (maybe_warn_for_bound): Dito.
> (warn_for_access): Dito.
> (check_access): Dito.
> (warn_dealloc_offset): Dito.
> (maybe_warn_alloc_args_overflow): Dito.
> (pass_waccess::check_strncat): Dito.
> (pass_waccess::maybe_check_access_sizes): Dito.
> (pass_waccess::maybe_check_dealloc_call): Dito.
> (pass_waccess::warn_invalid_pointer): Dito.
> (maybe_warn_mismatched_realloc): Dito.
> (pass_waccess::check_dangling_stores): Dito.
> (pass_waccess::execute): Reset last_function variable.
>
> diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
> index 61f9f0f3d310..94c043531988 100644
> --- a/gcc/gimple-ssa-warn-access.cc
> +++ b/gcc/gimple-ssa-warn-access.cc
> @@ -125,6 +125,21 @@ call_arg (tree expr, unsigned argno)
> return CALL_EXPR_ARG (expr, argno);
> }
>
> +/* Already printed inform for the function. */
> +static bool printed_function;
> +
> +/* Inform about the function stack unless warning is suppressed at LOC
> + with opt code OPT. */
> +static void
> +maybe_inform_function (location_t loc, int opt)
> +{
> + if (printed_function)
> + return;
> + printed_function = true;
> + if (!warning_suppressed_at (loc, (opt_code)opt))
> + inform (DECL_SOURCE_LOCATION (cfun->decl), "in function %qD", cfun->decl);
> +}
> +
> /* For a call EXPR at LOC to a function FNAME that expects a string
> in the argument ARG, issue a diagnostic due to it being a called
> with an argument that is a character array with no terminating
> @@ -162,6 +177,8 @@ warn_string_no_nul (location_t loc, GimpleOrTree expr, const char *fname,
>
> auto_diagnostic_group d;
>
> + maybe_inform_function (loc, opt);
> +
> const tree maxobjsize = max_object_size ();
> const wide_int maxsiz = wi::to_wide (maxobjsize);
> if (expr)
> @@ -485,6 +502,7 @@ maybe_warn_nonstring_arg (tree fndecl, GimpleOrTree exp)
> if (tree_int_cst_lt (maxobjsize, bndrng[0]))
> {
> bool warned = false;
> + maybe_inform_function (loc, OPT_Wstringop_overread);
> if (tree_int_cst_equal (bndrng[0], bndrng[1]))
> warned = warning_at (loc, OPT_Wstringop_overread,
> "%qD specified bound %E "
> @@ -638,6 +656,7 @@ maybe_warn_nonstring_arg (tree fndecl, GimpleOrTree exp)
> auto_diagnostic_group d;
> if (wi::ltu_p (asize, wibnd))
> {
> + maybe_inform_function (loc, OPT_Wstringop_overread);
> if (bndrng[0] == bndrng[1])
> warned = warning_at (loc, OPT_Wstringop_overread,
> "%qD argument %i declared attribute "
> @@ -723,6 +742,7 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
> auto_diagnostic_group d;
> if (tree_int_cst_lt (maxobjsize, bndrng[0]))
> {
> + maybe_inform_function (loc, opt);
> if (bndrng[0] == bndrng[1])
> warned = (func
> ? warning_at (loc, opt,
> @@ -760,7 +780,9 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
> else if (!size || tree_int_cst_le (bndrng[0], size))
> return false;
> else if (tree_int_cst_equal (bndrng[0], bndrng[1]))
> - warned = (func
> + {
> + maybe_inform_function (loc, opt);
> + warned = (func
> ? warning_at (loc, opt,
> (maybe
> ? G_("%qD specified bound %E may exceed "
> @@ -775,8 +797,11 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
> : G_("specified bound %E exceeds "
> "source size %E")),
> bndrng[0], size));
> + }
> else
> - warned = (func
> + {
> + maybe_inform_function (loc, opt);
> + warned = (func
> ? warning_at (loc, opt,
> (maybe
> ? G_("%qD specified bound [%E, %E] may "
> @@ -791,6 +816,7 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
> : G_("specified bound [%E, %E] exceeds "
> "source size %E")),
> bndrng[0], bndrng[1], size));
> + }
> if (warned)
> {
> if (pad && pad->src.ref
> @@ -816,7 +842,9 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
> if (tree_int_cst_lt (maxobjsize, bndrng[0]))
> {
> if (bndrng[0] == bndrng[1])
> - warned = (func
> + {
> + maybe_inform_function (loc, opt);
> + warned = (func
> ? warning_at (loc, opt,
> (maybe
> ? G_("%qD specified size %E may "
> @@ -831,8 +859,11 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
> : G_("specified size %E exceeds "
> "maximum object size %E")),
> bndrng[0], maxobjsize));
> + }
> else
> - warned = (func
> + {
> + maybe_inform_function (loc, opt);
> + warned = (func
> ? warning_at (loc, opt,
> (maybe
> ? G_("%qD specified size between %E and %E "
> @@ -847,11 +878,14 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
> : G_("specified size between %E and %E "
> "exceeds maximum object size %E")),
> bndrng[0], bndrng[1], maxobjsize));
> + }
> }
> else if (!size || tree_int_cst_le (bndrng[0], size))
> return false;
> else if (tree_int_cst_equal (bndrng[0], bndrng[1]))
> - warned = (func
> + {
> + maybe_inform_function (loc, opt);
> + warned = (func
> ? warning_at (loc, opt,
> (maybe
> ? G_("%qD specified bound %E may exceed "
> @@ -866,8 +900,11 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
> : G_("specified bound %E exceeds "
> "destination size %E")),
> bndrng[0], size));
> + }
> else
> - warned = (func
> + {
> + maybe_inform_function (loc, opt);
> + warned = (func
> ? warning_at (loc, opt,
> (maybe
> ? G_("%qD specified bound [%E, %E] may exceed "
> @@ -882,6 +919,7 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
> : G_("specified bound [%E, %E] exceeds "
> "destination size %E")),
> bndrng[0], bndrng[1], size));
> + }
>
> if (warned)
> {
> @@ -928,6 +966,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
>
> if (write && read)
> {
> + maybe_inform_function (loc, opt);
> if (tree_int_cst_equal (range[0], range[1]))
> warned = (func
> ? warning_n (loc, opt, tree_to_uhwi (range[0]),
> @@ -994,6 +1033,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
>
> if (write)
> {
> + maybe_inform_function (loc, opt);
> if (tree_int_cst_equal (range[0], range[1]))
> warned = (func
> ? warning_n (loc, opt, tree_to_uhwi (range[0]),
> @@ -1064,6 +1104,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
>
> if (read)
> {
> + maybe_inform_function (loc, OPT_Wstringop_overread);
> if (tree_int_cst_equal (range[0], range[1]))
> warned = (func
> ? warning_n (loc, OPT_Wstringop_overread,
> @@ -1134,6 +1175,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
> return warned;
> }
>
> + maybe_inform_function (loc, OPT_Wstringop_overread);
> if (tree_int_cst_equal (range[0], range[1])
> || tree_int_cst_sign_bit (range[1]))
> warned = (func
> @@ -1400,6 +1442,7 @@ check_access (GimpleOrTree exp, tree dstwrite,
> bool warned = false;
> if (dstwrite == slen && at_least_one)
> {
> + maybe_inform_function (loc, opt);
> /* This is a call to strcpy with a destination of 0 size
> and a source of unknown length. The call will write
> at least one byte past the end of the destination. */
> @@ -2046,6 +2089,7 @@ warn_dealloc_offset (location_t loc, gimple *call, const access_ref &aref)
> }
>
> auto_diagnostic_group d;
> + maybe_inform_function (loc, OPT_Wfree_nonheap_object);
> if (!warning_at (loc, OPT_Wfree_nonheap_object,
> "%qD called on pointer %qE with nonzero offset%s",
> dealloc_decl, aref.ref, offstr))
> @@ -2273,6 +2317,7 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
>
> if (tree_int_cst_lt (args[i], integer_zero_node))
> {
> + maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
> warned = warning_at (loc, OPT_Walloc_size_larger_than_,
> "argument %i value %qE is negative",
> idx[i] + 1, args[i]);
> @@ -2290,9 +2335,12 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
> ? IDENTIFIER_LENGTH (DECL_NAME (fn)) != 6
> : !lookup_attribute ("returns_nonnull",
> TYPE_ATTRIBUTES (fntype)))
> - warned = warning_at (loc, OPT_Walloc_zero,
> + {
> + maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
> + warned = warning_at (loc, OPT_Walloc_zero,
> "argument %i value is zero",
> idx[i] + 1);
> + }
> }
> else if (tree_int_cst_lt (maxobjsize, args[i]))
> {
> @@ -2308,6 +2356,7 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
> && integer_all_onesp (args[i]))
> continue;
>
> + maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
> warned = warning_at (loc, OPT_Walloc_size_larger_than_,
> "argument %i value %qE exceeds "
> "maximum object size %E",
> @@ -2322,6 +2371,7 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
> if (tree_int_cst_lt (argrange[i][0], integer_zero_node)
> && tree_int_cst_le (argrange[i][1], integer_zero_node))
> {
> + maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
> warned = warning_at (loc, OPT_Walloc_size_larger_than_,
> "argument %i range [%E, %E] is negative",
> idx[i] + 1,
> @@ -2329,6 +2379,7 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
> }
> else if (tree_int_cst_lt (maxobjsize, argrange[i][0]))
> {
> + maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
> warned = warning_at (loc, OPT_Walloc_size_larger_than_,
> "argument %i range [%E, %E] exceeds "
> "maximum object size %E",
> @@ -2359,18 +2410,24 @@ maybe_warn_alloc_args_overflow (gimple *stmt, const tree args[2],
> wide_int prod = wi::umul (x, y, &vflow);
>
> if (vflow)
> - warned = warning_at (loc, OPT_Walloc_size_larger_than_,
> + {
> + maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
> + warned = warning_at (loc, OPT_Walloc_size_larger_than_,
> "product %<%E * %E%> of arguments %i and %i "
> "exceeds %<SIZE_MAX%>",
> argrange[0][0], argrange[1][0],
> idx[0] + 1, idx[1] + 1);
> + }
> else if (wi::ltu_p (wi::to_wide (maxobjsize, szprec), prod))
> - warned = warning_at (loc, OPT_Walloc_size_larger_than_,
> + {
> + maybe_inform_function (loc, OPT_Walloc_size_larger_than_);
> + warned = warning_at (loc, OPT_Walloc_size_larger_than_,
> "product %<%E * %E%> of arguments %i and %i "
> "exceeds maximum object size %E",
> argrange[0][0], argrange[1][0],
> idx[0] + 1, idx[1] + 1,
> maxobjsize);
> + }
>
> if (warned)
> {
> @@ -2563,6 +2620,7 @@ pass_waccess::check_strncat (gcall *stmt)
> && tree_int_cst_equal (destsize, maxread))
> {
> location_t loc = get_location (stmt);
> + maybe_inform_function (loc, OPT_Wstringop_overflow_);
> warning_at (loc, OPT_Wstringop_overflow_,
> "%qD specified bound %E equals destination size",
> get_callee_fndecl (stmt), maxread);
> @@ -3454,6 +3512,7 @@ pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, tree fndecl, tree fntype,
> const std::string argtypestr
> = access.second.array_as_string (ptrtype);
>
> + maybe_inform_function (loc, OPT_Wstringop_overflow_);
> if (warning_at (loc, OPT_Wstringop_overflow_,
> "bound argument %i value %s is "
> "negative for a variable length array "
> @@ -3508,6 +3567,7 @@ pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, tree fndecl, tree fntype,
> different from also declaring the pointer argument with
> attribute nonnull when the function accepts null pointers
> only when the corresponding size is zero. */
> + maybe_inform_function (loc, OPT_Wnonnull);
> if (warning_at (loc, OPT_Wnonnull,
> "argument %i is null but "
> "the corresponding size argument "
> @@ -3706,6 +3766,7 @@ pass_waccess::maybe_check_dealloc_call (gcall *call)
> if (aref.ref_declared ())
> {
> auto_diagnostic_group d;
> + maybe_inform_function (loc, OPT_Wfree_nonheap_object);
> if (warning_at (loc, OPT_Wfree_nonheap_object,
> "%qD called on unallocated object %qD",
> dealloc_decl, ref))
> @@ -3725,6 +3786,7 @@ pass_waccess::maybe_check_dealloc_call (gcall *call)
> else if (CONSTANT_CLASS_P (ref))
> {
> auto_diagnostic_group d;
> + maybe_inform_function (loc, OPT_Wfree_nonheap_object);
> if (warning_at (loc, OPT_Wfree_nonheap_object,
> "%qD called on a pointer to an unallocated "
> "object %qE", dealloc_decl, ref))
> @@ -3767,6 +3829,7 @@ pass_waccess::maybe_check_dealloc_call (gcall *call)
> || DECL_IS_OPERATOR_DELETE_P (dealloc_decl)
> ? OPT_Wmismatched_new_delete
> : OPT_Wmismatched_dealloc);
> + maybe_inform_function (loc, opt);
> warned = warning_at (loc, opt,
> "%qD called on pointer returned "
> "from a mismatched allocation "
> @@ -3776,10 +3839,13 @@ pass_waccess::maybe_check_dealloc_call (gcall *call)
> else if (gimple_call_builtin_p (def_stmt, BUILT_IN_ALLOCA)
> || gimple_call_builtin_p (def_stmt,
> BUILT_IN_ALLOCA_WITH_ALIGN))
> - warned = warning_at (loc, OPT_Wfree_nonheap_object,
> + {
> + maybe_inform_function (loc, OPT_Wfree_nonheap_object);
> + warned = warning_at (loc, OPT_Wfree_nonheap_object,
> "%qD called on pointer to "
> "an unallocated object",
> dealloc_decl);
> + }
> else if (warn_dealloc_offset (loc, call, aref))
> return;
>
> @@ -3923,6 +3989,7 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple *use_stmt,
> const tree inval_decl = gimple_call_fndecl (inval_stmt);
>
> auto_diagnostic_group d;
> + maybe_inform_function (use_loc, OPT_Wuse_after_free);
> if ((ref && warning_at (use_loc, OPT_Wuse_after_free,
> (maybe
> ? G_("pointer %qE may be used after %qD")
> @@ -3949,6 +4016,7 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple *use_stmt,
> if (DECL_NAME (var))
> {
> auto_diagnostic_group d;
> + maybe_inform_function (use_loc, OPT_Wdangling_pointer_);
> if ((ref
> && warning_at (use_loc, OPT_Wdangling_pointer_,
> (maybe
> @@ -3967,6 +4035,7 @@ pass_waccess::warn_invalid_pointer (tree ref, gimple *use_stmt,
> return;
> }
>
> + maybe_inform_function (use_loc, OPT_Wdangling_pointer_);
> if ((ref
> && warning_at (use_loc, OPT_Wdangling_pointer_,
> (maybe
> @@ -4073,6 +4142,7 @@ maybe_warn_mismatched_realloc (tree ptr, gimple *realloc_stmt, gimple *stmt)
> location_t loc = gimple_location (stmt);
> tree realloc_decl = gimple_call_fndecl (realloc_stmt);
> tree dealloc_decl = gimple_call_fndecl (stmt);
> + maybe_inform_function (loc, OPT_Wmismatched_dealloc);
> if (ptr && !warning_at (loc, OPT_Wmismatched_dealloc,
> "%qD called on pointer %qE passed to mismatched "
> "allocation function %qD",
> @@ -4603,6 +4673,7 @@ pass_waccess::check_dangling_stores (basic_block bb,
>
> auto_diagnostic_group d;
> location_t loc = gimple_location (stmt);
> + maybe_inform_function (loc, OPT_Wdangling_pointer_);
> if (warning_at (loc, OPT_Wdangling_pointer_,
> "storing the address of local variable %qD in %qE",
> rhs_ref.ref, lhs))
> @@ -4750,6 +4821,7 @@ pass_waccess::check_call_dangling (gcall *call)
> unsigned
> pass_waccess::execute (function *fun)
> {
> + printed_function = false;
> calculate_dominance_info (CDI_DOMINATORS);
> calculate_dominance_info (CDI_POST_DOMINATORS);
>
>
>
>
> -Andi
David Malcolm <dmalcolm@redhat.com> writes:
> On Wed, 2024-10-30 at 17:33 +0000, Sam James wrote:
>> Qing Zhao <qing.zhao@oracle.com> writes:
>>
>> > > On Oct 30, 2024, at 10:48, David Malcolm <dmalcolm@redhat.com>
>> > > wrote:
>> > >
>> > > On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
>> > > > Qing Zhao <qing.zhao@oracle.com> writes:
>> > > >
>> > > > > Control this with a new option -fdiagnostics-details.
>> > > > >
>> > > > > [...]
>> > > >
>> > > > The patch doesn't apply for me on very latest trunk -- I think
>> > > > David's
>> > > > recent diag refactoring means it needs a slight rebase. Could
>> > > > you
>> > > > send
>> > > > that?
>> > >
>> > > If it's broken, it was probably by:
>> > >
>> > > r15-4610 ("Use unique_ptr in more places in
>> > > pretty_printer/diagnostics
>> > > [PR116613]")
>> > > https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=bf43fe6aa966eaf397ea3b8ebd6408d3d124e285
>> >
>> > Yes, due to the following change in the above commit:
>> >
>> > diff --git a/gcc/toplev.cc b/gcc/toplev.cc
>> > index
>> > 62034c32b4aff32cdf2cb051bf9d0803b4730b3f..a12a2e1afba15ba16f6ade624
>> > cde3e60907ba5d2 100644 (file)
>> > --- a/gcc/toplev.cc
>> > +++ b/gcc/toplev.cc
>> > @@ -42,6 +42,7 @@ along with GCC; see the file COPYING3. If not
>> > see
>> > #include "cgraph.h"
>> > #include "coverage.h"
>> > #include "diagnostic.h"
>> > +#include "pretty-print-urlifier.h"
>> > #include "varasm.h"
>> > #include "tree-inline.h"
>> > #include "realmpfr.h" /* For GMP/MPFR/MPC versions, in
>> > print_version. */
>> >
>> >
>> > [...]
>> > >
>>
>> To continue testing, I am using the attached hacked up patches
>
> Thanks; FWIW the fixes in those patches look correct to me.
Thanks! Need a bit more confidence I think.
>
> Dave
Hi, David,
> On Oct 30, 2024, at 14:54, David Malcolm <dmalcolm@redhat.com> wrote:
>
> On Wed, 2024-10-30 at 15:53 +0000, Qing Zhao wrote:
>>
>>
>>> On Oct 30, 2024, at 10:48, David Malcolm <dmalcolm@redhat.com>
>>> wrote:
>>>
>>> On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
>>>> Qing Zhao <qing.zhao@oracle.com> writes:
>>>>
>>>>> Control this with a new option -fdiagnostics-details.
>>>>>
>
> [...]
>
>>
>> I have a question on the changes to the “warning_at”: (there are a
>> lot of such changes for -Warray-bounds and -Wstringop-**)
>>
>> - 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_,
>>
>> The above is the current change.
>>
>> My concern with this change is:
>> even when -fdiagnostics_details is NOT on, the rich_location is
>> created.
>
> A rich_location instance is always constructed when emitting
> diagnostics; warning_at with a location_t simply makes a rich_location
> on the stack.
Okay, I see. Thanks for the explanation.
>>
>> How much is the additional overhead when using “rich_location *”
>> other than “location_t” as the 1st argument of warning_at?
>
> The warning_at overload taking a rich_location * takes a borrowed
> pointer to a rich_location; it doesn't take ownership. Hence, as
> written, the patch has a memory leak: every call to
> build_rich_location_with_diagnostic_path is using "new" to make a new
> rich_location instance on the heap, and they aren't being deleted.
Oops, good catch!
>
>>
>> Should I control the creation of “rich_location" with the flag
>> “flag_diagnostics_details” (Similar as I control the creation of
>> “move_history” data structure with the flag
>> “flag_diagnostics_details”?
>>
>> If so, how should I do it? Do you have a suggestion on a clean and
>> simply coding here (Sorry for the stupid question on this)
>
> You can probably do all of this on the stack; make a new rich_location
> subclass, with something like:
>
> class rich_location_with_details : public gcc_rich_location
> {
> public:
> rich_location_with_details (location_t location, gimple *stmt);
>
> private:
> class deferred_move_history_path {
> public:
> deferred_move_history_path (location_t location, gimple *stmt)
> : m_location (location), m_stmt (stmt)
> {
> }
>
> std::unique_ptr<diagnostic_path>
> make_path () const final override;
> /* TODO: you'll need to implement this; it will be called on
> demand if a diagnostic is acutally emitted for this
> rich_location. */
What do you mean by “it will be called on demand if a diagnostic is actually emitted”?
Do I need to do anything special in the code to call this “make_path”?
>
> location_t m_location;
> gimple *m_stmt;
> } m_deferred_move_history_path;
> };
>
> rich_location_with_details::
> rich_location_with_details (location_t location, gimple *stmt)
> : gcc_rich_location (location),
> m_deferred_move_history_path (location, stmt)
> {
> set_path (&m_deferred_move_history_path);
> }
>
> using class deferred_diagnostic_path from the attached patch (caveat: I
> haven't tried bootstrapping it yet).
So, I also need to add the new class “deferred_diangostic_path” ?
>
> With that support subclass, you should be able to do something like
> this to make them on the stack:
>
> rich_location_with_details richloc (location, stmt);
> warned = warning_at (&richloc, OPT_Warray_bounds_,
> "array subscript %E is outside array"
> " bounds of %qT", low_sub_org, artype);
>
> and no work will be done for path creation unless and until a
> diagnostic is actually emitted for richloc - the richloc ctor will just
> initialize the vtable and some location_t/gimple * fields, which ought
> to be very cheap for the "warning is disabled" case .
>
> I'll try bootstrapping the attached patch.
Will you commit the attached patch? (A little confused…)
Qing
>
> Hope this makes sense.
> Dave
> <0001-diagnostics-add-class-deferred_diagnostic_path.patch>
On Wed, 2024-10-30 at 20:51 +0000, Qing Zhao wrote:
> Hi, David,
>
> > On Oct 30, 2024, at 14:54, David Malcolm <dmalcolm@redhat.com>
> > wrote:
> >
> > On Wed, 2024-10-30 at 15:53 +0000, Qing Zhao wrote:
> > >
> > >
> > > > On Oct 30, 2024, at 10:48, David Malcolm <dmalcolm@redhat.com>
> > > > wrote:
> > > >
> > > > On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
> > > > > Qing Zhao <qing.zhao@oracle.com> writes:
> > > > >
> > > > > > Control this with a new option -fdiagnostics-details.
> > > > > >
> >
[...]
>
> > > Should I control the creation of “rich_location" with the flag
> > > “flag_diagnostics_details” (Similar as I control the creation of
> > > “move_history” data structure with the flag
> > > “flag_diagnostics_details”?
> > >
> > > If so, how should I do it? Do you have a suggestion on a clean
> > > and
> > > simply coding here (Sorry for the stupid question on this)
> >
> > You can probably do all of this on the stack; make a new
> > rich_location
> > subclass, with something like:
> >
> > class rich_location_with_details : public gcc_rich_location
> > {
> > public:
> > rich_location_with_details (location_t location, gimple *stmt);
> >
> > private:
> > class deferred_move_history_path {
> > public:
> > deferred_move_history_path (location_t location, gimple *stmt)
> > : m_location (location), m_stmt (stmt)
> > {
> > }
> >
> > std::unique_ptr<diagnostic_path>
> > make_path () const final override;
> > /* TODO: you'll need to implement this; it will be called on
> > demand if a diagnostic is acutally emitted for this
> > rich_location. */
>
> What do you mean by “it will be called on demand if a diagnostic is
> actually emitted”?
> Do I need to do anything special in the code to call this
> “make_path”?
I think I got the above wrong, sorry - I meant
deferred_move_history_path to be derived from the
deferred_diagnostic_path from my patch
I've rewritten my patch somewhat, and I pushed it to trunk as r15-4807
a few minutes ago:
https://gcc.gnu.org/pipermail/gcc-patches/2024-October/667070.html
I renamed "deferred_diagnostic_path" to "lazy_diagnostic_path" which
hopefully is a clearer name.
With that, I think you want something like rich_location subclass
(caveat: untested):
#include "lazy-diagnostic-path.h"
/* A rich_location subclass that lazily populates a diagnostic_path
with move history events, but only if the path is actually to be
used. */
class rich_location_with_details : public gcc_rich_location
{
public:
rich_location_with_details (location_t location, gimple *stmt)
: gcc_rich_location (location),
m_deferred_move_history_path (location, stmt)
{
set_path (&m_deferred_move_history_path);
}
private:
class lazy_move_history_path : public lazy_diagnostic_path {
public:
lazy_move_history_path (location_t location, gimple *stmt)
: m_location (location), m_stmt (stmt)
{
}
std::unique_ptr<diagnostic_path>
make_inner_path () const final override;
/* TODO: you'll need to implement this; it will be called on
demand if a diagnostic is actually emitted for this
rich_location. */
location_t m_location;
gimple *m_stmt;
} m_lazy_move_history_path;
};
...and with that support subclass, you should be able to do something like
this to make them on the stack:
rich_location_with_details richloc (location, stmt);
warned = warning_at (&richloc, OPT_Warray_bounds_,
"array subscript %E is outside array"
" bounds of %qT", low_sub_org, artype);
and all that will be done in richloc's constructor will be some cheap
field initializations.
Only if a diagnostic actually gets emitted will
lazy_move_history_path::make_inner_path
be called, at which point it can query the move history and make a path
with the appropriate events.
There's an example of this approach in test_emission in
https://gcc.gnu.org/pipermail/gcc-patches/2024-October/667070.html
which has types test_lazy_path and test_rich_location, which are
analogous to lazy_move_history_path and rich_location_with_details
above.
Hope this makes sense; sorry for being unclear. Alternatively I can
have a go at implementing this if you send me the latest version of
your patches.
Dave
> On Oct 31, 2024, at 12:55, David Malcolm <dmalcolm@redhat.com> wrote:
>
> On Wed, 2024-10-30 at 20:51 +0000, Qing Zhao wrote:
>> Hi, David,
>>
>>> On Oct 30, 2024, at 14:54, David Malcolm <dmalcolm@redhat.com>
>>> wrote:
>>>
>>> On Wed, 2024-10-30 at 15:53 +0000, Qing Zhao wrote:
>>>>
>>>>
>>>>> On Oct 30, 2024, at 10:48, David Malcolm <dmalcolm@redhat.com>
>>>>> wrote:
>>>>>
>>>>> On Wed, 2024-10-30 at 14:34 +0000, Sam James wrote:
>>>>>> Qing Zhao <qing.zhao@oracle.com> writes:
>>>>>>
>>>>>>> Control this with a new option -fdiagnostics-details.
>>>>>>>
>>>
>
> [...]
>
>>
>>>> Should I control the creation of “rich_location" with the flag
>>>> “flag_diagnostics_details” (Similar as I control the creation of
>>>> “move_history” data structure with the flag
>>>> “flag_diagnostics_details”?
>>>>
>>>> If so, how should I do it? Do you have a suggestion on a clean
>>>> and
>>>> simply coding here (Sorry for the stupid question on this)
>>>
>>> You can probably do all of this on the stack; make a new
>>> rich_location
>>> subclass, with something like:
>>>
>>> class rich_location_with_details : public gcc_rich_location
>>> {
>>> public:
>>> rich_location_with_details (location_t location, gimple *stmt);
>>>
>>> private:
>>> class deferred_move_history_path {
>>> public:
>>> deferred_move_history_path (location_t location, gimple *stmt)
>>> : m_location (location), m_stmt (stmt)
>>> {
>>> }
>>>
>>> std::unique_ptr<diagnostic_path>
>>> make_path () const final override;
>>> /* TODO: you'll need to implement this; it will be called on
>>> demand if a diagnostic is acutally emitted for this
>>> rich_location. */
>>
>> What do you mean by “it will be called on demand if a diagnostic is
>> actually emitted”?
>> Do I need to do anything special in the code to call this
>> “make_path”?
>
> I think I got the above wrong, sorry - I meant
> deferred_move_history_path to be derived from the
> deferred_diagnostic_path from my patch
Okay, I see.
>
> I've rewritten my patch somewhat, and I pushed it to trunk as r15-4807
> a few minutes ago:
> https://gcc.gnu.org/pipermail/gcc-patches/2024-October/667070.html
> I renamed "deferred_diagnostic_path" to "lazy_diagnostic_path" which
> hopefully is a clearer name.
>
>
> With that, I think you want something like rich_location subclass
> (caveat: untested):
>
> #include "lazy-diagnostic-path.h"
>
> /* A rich_location subclass that lazily populates a diagnostic_path
> with move history events, but only if the path is actually to be
> used. */
>
> class rich_location_with_details : public gcc_rich_location
> {
> public:
> rich_location_with_details (location_t location, gimple *stmt)
> : gcc_rich_location (location),
> m_deferred_move_history_path (location, stmt)
> {
> set_path (&m_deferred_move_history_path);
> }
>
> private:
> class lazy_move_history_path : public lazy_diagnostic_path {
> public:
> lazy_move_history_path (location_t location, gimple *stmt)
> : m_location (location), m_stmt (stmt)
> {
> }
>
> std::unique_ptr<diagnostic_path>
> make_inner_path () const final override;
> /* TODO: you'll need to implement this; it will be called on
> demand if a diagnostic is actually emitted for this
> rich_location. */
>
> location_t m_location;
> gimple *m_stmt;
> } m_lazy_move_history_path;
> };
>
> ...and with that support subclass, you should be able to do something like
> this to make them on the stack:
>
> rich_location_with_details richloc (location, stmt);
> warned = warning_at (&richloc, OPT_Warray_bounds_,
> "array subscript %E is outside array"
> " bounds of %qT", low_sub_org, artype);
>
> and all that will be done in richloc's constructor will be some cheap
> field initializations.
>
> Only if a diagnostic actually gets emitted will
> lazy_move_history_path::make_inner_path
> be called, at which point it can query the move history and make a path
> with the appropriate events.
Now, I understand. Thanks a lot for the detailed explanation.
>
> There's an example of this approach in test_emission in
> https://gcc.gnu.org/pipermail/gcc-patches/2024-October/667070.html
> which has types test_lazy_path and test_rich_location, which are
> analogous to lazy_move_history_path and rich_location_with_details
> above.
Thanks a lot for the example, it’s very helpful.
>
> Hope this makes sense; sorry for being unclear. Alternatively I can
> have a go at implementing this if you send me the latest version of
> your patches.
I will try to write this myself, if there is any issue, I will ask for your help.
Again, thanks a lot for the help.
Qing
>
> Dave
@@ -1432,6 +1432,8 @@ OBJS = \
df-problems.o \
df-scan.o \
dfp.o \
+ diagnostic-move-history.o \
+ move-history-diagnostic-path.o \
digraph.o \
dojump.o \
dominance.o \
@@ -1566,6 +1566,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-details
+Common Var(flag_diagnostics_details)
+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.
new file mode 100644
@@ -0,0 +1,264 @@
+/* Functions to handle move 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 "gimple.h"
+#include "gimple-iterator.h"
+#include "cfganal.h"
+#include "diagnostic-move-history.h"
+
+/* A mapping from a gimple to a pointer to the move history of it. */
+static move_history_map_t *move_history_map;
+
+/* Obstack for move history. */
+static struct obstack move_history_obstack;
+
+/* Create a new move history. */
+
+move_history_t
+create_move_history (location_t condition,
+ bool is_true_path,
+ enum move_reason reason,
+ move_history_t prev_move)
+{
+ static bool initialized = false;
+
+ if (!initialized)
+ {
+ gcc_obstack_init (&move_history_obstack);
+ initialized = true;
+ }
+
+ move_history_t move_history
+ = (move_history_t) obstack_alloc (&move_history_obstack,
+ sizeof (struct move_history));
+ move_history->condition = condition;
+ move_history->is_true_path = is_true_path;
+ move_history->reason = reason;
+ move_history->prev_move = prev_move;
+ return move_history;
+}
+
+/* Insert the move history for the gimple STMT assuming the linked list
+ of MV_HISTORY does not have duplications. It's the caller's
+ responsibility to make sure that the linked list of MV_HISTORY does
+ not have duplications. */
+
+void
+insert_move_history (gimple *stmt, move_history_t mv_history)
+{
+ if (!move_history_map)
+ move_history_map = new move_history_map_t;
+
+ move_history_map->put (stmt, mv_history);
+ return;
+}
+
+/* Get the move history for the gimple STMT, return NULL when there is
+ no associated move history. */
+
+move_history_t
+get_move_history (const gimple *stmt)
+{
+ if (!move_history_map)
+ return NULL;
+
+ if (const move_history_t *mv_history_p = move_history_map->get (stmt))
+ return *mv_history_p;
+
+ return NULL;
+}
+
+/* Remove the move history for STMT. */
+
+void
+remove_move_history (gimple *stmt)
+{
+ if (!move_history_map)
+ return;
+ move_history_map->remove (stmt);
+ return;
+}
+
+/* Check whether the cond_location, is_true_path and reason existed
+ * in the OLD_MOVE_HISTORY. */
+
+static bool
+is_move_history_existed (location_t cond_location, bool is_true_path,
+ enum move_reason reason,
+ move_history_t old_move_history)
+{
+ for (move_history_t cur_ch = old_move_history; cur_ch;
+ cur_ch = cur_ch->prev_move)
+ if ((cur_ch->condition == cond_location)
+ && (cur_ch->is_true_path == is_true_path)
+ && (cur_ch->reason == reason))
+ return true;
+
+ return false;
+}
+
+/* Set move history for the gimple STMT. Return TRUE when a new move
+ * history is created and inserted. Otherwise return FALSE. */
+
+bool
+set_move_history (gimple *stmt, location_t cond_location,
+ bool is_true_path, enum move_reason reason)
+{
+
+ /* First, get the old move history associated with this STMT. */
+ move_history_t old_mv_history = get_move_history (stmt);
+
+ /* If the current move history is not in the STMT's move history linked
+ list yet, create the new move history, put the old_move_history as the
+ prev_move of it. */
+ move_history_t new_mv_history = NULL;
+ if (!is_move_history_existed (cond_location, is_true_path,
+ reason, old_mv_history))
+ new_mv_history
+ = create_move_history (cond_location, is_true_path,
+ reason, old_mv_history);
+
+ /* Insert the move history into the hash map. */
+ if (new_mv_history)
+ {
+ insert_move_history (stmt, new_mv_history);
+ return true;
+ }
+
+ return false;
+}
+
+/* Reset all state for diagnostic-move-history.cc so that we can rerun the
+ compiler within the same process. For use by toplev::finalize. */
+
+void
+move_history_finalize (void)
+{
+ if (move_history_map)
+ {
+ delete move_history_map;
+ move_history_map = NULL;
+ }
+ obstack_free (&move_history_obstack, NULL);
+ return;
+}
+
+/* Given an edge ENTRY and whether the new code will be moved to the
+ destination of the edge, IS_DISTINATION, return the condition
+ statement in the source of the ENTRY if found. Return NULL otherwise.
+
+ When the condition statement is found, setting IS_TRUE_PATH to true
+ if the destination of the edge is on the true path of the condition.
+
+ IS_TRUE_PATH is only valid when the condition statement is found.
+
+ source
+ | ENTRY
+ V
+ destination
+
+*/
+
+static gimple *
+get_cond_stmt (edge entry, bool is_destination, bool *is_true_path)
+{
+ /* First, get the condition statement in the source of the
+ edge ENTRY. */
+ basic_block cond_block = entry->src;
+ gimple *cond_stmt = NULL;
+ gimple_stmt_iterator gsi;
+ *is_true_path = false;
+
+ /* if the cond_block ends with a conditional statement, get it. */
+ while (!cond_stmt && cond_block)
+ {
+ gsi = gsi_last_bb (cond_block);
+ if (!gsi_end_p (gsi)
+ && gsi_stmt (gsi)
+ && (gimple_code (gsi_stmt (gsi)) == GIMPLE_COND))
+ cond_stmt = gsi_stmt (gsi);
+ /* If there is no cond_stmt in the cond_block, search the single_pred
+ of it. */
+ if (!cond_stmt && single_pred_p (cond_block))
+ {
+ basic_block prev_cond_block = cond_block;
+ cond_block = single_pred (cond_block);
+ entry = find_edge (cond_block, prev_cond_block);
+ }
+ else
+ break;
+ }
+
+ bool is_branch_taken = (cond_stmt && (BRANCH_EDGE (cond_block) == entry));
+ *is_true_path = !(is_branch_taken ^ is_destination);
+
+ return cond_stmt;
+}
+
+/* Set move history to the stmt based on the edge ENTRY and whether this stmt
+ will be in the destination of the ENTRY.
+ The REASON indicates what kind of transformation contributing to the
+ statment movement. Return TRUE when the move history has been set
+ successfully. */
+
+bool
+set_move_history_to_stmt (gimple *stmt, edge entry,
+ bool is_destination, enum move_reason reason)
+{
+ bool is_true_path = false;
+ gimple *cond_stmt = get_cond_stmt (entry, is_destination, &is_true_path);
+
+ if (!cond_stmt)
+ return false;
+
+ set_move_history (stmt, gimple_location (cond_stmt),
+ is_true_path, reason);
+ return true;
+}
+
+/* Set move history to all the stmts in the basic block BB based on
+ the edge ENTRY and whether this basic block will be the destination
+ of the ENTRY.
+ The REASON indicates what kind of transformation contributing to the
+ statement move. Return TRUE when the move history has been set
+ successfully. */
+
+bool
+set_move_history_to_stmts_in_bb (basic_block bb, edge entry,
+ bool is_destination,
+ enum move_reason reason)
+{
+ bool is_true_path = false;
+ gimple_stmt_iterator gsi;
+ gimple *cond_stmt = get_cond_stmt (entry, is_destination, &is_true_path);
+
+ if (!cond_stmt)
+ return false;
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ set_move_history (gsi_stmt (gsi), gimple_location (cond_stmt),
+ is_true_path, reason);
+
+ return true;
+}
new file mode 100644
@@ -0,0 +1,92 @@
+/* Move history associated with a gimple statement to record its history
+ of movement due to different transformations.
+ The move 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_MOVE_HISTORY_H_INCLUDED
+#define DIAGNOSTIC_MOVE_HISTORY_H_INCLUDED
+
+#include "hash-map.h"
+#include "line-map.h"
+
+/* An enum for the reason why this move is made. Right now, there are
+ three reasons, we can add more if needed. */
+enum move_reason {
+ COPY_BY_THREAD_JUMP,
+ COPY_BY_ISOLATE_PATH,
+ MOVE_BY_SINK,
+ COPY_BY_MAX
+};
+
+/* This data structure records the information when a statement is
+ moved along control flow graph during different transformations.
+ Such information will be used by the later diagnostic messages
+ to report more contexts of the warnings or errors. */
+struct move_history {
+ /* The location of the condition statement that triggered the code
+ movement. */
+ location_t condition;
+
+ /* Whether this move is on the TRUE path of the condition. */
+ bool is_true_path;
+
+ /* The reason for the code movement. */
+ enum move_reason reason;
+
+ /* This statement itself might be a previous code movement. */
+ struct move_history *prev_move;
+};
+
+typedef struct move_history *move_history_t;
+
+/* Create a new move history. */
+extern move_history_t create_move_history (location_t, bool,
+ enum move_reason, move_history_t);
+
+typedef hash_map<const gimple *, move_history_t> move_history_map_t;
+
+/* Get the move history for the gimple STMT, return NULL when there is
+ no associated move history. */
+extern move_history_t get_move_history (const gimple *);
+
+/* Remove the move history for STMT from the move_history_map. */
+extern void remove_move_history (gimple *);
+
+/* Set move history for the gimple STMT. */
+extern bool set_move_history (gimple *, location_t,
+ bool, enum move_reason);
+
+/* Reset all state for diagnostic-move-history.cc so that we can rerun the
+ compiler within the same process. For use by toplev::finalize. */
+extern void move_history_finalize (void);
+
+/* Set move history to the stmt based on the edge ENTRY and whether this stmt
+ will be in the destination of the ENTRY. */
+extern bool set_move_history_to_stmt (gimple *, edge,
+ bool, enum move_reason);
+
+/* Set move history to all the stmts in the basic block based on
+ the entry edge and whether this basic block will be the destination
+ of the entry edge. */
+extern bool set_move_history_to_stmts_in_bb (basic_block, edge,
+ bool, enum move_reason);
+
+#endif // DIAGNOSTIC_MOVE_HISTORY_H_INCLUDED
@@ -324,6 +324,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-details
@item Warning Options
@xref{Warning Options,,Options to Request or Suppress Warnings}.
@@ -5609,6 +5610,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-details
+@item -fdiagnostics-details
+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
@@ -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-move-history.h"
+#include "move-history-diagnostic-path.h"
#include "intl.h"
#include "tree-vrp.h"
#include "alloc-pool.h"
@@ -262,6 +265,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 +284,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 +307,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 +325,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 +408,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 +442,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 +503,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 +606,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 +633,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 +658,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 +693,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 +739,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 +755,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 +852,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)
{
@@ -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 *);
@@ -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-move-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_move_history (stmt) != NULL)
+ remove_move_history (stmt);
}
/* Update the iterator and re-wire the links in I->SEQ. */
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see
#include "asan.h"
#include "cfgloop.h"
#include "gimple-range.h"
+#include "diagnostic-move-history.h"
/* Disable warnings about quoting issues in the pp_xxx calls below
that (intentionally) don't follow GCC diagnostic conventions. */
@@ -2686,6 +2687,9 @@ pp_gimple_stmt_1 (pretty_printer *pp, const gimple *gs, int spc,
&& (flags & TDF_ALIAS))
dump_ssaname_info (pp, gimple_get_lhs (gs), spc);
+ if (get_move_history (gs))
+ pp_printf (pp, "[MV_H] ");
+
switch (gimple_code (gs))
{
case GIMPLE_ASM:
@@ -37,6 +37,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-cfg.h"
#include "cfganal.h"
#include "intl.h"
+#include "diagnostic-move-history.h"
static bool cfg_altered;
@@ -170,6 +171,16 @@ isolate_path (basic_block bb, basic_block duplicate,
}
bb->count -= count;
+ /* Set the move history for all the stmts in both original and copied
+ basic blocks. The duplicated block will be the destination of the
+ incoming edge. */
+ if (flag_diagnostics_details)
+ {
+ set_move_history_to_stmts_in_bb (bb, e, false, COPY_BY_ISOLATE_PATH);
+ set_move_history_to_stmts_in_bb (duplicate, e,
+ true, COPY_BY_ISOLATE_PATH);
+ }
+
/* Complete the isolation step by redirecting E to reach DUPLICATE. */
e2 = redirect_edge_and_branch (e, duplicate);
if (e2)
@@ -56,6 +56,7 @@
#include "attr-fnspec.h"
#include "pointer-query.h"
#include "pretty-print-markup.h"
+#include "move-history-diagnostic-path.h"
/* Return true if tree node X has an associated location. */
@@ -168,17 +169,20 @@ warn_string_no_nul (location_t loc, GimpleOrTree expr, const char *fname,
if (expr)
{
tree func = get_callee_fndecl (expr);
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, expr);
+
if (bndrng)
{
if (wi::ltu_p (maxsiz, bndrng[0]))
- warned = warning_at (loc, opt,
+ warned = warning_at (richloc, opt,
"%qD specified bound %s exceeds "
"maximum object size %E",
func, bndstr, maxobjsize);
else
{
bool maybe = wi::to_wide (size) == bndrng[0];
- warned = warning_at (loc, opt,
+ warned = warning_at (richloc, opt,
exact
? G_("%qD specified bound %s exceeds "
"the size %E of unterminated array")
@@ -193,7 +197,7 @@ warn_string_no_nul (location_t loc, GimpleOrTree expr, const char *fname,
}
}
else
- warned = warning_at (loc, opt,
+ warned = warning_at (richloc, opt,
"%qD argument missing terminating nul",
func);
}
@@ -485,14 +489,17 @@ maybe_warn_nonstring_arg (tree fndecl, GimpleOrTree exp)
tree maxobjsize = max_object_size ();
if (tree_int_cst_lt (maxobjsize, bndrng[0]))
{
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, exp);
+
bool warned = false;
if (tree_int_cst_equal (bndrng[0], bndrng[1]))
- warned = warning_at (loc, OPT_Wstringop_overread,
+ warned = warning_at (richloc, OPT_Wstringop_overread,
"%qD specified bound %E "
"exceeds maximum object size %E",
fndecl, bndrng[0], maxobjsize);
else
- warned = warning_at (loc, OPT_Wstringop_overread,
+ warned = warning_at (richloc, OPT_Wstringop_overread,
"%qD specified bound [%E, %E] "
"exceeds maximum object size %E",
fndecl, bndrng[0], bndrng[1],
@@ -639,20 +646,22 @@ maybe_warn_nonstring_arg (tree fndecl, GimpleOrTree exp)
auto_diagnostic_group d;
if (wi::ltu_p (asize, wibnd))
{
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, exp);
if (bndrng[0] == bndrng[1])
- warned = warning_at (loc, OPT_Wstringop_overread,
+ warned = warning_at (richloc, OPT_Wstringop_overread,
"%qD argument %i declared attribute "
"%<nonstring%> is smaller than the specified "
"bound %wu",
fndecl, argno + 1, wibnd.to_uhwi ());
else if (wi::ltu_p (asize, wi::to_offset (bndrng[0])))
- warned = warning_at (loc, OPT_Wstringop_overread,
+ warned = warning_at (richloc, OPT_Wstringop_overread,
"%qD argument %i declared attribute "
"%<nonstring%> is smaller than "
"the specified bound [%E, %E]",
fndecl, argno + 1, bndrng[0], bndrng[1]);
else
- warned = warning_at (loc, OPT_Wstringop_overread,
+ warned = warning_at (richloc, OPT_Wstringop_overread,
"%qD argument %i declared attribute "
"%<nonstring%> may be smaller than "
"the specified bound [%E, %E]",
@@ -724,16 +733,18 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
auto_diagnostic_group d;
if (tree_int_cst_lt (maxobjsize, bndrng[0]))
{
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, exp);
if (bndrng[0] == bndrng[1])
warned = (func
- ? warning_at (loc, opt,
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD specified bound %E may "
"exceed maximum object size %E")
: G_("%qD specified bound %E "
"exceeds maximum object size %E")),
func, bndrng[0], maxobjsize)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("specified bound %E may "
"exceed maximum object size %E")
@@ -742,7 +753,7 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
bndrng[0], maxobjsize));
else
warned = (func
- ? warning_at (loc, opt,
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD specified bound [%E, %E] may "
"exceed maximum object size %E")
@@ -750,7 +761,7 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
"exceeds maximum object size %E")),
func,
bndrng[0], bndrng[1], maxobjsize)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("specified bound [%E, %E] may "
"exceed maximum object size %E")
@@ -761,37 +772,45 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
else if (!size || tree_int_cst_le (bndrng[0], size))
return false;
else if (tree_int_cst_equal (bndrng[0], bndrng[1]))
- warned = (func
- ? warning_at (loc, opt,
+ {
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, exp);
+ warned = (func
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD specified bound %E may exceed "
"source size %E")
: G_("%qD specified bound %E exceeds "
"source size %E")),
func, bndrng[0], size)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("specified bound %E may exceed "
"source size %E")
: G_("specified bound %E exceeds "
"source size %E")),
bndrng[0], size));
+ }
else
- warned = (func
- ? warning_at (loc, opt,
+ {
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, exp);
+ warned = (func
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD specified bound [%E, %E] may "
"exceed source size %E")
: G_("%qD specified bound [%E, %E] exceeds "
"source size %E")),
func, bndrng[0], bndrng[1], size)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("specified bound [%E, %E] may exceed "
"source size %E")
: G_("specified bound [%E, %E] exceeds "
"source size %E")),
bndrng[0], bndrng[1], size));
+ }
if (warned)
{
if (pad && pad->src.ref
@@ -816,16 +835,18 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
}
if (tree_int_cst_lt (maxobjsize, bndrng[0]))
{
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, exp);
if (bndrng[0] == bndrng[1])
warned = (func
- ? warning_at (loc, opt,
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD specified size %E may "
"exceed maximum object size %E")
: G_("%qD specified size %E "
"exceeds maximum object size %E")),
func, bndrng[0], maxobjsize)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("specified size %E may exceed "
"maximum object size %E")
@@ -834,14 +855,14 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
bndrng[0], maxobjsize));
else
warned = (func
- ? warning_at (loc, opt,
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD specified size between %E and %E "
"may exceed maximum object size %E")
: G_("%qD specified size between %E and %E "
"exceeds maximum object size %E")),
func, bndrng[0], bndrng[1], maxobjsize)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("specified size between %E and %E "
"may exceed maximum object size %E")
@@ -852,37 +873,45 @@ maybe_warn_for_bound (opt_code opt, location_t loc, GimpleOrTree exp, tree func,
else if (!size || tree_int_cst_le (bndrng[0], size))
return false;
else if (tree_int_cst_equal (bndrng[0], bndrng[1]))
- warned = (func
- ? warning_at (loc, opt,
+ {
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, exp);
+ warned = (func
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD specified bound %E may exceed "
"destination size %E")
: G_("%qD specified bound %E exceeds "
"destination size %E")),
func, bndrng[0], size)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("specified bound %E may exceed "
"destination size %E")
: G_("specified bound %E exceeds "
"destination size %E")),
bndrng[0], size));
+ }
else
- warned = (func
- ? warning_at (loc, opt,
+ {
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, exp);
+ warned = (func
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD specified bound [%E, %E] may exceed "
"destination size %E")
: G_("%qD specified bound [%E, %E] exceeds "
"destination size %E")),
func, bndrng[0], bndrng[1], size)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("specified bound [%E, %E] exceeds "
"destination size %E")
: G_("specified bound [%E, %E] exceeds "
"destination size %E")),
bndrng[0], bndrng[1], size));
+ }
if (warned)
{
@@ -927,11 +956,14 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
{
bool warned = false;
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, exp);
+
if (write && read)
{
if (tree_int_cst_equal (range[0], range[1]))
warned = (func
- ? warning_n (loc, opt, tree_to_uhwi (range[0]),
+ ? warning_n (richloc, opt, tree_to_uhwi (range[0]),
(maybe
? G_("%qD may access %E byte in a region "
"of size %E")
@@ -943,7 +975,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
: G_ ("%qD accessing %E bytes in a region "
"of size %E")),
func, range[0], size)
- : warning_n (loc, opt, tree_to_uhwi (range[0]),
+ : warning_n (richloc, opt, tree_to_uhwi (range[0]),
(maybe
? G_("may access %E byte in a region "
"of size %E")
@@ -959,14 +991,14 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
{
/* Avoid printing the upper bound if it's invalid. */
warned = (func
- ? warning_at (loc, opt,
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD may access %E or more bytes "
"in a region of size %E")
: G_("%qD accessing %E or more bytes "
"in a region of size %E")),
func, range[0], size)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("may access %E or more bytes "
"in a region of size %E")
@@ -976,14 +1008,14 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
}
else
warned = (func
- ? warning_at (loc, opt,
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD may access between %E and %E "
"bytes in a region of size %E")
: G_("%qD accessing between %E and %E "
"bytes in a region of size %E")),
func, range[0], range[1], size)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("may access between %E and %E bytes "
"in a region of size %E")
@@ -997,7 +1029,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
{
if (tree_int_cst_equal (range[0], range[1]))
warned = (func
- ? warning_n (loc, opt, tree_to_uhwi (range[0]),
+ ? warning_n (richloc, opt, tree_to_uhwi (range[0]),
(maybe
? G_("%qD may write %E byte into a region "
"of size %E")
@@ -1009,7 +1041,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
: G_("%qD writing %E bytes into a region "
"of size %E overflows the destination")),
func, range[0], size)
- : warning_n (loc, opt, tree_to_uhwi (range[0]),
+ : warning_n (richloc, opt, tree_to_uhwi (range[0]),
(maybe
? G_("may write %E byte into a region "
"of size %E")
@@ -1025,7 +1057,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
{
/* Avoid printing the upper bound if it's invalid. */
warned = (func
- ? warning_at (loc, opt,
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD may write %E or more bytes "
"into a region of size %E")
@@ -1033,7 +1065,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
"into a region of size %E overflows "
"the destination")),
func, range[0], size)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("may write %E or more bytes into "
"a region of size %E")
@@ -1044,7 +1076,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
}
else
warned = (func
- ? warning_at (loc, opt,
+ ? warning_at (richloc, opt,
(maybe
? G_("%qD may write between %E and %E bytes "
"into a region of size %E")
@@ -1052,7 +1084,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
"into a region of size %E overflows "
"the destination")),
func, range[0], range[1], size)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("may write between %E and %E bytes "
"into a region of size %E")
@@ -1067,7 +1099,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
{
if (tree_int_cst_equal (range[0], range[1]))
warned = (func
- ? warning_n (loc, OPT_Wstringop_overread,
+ ? warning_n (richloc, OPT_Wstringop_overread,
tree_to_uhwi (range[0]),
(maybe
? G_("%qD may read %E byte from a region "
@@ -1080,7 +1112,7 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
: G_("%qD reading %E bytes from a region "
"of size %E")),
func, range[0], size)
- : warning_n (loc, OPT_Wstringop_overread,
+ : warning_n (richloc, OPT_Wstringop_overread,
tree_to_uhwi (range[0]),
(maybe
? G_("may read %E byte from a region "
@@ -1097,14 +1129,14 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
{
/* Avoid printing the upper bound if it's invalid. */
warned = (func
- ? warning_at (loc, OPT_Wstringop_overread,
+ ? warning_at (richloc, OPT_Wstringop_overread,
(maybe
? G_("%qD may read %E or more bytes "
"from a region of size %E")
: G_("%qD reading %E or more bytes "
"from a region of size %E")),
func, range[0], size)
- : warning_at (loc, OPT_Wstringop_overread,
+ : warning_at (richloc, OPT_Wstringop_overread,
(maybe
? G_("may read %E or more bytes "
"from a region of size %E")
@@ -1114,14 +1146,14 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
}
else
warned = (func
- ? warning_at (loc, OPT_Wstringop_overread,
+ ? warning_at (richloc, OPT_Wstringop_overread,
(maybe
? G_("%qD may read between %E and %E bytes "
"from a region of size %E")
: G_("%qD reading between %E and %E bytes "
"from a region of size %E")),
func, range[0], range[1], size)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
(maybe
? G_("may read between %E and %E bytes "
"from a region of size %E")
@@ -1138,12 +1170,12 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
if (tree_int_cst_equal (range[0], range[1])
|| tree_int_cst_sign_bit (range[1]))
warned = (func
- ? warning_n (loc, OPT_Wstringop_overread,
+ ? warning_n (richloc, OPT_Wstringop_overread,
tree_to_uhwi (range[0]),
"%qD expecting %E byte in a region of size %E",
"%qD expecting %E bytes in a region of size %E",
func, range[0], size)
- : warning_n (loc, OPT_Wstringop_overread,
+ : warning_n (richloc, OPT_Wstringop_overread,
tree_to_uhwi (range[0]),
"expecting %E byte in a region of size %E",
"expecting %E bytes in a region of size %E",
@@ -1152,22 +1184,22 @@ warn_for_access (location_t loc, tree func, GimpleOrTree exp, int opt,
{
/* Avoid printing the upper bound if it's invalid. */
warned = (func
- ? warning_at (loc, OPT_Wstringop_overread,
+ ? warning_at (richloc, OPT_Wstringop_overread,
"%qD expecting %E or more bytes in a region "
"of size %E",
func, range[0], size)
- : warning_at (loc, OPT_Wstringop_overread,
+ : warning_at (richloc, OPT_Wstringop_overread,
"expecting %E or more bytes in a region "
"of size %E",
range[0], size));
}
else
warned = (func
- ? warning_at (loc, OPT_Wstringop_overread,
+ ? warning_at (richloc, OPT_Wstringop_overread,
"%qD expecting between %E and %E bytes in "
"a region of size %E",
func, range[0], range[1], size)
- : warning_at (loc, OPT_Wstringop_overread,
+ : warning_at (richloc, OPT_Wstringop_overread,
"expecting between %E and %E bytes in "
"a region of size %E",
range[0], range[1], size));
@@ -1398,6 +1430,9 @@ check_access (GimpleOrTree exp, tree dstwrite,
auto_diagnostic_group d;
location_t loc = get_location (exp);
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, exp);
+
bool warned = false;
if (dstwrite == slen && at_least_one)
{
@@ -1405,12 +1440,12 @@ check_access (GimpleOrTree exp, tree dstwrite,
and a source of unknown length. The call will write
at least one byte past the end of the destination. */
warned = (func
- ? warning_at (loc, opt,
+ ? warning_at (richloc, opt,
"%qD writing %E or more bytes into "
"a region of size %E overflows "
"the destination",
func, range[0], dstsize)
- : warning_at (loc, opt,
+ : warning_at (richloc, opt,
"writing %E or more bytes into "
"a region of size %E overflows "
"the destination",
@@ -2564,7 +2599,9 @@ pass_waccess::check_strncat (gcall *stmt)
&& tree_int_cst_equal (destsize, maxread))
{
location_t loc = get_location (stmt);
- warning_at (loc, OPT_Wstringop_overflow_,
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, stmt);
+ warning_at (richloc, OPT_Wstringop_overflow_,
"%qD specified bound %E equals destination size",
get_callee_fndecl (stmt), maxread);
@@ -3445,13 +3482,15 @@ pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, tree fndecl, tree fntype,
&& tree_int_cst_sgn (sizrng[0]) < 0
&& tree_int_cst_sgn (sizrng[1]) < 0)
{
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, stmt);
/* Warn about negative sizes. */
if (access.second.internal_p)
{
const std::string argtypestr
= access.second.array_as_string (ptrtype);
- if (warning_at (loc, OPT_Wstringop_overflow_,
+ if (warning_at (richloc, OPT_Wstringop_overflow_,
"bound argument %i value %s is "
"negative for a variable length array "
"argument %i of type %s",
@@ -3459,7 +3498,7 @@ pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, tree fndecl, tree fntype,
ptridx + 1, argtypestr.c_str ()))
arg_warned = OPT_Wstringop_overflow_;
}
- else if (warning_at (loc, OPT_Wstringop_overflow_,
+ else if (warning_at (richloc, OPT_Wstringop_overflow_,
"argument %i value %s is negative",
sizidx + 1, sizstr))
arg_warned = OPT_Wstringop_overflow_;
@@ -40,6 +40,8 @@
#include "tree-object-size.h"
#include "calls.h"
#include "cfgloop.h"
+#include "diagnostic-move-history.h"
+#include "move-history-diagnostic-path.h"
#include "intl.h"
#include "gimple-range.h"
@@ -1693,6 +1695,9 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
location_t loc = gimple_location (call);
const offset_int maxobjsize = ref.maxobjsize;
+ rich_location *richloc
+ = build_rich_location_with_diagnostic_path (loc, call);
+
/* Check for excessive size first and regardless of warning options
since the result is used to make codegen decisions. */
if (ref.sizrange[0] > maxobjsize)
@@ -1709,13 +1714,13 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
if (warn_stringop_overflow)
{
if (ref.sizrange[0] == ref.sizrange[1])
- warned = warning_at (loc, opt,
+ warned = warning_at (richloc, opt,
"%qD specified bound %wu "
"exceeds maximum object size %wu",
func, ref.sizrange[0].to_uhwi (),
maxobjsize.to_uhwi ());
else
- warned = warning_at (loc, opt,
+ warned = warning_at (richloc, opt,
"%qD specified bound between %wu and %wu "
"exceeds maximum object size %wu",
func, ref.sizrange[0].to_uhwi (),
@@ -1776,7 +1781,7 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
&& TREE_CODE (type = TREE_TYPE (ref.base)) == ARRAY_TYPE)
{
auto_diagnostic_group d;
- if (warning_at (loc, opt,
+ if (warning_at (richloc, opt,
"%qD pointer overflow between offset %s "
"and size %s accessing array %qD with type %qT",
func, rangestr[0], rangestr[1], ref.base, type))
@@ -1786,13 +1791,13 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
warned = true;
}
else
- warned = warning_at (loc, opt,
+ warned = warning_at (richloc, opt,
"%qD pointer overflow between offset %s "
"and size %s",
func, rangestr[0], rangestr[1]);
}
else
- warned = warning_at (loc, opt,
+ warned = warning_at (richloc, opt,
"%qD pointer overflow between offset %s "
"and size %s",
func, rangestr[0], rangestr[1]);
@@ -1808,7 +1813,7 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
{
auto_diagnostic_group d;
if ((ref.basesize < maxobjsize
- && warning_at (loc, opt,
+ && warning_at (richloc, opt,
form
? G_("%qD forming offset %s is out of "
"the bounds [0, %wu] of object %qD with "
@@ -1817,7 +1822,7 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
"[0, %wu] of object %qD with type %qT"),
func, rangestr[0], ref.basesize.to_uhwi (),
ref.base, TREE_TYPE (ref.base)))
- || warning_at (loc, opt,
+ || warning_at (richloc, opt,
form
? G_("%qD forming offset %s is out of "
"the bounds of object %qD with type %qT")
@@ -1832,7 +1837,7 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
}
}
else if (ref.basesize < maxobjsize)
- warned = warning_at (loc, opt,
+ warned = warning_at (richloc, opt,
form
? G_("%qD forming offset %s is out "
"of the bounds [0, %wu]")
@@ -1840,7 +1845,7 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
"of the bounds [0, %wu]"),
func, rangestr[0], ref.basesize.to_uhwi ());
else
- warned = warning_at (loc, opt,
+ warned = warning_at (richloc, opt,
form
? G_("%qD forming offset %s is out of bounds")
: G_("%qD offset %s is out of bounds"),
@@ -1854,7 +1859,7 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
type = TREE_TYPE (type);
type = TYPE_MAIN_VARIANT (type);
- if (warning_at (loc, opt,
+ if (warning_at (richloc, opt,
"%qD offset %s from the object at %qE is out "
"of the bounds of %qT",
func, rangestr[0], ref.base, type))
@@ -1872,7 +1877,7 @@ maybe_diag_access_bounds (gimple *call, tree func, int strict,
tree refop = TREE_OPERAND (ref.ref, 0);
tree type = TYPE_MAIN_VARIANT (TREE_TYPE (ref.ref));
- if (warning_at (loc, opt,
+ if (warning_at (richloc, opt,
"%qD offset %s from the object at %qE is out "
"of the bounds of referenced subobject %qD with "
"type %qT at offset %wi",
new file mode 100644
@@ -0,0 +1,119 @@
+/* Classes for implementing diagnostic paths for move_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 "move-history-diagnostic-path.h"
+
+bool
+move_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
+move_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 move_history. */
+void
+move_history_diagnostic_path::populate_path_from_move_history ()
+{
+ if (!m_move_history)
+ return;
+
+ for (move_history_t cur_ch = m_move_history; cur_ch;
+ cur_ch = cur_ch->prev_move)
+ 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;
+}
+
+/* Build a rich location for LOCATION, and populate the diagonistic path
+ for it corresponding to the containing gimple stmt. */
+
+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);
+
+ move_history_t mv_history = stmt ? get_move_history (stmt) : NULL;
+ move_history_diagnostic_path *path
+ = new move_history_diagnostic_path (global_dc->m_printer,
+ mv_history, location);
+ path->populate_path_from_move_history ();
+
+ richloc->set_path (path);
+ return richloc;
+}
+
+/* Overload of the build_rich_location_with_diagnostic_patch for TREE node. */
+
+rich_location *
+build_rich_location_with_diagnostic_path (location_t location,
+ tree exp ATTRIBUTE_UNUSED)
+{
+ /* Generate a rich location for this location.
+ Right now, there is no move_history_t data structure attached to TREE,
+ therefore no move_history_diagnostic_path is generated. */
+ rich_location *richloc = new rich_location (line_table, location);
+
+ return richloc;
+}
new file mode 100644
@@ -0,0 +1,96 @@
+/* Classes for implementing diagnostic paths for move_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_MOVE_HISTORY_DIAGNOSTIC_PATH_H
+#define GCC_MOVE_HISTORY_DIAGNOSTIC_PATH_H
+
+#include "diagnostic-path.h"
+#include "simple-diagnostic-path.h"
+#include "diagnostic-move-history.h"
+
+/* An implementation of diagnostic_path for move_history_t, as a
+ vector of simple_diagnostic_event instances. */
+
+class move_history_diagnostic_path : public diagnostic_path
+{
+ public:
+ move_history_diagnostic_path (pretty_printer *event_pp,
+ move_history_t mv_history,
+ location_t loc)
+ : diagnostic_path (),
+ m_thread ("main"),
+ m_event_pp (event_pp),
+ m_move_history (mv_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_move_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 move_history associated with this path. */
+ move_history_t m_move_history;
+
+ /* The location for the gimple statement where the
+ diagnostic message emitted. */
+ location_t m_location;
+
+};
+
+/* Build a rich location for LOCATION, and populate the diagonistic path
+ for it corresponding to the containing gimple stmt. */
+extern rich_location *
+build_rich_location_with_diagnostic_path (location_t, gimple *);
+
+/* Overload of the build_rich_location_with_diagnostic_patch for TREE node. */
+extern rich_location *
+build_rich_location_with_diagnostic_path (location_t, tree ATTRIBUTE_UNUSED);
+
+#endif /* ! GCC_MOVE_HISTORY_DIAGNOSTIC_PATH_H */
new file mode 100644
@@ -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-details" } */
+/* { 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 "" } */
new file mode 100644
@@ -0,0 +1,36 @@
+/* PR tree-optimization/109071 need more context for -Warray-bounds warnings
+ due to code duplication from jump threading.
+ test case is from PR88771, which is a duplication of PR109071. */
+/* { dg-options "-O2 -fdiagnostics-details" } */
+/* { dg-additional-options "-fdiagnostics-show-line-numbers -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+/* { dg-enable-nn-line-numbers "" } */
+typedef struct {
+ int a;
+} * b;
+
+char *c, *x;
+int f;
+
+void d() {
+ b e;
+ char a = f + 1 ?: f;
+ __builtin_strncpy(c, x, f); /* { dg-warning "exceeds maximum object size" } */
+ if (a)
+ e->a = 0;
+}
+/* { dg-begin-multiline-output "" }
+ NN | __builtin_strncpy(c, x, f);
+ | ^~~~~~~~~~~~~~~~~~~~~~~~~~
+ 'd': events 1-2
+ NN | char a = f + 1 ?: f;
+ | ^
+ | |
+ | (1) when the condition is evaluated to false
+ { dg-end-multiline-output "" } */
+
+/* { dg-begin-multiline-output "" }
+ NN | __builtin_strncpy(c, x, f);
+ | ~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | |
+ | (2) out of array bounds here
+ { dg-end-multiline-output "" } */
new file mode 100644
@@ -0,0 +1,50 @@
+/* PR tree-optimization/109071 need more context for -Warray-bounds warnings
+ due to code duplication from jump threading.
+ test case is from PR85788, which is a duplication of PR109071. */
+/* { dg-options "-O2 -Warray-bounds -fdiagnostics-details" } */
+/* { dg-additional-options "-fdiagnostics-show-line-numbers -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+/* { dg-enable-nn-line-numbers "" } */
+int b=10;
+int *d = &b, *e;
+void a(void *k, long l) {
+ long f = __builtin_object_size(k, 0);
+ __builtin___memset_chk(k, b, l, f); /* { dg-warning "is out of the bounds" } */
+}
+typedef struct {
+ int g;
+ int h;
+ char i[8000 * 8];
+} j;
+static void make_str_raster(j *k) {
+ int *c = d;
+ for (; c; c = e)
+ k->g = k->h = 32767;
+
+ a(k->i, k->g / 8 * k->h);
+ for (; d;)
+ ;
+}
+j m;
+void n() { make_str_raster(&m); }
+/* { dg-begin-multiline-output "" }
+ NN | __builtin___memset_chk(k, b, l, f);
+ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 'n': events 1-2
+ NN | __builtin___memset_chk(k, b, l, f);
+ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | |
+ | (2) out of array bounds here
+......
+ { dg-end-multiline-output "" } */
+
+/* { dg-begin-multiline-output "" }
+ NN | for (; c; c = e)
+ | ^
+ | |
+ | (1) when the condition is evaluated to true
+ { dg-end-multiline-output "" } */
+
+/* { dg-begin-multiline-output "" }
+ NN | j m;
+ | ^
+ { dg-end-multiline-output "" } */
new file mode 100644
@@ -0,0 +1,42 @@
+/* PR tree-optimization/109071 need more context for -Warray-bounds warnings
+ due to code duplication from jump threading.
+ test case is from PR108770, which is a duplication of PR109071. */
+/* { dg-options "-O2 -Warray-bounds -fdiagnostics-details" } */
+/* { dg-additional-options "-fdiagnostics-show-line-numbers -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+/* { dg-enable-nn-line-numbers "" } */
+extern void put(int i);
+int check_idx(int i) {
+ if (i > 1)
+ put(i);
+ return i;
+}
+const char *arr[] = {"A", 0};
+void init() {
+ int i = 0;
+ while (arr[check_idx(i)] != 0) { /* { dg-warning "is above array bounds of" } */
+ if (arr[check_idx(i)]) {}
+ i++;
+ }
+}
+/* { dg-begin-multiline-output "" }
+ NN | while (arr[check_idx(i)] != 0) {
+ | ~~~^~~~~~~~~~~~~~
+ 'init': events 1-2
+ NN | if (i > 1)
+ | ^
+ | |
+ | (1) when the condition is evaluated to true
+......
+ { dg-end-multiline-output "" } */
+
+/* { dg-begin-multiline-output "" }
+ NN | while (arr[check_idx(i)] != 0) {
+ | ~~~~~~~~~~~~~~~~~
+ | |
+ | (2) out of array bounds here
+ { dg-end-multiline-output "" } */
+
+/* { dg-begin-multiline-output "" }
+ NN | const char *arr[] = {"A", 0};
+ | ^~~
+ { dg-end-multiline-output "" } */
new file mode 100644
@@ -0,0 +1,41 @@
+/* PR tree-optimization/109071 need more context for -Warray-bounds warnings
+ due to code duplication from jump threading.
+ test case is from PR106762, which is a duplication of PR109071. */
+/* { dg-options "-O2 -Warray-bounds -fdiagnostics-details" } */
+/* { dg-additional-options "-fdiagnostics-show-line-numbers -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+/* { dg-enable-nn-line-numbers "" } */
+typedef long unsigned int size_t;
+
+struct obj_t { size_t field0; size_t field1; };
+struct obj_array_t { size_t objcnt; struct obj_t* objary; };
+
+extern void *memset (void *__s, int __c, size_t __n) __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__nonnull__(1)));
+
+void bug(struct obj_array_t* ary)
+{
+ size_t idx = 0;
+ struct obj_t* obj;
+ if (idx < ary->objcnt)
+ obj = &ary->objary[idx];
+ else
+ obj = 0;
+ memset(&obj->field1, 0xff, sizeof(obj->field1)); /* { dg-warning "is out of the bounds" } */
+ obj->field0 = 0;
+}
+/* { dg-begin-multiline-output "" }
+ NN | memset(&obj->field1, 0xff, sizeof(obj->field1));
+ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 'bug': events 1-2
+ NN | if (idx < ary->objcnt)
+ | ^
+ | |
+ | (1) when the condition is evaluated to false
+......
+ { dg-end-multiline-output "" } */
+
+/* { dg-begin-multiline-output "" }
+ NN | memset(&obj->field1, 0xff, sizeof(obj->field1));
+ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | |
+ | (2) out of array bounds here
+ { dg-end-multiline-output "" } */
new file mode 100644
@@ -0,0 +1,33 @@
+/* PR tree-optimization/109071 need more context for -Warray-bounds warnings
+ due to code duplication from jump threading.
+ test case is from PR115274, which is a duplication of PR109071. */
+/* { dg-options "-O2 -Wstringop-overread -fdiagnostics-details" } */
+/* { dg-additional-options "-fdiagnostics-show-line-numbers -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+/* { dg-enable-nn-line-numbers "" } */
+#include <string.h>
+char *c;
+void a();
+int b(char *d) { return strlen(d); } /* { dg-warning "or more bytes from a region of size 0" } */
+void e() {
+ long f = 1;
+ f = b(c + f);
+ if (c == 0)
+ a(f);
+}
+/* { dg-begin-multiline-output "" }
+ NN | int b(char *d) { return strlen(d); }
+ | ^~~~~~~~~
+ 'e': events 1-2
+ NN | int b(char *d) { return strlen(d); }
+ | ~~~~~~~~~
+ | |
+ | (2) out of array bounds here
+......
+ { dg-end-multiline-output "" } */
+
+/* { dg-begin-multiline-output "" }
+ NN | if (c == 0)
+ | ^
+ | |
+ | (1) when the condition is evaluated to true
+ { dg-end-multiline-output "" } */
new file mode 100644
@@ -0,0 +1,49 @@
+/* PR tree-optimization/109071 need more context for -Warray-bounds warnings
+ due to code duplication from jump threading.
+ test case is from PR117179, which is a duplication of PR109071. */
+/* { dg-options "-O2 -Warray-bounds -fdiagnostics-details" } */
+/* { dg-additional-options "-fdiagnostics-show-line-numbers -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+/* { dg-enable-nn-line-numbers "" } */
+const char* commands[] = {"a", "b"};
+
+int setval_internal(int comind)
+{
+ if (comind > sizeof(commands)/sizeof(commands[0])) {
+ return 0;
+ }
+
+ return 1;
+}
+
+_Bool setval_internal_tilde(int comind, const char *com,
+ const char *val)
+{
+ int ret = setval_internal(comind);
+ if (commands[comind] == "b" && /* { dg-warning "is outside array bounds of" } */
+
+ ret)
+ return 1;
+ return 0;
+}
+/* { dg-begin-multiline-output "" }
+ NN | if (commands[comind] == "b" &&
+ | ~~~~~~~~^~~~~~~~
+ 'setval_internal_tilde': events 1-2
+ NN | if (comind > sizeof(commands)/sizeof(commands[0])) {
+ | ^
+ | |
+ | (1) when the condition is evaluated to true
+......
+ { dg-end-multiline-output "" } */
+
+/* { dg-begin-multiline-output "" }
+ NN | if (commands[comind] == "b" &&
+ | ~~~~~~~~~~~~~~~~
+ | |
+ | (2) out of array bounds here
+ { dg-end-multiline-output "" } */
+
+/* { dg-begin-multiline-output "" }
+ NN | const char* commands[] = {"a", "b"};
+ | ^~~~~~~~
+ { dg-end-multiline-output "" } */
@@ -42,6 +42,7 @@ along with GCC; see the file COPYING3. If not see
#include "cgraph.h"
#include "coverage.h"
#include "diagnostic.h"
+#include "diagnostic-move-history.h"
#include "varasm.h"
#include "tree-inline.h"
#include "realmpfr.h" /* For GMP/MPFR/MPC versions, in print_version. */
@@ -2434,6 +2435,8 @@ toplev::finalize (void)
tree_cc_finalize ();
reginfo_cc_finalize ();
+ move_history_finalize ();
+
/* save_decoded_options uses opts_obstack, so these must
be cleaned up together. */
obstack_free (&opts_obstack, NULL);
@@ -36,6 +36,7 @@ along with GCC; see the file COPYING3. If not see
#include "cfgloop.h"
#include "tree-eh.h"
#include "tree-ssa-live.h"
+#include "diagnostic-move-history.h"
/* TODO:
1. Sinking store only using scalar promotion (IE without moving the RHS):
@@ -710,6 +711,15 @@ sink_code_in_bb (basic_block bb, virtual_operand_live &vop_live)
bb->index, (gsi_bb (togsi))->index);
}
+ /* Set the move history for the stmt that is sinked from BB to
+ gsi_bb (togsi). This stmt is on the path from BB to
+ gsi_bb (togsi). */
+ if (flag_diagnostics_details)
+ {
+ edge entry = find_edge (bb, gsi_bb (togsi));
+ set_move_history_to_stmt (stmt, entry, true, MOVE_BY_SINK);
+ }
+
/* Update virtual operands of statements in the path we
do not sink to. */
if (gimple_vdef (stmt))
@@ -37,6 +37,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-move-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
@@ -1342,6 +1343,17 @@ ssa_redirect_edges (struct redirection_data **slot,
{
edge e2;
+ /* Set the move history for all the stmts in both original and copied
+ basic blocks. The duplicated block will be the destination of the
+ incoming edge. */
+ if (flag_diagnostics_details)
+ {
+ set_move_history_to_stmts_in_bb (e->dest, e, false,
+ COPY_BY_THREAD_JUMP);
+ set_move_history_to_stmts_in_bb (rd->dup_blocks[0], e,
+ true, COPY_BY_THREAD_JUMP);
+ }
+
if (dump_file && (dump_flags & TDF_DETAILS))
fprintf (dump_file, " Threaded jump %d --> %d to %d\n",
e->src->index, e->dest->index, rd->dup_blocks[0]->index);
@@ -2420,6 +2432,19 @@ 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 move history for all the stmts in both original and copied
+ basic blocks. The copied regions will be the destination of the
+ entry edge. */
+ for (i = 0; i < n_region; i++)
+ if (flag_diagnostics_details)
+ {
+ set_move_history_to_stmts_in_bb (region[i], entry, false,
+ COPY_BY_THREAD_JUMP);
+ set_move_history_to_stmts_in_bb (region_copy[i], entry,
+ true, COPY_BY_THREAD_JUMP);
+ }
+
+
/* 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