On Fri, Feb 20, 2026 at 1:19 PM Siddhesh Poyarekar <[email protected]> wrote:
>
> On 2026-02-20 03:41, Richard Biener wrote:
> > On Thu, Feb 19, 2026 at 10:51 PM Siddhesh Poyarekar <[email protected]> 
> > wrote:
> >>
> >> When checking for use of a dangling pointer, use_after_inval_p only
> >> checks to see if there's a direct path between the use and the exit
> >> block and that there's a clobber in the way.  This is insufficient to
> >> prove dangling pointer access since the basic premise of the use being
> >> reachable from End Of Scope clobber is not tested.
> >>
> >> If there's a straight, potentially failing path from use to the exit
> >> block, add another test to make sure that the use block is actually
> >> reachable from the invalidating block before warning.
> >>
> >> gcc/ChangeLog:
> >>
> >>          PR middle-end/110091
> >>          PR middle-end/124141
> >>          * gimple-ssa-warn-access.cc (reachable_path_p): New function.
> >>          (pass_waccess::use_after_inval_p): Use it.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >>          PR middle-end/110091
> >>          PR middle-end/124141
> >>          * c-c++-common/Wdangling-pointer-pr110091.c: New test.
> >>
> >> Signed-off-by: Siddhesh Poyarekar <[email protected]>
> >> ---
> >> Changes from v2:
> >> - Walk backwards from use_bb and use nearest common dominator as the end
> >>    point.
> >> - Clean up and tighten the code a bit.
> >> - Use a stack based worklist for DFS instead of recursion.
> >>
> >> Testing:
> >> - x86_64 bootstrap in progress
> >> - Tested x86_64, i686 and the sqlite reproducer
> >>
> >>   gcc/gimple-ssa-warn-access.cc                 | 52 +++++++++++++++++--
> >>   .../c-c++-common/Wdangling-pointer-pr110091.c | 40 ++++++++++++++
> >>   2 files changed, 88 insertions(+), 4 deletions(-)
> >>   create mode 100644 
> >> gcc/testsuite/c-c++-common/Wdangling-pointer-pr110091.c
> >>
> >> diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
> >> index c8f10cae32f..64c4b99d80d 100644
> >> --- a/gcc/gimple-ssa-warn-access.cc
> >> +++ b/gcc/gimple-ssa-warn-access.cc
> >> @@ -3854,6 +3854,46 @@ pass_waccess::maybe_check_dealloc_call (gcall *call)
> >>       }
> >>   }
> >>
> >> +/* Return true if there is a path from FROM to TO.  */
> >> +
> >> +static bool
> >> +reachable_path_p (basic_block from, basic_block to)
> >> +{
> >> +  edge_iterator ei;
> >> +  edge e;
> >> +  auto_vec<basic_block> worklist;
> >> +  hash_set<basic_block> visited;
> >> +
> >> +  if (dominated_by_p (CDI_POST_DOMINATORS, from, to))
> >> +    return true;
> >> +
> >> +  /* Stop at the common dominator for both BBs.  */
> >> +  visited.add (nearest_common_dominator (CDI_DOMINATORS, from, to));
> >> +
> >> +  /* Staring at TO, walk backwards through each predecessor, focusing on
> >> +     unique, normal, forward edges.  */
> >> +  worklist.safe_push (to);
> >> +  while (worklist.length () > 0)
> >> +    {
> >> +      worklist.pop ();
> >> +      FOR_EACH_EDGE (e, ei, to->preds)
> >
> > Errr, you are walking the same BB all the time?  You want
> >
> >     to = worklist.pop ()?
>
> /o\ yes, sorry this was quite tardy :/
>
> >> +       {
> >> +         if (e->flags & (EDGE_EH | EDGE_ABNORMAL | EDGE_DFS_BACK))
> >> +           continue;
> >> +
> >> +         if (visited.add (e->src))
> >> +           continue;
> >> +
> >> +         if (dominated_by_p (CDI_POST_DOMINATORS, from, e->src))
> >> +           return true;
> >
> > I think this is a) wrong (a single-pred to would immediately claim there's
> > a path from from to to), you want dominated_by_p  (CDI_DOMIANTORS,
> > e->src, from),
>
> Ack, I need a dominator check, not post.
>
> > or rather e->src == from || dominated_by_p ...
>
>
> I had this in there but removed it because I reasoned that a block
> should dominate itself.  A cursory look at dominated_by_p and et_below
> confirmed it for me, but maybe I misread them?

No, a block indeed dominates itself.

>
> > note for from == to this function would the return false, so this
> > check belongs right after
> > the to = worklist .pop() and check 'to'.
> >
> > This all means you need some "positive" check for this, if this passed
> > testing or test-coverage
> > for the requirement of reachable_path is very weak.   In particluar ...
>
> The added test should have covered this (because it relies on this
> function returning false for its case), but it somehow slips through,
> maybe because of some peculiarity of distance between the nodes.  I'll
> take a closer look and see if I need additional tests.
>
> >> +
> >> +         worklist.safe_push (e->src);
> >> +       }
> >> +    }
> >> +
> >> +  return false;
> >> +}
> >> +
> >>   /* Return true if either USE_STMT's basic block (that of a pointer's use)
> >>      is dominated by INVAL_STMT's (that of a pointer's invalidating 
> >> statement,
> >>      which is either a clobber or a deallocation call), or if they're in
> >> @@ -3907,10 +3947,14 @@ pass_waccess::use_after_inval_p (gimple 
> >> *inval_stmt, gimple *use_stmt,
> >>            gsi = gsi_start_bb (bb);
> >>          }
> >>
> >> -      /* The use is one of a dangling pointer if a clobber of the variable
> >> -        [the pointer points to] has not been found before the function 
> >> exit
> >> -        point.  */
> >> -      return bb == EXIT_BLOCK_PTR_FOR_FN (cfun);
> >> +      /* Clobber of the variable [the pointer points to] has not been 
> >> found
> >> +        before the function exit point.  If there's a path from INVAL_BB 
> >> to
> >> +        USE_BB, then this is an access through a dangling pointer.  */
> >> +      if (bb == EXIT_BLOCK_PTR_FOR_FN (cfun)
> >> +         && reachable_path_p (inval_bb, use_bb))
> >
> > ... I wonder if your analysis of the original issue is correct.  I've
> > sofar only looked at
> > your reachable_path_p implementation.
>
> I'm still pretty confident of my analysis and that I'm repeatedly
> goofing up the implementation.  However I'll critically review my
> analysis anyway, sorry for bringing in half-baked implementations to you :/
>
> Thanks,
> Sid
>

Reply via email to