On Sat, Jun 22, 2024 at 12:26 AM David Malcolm <dmalc...@redhat.com> wrote:
>
> PR analyzer/115564 reports a missing warning from the analyzer
> on this infinite loop at -O2 and above:
>
> void test (unsigned b)
> {
> for (unsigned i = b; i >= 0; --i) {}
> }
>
> The issue is that there are no useful location_t values in the CFG
> by the time the analyzer sees it: two basic blocks with no
> statements, connected by edges with UNKNOWN_LOCATION for their
> "goto_locus" values. The analyzer's attempts to get a location for the
> loop fail with "UNKNOWN_LOCATION", and so it gives up on the warning.
>
> Root cause is that the edge in question is created by gimple_split_edge
> within the loop optimizer, and gimple_split_edge creates the new edge
> with UNKNOWN_LOCATION.
>
> This patch tweaks gimple_split_edge to copy edge_in->goto_locus's to the
> new edge, so that the edge seen by the analyzer has a useful goto_locus
> value, fixing the issue.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> Successful run of analyzer integration tests on x86_64-pc-linux-gnu,
> which shows 8 new true positives from -Wanalyzer-infinite-loop with
> the patch.
Is the edge the goto_locus is copied from not surviving? Does this
maybe mean we should, when removing a forwarder(?), "merge"
the goto_locus of the incoming and outgoing edge from the forwarder?
That said, I'm not opposed to this change but I wonder whether the
fix is in the wrong place?
Richard.
> OK for trunk?
>
> gcc/testsuite/ChangeLog:
> PR analyzer/115564
> * c-c++-common/analyzer/infinite-loop-pr115564.c: New test.
>
> gcc/ChangeLog:
> PR analyzer/115564
> * tree-cfg.cc (gimple_split_edge): Propagate any source location
> from EDGE_IN to the new edge.
>
> Signed-off-by: David Malcolm <dmalc...@redhat.com>
> ---
> .../c-c++-common/analyzer/infinite-loop-pr115564.c | 8 ++++++++
> gcc/tree-cfg.cc | 3 +++
> 2 files changed, 11 insertions(+)
> create mode 100644
> gcc/testsuite/c-c++-common/analyzer/infinite-loop-pr115564.c
>
> diff --git a/gcc/testsuite/c-c++-common/analyzer/infinite-loop-pr115564.c
> b/gcc/testsuite/c-c++-common/analyzer/infinite-loop-pr115564.c
> new file mode 100644
> index 000000000000..950d92dd1254
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/analyzer/infinite-loop-pr115564.c
> @@ -0,0 +1,8 @@
> +/* Verify that we detect the infinite loop below even at -O2. */
> +
> +/* { dg-additional-options "-O2" } */
> +
> +void test (unsigned b)
> +{
> + for (unsigned i = b; i >= 0; --i) {} /* { dg-warning "infinite loop" } */
> +}
> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> index 7fb7b92966be..45c0eef6c095 100644
> --- a/gcc/tree-cfg.cc
> +++ b/gcc/tree-cfg.cc
> @@ -3061,6 +3061,9 @@ gimple_split_edge (edge edge_in)
> /* set_phi_nodes sets the BB of the PHI nodes, so do it manually here. */
> dest->il.gimple.phi_nodes = saved_phis;
>
> + /* Propagate any source location from EDGE_IN to the new edge. */
> + new_edge->goto_locus = edge_in->goto_locus;
> +
> return new_bb;
> }
>
> --
> 2.26.3
>
