diff mbox series

[v2,1/2] add -Wuse-after-free

Message ID 13a79548-162e-e499-559e-c29a77b1d188@gmail.com
State New
Headers show
Series [v2,1/2] add -Wuse-after-free | expand

Commit Message

Martin Sebor Nov. 30, 2021, 10:32 p.m. UTC
Attached is a revised patch with the following changes based
on your comments:

1) Set and use statement uids to determine which statement
    precedes which in the same basic block.
2) Avoid testing flag_isolate_erroneous_paths_dereference.
3) Use post-dominance to decide whether to use the "maybe"
    phrasing vs a definite form.

David raised (and in our offline discussion today reiterated)
an objection to the default setting of the option being
the strictest.  I have not changed that in this revision.
See my rationale for this choice in my reply below:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html

Martin

On 11/23/21 2:16 PM, Martin Sebor wrote:
> On 11/22/21 6:32 PM, Jeff Law wrote:
>>
>>
>> On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote:
>>> Patch 1 in the series detects a small subset of uses of pointers
>>> made indeterminate by calls to deallocation functions like free
>>> or C++ operator delete.  To control the conditions the warnings
>>> are issued under the new -Wuse-after-free= option provides three
>>> levels.  At the lowest level the warning triggers only for
>>> unconditional uses of freed pointers and doesn't warn for uses
>>> in equality expressions.  Level 2 warns also for come conditional
>>> uses, and level 3 also for uses in equality expressions.
>>>
>>> I debated whether to make level 2 or 3 the default included in
>>> -Wall.  I decided on 3 for two reasons: 1) to raise awareness
>>> of both the problem and GCC's new ability to detect it: using
>>> a pointer after it's been freed, even only in principle, by
>>> a successful call to realloc, is undefined, and 2) because
>>> it's trivial to lower the level either globally, or locally
>>> by suppressing the warning around such misuses.
>>>
>>> I've tested the patch on x86_64-linux and by building Glibc
>>> and Binutils/GDB.  It triggers a number of times in each, all
>>> due to comparing invalidated pointers for equality (i.e., level
>>> 3).  I have suppressed these in GCC (libiberty) by a #pragma,
>>> and will see how the Glibc folks want to deal with theirs (I
>>> track them in BZ #28521).
>>>
>>> The tests contain a number of xfails due to limitations I'm
>>> aware of.  I marked them pr?????? until the patch is approved.
>>> I will open bugs for them before committing if I don't resolve
>>> them in a followup.
>>>
>>> Martin
>>>
>>> gcc-63272-1.diff
>>>
>>> Add -Wuse-after-free.
>>>
>>> gcc/c-family/ChangeLog
>>>
>>>     * c.opt (-Wuse-after-free): New options.
>>>
>>> gcc/ChangeLog:
>>>
>>>     * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle
>>>     OPT_Wreturn_local_addr and OPT_Wuse_after_free_.
>>>     * diagnostic-spec.h (NW_DANGLING): New enumerator.
>>>     * doc/invoke.texi (-Wuse-after-free): Document new option.
>>>     * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename...
>>>     (pass_waccess::check_call_access): ...to this.
>>>     (pass_waccess::check): Rename...
>>>     (pass_waccess::check_block): ...to this.
>>>     (pass_waccess::check_pointer_uses): New function.
>>>     (pass_waccess::gimple_call_return_arg): New function.
>>>     (pass_waccess::warn_invalid_pointer): New function.
>>>     (pass_waccess::check_builtin): Handle free and realloc.
>>>     (gimple_use_after_inval_p): New function.
>>>     (get_realloc_lhs): New function.
>>>     (maybe_warn_mismatched_realloc): New function.
>>>     (pointers_related_p): New function.
>>>     (pass_waccess::check_call): Call check_pointer_uses.
>>>     (pass_waccess::execute): Compute and free dominance info.
>>>
>>> libcpp/ChangeLog:
>>>
>>>     * files.c (_cpp_find_file): Substitute a valid pointer for
>>>     an invalid one to avoid -Wuse-0after-free.
>>>
>>> libiberty/ChangeLog:
>>>
>>>     * regex.c: Suppress -Wuse-after-free.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free.
>>>     * gcc.dg/Wmismatched-dealloc-3.c: Same.
>>>     * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free.
>>>     * gcc.dg/attr-alloc_size-7.c: Same.
>>>     * c-c++-common/Wuse-after-free-2.c: New test.
>>>     * c-c++-common/Wuse-after-free-3.c: New test.
>>>     * c-c++-common/Wuse-after-free-4.c: New test.
>>>     * c-c++-common/Wuse-after-free-5.c: New test.
>>>     * c-c++-common/Wuse-after-free-6.c: New test.
>>>     * c-c++-common/Wuse-after-free-7.c: New test.
>>>     * c-c++-common/Wuse-after-free.c: New test.
>>>     * g++.dg/warn/Wdangling-pointer.C: New test.
>>>     * g++.dg/warn/Wmismatched-dealloc-3.C: New test.
>>>     * g++.dg/warn/Wuse-after-free.C: New test.
>>>
>>> diff --git a/gcc/gimple-ssa-warn-access.cc 
>>> b/gcc/gimple-ssa-warn-access.cc
>>> index 63fc27a1487..2065402a2b9 100644
>>> --- a/gcc/gimple-ssa-warn-access.cc
>>> +++ b/gcc/gimple-ssa-warn-access.cc
>>>
>>> @@ -3397,33 +3417,460 @@ pass_waccess::maybe_check_dealloc_call 
>>> (gcall *call)
>>>       }
>>>   }
>>> +/* 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
>>> +   the same block, USE_STMT follows INVAL_STMT.  */
>>> +
>>> +static bool
>>> +gimple_use_after_inval_p (gimple *inval_stmt, gimple *use_stmt,
>>> +              bool last_block = false)
>>> +{
>>> +  tree clobvar =
>>> +    gimple_clobber_p (inval_stmt) ? gimple_assign_lhs (inval_stmt) : 
>>> NULL_TREE;
>>> +
>>> +  basic_block inval_bb = gimple_bb (inval_stmt);
>>> +  basic_block use_bb = gimple_bb (use_stmt);
>>> +
>>> +  if (inval_bb != use_bb)
>>> +    {
>>> +      if (dominated_by_p (CDI_DOMINATORS, use_bb, inval_bb))
>>> +    return true;
>>> +
>>> +      if (!clobvar || !last_block)
>>> +    return false;
>>> +
>>> +      auto gsi = gsi_for_stmt (use_stmt);
>>> +
>>> +      auto_bitmap visited;
>>> +
>>> +      /* A use statement in the last basic block in a function or 
>>> one that
>>> +     falls through to it is after any other prior clobber of the used
>>> +     variable unless it's followed by a clobber of the same 
>>> variable. */
>>> +      basic_block bb = use_bb;
>>> +      while (bb != inval_bb
>>> +         && single_succ_p (bb)
>>> +         && !(single_succ_edge (bb)->flags & (EDGE_EH|EDGE_DFS_BACK)))
>>> +    {
>>> +      if (!bitmap_set_bit (visited, bb->index))
>>> +        /* Avoid cycles. */
>>> +        return true;
>>> +
>>> +      for (; !gsi_end_p (gsi); gsi_next_nondebug (&gsi))
>>> +        {
>>> +          gimple *stmt = gsi_stmt (gsi);
>>> +          if (gimple_clobber_p (stmt))
>>> +        {
>>> +          if (clobvar == gimple_assign_lhs (stmt))
>>> +            /* The use is followed by a clobber.  */
>>> +            return false;
>>> +        }
>>> +        }
>>> +
>>> +      bb = single_succ (bb);
>>> +      gsi = gsi_start_bb (bb);
>>> +    }
>>> +
>>> +      return bb == EXIT_BLOCK_PTR_FOR_FN (cfun);
>>> +    }
>> ?!?  I would have thought the block dominance test plus checking UIDs 
>> if the two statements are in the same block would be all you need.  
>> Can you elaborate more on what that hunk above is trying to do?
> 
> The loop is entered only for -Wdangling-pointer.  It looks for
> the first clobber of the CLOBVAR variable (one whose clobber
> statement has been seen during the CFG traversal and whose use
> is being validated) in the successors along the single edge
> from the use block.  If the search finds a clobber, the use
> is valid.  If it doesn't, the use is one of a variable having
> gone out of scope (the clobber must be before the use).
> 
> Among the cases the loop handles is the one in PR 63272
> (the request for -Wdangling-pointer) where the use neither
> follows the clobber in the same block nor dominated by it.
> 
> There may be a way to optimize it somehow but because it's
> a search I don't think a simple UID check alone would be
> enough.
> 
>>> +
>>> +  for (auto si = gsi_for_stmt (inval_stmt); !gsi_end_p (si);
>>> +       gsi_next_nondebug (&si))
>>> +    {
>>> +      gimple *stmt = gsi_stmt (si);
>>> +      if (stmt == use_stmt)
>>> +    return true;
>>> +    }
>>> +
>>> +  return false;
>>> +}
>> So from a compile-time standpoint, would it be better to to assign 
>> UIDs to each statement so that within a block you can just compare the 
>> UIDs? That's a pretty standard way to deal with the problem of 
>> statement domination within a block if we're going to be doing 
>> multiple queries.
> 
> I'd considered it but because statement UIDs don't exist at
> the start of a pass, assigning them means either traversing all
> statements in the whole CFG first, even in functions with no
> deallocation calls or clobbers, or doing it lazily, after
> the first such statement has been seen.  It might ultimately
> be worthwhile if more warnings(*) end up relying on it but at
> this point I'm not sure the optimization wouldn't end up slowing
> things down on average.
> 
> For some data, in a GCC bootstrap, each statement visited by
> this loop is visited on average twice (2.2 times), and
> the average sequence of statements traversed by the loop is
> 2.65, with a maximum of 22 times and 18 statements, respectively.
> So still not sure it would be a win.
> 
> Let me know if this is something you think I need to pursue at
> this stage.
> 
> [*] I think simple memory/resource leak detection might perhaps
> be one.
> 
>>> +
>>> +/* Return true if P and Q point to the same object, and false if they
>>> +   either don't or their relationship cannot be determined.  */
>>> +
>>> +static bool
>>> +pointers_related_p (gimple *stmt, tree p, tree q, pointer_query &qry)
>>> +{
>>> +  if (!ptr_derefs_may_alias_p (p, q))
>>> +    return false;
>> Hmm, I guess that you don't need to worry about the case where P and Q 
>> point to different elements within an array.  They point to different 
>> final objects, though they do share a common enclosing object. 
>> Similarly for P & Q pointing to different members within a structure.
> 
> Right.  The if statement is an optimization to avoid having to
> determine the identity of the complete objects that P and Q
> point to.  That's done by the calls to get_ref() below (for
> complete objects; as you note, we don't care about subobjects
> for this).
> 
>>> +
>>> +/* For a STMT either a call to a deallocation function or a clobber, 
>>> warn
>>> +   for uses of the pointer PTR it was called with (including its copies
>>> +   or others derived from it by pointer arithmetic).  */
>>> +
>>> +void
>>> +pass_waccess::check_pointer_uses (gimple *stmt, tree ptr)
>>> +{
>>> +  gcc_assert (TREE_CODE (ptr) == SSA_NAME);
>>> +
>>> +  const bool check_dangling = !is_gimple_call (stmt);
>>> +  basic_block stmt_bb = gimple_bb (stmt);
>>> +
>>> +  /* If the deallocation (or clobber) statement dominates more than
>>> +     a single basic block issue a "maybe" k
>> That seems wrong.   What you're looking for is a post-dominance 
>> relationship I think.   If the sink (free/delete) is post-dominated by 
>> the use, then it's a "must", if it's not post-dominated, then it's a 
>> maybe.  Of course, that means you need to build post-dominators.
> 
> I'm sure you're right in general.  To avoid false positives
> the warning is very simplistic and only considers straight
> paths through the CFG, so I'm not sure this matters.  But
> I'm fine with using the post-dominance test instead if you
> thin it's worthwhile (it doesn't change any tests).
> 
>>> +
>>> +      if (check_dangling
>>> +          && gimple_code (use_stmt) == GIMPLE_RETURN
>>> +          && optimize && flag_isolate_erroneous_paths_dereference)
>>> +        /* Avoid interfering with -Wreturn-local-addr (which runs only
>>> +           with optimization enabled).  */
>>> +        continue;
>> Umm, that looks like a hack.  I can't think of a good reason why 
>> removal of erroneous paths should gate any of this code.  ISTM that 
>> you're likely papering over a problem elsewhere.
> 
> This code avoids issuing -Wdangling-pointer for problems that
> will later be diagnosed by -Wreturn-local-addr.  E.g., in this
> case from Wreturn-local-addr-2.c:
> 
>    ATTR (noipa) void*
>    return_array_plus_var (int i)
>    {
>        int a[32];
>        void *p = a + i;
>      sink (p);
>      return p;         /* { dg-warning "function returns address of 
> local" } */
>    }
> 
> Without the test we'd end up with
> 
>    warning: using dangling pointer ‘p’ to ‘a’ [-Wdangling-pointer=]
> 
> in addition to -Wreturn-local-addr (and a whole slew of
> failures in the -Wreturn-local-addr tests).
> 
> -Wreturn-local-addr only runs when
> flag_isolate_erroneous_paths_dereference is nonzero, so
> the conditional makes sure -Wdangling-pointer is issued when
> either the flag or -Wreturn-local-addr is disabled.  I think
> that works as expected (i.e., there's no problem elsewhere).
> 
> I could have the code issue -Wdangling-pointer and suppress
> -Wreturn-local-addr but that doesn't seem right since
> the pointer hasn't gone out of scope yet at the point it's
> returned.
> 
> Alternatively, I could change this instance of
> -Wdangling-pointer to -Wreturn-local-addr but that also
> doesn't seem like good design since we have a whole pass
> dedicated to the latter warning.
> 
> I can't think of any other more elegant solutions but I'm open
> to suggestions.
> 
> Martin

Comments

Martin Sebor Dec. 7, 2021, 12:50 a.m. UTC | #1
Ping:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html

On 11/30/21 3:32 PM, Martin Sebor wrote:
> Attached is a revised patch with the following changes based
> on your comments:
> 
> 1) Set and use statement uids to determine which statement
>     precedes which in the same basic block.
> 2) Avoid testing flag_isolate_erroneous_paths_dereference.
> 3) Use post-dominance to decide whether to use the "maybe"
>     phrasing vs a definite form.
> 
> David raised (and in our offline discussion today reiterated)
> an objection to the default setting of the option being
> the strictest.  I have not changed that in this revision.
> See my rationale for this choice in my reply below:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html
> 
> Martin
> 
> On 11/23/21 2:16 PM, Martin Sebor wrote:
>> On 11/22/21 6:32 PM, Jeff Law wrote:
>>>
>>>
>>> On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote:
>>>> Patch 1 in the series detects a small subset of uses of pointers
>>>> made indeterminate by calls to deallocation functions like free
>>>> or C++ operator delete.  To control the conditions the warnings
>>>> are issued under the new -Wuse-after-free= option provides three
>>>> levels.  At the lowest level the warning triggers only for
>>>> unconditional uses of freed pointers and doesn't warn for uses
>>>> in equality expressions.  Level 2 warns also for come conditional
>>>> uses, and level 3 also for uses in equality expressions.
>>>>
>>>> I debated whether to make level 2 or 3 the default included in
>>>> -Wall.  I decided on 3 for two reasons: 1) to raise awareness
>>>> of both the problem and GCC's new ability to detect it: using
>>>> a pointer after it's been freed, even only in principle, by
>>>> a successful call to realloc, is undefined, and 2) because
>>>> it's trivial to lower the level either globally, or locally
>>>> by suppressing the warning around such misuses.
>>>>
>>>> I've tested the patch on x86_64-linux and by building Glibc
>>>> and Binutils/GDB.  It triggers a number of times in each, all
>>>> due to comparing invalidated pointers for equality (i.e., level
>>>> 3).  I have suppressed these in GCC (libiberty) by a #pragma,
>>>> and will see how the Glibc folks want to deal with theirs (I
>>>> track them in BZ #28521).
>>>>
>>>> The tests contain a number of xfails due to limitations I'm
>>>> aware of.  I marked them pr?????? until the patch is approved.
>>>> I will open bugs for them before committing if I don't resolve
>>>> them in a followup.
>>>>
>>>> Martin
>>>>
>>>> gcc-63272-1.diff
>>>>
>>>> Add -Wuse-after-free.
>>>>
>>>> gcc/c-family/ChangeLog
>>>>
>>>>     * c.opt (-Wuse-after-free): New options.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>     * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle
>>>>     OPT_Wreturn_local_addr and OPT_Wuse_after_free_.
>>>>     * diagnostic-spec.h (NW_DANGLING): New enumerator.
>>>>     * doc/invoke.texi (-Wuse-after-free): Document new option.
>>>>     * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename...
>>>>     (pass_waccess::check_call_access): ...to this.
>>>>     (pass_waccess::check): Rename...
>>>>     (pass_waccess::check_block): ...to this.
>>>>     (pass_waccess::check_pointer_uses): New function.
>>>>     (pass_waccess::gimple_call_return_arg): New function.
>>>>     (pass_waccess::warn_invalid_pointer): New function.
>>>>     (pass_waccess::check_builtin): Handle free and realloc.
>>>>     (gimple_use_after_inval_p): New function.
>>>>     (get_realloc_lhs): New function.
>>>>     (maybe_warn_mismatched_realloc): New function.
>>>>     (pointers_related_p): New function.
>>>>     (pass_waccess::check_call): Call check_pointer_uses.
>>>>     (pass_waccess::execute): Compute and free dominance info.
>>>>
>>>> libcpp/ChangeLog:
>>>>
>>>>     * files.c (_cpp_find_file): Substitute a valid pointer for
>>>>     an invalid one to avoid -Wuse-0after-free.
>>>>
>>>> libiberty/ChangeLog:
>>>>
>>>>     * regex.c: Suppress -Wuse-after-free.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>     * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free.
>>>>     * gcc.dg/Wmismatched-dealloc-3.c: Same.
>>>>     * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free.
>>>>     * gcc.dg/attr-alloc_size-7.c: Same.
>>>>     * c-c++-common/Wuse-after-free-2.c: New test.
>>>>     * c-c++-common/Wuse-after-free-3.c: New test.
>>>>     * c-c++-common/Wuse-after-free-4.c: New test.
>>>>     * c-c++-common/Wuse-after-free-5.c: New test.
>>>>     * c-c++-common/Wuse-after-free-6.c: New test.
>>>>     * c-c++-common/Wuse-after-free-7.c: New test.
>>>>     * c-c++-common/Wuse-after-free.c: New test.
>>>>     * g++.dg/warn/Wdangling-pointer.C: New test.
>>>>     * g++.dg/warn/Wmismatched-dealloc-3.C: New test.
>>>>     * g++.dg/warn/Wuse-after-free.C: New test.
>>>>
>>>> diff --git a/gcc/gimple-ssa-warn-access.cc 
>>>> b/gcc/gimple-ssa-warn-access.cc
>>>> index 63fc27a1487..2065402a2b9 100644
>>>> --- a/gcc/gimple-ssa-warn-access.cc
>>>> +++ b/gcc/gimple-ssa-warn-access.cc
>>>>
>>>> @@ -3397,33 +3417,460 @@ pass_waccess::maybe_check_dealloc_call 
>>>> (gcall *call)
>>>>       }
>>>>   }
>>>> +/* 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
>>>> +   the same block, USE_STMT follows INVAL_STMT.  */
>>>> +
>>>> +static bool
>>>> +gimple_use_after_inval_p (gimple *inval_stmt, gimple *use_stmt,
>>>> +              bool last_block = false)
>>>> +{
>>>> +  tree clobvar =
>>>> +    gimple_clobber_p (inval_stmt) ? gimple_assign_lhs (inval_stmt) 
>>>> : NULL_TREE;
>>>> +
>>>> +  basic_block inval_bb = gimple_bb (inval_stmt);
>>>> +  basic_block use_bb = gimple_bb (use_stmt);
>>>> +
>>>> +  if (inval_bb != use_bb)
>>>> +    {
>>>> +      if (dominated_by_p (CDI_DOMINATORS, use_bb, inval_bb))
>>>> +    return true;
>>>> +
>>>> +      if (!clobvar || !last_block)
>>>> +    return false;
>>>> +
>>>> +      auto gsi = gsi_for_stmt (use_stmt);
>>>> +
>>>> +      auto_bitmap visited;
>>>> +
>>>> +      /* A use statement in the last basic block in a function or 
>>>> one that
>>>> +     falls through to it is after any other prior clobber of the used
>>>> +     variable unless it's followed by a clobber of the same 
>>>> variable. */
>>>> +      basic_block bb = use_bb;
>>>> +      while (bb != inval_bb
>>>> +         && single_succ_p (bb)
>>>> +         && !(single_succ_edge (bb)->flags & (EDGE_EH|EDGE_DFS_BACK)))
>>>> +    {
>>>> +      if (!bitmap_set_bit (visited, bb->index))
>>>> +        /* Avoid cycles. */
>>>> +        return true;
>>>> +
>>>> +      for (; !gsi_end_p (gsi); gsi_next_nondebug (&gsi))
>>>> +        {
>>>> +          gimple *stmt = gsi_stmt (gsi);
>>>> +          if (gimple_clobber_p (stmt))
>>>> +        {
>>>> +          if (clobvar == gimple_assign_lhs (stmt))
>>>> +            /* The use is followed by a clobber.  */
>>>> +            return false;
>>>> +        }
>>>> +        }
>>>> +
>>>> +      bb = single_succ (bb);
>>>> +      gsi = gsi_start_bb (bb);
>>>> +    }
>>>> +
>>>> +      return bb == EXIT_BLOCK_PTR_FOR_FN (cfun);
>>>> +    }
>>> ?!?  I would have thought the block dominance test plus checking UIDs 
>>> if the two statements are in the same block would be all you need. 
>>> Can you elaborate more on what that hunk above is trying to do?
>>
>> The loop is entered only for -Wdangling-pointer.  It looks for
>> the first clobber of the CLOBVAR variable (one whose clobber
>> statement has been seen during the CFG traversal and whose use
>> is being validated) in the successors along the single edge
>> from the use block.  If the search finds a clobber, the use
>> is valid.  If it doesn't, the use is one of a variable having
>> gone out of scope (the clobber must be before the use).
>>
>> Among the cases the loop handles is the one in PR 63272
>> (the request for -Wdangling-pointer) where the use neither
>> follows the clobber in the same block nor dominated by it.
>>
>> There may be a way to optimize it somehow but because it's
>> a search I don't think a simple UID check alone would be
>> enough.
>>
>>>> +
>>>> +  for (auto si = gsi_for_stmt (inval_stmt); !gsi_end_p (si);
>>>> +       gsi_next_nondebug (&si))
>>>> +    {
>>>> +      gimple *stmt = gsi_stmt (si);
>>>> +      if (stmt == use_stmt)
>>>> +    return true;
>>>> +    }
>>>> +
>>>> +  return false;
>>>> +}
>>> So from a compile-time standpoint, would it be better to to assign 
>>> UIDs to each statement so that within a block you can just compare 
>>> the UIDs? That's a pretty standard way to deal with the problem of 
>>> statement domination within a block if we're going to be doing 
>>> multiple queries.
>>
>> I'd considered it but because statement UIDs don't exist at
>> the start of a pass, assigning them means either traversing all
>> statements in the whole CFG first, even in functions with no
>> deallocation calls or clobbers, or doing it lazily, after
>> the first such statement has been seen.  It might ultimately
>> be worthwhile if more warnings(*) end up relying on it but at
>> this point I'm not sure the optimization wouldn't end up slowing
>> things down on average.
>>
>> For some data, in a GCC bootstrap, each statement visited by
>> this loop is visited on average twice (2.2 times), and
>> the average sequence of statements traversed by the loop is
>> 2.65, with a maximum of 22 times and 18 statements, respectively.
>> So still not sure it would be a win.
>>
>> Let me know if this is something you think I need to pursue at
>> this stage.
>>
>> [*] I think simple memory/resource leak detection might perhaps
>> be one.
>>
>>>> +
>>>> +/* Return true if P and Q point to the same object, and false if they
>>>> +   either don't or their relationship cannot be determined.  */
>>>> +
>>>> +static bool
>>>> +pointers_related_p (gimple *stmt, tree p, tree q, pointer_query &qry)
>>>> +{
>>>> +  if (!ptr_derefs_may_alias_p (p, q))
>>>> +    return false;
>>> Hmm, I guess that you don't need to worry about the case where P and 
>>> Q point to different elements within an array.  They point to 
>>> different final objects, though they do share a common enclosing 
>>> object. Similarly for P & Q pointing to different members within a 
>>> structure.
>>
>> Right.  The if statement is an optimization to avoid having to
>> determine the identity of the complete objects that P and Q
>> point to.  That's done by the calls to get_ref() below (for
>> complete objects; as you note, we don't care about subobjects
>> for this).
>>
>>>> +
>>>> +/* For a STMT either a call to a deallocation function or a 
>>>> clobber, warn
>>>> +   for uses of the pointer PTR it was called with (including its 
>>>> copies
>>>> +   or others derived from it by pointer arithmetic).  */
>>>> +
>>>> +void
>>>> +pass_waccess::check_pointer_uses (gimple *stmt, tree ptr)
>>>> +{
>>>> +  gcc_assert (TREE_CODE (ptr) == SSA_NAME);
>>>> +
>>>> +  const bool check_dangling = !is_gimple_call (stmt);
>>>> +  basic_block stmt_bb = gimple_bb (stmt);
>>>> +
>>>> +  /* If the deallocation (or clobber) statement dominates more than
>>>> +     a single basic block issue a "maybe" k
>>> That seems wrong.   What you're looking for is a post-dominance 
>>> relationship I think.   If the sink (free/delete) is post-dominated 
>>> by the use, then it's a "must", if it's not post-dominated, then it's 
>>> a maybe.  Of course, that means you need to build post-dominators.
>>
>> I'm sure you're right in general.  To avoid false positives
>> the warning is very simplistic and only considers straight
>> paths through the CFG, so I'm not sure this matters.  But
>> I'm fine with using the post-dominance test instead if you
>> thin it's worthwhile (it doesn't change any tests).
>>
>>>> +
>>>> +      if (check_dangling
>>>> +          && gimple_code (use_stmt) == GIMPLE_RETURN
>>>> +          && optimize && flag_isolate_erroneous_paths_dereference)
>>>> +        /* Avoid interfering with -Wreturn-local-addr (which runs only
>>>> +           with optimization enabled).  */
>>>> +        continue;
>>> Umm, that looks like a hack.  I can't think of a good reason why 
>>> removal of erroneous paths should gate any of this code.  ISTM that 
>>> you're likely papering over a problem elsewhere.
>>
>> This code avoids issuing -Wdangling-pointer for problems that
>> will later be diagnosed by -Wreturn-local-addr.  E.g., in this
>> case from Wreturn-local-addr-2.c:
>>
>>    ATTR (noipa) void*
>>    return_array_plus_var (int i)
>>    {
>>        int a[32];
>>        void *p = a + i;
>>      sink (p);
>>      return p;         /* { dg-warning "function returns address of 
>> local" } */
>>    }
>>
>> Without the test we'd end up with
>>
>>    warning: using dangling pointer ‘p’ to ‘a’ [-Wdangling-pointer=]
>>
>> in addition to -Wreturn-local-addr (and a whole slew of
>> failures in the -Wreturn-local-addr tests).
>>
>> -Wreturn-local-addr only runs when
>> flag_isolate_erroneous_paths_dereference is nonzero, so
>> the conditional makes sure -Wdangling-pointer is issued when
>> either the flag or -Wreturn-local-addr is disabled.  I think
>> that works as expected (i.e., there's no problem elsewhere).
>>
>> I could have the code issue -Wdangling-pointer and suppress
>> -Wreturn-local-addr but that doesn't seem right since
>> the pointer hasn't gone out of scope yet at the point it's
>> returned.
>>
>> Alternatively, I could change this instance of
>> -Wdangling-pointer to -Wreturn-local-addr but that also
>> doesn't seem like good design since we have a whole pass
>> dedicated to the latter warning.
>>
>> I can't think of any other more elegant solutions but I'm open
>> to suggestions.
>>
>> Martin
>
Martin Sebor Dec. 13, 2021, 4:48 p.m. UTC | #2
Ping.

Jeff, I addressed your comments in the updated patch.  If there
are no other changes is the last revision okay to commit?

https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html

On 12/6/21 5:50 PM, Martin Sebor wrote:
> Ping:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html
> 
> On 11/30/21 3:32 PM, Martin Sebor wrote:
>> Attached is a revised patch with the following changes based
>> on your comments:
>>
>> 1) Set and use statement uids to determine which statement
>>     precedes which in the same basic block.
>> 2) Avoid testing flag_isolate_erroneous_paths_dereference.
>> 3) Use post-dominance to decide whether to use the "maybe"
>>     phrasing vs a definite form.
>>
>> David raised (and in our offline discussion today reiterated)
>> an objection to the default setting of the option being
>> the strictest.  I have not changed that in this revision.
>> See my rationale for this choice in my reply below:
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html
>>
>> Martin
>>
>> On 11/23/21 2:16 PM, Martin Sebor wrote:
>>> On 11/22/21 6:32 PM, Jeff Law wrote:
>>>>
>>>>
>>>> On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote:
>>>>> Patch 1 in the series detects a small subset of uses of pointers
>>>>> made indeterminate by calls to deallocation functions like free
>>>>> or C++ operator delete.  To control the conditions the warnings
>>>>> are issued under the new -Wuse-after-free= option provides three
>>>>> levels.  At the lowest level the warning triggers only for
>>>>> unconditional uses of freed pointers and doesn't warn for uses
>>>>> in equality expressions.  Level 2 warns also for come conditional
>>>>> uses, and level 3 also for uses in equality expressions.
>>>>>
>>>>> I debated whether to make level 2 or 3 the default included in
>>>>> -Wall.  I decided on 3 for two reasons: 1) to raise awareness
>>>>> of both the problem and GCC's new ability to detect it: using
>>>>> a pointer after it's been freed, even only in principle, by
>>>>> a successful call to realloc, is undefined, and 2) because
>>>>> it's trivial to lower the level either globally, or locally
>>>>> by suppressing the warning around such misuses.
>>>>>
>>>>> I've tested the patch on x86_64-linux and by building Glibc
>>>>> and Binutils/GDB.  It triggers a number of times in each, all
>>>>> due to comparing invalidated pointers for equality (i.e., level
>>>>> 3).  I have suppressed these in GCC (libiberty) by a #pragma,
>>>>> and will see how the Glibc folks want to deal with theirs (I
>>>>> track them in BZ #28521).
>>>>>
>>>>> The tests contain a number of xfails due to limitations I'm
>>>>> aware of.  I marked them pr?????? until the patch is approved.
>>>>> I will open bugs for them before committing if I don't resolve
>>>>> them in a followup.
>>>>>
>>>>> Martin
>>>>>
>>>>> gcc-63272-1.diff
>>>>>
>>>>> Add -Wuse-after-free.
>>>>>
>>>>> gcc/c-family/ChangeLog
>>>>>
>>>>>     * c.opt (-Wuse-after-free): New options.
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>>     * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle
>>>>>     OPT_Wreturn_local_addr and OPT_Wuse_after_free_.
>>>>>     * diagnostic-spec.h (NW_DANGLING): New enumerator.
>>>>>     * doc/invoke.texi (-Wuse-after-free): Document new option.
>>>>>     * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename...
>>>>>     (pass_waccess::check_call_access): ...to this.
>>>>>     (pass_waccess::check): Rename...
>>>>>     (pass_waccess::check_block): ...to this.
>>>>>     (pass_waccess::check_pointer_uses): New function.
>>>>>     (pass_waccess::gimple_call_return_arg): New function.
>>>>>     (pass_waccess::warn_invalid_pointer): New function.
>>>>>     (pass_waccess::check_builtin): Handle free and realloc.
>>>>>     (gimple_use_after_inval_p): New function.
>>>>>     (get_realloc_lhs): New function.
>>>>>     (maybe_warn_mismatched_realloc): New function.
>>>>>     (pointers_related_p): New function.
>>>>>     (pass_waccess::check_call): Call check_pointer_uses.
>>>>>     (pass_waccess::execute): Compute and free dominance info.
>>>>>
>>>>> libcpp/ChangeLog:
>>>>>
>>>>>     * files.c (_cpp_find_file): Substitute a valid pointer for
>>>>>     an invalid one to avoid -Wuse-0after-free.
>>>>>
>>>>> libiberty/ChangeLog:
>>>>>
>>>>>     * regex.c: Suppress -Wuse-after-free.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>>     * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free.
>>>>>     * gcc.dg/Wmismatched-dealloc-3.c: Same.
>>>>>     * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free.
>>>>>     * gcc.dg/attr-alloc_size-7.c: Same.
>>>>>     * c-c++-common/Wuse-after-free-2.c: New test.
>>>>>     * c-c++-common/Wuse-after-free-3.c: New test.
>>>>>     * c-c++-common/Wuse-after-free-4.c: New test.
>>>>>     * c-c++-common/Wuse-after-free-5.c: New test.
>>>>>     * c-c++-common/Wuse-after-free-6.c: New test.
>>>>>     * c-c++-common/Wuse-after-free-7.c: New test.
>>>>>     * c-c++-common/Wuse-after-free.c: New test.
>>>>>     * g++.dg/warn/Wdangling-pointer.C: New test.
>>>>>     * g++.dg/warn/Wmismatched-dealloc-3.C: New test.
>>>>>     * g++.dg/warn/Wuse-after-free.C: New test.
>>>>>
>>>>> diff --git a/gcc/gimple-ssa-warn-access.cc 
>>>>> b/gcc/gimple-ssa-warn-access.cc
>>>>> index 63fc27a1487..2065402a2b9 100644
>>>>> --- a/gcc/gimple-ssa-warn-access.cc
>>>>> +++ b/gcc/gimple-ssa-warn-access.cc
>>>>>
>>>>> @@ -3397,33 +3417,460 @@ pass_waccess::maybe_check_dealloc_call 
>>>>> (gcall *call)
>>>>>       }
>>>>>   }
>>>>> +/* 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
>>>>> +   the same block, USE_STMT follows INVAL_STMT.  */
>>>>> +
>>>>> +static bool
>>>>> +gimple_use_after_inval_p (gimple *inval_stmt, gimple *use_stmt,
>>>>> +              bool last_block = false)
>>>>> +{
>>>>> +  tree clobvar =
>>>>> +    gimple_clobber_p (inval_stmt) ? gimple_assign_lhs (inval_stmt) 
>>>>> : NULL_TREE;
>>>>> +
>>>>> +  basic_block inval_bb = gimple_bb (inval_stmt);
>>>>> +  basic_block use_bb = gimple_bb (use_stmt);
>>>>> +
>>>>> +  if (inval_bb != use_bb)
>>>>> +    {
>>>>> +      if (dominated_by_p (CDI_DOMINATORS, use_bb, inval_bb))
>>>>> +    return true;
>>>>> +
>>>>> +      if (!clobvar || !last_block)
>>>>> +    return false;
>>>>> +
>>>>> +      auto gsi = gsi_for_stmt (use_stmt);
>>>>> +
>>>>> +      auto_bitmap visited;
>>>>> +
>>>>> +      /* A use statement in the last basic block in a function or 
>>>>> one that
>>>>> +     falls through to it is after any other prior clobber of the used
>>>>> +     variable unless it's followed by a clobber of the same 
>>>>> variable. */
>>>>> +      basic_block bb = use_bb;
>>>>> +      while (bb != inval_bb
>>>>> +         && single_succ_p (bb)
>>>>> +         && !(single_succ_edge (bb)->flags & 
>>>>> (EDGE_EH|EDGE_DFS_BACK)))
>>>>> +    {
>>>>> +      if (!bitmap_set_bit (visited, bb->index))
>>>>> +        /* Avoid cycles. */
>>>>> +        return true;
>>>>> +
>>>>> +      for (; !gsi_end_p (gsi); gsi_next_nondebug (&gsi))
>>>>> +        {
>>>>> +          gimple *stmt = gsi_stmt (gsi);
>>>>> +          if (gimple_clobber_p (stmt))
>>>>> +        {
>>>>> +          if (clobvar == gimple_assign_lhs (stmt))
>>>>> +            /* The use is followed by a clobber.  */
>>>>> +            return false;
>>>>> +        }
>>>>> +        }
>>>>> +
>>>>> +      bb = single_succ (bb);
>>>>> +      gsi = gsi_start_bb (bb);
>>>>> +    }
>>>>> +
>>>>> +      return bb == EXIT_BLOCK_PTR_FOR_FN (cfun);
>>>>> +    }
>>>> ?!?  I would have thought the block dominance test plus checking 
>>>> UIDs if the two statements are in the same block would be all you 
>>>> need. Can you elaborate more on what that hunk above is trying to do?
>>>
>>> The loop is entered only for -Wdangling-pointer.  It looks for
>>> the first clobber of the CLOBVAR variable (one whose clobber
>>> statement has been seen during the CFG traversal and whose use
>>> is being validated) in the successors along the single edge
>>> from the use block.  If the search finds a clobber, the use
>>> is valid.  If it doesn't, the use is one of a variable having
>>> gone out of scope (the clobber must be before the use).
>>>
>>> Among the cases the loop handles is the one in PR 63272
>>> (the request for -Wdangling-pointer) where the use neither
>>> follows the clobber in the same block nor dominated by it.
>>>
>>> There may be a way to optimize it somehow but because it's
>>> a search I don't think a simple UID check alone would be
>>> enough.
>>>
>>>>> +
>>>>> +  for (auto si = gsi_for_stmt (inval_stmt); !gsi_end_p (si);
>>>>> +       gsi_next_nondebug (&si))
>>>>> +    {
>>>>> +      gimple *stmt = gsi_stmt (si);
>>>>> +      if (stmt == use_stmt)
>>>>> +    return true;
>>>>> +    }
>>>>> +
>>>>> +  return false;
>>>>> +}
>>>> So from a compile-time standpoint, would it be better to to assign 
>>>> UIDs to each statement so that within a block you can just compare 
>>>> the UIDs? That's a pretty standard way to deal with the problem of 
>>>> statement domination within a block if we're going to be doing 
>>>> multiple queries.
>>>
>>> I'd considered it but because statement UIDs don't exist at
>>> the start of a pass, assigning them means either traversing all
>>> statements in the whole CFG first, even in functions with no
>>> deallocation calls or clobbers, or doing it lazily, after
>>> the first such statement has been seen.  It might ultimately
>>> be worthwhile if more warnings(*) end up relying on it but at
>>> this point I'm not sure the optimization wouldn't end up slowing
>>> things down on average.
>>>
>>> For some data, in a GCC bootstrap, each statement visited by
>>> this loop is visited on average twice (2.2 times), and
>>> the average sequence of statements traversed by the loop is
>>> 2.65, with a maximum of 22 times and 18 statements, respectively.
>>> So still not sure it would be a win.
>>>
>>> Let me know if this is something you think I need to pursue at
>>> this stage.
>>>
>>> [*] I think simple memory/resource leak detection might perhaps
>>> be one.
>>>
>>>>> +
>>>>> +/* Return true if P and Q point to the same object, and false if they
>>>>> +   either don't or their relationship cannot be determined.  */
>>>>> +
>>>>> +static bool
>>>>> +pointers_related_p (gimple *stmt, tree p, tree q, pointer_query &qry)
>>>>> +{
>>>>> +  if (!ptr_derefs_may_alias_p (p, q))
>>>>> +    return false;
>>>> Hmm, I guess that you don't need to worry about the case where P and 
>>>> Q point to different elements within an array.  They point to 
>>>> different final objects, though they do share a common enclosing 
>>>> object. Similarly for P & Q pointing to different members within a 
>>>> structure.
>>>
>>> Right.  The if statement is an optimization to avoid having to
>>> determine the identity of the complete objects that P and Q
>>> point to.  That's done by the calls to get_ref() below (for
>>> complete objects; as you note, we don't care about subobjects
>>> for this).
>>>
>>>>> +
>>>>> +/* For a STMT either a call to a deallocation function or a 
>>>>> clobber, warn
>>>>> +   for uses of the pointer PTR it was called with (including its 
>>>>> copies
>>>>> +   or others derived from it by pointer arithmetic).  */
>>>>> +
>>>>> +void
>>>>> +pass_waccess::check_pointer_uses (gimple *stmt, tree ptr)
>>>>> +{
>>>>> +  gcc_assert (TREE_CODE (ptr) == SSA_NAME);
>>>>> +
>>>>> +  const bool check_dangling = !is_gimple_call (stmt);
>>>>> +  basic_block stmt_bb = gimple_bb (stmt);
>>>>> +
>>>>> +  /* If the deallocation (or clobber) statement dominates more than
>>>>> +     a single basic block issue a "maybe" k
>>>> That seems wrong.   What you're looking for is a post-dominance 
>>>> relationship I think.   If the sink (free/delete) is post-dominated 
>>>> by the use, then it's a "must", if it's not post-dominated, then 
>>>> it's a maybe.  Of course, that means you need to build post-dominators.
>>>
>>> I'm sure you're right in general.  To avoid false positives
>>> the warning is very simplistic and only considers straight
>>> paths through the CFG, so I'm not sure this matters.  But
>>> I'm fine with using the post-dominance test instead if you
>>> thin it's worthwhile (it doesn't change any tests).
>>>
>>>>> +
>>>>> +      if (check_dangling
>>>>> +          && gimple_code (use_stmt) == GIMPLE_RETURN
>>>>> +          && optimize && flag_isolate_erroneous_paths_dereference)
>>>>> +        /* Avoid interfering with -Wreturn-local-addr (which runs 
>>>>> only
>>>>> +           with optimization enabled).  */
>>>>> +        continue;
>>>> Umm, that looks like a hack.  I can't think of a good reason why 
>>>> removal of erroneous paths should gate any of this code.  ISTM that 
>>>> you're likely papering over a problem elsewhere.
>>>
>>> This code avoids issuing -Wdangling-pointer for problems that
>>> will later be diagnosed by -Wreturn-local-addr.  E.g., in this
>>> case from Wreturn-local-addr-2.c:
>>>
>>>    ATTR (noipa) void*
>>>    return_array_plus_var (int i)
>>>    {
>>>        int a[32];
>>>        void *p = a + i;
>>>      sink (p);
>>>      return p;         /* { dg-warning "function returns address of 
>>> local" } */
>>>    }
>>>
>>> Without the test we'd end up with
>>>
>>>    warning: using dangling pointer ‘p’ to ‘a’ [-Wdangling-pointer=]
>>>
>>> in addition to -Wreturn-local-addr (and a whole slew of
>>> failures in the -Wreturn-local-addr tests).
>>>
>>> -Wreturn-local-addr only runs when
>>> flag_isolate_erroneous_paths_dereference is nonzero, so
>>> the conditional makes sure -Wdangling-pointer is issued when
>>> either the flag or -Wreturn-local-addr is disabled.  I think
>>> that works as expected (i.e., there's no problem elsewhere).
>>>
>>> I could have the code issue -Wdangling-pointer and suppress
>>> -Wreturn-local-addr but that doesn't seem right since
>>> the pointer hasn't gone out of scope yet at the point it's
>>> returned.
>>>
>>> Alternatively, I could change this instance of
>>> -Wdangling-pointer to -Wreturn-local-addr but that also
>>> doesn't seem like good design since we have a whole pass
>>> dedicated to the latter warning.
>>>
>>> I can't think of any other more elegant solutions but I'm open
>>> to suggestions.
>>>
>>> Martin
>>
>
Martin Sebor Jan. 4, 2022, 6:01 p.m. UTC | #3
Ping.  (CC'ing Jason as requested.)

https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html

On 12/13/21 9:48 AM, Martin Sebor wrote:
> Ping.
> 
> Jeff, I addressed your comments in the updated patch.  If there
> are no other changes is the last revision okay to commit?
> 
> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html
> 
> On 12/6/21 5:50 PM, Martin Sebor wrote:
>> Ping:
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html
>>
>> On 11/30/21 3:32 PM, Martin Sebor wrote:
>>> Attached is a revised patch with the following changes based
>>> on your comments:
>>>
>>> 1) Set and use statement uids to determine which statement
>>>     precedes which in the same basic block.
>>> 2) Avoid testing flag_isolate_erroneous_paths_dereference.
>>> 3) Use post-dominance to decide whether to use the "maybe"
>>>     phrasing vs a definite form.
>>>
>>> David raised (and in our offline discussion today reiterated)
>>> an objection to the default setting of the option being
>>> the strictest.  I have not changed that in this revision.
>>> See my rationale for this choice in my reply below:
>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html
>>>
>>> Martin
>>>
>>> On 11/23/21 2:16 PM, Martin Sebor wrote:
>>>> On 11/22/21 6:32 PM, Jeff Law wrote:
>>>>>
>>>>>
>>>>> On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote:
>>>>>> Patch 1 in the series detects a small subset of uses of pointers
>>>>>> made indeterminate by calls to deallocation functions like free
>>>>>> or C++ operator delete.  To control the conditions the warnings
>>>>>> are issued under the new -Wuse-after-free= option provides three
>>>>>> levels.  At the lowest level the warning triggers only for
>>>>>> unconditional uses of freed pointers and doesn't warn for uses
>>>>>> in equality expressions.  Level 2 warns also for come conditional
>>>>>> uses, and level 3 also for uses in equality expressions.
>>>>>>
>>>>>> I debated whether to make level 2 or 3 the default included in
>>>>>> -Wall.  I decided on 3 for two reasons: 1) to raise awareness
>>>>>> of both the problem and GCC's new ability to detect it: using
>>>>>> a pointer after it's been freed, even only in principle, by
>>>>>> a successful call to realloc, is undefined, and 2) because
>>>>>> it's trivial to lower the level either globally, or locally
>>>>>> by suppressing the warning around such misuses.
>>>>>>
>>>>>> I've tested the patch on x86_64-linux and by building Glibc
>>>>>> and Binutils/GDB.  It triggers a number of times in each, all
>>>>>> due to comparing invalidated pointers for equality (i.e., level
>>>>>> 3).  I have suppressed these in GCC (libiberty) by a #pragma,
>>>>>> and will see how the Glibc folks want to deal with theirs (I
>>>>>> track them in BZ #28521).
>>>>>>
>>>>>> The tests contain a number of xfails due to limitations I'm
>>>>>> aware of.  I marked them pr?????? until the patch is approved.
>>>>>> I will open bugs for them before committing if I don't resolve
>>>>>> them in a followup.
>>>>>>
>>>>>> Martin
>>>>>>
>>>>>> gcc-63272-1.diff
>>>>>>
>>>>>> Add -Wuse-after-free.
>>>>>>
>>>>>> gcc/c-family/ChangeLog
>>>>>>
>>>>>>     * c.opt (-Wuse-after-free): New options.
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>>     * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle
>>>>>>     OPT_Wreturn_local_addr and OPT_Wuse_after_free_.
>>>>>>     * diagnostic-spec.h (NW_DANGLING): New enumerator.
>>>>>>     * doc/invoke.texi (-Wuse-after-free): Document new option.
>>>>>>     * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename...
>>>>>>     (pass_waccess::check_call_access): ...to this.
>>>>>>     (pass_waccess::check): Rename...
>>>>>>     (pass_waccess::check_block): ...to this.
>>>>>>     (pass_waccess::check_pointer_uses): New function.
>>>>>>     (pass_waccess::gimple_call_return_arg): New function.
>>>>>>     (pass_waccess::warn_invalid_pointer): New function.
>>>>>>     (pass_waccess::check_builtin): Handle free and realloc.
>>>>>>     (gimple_use_after_inval_p): New function.
>>>>>>     (get_realloc_lhs): New function.
>>>>>>     (maybe_warn_mismatched_realloc): New function.
>>>>>>     (pointers_related_p): New function.
>>>>>>     (pass_waccess::check_call): Call check_pointer_uses.
>>>>>>     (pass_waccess::execute): Compute and free dominance info.
>>>>>>
>>>>>> libcpp/ChangeLog:
>>>>>>
>>>>>>     * files.c (_cpp_find_file): Substitute a valid pointer for
>>>>>>     an invalid one to avoid -Wuse-0after-free.
>>>>>>
>>>>>> libiberty/ChangeLog:
>>>>>>
>>>>>>     * regex.c: Suppress -Wuse-after-free.
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>>     * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free.
>>>>>>     * gcc.dg/Wmismatched-dealloc-3.c: Same.
>>>>>>     * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free.
>>>>>>     * gcc.dg/attr-alloc_size-7.c: Same.
>>>>>>     * c-c++-common/Wuse-after-free-2.c: New test.
>>>>>>     * c-c++-common/Wuse-after-free-3.c: New test.
>>>>>>     * c-c++-common/Wuse-after-free-4.c: New test.
>>>>>>     * c-c++-common/Wuse-after-free-5.c: New test.
>>>>>>     * c-c++-common/Wuse-after-free-6.c: New test.
>>>>>>     * c-c++-common/Wuse-after-free-7.c: New test.
>>>>>>     * c-c++-common/Wuse-after-free.c: New test.
>>>>>>     * g++.dg/warn/Wdangling-pointer.C: New test.
>>>>>>     * g++.dg/warn/Wmismatched-dealloc-3.C: New test.
>>>>>>     * g++.dg/warn/Wuse-after-free.C: New test.
>>>>>>
>>>>>> diff --git a/gcc/gimple-ssa-warn-access.cc 
>>>>>> b/gcc/gimple-ssa-warn-access.cc
>>>>>> index 63fc27a1487..2065402a2b9 100644
>>>>>> --- a/gcc/gimple-ssa-warn-access.cc
>>>>>> +++ b/gcc/gimple-ssa-warn-access.cc
>>>>>>
>>>>>> @@ -3397,33 +3417,460 @@ pass_waccess::maybe_check_dealloc_call 
>>>>>> (gcall *call)
>>>>>>       }
>>>>>>   }
>>>>>> +/* 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
>>>>>> +   the same block, USE_STMT follows INVAL_STMT.  */
>>>>>> +
>>>>>> +static bool
>>>>>> +gimple_use_after_inval_p (gimple *inval_stmt, gimple *use_stmt,
>>>>>> +              bool last_block = false)
>>>>>> +{
>>>>>> +  tree clobvar =
>>>>>> +    gimple_clobber_p (inval_stmt) ? gimple_assign_lhs 
>>>>>> (inval_stmt) : NULL_TREE;
>>>>>> +
>>>>>> +  basic_block inval_bb = gimple_bb (inval_stmt);
>>>>>> +  basic_block use_bb = gimple_bb (use_stmt);
>>>>>> +
>>>>>> +  if (inval_bb != use_bb)
>>>>>> +    {
>>>>>> +      if (dominated_by_p (CDI_DOMINATORS, use_bb, inval_bb))
>>>>>> +    return true;
>>>>>> +
>>>>>> +      if (!clobvar || !last_block)
>>>>>> +    return false;
>>>>>> +
>>>>>> +      auto gsi = gsi_for_stmt (use_stmt);
>>>>>> +
>>>>>> +      auto_bitmap visited;
>>>>>> +
>>>>>> +      /* A use statement in the last basic block in a function or 
>>>>>> one that
>>>>>> +     falls through to it is after any other prior clobber of the 
>>>>>> used
>>>>>> +     variable unless it's followed by a clobber of the same 
>>>>>> variable. */
>>>>>> +      basic_block bb = use_bb;
>>>>>> +      while (bb != inval_bb
>>>>>> +         && single_succ_p (bb)
>>>>>> +         && !(single_succ_edge (bb)->flags & 
>>>>>> (EDGE_EH|EDGE_DFS_BACK)))
>>>>>> +    {
>>>>>> +      if (!bitmap_set_bit (visited, bb->index))
>>>>>> +        /* Avoid cycles. */
>>>>>> +        return true;
>>>>>> +
>>>>>> +      for (; !gsi_end_p (gsi); gsi_next_nondebug (&gsi))
>>>>>> +        {
>>>>>> +          gimple *stmt = gsi_stmt (gsi);
>>>>>> +          if (gimple_clobber_p (stmt))
>>>>>> +        {
>>>>>> +          if (clobvar == gimple_assign_lhs (stmt))
>>>>>> +            /* The use is followed by a clobber.  */
>>>>>> +            return false;
>>>>>> +        }
>>>>>> +        }
>>>>>> +
>>>>>> +      bb = single_succ (bb);
>>>>>> +      gsi = gsi_start_bb (bb);
>>>>>> +    }
>>>>>> +
>>>>>> +      return bb == EXIT_BLOCK_PTR_FOR_FN (cfun);
>>>>>> +    }
>>>>> ?!?  I would have thought the block dominance test plus checking 
>>>>> UIDs if the two statements are in the same block would be all you 
>>>>> need. Can you elaborate more on what that hunk above is trying to do?
>>>>
>>>> The loop is entered only for -Wdangling-pointer.  It looks for
>>>> the first clobber of the CLOBVAR variable (one whose clobber
>>>> statement has been seen during the CFG traversal and whose use
>>>> is being validated) in the successors along the single edge
>>>> from the use block.  If the search finds a clobber, the use
>>>> is valid.  If it doesn't, the use is one of a variable having
>>>> gone out of scope (the clobber must be before the use).
>>>>
>>>> Among the cases the loop handles is the one in PR 63272
>>>> (the request for -Wdangling-pointer) where the use neither
>>>> follows the clobber in the same block nor dominated by it.
>>>>
>>>> There may be a way to optimize it somehow but because it's
>>>> a search I don't think a simple UID check alone would be
>>>> enough.
>>>>
>>>>>> +
>>>>>> +  for (auto si = gsi_for_stmt (inval_stmt); !gsi_end_p (si);
>>>>>> +       gsi_next_nondebug (&si))
>>>>>> +    {
>>>>>> +      gimple *stmt = gsi_stmt (si);
>>>>>> +      if (stmt == use_stmt)
>>>>>> +    return true;
>>>>>> +    }
>>>>>> +
>>>>>> +  return false;
>>>>>> +}
>>>>> So from a compile-time standpoint, would it be better to to assign 
>>>>> UIDs to each statement so that within a block you can just compare 
>>>>> the UIDs? That's a pretty standard way to deal with the problem of 
>>>>> statement domination within a block if we're going to be doing 
>>>>> multiple queries.
>>>>
>>>> I'd considered it but because statement UIDs don't exist at
>>>> the start of a pass, assigning them means either traversing all
>>>> statements in the whole CFG first, even in functions with no
>>>> deallocation calls or clobbers, or doing it lazily, after
>>>> the first such statement has been seen.  It might ultimately
>>>> be worthwhile if more warnings(*) end up relying on it but at
>>>> this point I'm not sure the optimization wouldn't end up slowing
>>>> things down on average.
>>>>
>>>> For some data, in a GCC bootstrap, each statement visited by
>>>> this loop is visited on average twice (2.2 times), and
>>>> the average sequence of statements traversed by the loop is
>>>> 2.65, with a maximum of 22 times and 18 statements, respectively.
>>>> So still not sure it would be a win.
>>>>
>>>> Let me know if this is something you think I need to pursue at
>>>> this stage.
>>>>
>>>> [*] I think simple memory/resource leak detection might perhaps
>>>> be one.
>>>>
>>>>>> +
>>>>>> +/* Return true if P and Q point to the same object, and false if 
>>>>>> they
>>>>>> +   either don't or their relationship cannot be determined.  */
>>>>>> +
>>>>>> +static bool
>>>>>> +pointers_related_p (gimple *stmt, tree p, tree q, pointer_query 
>>>>>> &qry)
>>>>>> +{
>>>>>> +  if (!ptr_derefs_may_alias_p (p, q))
>>>>>> +    return false;
>>>>> Hmm, I guess that you don't need to worry about the case where P 
>>>>> and Q point to different elements within an array.  They point to 
>>>>> different final objects, though they do share a common enclosing 
>>>>> object. Similarly for P & Q pointing to different members within a 
>>>>> structure.
>>>>
>>>> Right.  The if statement is an optimization to avoid having to
>>>> determine the identity of the complete objects that P and Q
>>>> point to.  That's done by the calls to get_ref() below (for
>>>> complete objects; as you note, we don't care about subobjects
>>>> for this).
>>>>
>>>>>> +
>>>>>> +/* For a STMT either a call to a deallocation function or a 
>>>>>> clobber, warn
>>>>>> +   for uses of the pointer PTR it was called with (including its 
>>>>>> copies
>>>>>> +   or others derived from it by pointer arithmetic).  */
>>>>>> +
>>>>>> +void
>>>>>> +pass_waccess::check_pointer_uses (gimple *stmt, tree ptr)
>>>>>> +{
>>>>>> +  gcc_assert (TREE_CODE (ptr) == SSA_NAME);
>>>>>> +
>>>>>> +  const bool check_dangling = !is_gimple_call (stmt);
>>>>>> +  basic_block stmt_bb = gimple_bb (stmt);
>>>>>> +
>>>>>> +  /* If the deallocation (or clobber) statement dominates more than
>>>>>> +     a single basic block issue a "maybe" k
>>>>> That seems wrong.   What you're looking for is a post-dominance 
>>>>> relationship I think.   If the sink (free/delete) is post-dominated 
>>>>> by the use, then it's a "must", if it's not post-dominated, then 
>>>>> it's a maybe.  Of course, that means you need to build 
>>>>> post-dominators.
>>>>
>>>> I'm sure you're right in general.  To avoid false positives
>>>> the warning is very simplistic and only considers straight
>>>> paths through the CFG, so I'm not sure this matters.  But
>>>> I'm fine with using the post-dominance test instead if you
>>>> thin it's worthwhile (it doesn't change any tests).
>>>>
>>>>>> +
>>>>>> +      if (check_dangling
>>>>>> +          && gimple_code (use_stmt) == GIMPLE_RETURN
>>>>>> +          && optimize && flag_isolate_erroneous_paths_dereference)
>>>>>> +        /* Avoid interfering with -Wreturn-local-addr (which runs 
>>>>>> only
>>>>>> +           with optimization enabled).  */
>>>>>> +        continue;
>>>>> Umm, that looks like a hack.  I can't think of a good reason why 
>>>>> removal of erroneous paths should gate any of this code.  ISTM that 
>>>>> you're likely papering over a problem elsewhere.
>>>>
>>>> This code avoids issuing -Wdangling-pointer for problems that
>>>> will later be diagnosed by -Wreturn-local-addr.  E.g., in this
>>>> case from Wreturn-local-addr-2.c:
>>>>
>>>>    ATTR (noipa) void*
>>>>    return_array_plus_var (int i)
>>>>    {
>>>>        int a[32];
>>>>        void *p = a + i;
>>>>      sink (p);
>>>>      return p;         /* { dg-warning "function returns address of 
>>>> local" } */
>>>>    }
>>>>
>>>> Without the test we'd end up with
>>>>
>>>>    warning: using dangling pointer ‘p’ to ‘a’ [-Wdangling-pointer=]
>>>>
>>>> in addition to -Wreturn-local-addr (and a whole slew of
>>>> failures in the -Wreturn-local-addr tests).
>>>>
>>>> -Wreturn-local-addr only runs when
>>>> flag_isolate_erroneous_paths_dereference is nonzero, so
>>>> the conditional makes sure -Wdangling-pointer is issued when
>>>> either the flag or -Wreturn-local-addr is disabled.  I think
>>>> that works as expected (i.e., there's no problem elsewhere).
>>>>
>>>> I could have the code issue -Wdangling-pointer and suppress
>>>> -Wreturn-local-addr but that doesn't seem right since
>>>> the pointer hasn't gone out of scope yet at the point it's
>>>> returned.
>>>>
>>>> Alternatively, I could change this instance of
>>>> -Wdangling-pointer to -Wreturn-local-addr but that also
>>>> doesn't seem like good design since we have a whole pass
>>>> dedicated to the latter warning.
>>>>
>>>> I can't think of any other more elegant solutions but I'm open
>>>> to suggestions.
>>>>
>>>> Martin
>>>
>>
>
Martin Sebor Jan. 10, 2022, 9:58 p.m. UTC | #4
Last ping before stage 3 ends:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html

On 1/4/22 11:01, Martin Sebor wrote:
> Ping.  (CC'ing Jason as requested.)
> 
> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html
> 
> On 12/13/21 9:48 AM, Martin Sebor wrote:
>> Ping.
>>
>> Jeff, I addressed your comments in the updated patch.  If there
>> are no other changes is the last revision okay to commit?
>>
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html
>>
>> On 12/6/21 5:50 PM, Martin Sebor wrote:
>>> Ping:
>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html
>>>
>>> On 11/30/21 3:32 PM, Martin Sebor wrote:
>>>> Attached is a revised patch with the following changes based
>>>> on your comments:
>>>>
>>>> 1) Set and use statement uids to determine which statement
>>>>     precedes which in the same basic block.
>>>> 2) Avoid testing flag_isolate_erroneous_paths_dereference.
>>>> 3) Use post-dominance to decide whether to use the "maybe"
>>>>     phrasing vs a definite form.
>>>>
>>>> David raised (and in our offline discussion today reiterated)
>>>> an objection to the default setting of the option being
>>>> the strictest.  I have not changed that in this revision.
>>>> See my rationale for this choice in my reply below:
>>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html
>>>>
>>>> Martin
>>>>
>>>> On 11/23/21 2:16 PM, Martin Sebor wrote:
>>>>> On 11/22/21 6:32 PM, Jeff Law wrote:
>>>>>>
>>>>>>
>>>>>> On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote:
>>>>>>> Patch 1 in the series detects a small subset of uses of pointers
>>>>>>> made indeterminate by calls to deallocation functions like free
>>>>>>> or C++ operator delete.  To control the conditions the warnings
>>>>>>> are issued under the new -Wuse-after-free= option provides three
>>>>>>> levels.  At the lowest level the warning triggers only for
>>>>>>> unconditional uses of freed pointers and doesn't warn for uses
>>>>>>> in equality expressions.  Level 2 warns also for come conditional
>>>>>>> uses, and level 3 also for uses in equality expressions.
>>>>>>>
>>>>>>> I debated whether to make level 2 or 3 the default included in
>>>>>>> -Wall.  I decided on 3 for two reasons: 1) to raise awareness
>>>>>>> of both the problem and GCC's new ability to detect it: using
>>>>>>> a pointer after it's been freed, even only in principle, by
>>>>>>> a successful call to realloc, is undefined, and 2) because
>>>>>>> it's trivial to lower the level either globally, or locally
>>>>>>> by suppressing the warning around such misuses.
>>>>>>>
>>>>>>> I've tested the patch on x86_64-linux and by building Glibc
>>>>>>> and Binutils/GDB.  It triggers a number of times in each, all
>>>>>>> due to comparing invalidated pointers for equality (i.e., level
>>>>>>> 3).  I have suppressed these in GCC (libiberty) by a #pragma,
>>>>>>> and will see how the Glibc folks want to deal with theirs (I
>>>>>>> track them in BZ #28521).
>>>>>>>
>>>>>>> The tests contain a number of xfails due to limitations I'm
>>>>>>> aware of.  I marked them pr?????? until the patch is approved.
>>>>>>> I will open bugs for them before committing if I don't resolve
>>>>>>> them in a followup.
>>>>>>>
>>>>>>> Martin
>>>>>>>
>>>>>>> gcc-63272-1.diff
>>>>>>>
>>>>>>> Add -Wuse-after-free.
>>>>>>>
>>>>>>> gcc/c-family/ChangeLog
>>>>>>>
>>>>>>>     * c.opt (-Wuse-after-free): New options.
>>>>>>>
>>>>>>> gcc/ChangeLog:
>>>>>>>
>>>>>>>     * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle
>>>>>>>     OPT_Wreturn_local_addr and OPT_Wuse_after_free_.
>>>>>>>     * diagnostic-spec.h (NW_DANGLING): New enumerator.
>>>>>>>     * doc/invoke.texi (-Wuse-after-free): Document new option.
>>>>>>>     * gimple-ssa-warn-access.cc (pass_waccess::check_call): 
>>>>>>> Rename...
>>>>>>>     (pass_waccess::check_call_access): ...to this.
>>>>>>>     (pass_waccess::check): Rename...
>>>>>>>     (pass_waccess::check_block): ...to this.
>>>>>>>     (pass_waccess::check_pointer_uses): New function.
>>>>>>>     (pass_waccess::gimple_call_return_arg): New function.
>>>>>>>     (pass_waccess::warn_invalid_pointer): New function.
>>>>>>>     (pass_waccess::check_builtin): Handle free and realloc.
>>>>>>>     (gimple_use_after_inval_p): New function.
>>>>>>>     (get_realloc_lhs): New function.
>>>>>>>     (maybe_warn_mismatched_realloc): New function.
>>>>>>>     (pointers_related_p): New function.
>>>>>>>     (pass_waccess::check_call): Call check_pointer_uses.
>>>>>>>     (pass_waccess::execute): Compute and free dominance info.
>>>>>>>
>>>>>>> libcpp/ChangeLog:
>>>>>>>
>>>>>>>     * files.c (_cpp_find_file): Substitute a valid pointer for
>>>>>>>     an invalid one to avoid -Wuse-0after-free.
>>>>>>>
>>>>>>> libiberty/ChangeLog:
>>>>>>>
>>>>>>>     * regex.c: Suppress -Wuse-after-free.
>>>>>>>
>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>
>>>>>>>     * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free.
>>>>>>>     * gcc.dg/Wmismatched-dealloc-3.c: Same.
>>>>>>>     * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free.
>>>>>>>     * gcc.dg/attr-alloc_size-7.c: Same.
>>>>>>>     * c-c++-common/Wuse-after-free-2.c: New test.
>>>>>>>     * c-c++-common/Wuse-after-free-3.c: New test.
>>>>>>>     * c-c++-common/Wuse-after-free-4.c: New test.
>>>>>>>     * c-c++-common/Wuse-after-free-5.c: New test.
>>>>>>>     * c-c++-common/Wuse-after-free-6.c: New test.
>>>>>>>     * c-c++-common/Wuse-after-free-7.c: New test.
>>>>>>>     * c-c++-common/Wuse-after-free.c: New test.
>>>>>>>     * g++.dg/warn/Wdangling-pointer.C: New test.
>>>>>>>     * g++.dg/warn/Wmismatched-dealloc-3.C: New test.
>>>>>>>     * g++.dg/warn/Wuse-after-free.C: New test.
>>>>>>>
>>>>>>> diff --git a/gcc/gimple-ssa-warn-access.cc 
>>>>>>> b/gcc/gimple-ssa-warn-access.cc
>>>>>>> index 63fc27a1487..2065402a2b9 100644
>>>>>>> --- a/gcc/gimple-ssa-warn-access.cc
>>>>>>> +++ b/gcc/gimple-ssa-warn-access.cc
>>>>>>>
>>>>>>> @@ -3397,33 +3417,460 @@ pass_waccess::maybe_check_dealloc_call 
>>>>>>> (gcall *call)
>>>>>>>       }
>>>>>>>   }
>>>>>>> +/* 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
>>>>>>> +   the same block, USE_STMT follows INVAL_STMT.  */
>>>>>>> +
>>>>>>> +static bool
>>>>>>> +gimple_use_after_inval_p (gimple *inval_stmt, gimple *use_stmt,
>>>>>>> +              bool last_block = false)
>>>>>>> +{
>>>>>>> +  tree clobvar =
>>>>>>> +    gimple_clobber_p (inval_stmt) ? gimple_assign_lhs 
>>>>>>> (inval_stmt) : NULL_TREE;
>>>>>>> +
>>>>>>> +  basic_block inval_bb = gimple_bb (inval_stmt);
>>>>>>> +  basic_block use_bb = gimple_bb (use_stmt);
>>>>>>> +
>>>>>>> +  if (inval_bb != use_bb)
>>>>>>> +    {
>>>>>>> +      if (dominated_by_p (CDI_DOMINATORS, use_bb, inval_bb))
>>>>>>> +    return true;
>>>>>>> +
>>>>>>> +      if (!clobvar || !last_block)
>>>>>>> +    return false;
>>>>>>> +
>>>>>>> +      auto gsi = gsi_for_stmt (use_stmt);
>>>>>>> +
>>>>>>> +      auto_bitmap visited;
>>>>>>> +
>>>>>>> +      /* A use statement in the last basic block in a function 
>>>>>>> or one that
>>>>>>> +     falls through to it is after any other prior clobber of the 
>>>>>>> used
>>>>>>> +     variable unless it's followed by a clobber of the same 
>>>>>>> variable. */
>>>>>>> +      basic_block bb = use_bb;
>>>>>>> +      while (bb != inval_bb
>>>>>>> +         && single_succ_p (bb)
>>>>>>> +         && !(single_succ_edge (bb)->flags & 
>>>>>>> (EDGE_EH|EDGE_DFS_BACK)))
>>>>>>> +    {
>>>>>>> +      if (!bitmap_set_bit (visited, bb->index))
>>>>>>> +        /* Avoid cycles. */
>>>>>>> +        return true;
>>>>>>> +
>>>>>>> +      for (; !gsi_end_p (gsi); gsi_next_nondebug (&gsi))
>>>>>>> +        {
>>>>>>> +          gimple *stmt = gsi_stmt (gsi);
>>>>>>> +          if (gimple_clobber_p (stmt))
>>>>>>> +        {
>>>>>>> +          if (clobvar == gimple_assign_lhs (stmt))
>>>>>>> +            /* The use is followed by a clobber.  */
>>>>>>> +            return false;
>>>>>>> +        }
>>>>>>> +        }
>>>>>>> +
>>>>>>> +      bb = single_succ (bb);
>>>>>>> +      gsi = gsi_start_bb (bb);
>>>>>>> +    }
>>>>>>> +
>>>>>>> +      return bb == EXIT_BLOCK_PTR_FOR_FN (cfun);
>>>>>>> +    }
>>>>>> ?!?  I would have thought the block dominance test plus checking 
>>>>>> UIDs if the two statements are in the same block would be all you 
>>>>>> need. Can you elaborate more on what that hunk above is trying to do?
>>>>>
>>>>> The loop is entered only for -Wdangling-pointer.  It looks for
>>>>> the first clobber of the CLOBVAR variable (one whose clobber
>>>>> statement has been seen during the CFG traversal and whose use
>>>>> is being validated) in the successors along the single edge
>>>>> from the use block.  If the search finds a clobber, the use
>>>>> is valid.  If it doesn't, the use is one of a variable having
>>>>> gone out of scope (the clobber must be before the use).
>>>>>
>>>>> Among the cases the loop handles is the one in PR 63272
>>>>> (the request for -Wdangling-pointer) where the use neither
>>>>> follows the clobber in the same block nor dominated by it.
>>>>>
>>>>> There may be a way to optimize it somehow but because it's
>>>>> a search I don't think a simple UID check alone would be
>>>>> enough.
>>>>>
>>>>>>> +
>>>>>>> +  for (auto si = gsi_for_stmt (inval_stmt); !gsi_end_p (si);
>>>>>>> +       gsi_next_nondebug (&si))
>>>>>>> +    {
>>>>>>> +      gimple *stmt = gsi_stmt (si);
>>>>>>> +      if (stmt == use_stmt)
>>>>>>> +    return true;
>>>>>>> +    }
>>>>>>> +
>>>>>>> +  return false;
>>>>>>> +}
>>>>>> So from a compile-time standpoint, would it be better to to assign 
>>>>>> UIDs to each statement so that within a block you can just compare 
>>>>>> the UIDs? That's a pretty standard way to deal with the problem of 
>>>>>> statement domination within a block if we're going to be doing 
>>>>>> multiple queries.
>>>>>
>>>>> I'd considered it but because statement UIDs don't exist at
>>>>> the start of a pass, assigning them means either traversing all
>>>>> statements in the whole CFG first, even in functions with no
>>>>> deallocation calls or clobbers, or doing it lazily, after
>>>>> the first such statement has been seen.  It might ultimately
>>>>> be worthwhile if more warnings(*) end up relying on it but at
>>>>> this point I'm not sure the optimization wouldn't end up slowing
>>>>> things down on average.
>>>>>
>>>>> For some data, in a GCC bootstrap, each statement visited by
>>>>> this loop is visited on average twice (2.2 times), and
>>>>> the average sequence of statements traversed by the loop is
>>>>> 2.65, with a maximum of 22 times and 18 statements, respectively.
>>>>> So still not sure it would be a win.
>>>>>
>>>>> Let me know if this is something you think I need to pursue at
>>>>> this stage.
>>>>>
>>>>> [*] I think simple memory/resource leak detection might perhaps
>>>>> be one.
>>>>>
>>>>>>> +
>>>>>>> +/* Return true if P and Q point to the same object, and false if 
>>>>>>> they
>>>>>>> +   either don't or their relationship cannot be determined.  */
>>>>>>> +
>>>>>>> +static bool
>>>>>>> +pointers_related_p (gimple *stmt, tree p, tree q, pointer_query 
>>>>>>> &qry)
>>>>>>> +{
>>>>>>> +  if (!ptr_derefs_may_alias_p (p, q))
>>>>>>> +    return false;
>>>>>> Hmm, I guess that you don't need to worry about the case where P 
>>>>>> and Q point to different elements within an array.  They point to 
>>>>>> different final objects, though they do share a common enclosing 
>>>>>> object. Similarly for P & Q pointing to different members within a 
>>>>>> structure.
>>>>>
>>>>> Right.  The if statement is an optimization to avoid having to
>>>>> determine the identity of the complete objects that P and Q
>>>>> point to.  That's done by the calls to get_ref() below (for
>>>>> complete objects; as you note, we don't care about subobjects
>>>>> for this).
>>>>>
>>>>>>> +
>>>>>>> +/* For a STMT either a call to a deallocation function or a 
>>>>>>> clobber, warn
>>>>>>> +   for uses of the pointer PTR it was called with (including its 
>>>>>>> copies
>>>>>>> +   or others derived from it by pointer arithmetic).  */
>>>>>>> +
>>>>>>> +void
>>>>>>> +pass_waccess::check_pointer_uses (gimple *stmt, tree ptr)
>>>>>>> +{
>>>>>>> +  gcc_assert (TREE_CODE (ptr) == SSA_NAME);
>>>>>>> +
>>>>>>> +  const bool check_dangling = !is_gimple_call (stmt);
>>>>>>> +  basic_block stmt_bb = gimple_bb (stmt);
>>>>>>> +
>>>>>>> +  /* If the deallocation (or clobber) statement dominates more than
>>>>>>> +     a single basic block issue a "maybe" k
>>>>>> That seems wrong.   What you're looking for is a post-dominance 
>>>>>> relationship I think.   If the sink (free/delete) is 
>>>>>> post-dominated by the use, then it's a "must", if it's not 
>>>>>> post-dominated, then it's a maybe.  Of course, that means you need 
>>>>>> to build post-dominators.
>>>>>
>>>>> I'm sure you're right in general.  To avoid false positives
>>>>> the warning is very simplistic and only considers straight
>>>>> paths through the CFG, so I'm not sure this matters.  But
>>>>> I'm fine with using the post-dominance test instead if you
>>>>> thin it's worthwhile (it doesn't change any tests).
>>>>>
>>>>>>> +
>>>>>>> +      if (check_dangling
>>>>>>> +          && gimple_code (use_stmt) == GIMPLE_RETURN
>>>>>>> +          && optimize && flag_isolate_erroneous_paths_dereference)
>>>>>>> +        /* Avoid interfering with -Wreturn-local-addr (which 
>>>>>>> runs only
>>>>>>> +           with optimization enabled).  */
>>>>>>> +        continue;
>>>>>> Umm, that looks like a hack.  I can't think of a good reason why 
>>>>>> removal of erroneous paths should gate any of this code.  ISTM 
>>>>>> that you're likely papering over a problem elsewhere.
>>>>>
>>>>> This code avoids issuing -Wdangling-pointer for problems that
>>>>> will later be diagnosed by -Wreturn-local-addr.  E.g., in this
>>>>> case from Wreturn-local-addr-2.c:
>>>>>
>>>>>    ATTR (noipa) void*
>>>>>    return_array_plus_var (int i)
>>>>>    {
>>>>>        int a[32];
>>>>>        void *p = a + i;
>>>>>      sink (p);
>>>>>      return p;         /* { dg-warning "function returns address of 
>>>>> local" } */
>>>>>    }
>>>>>
>>>>> Without the test we'd end up with
>>>>>
>>>>>    warning: using dangling pointer ‘p’ to ‘a’ [-Wdangling-pointer=]
>>>>>
>>>>> in addition to -Wreturn-local-addr (and a whole slew of
>>>>> failures in the -Wreturn-local-addr tests).
>>>>>
>>>>> -Wreturn-local-addr only runs when
>>>>> flag_isolate_erroneous_paths_dereference is nonzero, so
>>>>> the conditional makes sure -Wdangling-pointer is issued when
>>>>> either the flag or -Wreturn-local-addr is disabled.  I think
>>>>> that works as expected (i.e., there's no problem elsewhere).
>>>>>
>>>>> I could have the code issue -Wdangling-pointer and suppress
>>>>> -Wreturn-local-addr but that doesn't seem right since
>>>>> the pointer hasn't gone out of scope yet at the point it's
>>>>> returned.
>>>>>
>>>>> Alternatively, I could change this instance of
>>>>> -Wdangling-pointer to -Wreturn-local-addr but that also
>>>>> doesn't seem like good design since we have a whole pass
>>>>> dedicated to the latter warning.
>>>>>
>>>>> I can't think of any other more elegant solutions but I'm open
>>>>> to suggestions.
>>>>>
>>>>> Martin
>>>>
>>>
>>
>
Jason Merrill Jan. 11, 2022, 10:40 p.m. UTC | #5
On 11/30/21 17:32, Martin Sebor via Gcc-patches wrote:
> Attached is a revised patch with the following changes based
> on your comments:
> 
> 1) Set and use statement uids to determine which statement
>     precedes which in the same basic block.
> 2) Avoid testing flag_isolate_erroneous_paths_dereference.
> 3) Use post-dominance to decide whether to use the "maybe"
>     phrasing vs a definite form.
> 
> David raised (and in our offline discussion today reiterated)
> an objection to the default setting of the option being
> the strictest.  I have not changed that in this revision.
> See my rationale for this choice in my reply below:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html

In the latest C2x draft I see in the list of undefined behavior

"The value of a pointer that refers to space deallocated by a call to 
the free or realloc function is used (7.22.3)."

So the case that would be technically undefined would be comparing the 
reallocated pointer to the old pointer which has been deallocated.

The C++ draft is more nuanced: it says, "When the end of the duration of 
a region of storage is reached, the values of all pointers representing 
the address of any part of that region of storage become invalid pointer 
values (6.8.3).  Indirection through an invalid pointer value and 
passing an invalid pointer value to a deallocation function have 
undefined behavior.  Any other use of an invalid pointer value has 
implementation-defined behavior."

So the case above is implementation-defined in C++, not undefined.

Let's put =2 in -Wall for now.

With that change, this and the -Wdangling-pointer patch are OK on Friday 
afternoon if there are no other comments before then.

> On 11/23/21 2:16 PM, Martin Sebor wrote:
>> On 11/22/21 6:32 PM, Jeff Law wrote:
>>>
>>>
>>> On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote:
>>>> Patch 1 in the series detects a small subset of uses of pointers
>>>> made indeterminate by calls to deallocation functions like free
>>>> or C++ operator delete.  To control the conditions the warnings
>>>> are issued under the new -Wuse-after-free= option provides three
>>>> levels.  At the lowest level the warning triggers only for
>>>> unconditional uses of freed pointers and doesn't warn for uses
>>>> in equality expressions.  Level 2 warns also for come conditional
>>>> uses, and level 3 also for uses in equality expressions.
>>>>
>>>> I debated whether to make level 2 or 3 the default included in
>>>> -Wall.  I decided on 3 for two reasons: 1) to raise awareness
>>>> of both the problem and GCC's new ability to detect it: using
>>>> a pointer after it's been freed, even only in principle, by
>>>> a successful call to realloc, is undefined, and 2) because
>>>> it's trivial to lower the level either globally, or locally
>>>> by suppressing the warning around such misuses.
>>>>
>>>> I've tested the patch on x86_64-linux and by building Glibc
>>>> and Binutils/GDB.  It triggers a number of times in each, all
>>>> due to comparing invalidated pointers for equality (i.e., level
>>>> 3).  I have suppressed these in GCC (libiberty) by a #pragma,
>>>> and will see how the Glibc folks want to deal with theirs (I
>>>> track them in BZ #28521).
>>>>
>>>> The tests contain a number of xfails due to limitations I'm
>>>> aware of.  I marked them pr?????? until the patch is approved.
>>>> I will open bugs for them before committing if I don't resolve
>>>> them in a followup.
>>>>
>>>> Martin
>>>>
>>>> gcc-63272-1.diff
>>>>
>>>> Add -Wuse-after-free.
>>>>
>>>> gcc/c-family/ChangeLog
>>>>
>>>>     * c.opt (-Wuse-after-free): New options.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>     * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle
>>>>     OPT_Wreturn_local_addr and OPT_Wuse_after_free_.
>>>>     * diagnostic-spec.h (NW_DANGLING): New enumerator.
>>>>     * doc/invoke.texi (-Wuse-after-free): Document new option.
>>>>     * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename...
>>>>     (pass_waccess::check_call_access): ...to this.
>>>>     (pass_waccess::check): Rename...
>>>>     (pass_waccess::check_block): ...to this.
>>>>     (pass_waccess::check_pointer_uses): New function.
>>>>     (pass_waccess::gimple_call_return_arg): New function.
>>>>     (pass_waccess::warn_invalid_pointer): New function.
>>>>     (pass_waccess::check_builtin): Handle free and realloc.
>>>>     (gimple_use_after_inval_p): New function.
>>>>     (get_realloc_lhs): New function.
>>>>     (maybe_warn_mismatched_realloc): New function.
>>>>     (pointers_related_p): New function.
>>>>     (pass_waccess::check_call): Call check_pointer_uses.
>>>>     (pass_waccess::execute): Compute and free dominance info.
>>>>
>>>> libcpp/ChangeLog:
>>>>
>>>>     * files.c (_cpp_find_file): Substitute a valid pointer for
>>>>     an invalid one to avoid -Wuse-0after-free.
>>>>
>>>> libiberty/ChangeLog:
>>>>
>>>>     * regex.c: Suppress -Wuse-after-free.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>     * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free.
>>>>     * gcc.dg/Wmismatched-dealloc-3.c: Same.
>>>>     * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free.
>>>>     * gcc.dg/attr-alloc_size-7.c: Same.
>>>>     * c-c++-common/Wuse-after-free-2.c: New test.
>>>>     * c-c++-common/Wuse-after-free-3.c: New test.
>>>>     * c-c++-common/Wuse-after-free-4.c: New test.
>>>>     * c-c++-common/Wuse-after-free-5.c: New test.
>>>>     * c-c++-common/Wuse-after-free-6.c: New test.
>>>>     * c-c++-common/Wuse-after-free-7.c: New test.
>>>>     * c-c++-common/Wuse-after-free.c: New test.
>>>>     * g++.dg/warn/Wdangling-pointer.C: New test.
>>>>     * g++.dg/warn/Wmismatched-dealloc-3.C: New test.
>>>>     * g++.dg/warn/Wuse-after-free.C: New test.
>>>>
>>>> diff --git a/gcc/gimple-ssa-warn-access.cc 
>>>> b/gcc/gimple-ssa-warn-access.cc
>>>> index 63fc27a1487..2065402a2b9 100644
>>>> --- a/gcc/gimple-ssa-warn-access.cc
>>>> +++ b/gcc/gimple-ssa-warn-access.cc
>>>>
>>>> @@ -3397,33 +3417,460 @@ pass_waccess::maybe_check_dealloc_call 
>>>> (gcall *call)
>>>>       }
>>>>   }
>>>> +/* 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
>>>> +   the same block, USE_STMT follows INVAL_STMT.  */
>>>> +
>>>> +static bool
>>>> +gimple_use_after_inval_p (gimple *inval_stmt, gimple *use_stmt,
>>>> +              bool last_block = false)
>>>> +{
>>>> +  tree clobvar =
>>>> +    gimple_clobber_p (inval_stmt) ? gimple_assign_lhs (inval_stmt) 
>>>> : NULL_TREE;
>>>> +
>>>> +  basic_block inval_bb = gimple_bb (inval_stmt);
>>>> +  basic_block use_bb = gimple_bb (use_stmt);
>>>> +
>>>> +  if (inval_bb != use_bb)
>>>> +    {
>>>> +      if (dominated_by_p (CDI_DOMINATORS, use_bb, inval_bb))
>>>> +    return true;
>>>> +
>>>> +      if (!clobvar || !last_block)
>>>> +    return false;
>>>> +
>>>> +      auto gsi = gsi_for_stmt (use_stmt);
>>>> +
>>>> +      auto_bitmap visited;
>>>> +
>>>> +      /* A use statement in the last basic block in a function or 
>>>> one that
>>>> +     falls through to it is after any other prior clobber of the used
>>>> +     variable unless it's followed by a clobber of the same 
>>>> variable. */
>>>> +      basic_block bb = use_bb;
>>>> +      while (bb != inval_bb
>>>> +         && single_succ_p (bb)
>>>> +         && !(single_succ_edge (bb)->flags & (EDGE_EH|EDGE_DFS_BACK)))
>>>> +    {
>>>> +      if (!bitmap_set_bit (visited, bb->index))
>>>> +        /* Avoid cycles. */
>>>> +        return true;
>>>> +
>>>> +      for (; !gsi_end_p (gsi); gsi_next_nondebug (&gsi))
>>>> +        {
>>>> +          gimple *stmt = gsi_stmt (gsi);
>>>> +          if (gimple_clobber_p (stmt))
>>>> +        {
>>>> +          if (clobvar == gimple_assign_lhs (stmt))
>>>> +            /* The use is followed by a clobber.  */
>>>> +            return false;
>>>> +        }
>>>> +        }
>>>> +
>>>> +      bb = single_succ (bb);
>>>> +      gsi = gsi_start_bb (bb);
>>>> +    }
>>>> +
>>>> +      return bb == EXIT_BLOCK_PTR_FOR_FN (cfun);
>>>> +    }
>>> ?!?  I would have thought the block dominance test plus checking UIDs 
>>> if the two statements are in the same block would be all you need. 
>>> Can you elaborate more on what that hunk above is trying to do?
>>
>> The loop is entered only for -Wdangling-pointer.  It looks for
>> the first clobber of the CLOBVAR variable (one whose clobber
>> statement has been seen during the CFG traversal and whose use
>> is being validated) in the successors along the single edge
>> from the use block.  If the search finds a clobber, the use
>> is valid.  If it doesn't, the use is one of a variable having
>> gone out of scope (the clobber must be before the use).
>>
>> Among the cases the loop handles is the one in PR 63272
>> (the request for -Wdangling-pointer) where the use neither
>> follows the clobber in the same block nor dominated by it.
>>
>> There may be a way to optimize it somehow but because it's
>> a search I don't think a simple UID check alone would be
>> enough.
>>
>>>> +
>>>> +  for (auto si = gsi_for_stmt (inval_stmt); !gsi_end_p (si);
>>>> +       gsi_next_nondebug (&si))
>>>> +    {
>>>> +      gimple *stmt = gsi_stmt (si);
>>>> +      if (stmt == use_stmt)
>>>> +    return true;
>>>> +    }
>>>> +
>>>> +  return false;
>>>> +}
>>> So from a compile-time standpoint, would it be better to to assign 
>>> UIDs to each statement so that within a block you can just compare 
>>> the UIDs? That's a pretty standard way to deal with the problem of 
>>> statement domination within a block if we're going to be doing 
>>> multiple queries.
>>
>> I'd considered it but because statement UIDs don't exist at
>> the start of a pass, assigning them means either traversing all
>> statements in the whole CFG first, even in functions with no
>> deallocation calls or clobbers, or doing it lazily, after
>> the first such statement has been seen.  It might ultimately
>> be worthwhile if more warnings(*) end up relying on it but at
>> this point I'm not sure the optimization wouldn't end up slowing
>> things down on average.
>>
>> For some data, in a GCC bootstrap, each statement visited by
>> this loop is visited on average twice (2.2 times), and
>> the average sequence of statements traversed by the loop is
>> 2.65, with a maximum of 22 times and 18 statements, respectively.
>> So still not sure it would be a win.
>>
>> Let me know if this is something you think I need to pursue at
>> this stage.
>>
>> [*] I think simple memory/resource leak detection might perhaps
>> be one.
>>
>>>> +
>>>> +/* Return true if P and Q point to the same object, and false if they
>>>> +   either don't or their relationship cannot be determined.  */
>>>> +
>>>> +static bool
>>>> +pointers_related_p (gimple *stmt, tree p, tree q, pointer_query &qry)
>>>> +{
>>>> +  if (!ptr_derefs_may_alias_p (p, q))
>>>> +    return false;
>>> Hmm, I guess that you don't need to worry about the case where P and 
>>> Q point to different elements within an array.  They point to 
>>> different final objects, though they do share a common enclosing 
>>> object. Similarly for P & Q pointing to different members within a 
>>> structure.
>>
>> Right.  The if statement is an optimization to avoid having to
>> determine the identity of the complete objects that P and Q
>> point to.  That's done by the calls to get_ref() below (for
>> complete objects; as you note, we don't care about subobjects
>> for this).
>>
>>>> +
>>>> +/* For a STMT either a call to a deallocation function or a 
>>>> clobber, warn
>>>> +   for uses of the pointer PTR it was called with (including its 
>>>> copies
>>>> +   or others derived from it by pointer arithmetic).  */
>>>> +
>>>> +void
>>>> +pass_waccess::check_pointer_uses (gimple *stmt, tree ptr)
>>>> +{
>>>> +  gcc_assert (TREE_CODE (ptr) == SSA_NAME);
>>>> +
>>>> +  const bool check_dangling = !is_gimple_call (stmt);
>>>> +  basic_block stmt_bb = gimple_bb (stmt);
>>>> +
>>>> +  /* If the deallocation (or clobber) statement dominates more than
>>>> +     a single basic block issue a "maybe" k
>>> That seems wrong.   What you're looking for is a post-dominance 
>>> relationship I think.   If the sink (free/delete) is post-dominated 
>>> by the use, then it's a "must", if it's not post-dominated, then it's 
>>> a maybe.  Of course, that means you need to build post-dominators.
>>
>> I'm sure you're right in general.  To avoid false positives
>> the warning is very simplistic and only considers straight
>> paths through the CFG, so I'm not sure this matters.  But
>> I'm fine with using the post-dominance test instead if you
>> thin it's worthwhile (it doesn't change any tests).
>>
>>>> +
>>>> +      if (check_dangling
>>>> +          && gimple_code (use_stmt) == GIMPLE_RETURN
>>>> +          && optimize && flag_isolate_erroneous_paths_dereference)
>>>> +        /* Avoid interfering with -Wreturn-local-addr (which runs only
>>>> +           with optimization enabled).  */
>>>> +        continue;
>>> Umm, that looks like a hack.  I can't think of a good reason why 
>>> removal of erroneous paths should gate any of this code.  ISTM that 
>>> you're likely papering over a problem elsewhere.
>>
>> This code avoids issuing -Wdangling-pointer for problems that
>> will later be diagnosed by -Wreturn-local-addr.  E.g., in this
>> case from Wreturn-local-addr-2.c:
>>
>>    ATTR (noipa) void*
>>    return_array_plus_var (int i)
>>    {
>>        int a[32];
>>        void *p = a + i;
>>      sink (p);
>>      return p;         /* { dg-warning "function returns address of 
>> local" } */
>>    }
>>
>> Without the test we'd end up with
>>
>>    warning: using dangling pointer ‘p’ to ‘a’ [-Wdangling-pointer=]
>>
>> in addition to -Wreturn-local-addr (and a whole slew of
>> failures in the -Wreturn-local-addr tests).
>>
>> -Wreturn-local-addr only runs when
>> flag_isolate_erroneous_paths_dereference is nonzero, so
>> the conditional makes sure -Wdangling-pointer is issued when
>> either the flag or -Wreturn-local-addr is disabled.  I think
>> that works as expected (i.e., there's no problem elsewhere).
>>
>> I could have the code issue -Wdangling-pointer and suppress
>> -Wreturn-local-addr but that doesn't seem right since
>> the pointer hasn't gone out of scope yet at the point it's
>> returned.
>>
>> Alternatively, I could change this instance of
>> -Wdangling-pointer to -Wreturn-local-addr but that also
>> doesn't seem like good design since we have a whole pass
>> dedicated to the latter warning.
>>
>> I can't think of any other more elegant solutions but I'm open
>> to suggestions.
>>
>> Martin
>
Martin Sebor Jan. 16, 2022, midnight UTC | #6
On 1/11/22 15:40, Jason Merrill wrote:
> On 11/30/21 17:32, Martin Sebor via Gcc-patches wrote:
>> Attached is a revised patch with the following changes based
>> on your comments:
>>
>> 1) Set and use statement uids to determine which statement
>>     precedes which in the same basic block.
>> 2) Avoid testing flag_isolate_erroneous_paths_dereference.
>> 3) Use post-dominance to decide whether to use the "maybe"
>>     phrasing vs a definite form.
>>
>> David raised (and in our offline discussion today reiterated)
>> an objection to the default setting of the option being
>> the strictest.  I have not changed that in this revision.
>> See my rationale for this choice in my reply below:
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html
> 
> In the latest C2x draft I see in the list of undefined behavior
> 
> "The value of a pointer that refers to space deallocated by a call to 
> the free or realloc function is used (7.22.3)."
> 
> So the case that would be technically undefined would be comparing the 
> reallocated pointer to the old pointer which has been deallocated.
> 
> The C++ draft is more nuanced: it says, "When the end of the duration of 
> a region of storage is reached, the values of all pointers representing 
> the address of any part of that region of storage become invalid pointer 
> values (6.8.3).  Indirection through an invalid pointer value and 
> passing an invalid pointer value to a deallocation function have 
> undefined behavior.  Any other use of an invalid pointer value has 
> implementation-defined behavior."
> 
> So the case above is implementation-defined in C++, not undefined.
> 
> Let's put =2 in -Wall for now.
> 
> With that change, this and the -Wdangling-pointer patch are OK on Friday 
> afternoon if there are no other comments before then.
I've adjusted both patches and pushed r12-6605 and r12-6606 after
retesting with a few packages.  In -Wdangling-pointer I reworded
the warning to refer to "unnamed temporary" instead of "compound
literal" since it triggers for those as well.

While rebuilding LLVM and a few its projects with the patches
I noticed the -Wdangling-pointer change exposes what seems like
a Ranger bug for one translation unit (it enters an infinite loop):
I opened pr104038 for it.

Martin

> 
>> On 11/23/21 2:16 PM, Martin Sebor wrote:
>>> On 11/22/21 6:32 PM, Jeff Law wrote:
>>>>
>>>>
>>>> On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote:
>>>>> Patch 1 in the series detects a small subset of uses of pointers
>>>>> made indeterminate by calls to deallocation functions like free
>>>>> or C++ operator delete.  To control the conditions the warnings
>>>>> are issued under the new -Wuse-after-free= option provides three
>>>>> levels.  At the lowest level the warning triggers only for
>>>>> unconditional uses of freed pointers and doesn't warn for uses
>>>>> in equality expressions.  Level 2 warns also for come conditional
>>>>> uses, and level 3 also for uses in equality expressions.
>>>>>
>>>>> I debated whether to make level 2 or 3 the default included in
>>>>> -Wall.  I decided on 3 for two reasons: 1) to raise awareness
>>>>> of both the problem and GCC's new ability to detect it: using
>>>>> a pointer after it's been freed, even only in principle, by
>>>>> a successful call to realloc, is undefined, and 2) because
>>>>> it's trivial to lower the level either globally, or locally
>>>>> by suppressing the warning around such misuses.
>>>>>
>>>>> I've tested the patch on x86_64-linux and by building Glibc
>>>>> and Binutils/GDB.  It triggers a number of times in each, all
>>>>> due to comparing invalidated pointers for equality (i.e., level
>>>>> 3).  I have suppressed these in GCC (libiberty) by a #pragma,
>>>>> and will see how the Glibc folks want to deal with theirs (I
>>>>> track them in BZ #28521).
>>>>>
>>>>> The tests contain a number of xfails due to limitations I'm
>>>>> aware of.  I marked them pr?????? until the patch is approved.
>>>>> I will open bugs for them before committing if I don't resolve
>>>>> them in a followup.
>>>>>
>>>>> Martin
>>>>>
>>>>> gcc-63272-1.diff
>>>>>
>>>>> Add -Wuse-after-free.
>>>>>
>>>>> gcc/c-family/ChangeLog
>>>>>
>>>>>     * c.opt (-Wuse-after-free): New options.
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>>     * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle
>>>>>     OPT_Wreturn_local_addr and OPT_Wuse_after_free_.
>>>>>     * diagnostic-spec.h (NW_DANGLING): New enumerator.
>>>>>     * doc/invoke.texi (-Wuse-after-free): Document new option.
>>>>>     * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename...
>>>>>     (pass_waccess::check_call_access): ...to this.
>>>>>     (pass_waccess::check): Rename...
>>>>>     (pass_waccess::check_block): ...to this.
>>>>>     (pass_waccess::check_pointer_uses): New function.
>>>>>     (pass_waccess::gimple_call_return_arg): New function.
>>>>>     (pass_waccess::warn_invalid_pointer): New function.
>>>>>     (pass_waccess::check_builtin): Handle free and realloc.
>>>>>     (gimple_use_after_inval_p): New function.
>>>>>     (get_realloc_lhs): New function.
>>>>>     (maybe_warn_mismatched_realloc): New function.
>>>>>     (pointers_related_p): New function.
>>>>>     (pass_waccess::check_call): Call check_pointer_uses.
>>>>>     (pass_waccess::execute): Compute and free dominance info.
>>>>>
>>>>> libcpp/ChangeLog:
>>>>>
>>>>>     * files.c (_cpp_find_file): Substitute a valid pointer for
>>>>>     an invalid one to avoid -Wuse-0after-free.
>>>>>
>>>>> libiberty/ChangeLog:
>>>>>
>>>>>     * regex.c: Suppress -Wuse-after-free.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>>     * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free.
>>>>>     * gcc.dg/Wmismatched-dealloc-3.c: Same.
>>>>>     * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free.
>>>>>     * gcc.dg/attr-alloc_size-7.c: Same.
>>>>>     * c-c++-common/Wuse-after-free-2.c: New test.
>>>>>     * c-c++-common/Wuse-after-free-3.c: New test.
>>>>>     * c-c++-common/Wuse-after-free-4.c: New test.
>>>>>     * c-c++-common/Wuse-after-free-5.c: New test.
>>>>>     * c-c++-common/Wuse-after-free-6.c: New test.
>>>>>     * c-c++-common/Wuse-after-free-7.c: New test.
>>>>>     * c-c++-common/Wuse-after-free.c: New test.
>>>>>     * g++.dg/warn/Wdangling-pointer.C: New test.
>>>>>     * g++.dg/warn/Wmismatched-dealloc-3.C: New test.
>>>>>     * g++.dg/warn/Wuse-after-free.C: New test.
>>>>>
>>>>> diff --git a/gcc/gimple-ssa-warn-access.cc 
>>>>> b/gcc/gimple-ssa-warn-access.cc
>>>>> index 63fc27a1487..2065402a2b9 100644
>>>>> --- a/gcc/gimple-ssa-warn-access.cc
>>>>> +++ b/gcc/gimple-ssa-warn-access.cc
>>>>>
>>>>> @@ -3397,33 +3417,460 @@ pass_waccess::maybe_check_dealloc_call 
>>>>> (gcall *call)
>>>>>       }
>>>>>   }
>>>>> +/* 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
>>>>> +   the same block, USE_STMT follows INVAL_STMT.  */
>>>>> +
>>>>> +static bool
>>>>> +gimple_use_after_inval_p (gimple *inval_stmt, gimple *use_stmt,
>>>>> +              bool last_block = false)
>>>>> +{
>>>>> +  tree clobvar =
>>>>> +    gimple_clobber_p (inval_stmt) ? gimple_assign_lhs (inval_stmt) 
>>>>> : NULL_TREE;
>>>>> +
>>>>> +  basic_block inval_bb = gimple_bb (inval_stmt);
>>>>> +  basic_block use_bb = gimple_bb (use_stmt);
>>>>> +
>>>>> +  if (inval_bb != use_bb)
>>>>> +    {
>>>>> +      if (dominated_by_p (CDI_DOMINATORS, use_bb, inval_bb))
>>>>> +    return true;
>>>>> +
>>>>> +      if (!clobvar || !last_block)
>>>>> +    return false;
>>>>> +
>>>>> +      auto gsi = gsi_for_stmt (use_stmt);
>>>>> +
>>>>> +      auto_bitmap visited;
>>>>> +
>>>>> +      /* A use statement in the last basic block in a function or 
>>>>> one that
>>>>> +     falls through to it is after any other prior clobber of the used
>>>>> +     variable unless it's followed by a clobber of the same 
>>>>> variable. */
>>>>> +      basic_block bb = use_bb;
>>>>> +      while (bb != inval_bb
>>>>> +         && single_succ_p (bb)
>>>>> +         && !(single_succ_edge (bb)->flags & 
>>>>> (EDGE_EH|EDGE_DFS_BACK)))
>>>>> +    {
>>>>> +      if (!bitmap_set_bit (visited, bb->index))
>>>>> +        /* Avoid cycles. */
>>>>> +        return true;
>>>>> +
>>>>> +      for (; !gsi_end_p (gsi); gsi_next_nondebug (&gsi))
>>>>> +        {
>>>>> +          gimple *stmt = gsi_stmt (gsi);
>>>>> +          if (gimple_clobber_p (stmt))
>>>>> +        {
>>>>> +          if (clobvar == gimple_assign_lhs (stmt))
>>>>> +            /* The use is followed by a clobber.  */
>>>>> +            return false;
>>>>> +        }
>>>>> +        }
>>>>> +
>>>>> +      bb = single_succ (bb);
>>>>> +      gsi = gsi_start_bb (bb);
>>>>> +    }
>>>>> +
>>>>> +      return bb == EXIT_BLOCK_PTR_FOR_FN (cfun);
>>>>> +    }
>>>> ?!?  I would have thought the block dominance test plus checking 
>>>> UIDs if the two statements are in the same block would be all you 
>>>> need. Can you elaborate more on what that hunk above is trying to do?
>>>
>>> The loop is entered only for -Wdangling-pointer.  It looks for
>>> the first clobber of the CLOBVAR variable (one whose clobber
>>> statement has been seen during the CFG traversal and whose use
>>> is being validated) in the successors along the single edge
>>> from the use block.  If the search finds a clobber, the use
>>> is valid.  If it doesn't, the use is one of a variable having
>>> gone out of scope (the clobber must be before the use).
>>>
>>> Among the cases the loop handles is the one in PR 63272
>>> (the request for -Wdangling-pointer) where the use neither
>>> follows the clobber in the same block nor dominated by it.
>>>
>>> There may be a way to optimize it somehow but because it's
>>> a search I don't think a simple UID check alone would be
>>> enough.
>>>
>>>>> +
>>>>> +  for (auto si = gsi_for_stmt (inval_stmt); !gsi_end_p (si);
>>>>> +       gsi_next_nondebug (&si))
>>>>> +    {
>>>>> +      gimple *stmt = gsi_stmt (si);
>>>>> +      if (stmt == use_stmt)
>>>>> +    return true;
>>>>> +    }
>>>>> +
>>>>> +  return false;
>>>>> +}
>>>> So from a compile-time standpoint, would it be better to to assign 
>>>> UIDs to each statement so that within a block you can just compare 
>>>> the UIDs? That's a pretty standard way to deal with the problem of 
>>>> statement domination within a block if we're going to be doing 
>>>> multiple queries.
>>>
>>> I'd considered it but because statement UIDs don't exist at
>>> the start of a pass, assigning them means either traversing all
>>> statements in the whole CFG first, even in functions with no
>>> deallocation calls or clobbers, or doing it lazily, after
>>> the first such statement has been seen.  It might ultimately
>>> be worthwhile if more warnings(*) end up relying on it but at
>>> this point I'm not sure the optimization wouldn't end up slowing
>>> things down on average.
>>>
>>> For some data, in a GCC bootstrap, each statement visited by
>>> this loop is visited on average twice (2.2 times), and
>>> the average sequence of statements traversed by the loop is
>>> 2.65, with a maximum of 22 times and 18 statements, respectively.
>>> So still not sure it would be a win.
>>>
>>> Let me know if this is something you think I need to pursue at
>>> this stage.
>>>
>>> [*] I think simple memory/resource leak detection might perhaps
>>> be one.
>>>
>>>>> +
>>>>> +/* Return true if P and Q point to the same object, and false if they
>>>>> +   either don't or their relationship cannot be determined.  */
>>>>> +
>>>>> +static bool
>>>>> +pointers_related_p (gimple *stmt, tree p, tree q, pointer_query &qry)
>>>>> +{
>>>>> +  if (!ptr_derefs_may_alias_p (p, q))
>>>>> +    return false;
>>>> Hmm, I guess that you don't need to worry about the case where P and 
>>>> Q point to different elements within an array.  They point to 
>>>> different final objects, though they do share a common enclosing 
>>>> object. Similarly for P & Q pointing to different members within a 
>>>> structure.
>>>
>>> Right.  The if statement is an optimization to avoid having to
>>> determine the identity of the complete objects that P and Q
>>> point to.  That's done by the calls to get_ref() below (for
>>> complete objects; as you note, we don't care about subobjects
>>> for this).
>>>
>>>>> +
>>>>> +/* For a STMT either a call to a deallocation function or a 
>>>>> clobber, warn
>>>>> +   for uses of the pointer PTR it was called with (including its 
>>>>> copies
>>>>> +   or others derived from it by pointer arithmetic).  */
>>>>> +
>>>>> +void
>>>>> +pass_waccess::check_pointer_uses (gimple *stmt, tree ptr)
>>>>> +{
>>>>> +  gcc_assert (TREE_CODE (ptr) == SSA_NAME);
>>>>> +
>>>>> +  const bool check_dangling = !is_gimple_call (stmt);
>>>>> +  basic_block stmt_bb = gimple_bb (stmt);
>>>>> +
>>>>> +  /* If the deallocation (or clobber) statement dominates more than
>>>>> +     a single basic block issue a "maybe" k
>>>> That seems wrong.   What you're looking for is a post-dominance 
>>>> relationship I think.   If the sink (free/delete) is post-dominated 
>>>> by the use, then it's a "must", if it's not post-dominated, then 
>>>> it's a maybe.  Of course, that means you need to build post-dominators.
>>>
>>> I'm sure you're right in general.  To avoid false positives
>>> the warning is very simplistic and only considers straight
>>> paths through the CFG, so I'm not sure this matters.  But
>>> I'm fine with using the post-dominance test instead if you
>>> thin it's worthwhile (it doesn't change any tests).
>>>
>>>>> +
>>>>> +      if (check_dangling
>>>>> +          && gimple_code (use_stmt) == GIMPLE_RETURN
>>>>> +          && optimize && flag_isolate_erroneous_paths_dereference)
>>>>> +        /* Avoid interfering with -Wreturn-local-addr (which runs 
>>>>> only
>>>>> +           with optimization enabled).  */
>>>>> +        continue;
>>>> Umm, that looks like a hack.  I can't think of a good reason why 
>>>> removal of erroneous paths should gate any of this code.  ISTM that 
>>>> you're likely papering over a problem elsewhere.
>>>
>>> This code avoids issuing -Wdangling-pointer for problems that
>>> will later be diagnosed by -Wreturn-local-addr.  E.g., in this
>>> case from Wreturn-local-addr-2.c:
>>>
>>>    ATTR (noipa) void*
>>>    return_array_plus_var (int i)
>>>    {
>>>        int a[32];
>>>        void *p = a + i;
>>>      sink (p);
>>>      return p;         /* { dg-warning "function returns address of 
>>> local" } */
>>>    }
>>>
>>> Without the test we'd end up with
>>>
>>>    warning: using dangling pointer ‘p’ to ‘a’ [-Wdangling-pointer=]
>>>
>>> in addition to -Wreturn-local-addr (and a whole slew of
>>> failures in the -Wreturn-local-addr tests).
>>>
>>> -Wreturn-local-addr only runs when
>>> flag_isolate_erroneous_paths_dereference is nonzero, so
>>> the conditional makes sure -Wdangling-pointer is issued when
>>> either the flag or -Wreturn-local-addr is disabled.  I think
>>> that works as expected (i.e., there's no problem elsewhere).
>>>
>>> I could have the code issue -Wdangling-pointer and suppress
>>> -Wreturn-local-addr but that doesn't seem right since
>>> the pointer hasn't gone out of scope yet at the point it's
>>> returned.
>>>
>>> Alternatively, I could change this instance of
>>> -Wdangling-pointer to -Wreturn-local-addr but that also
>>> doesn't seem like good design since we have a whole pass
>>> dedicated to the latter warning.
>>>
>>> I can't think of any other more elegant solutions but I'm open
>>> to suggestions.
>>>
>>> Martin
>>
>
Jeff Law Jan. 19, 2022, 10:53 p.m. UTC | #7
On 1/11/2022 3:40 PM, Jason Merrill wrote:
> On 11/30/21 17:32, Martin Sebor via Gcc-patches wrote:
>> Attached is a revised patch with the following changes based
>> on your comments:
>>
>> 1) Set and use statement uids to determine which statement
>>     precedes which in the same basic block.
>> 2) Avoid testing flag_isolate_erroneous_paths_dereference.
>> 3) Use post-dominance to decide whether to use the "maybe"
>>     phrasing vs a definite form.
>>
>> David raised (and in our offline discussion today reiterated)
>> an objection to the default setting of the option being
>> the strictest.  I have not changed that in this revision.
>> See my rationale for this choice in my reply below:
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html
>
> In the latest C2x draft I see in the list of undefined behavior
>
> "The value of a pointer that refers to space deallocated by a call to 
> the free or realloc function is used (7.22.3)."
>
> So the case that would be technically undefined would be comparing the 
> reallocated pointer to the old pointer which has been deallocated.
>
> The C++ draft is more nuanced: it says, "When the end of the duration 
> of a region of storage is reached, the values of all pointers 
> representing the address of any part of that region of storage become 
> invalid pointer values (6.8.3).  Indirection through an invalid 
> pointer value and passing an invalid pointer value to a deallocation 
> function have undefined behavior.  Any other use of an invalid pointer 
> value has implementation-defined behavior."
>
> So the case above is implementation-defined in C++, not undefined.
>
> Let's put =2 in -Wall for now.
>
> With that change, this and the -Wdangling-pointer patch are OK on 
> Friday afternoon if there are no other comments before then.
THanks for picking this up.  I've been busier than expected the last 
several weeks.

jeff
diff mbox series

Patch

Add -Wuse-after-free.

gcc/c-family/ChangeLog

	* c.opt (-Wuse-after-free): New options.

gcc/ChangeLog:

	* diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle
	OPT_Wreturn_local_addr and OPT_Wuse_after_free_.
	* diagnostic-spec.h (NW_DANGLING): New enumerator.
	* doc/invoke.texi (-Wuse-after-free): Document new option.
	* gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename...
	(pass_waccess::check_call_access): ...to this.
	(pass_waccess::check): Rename...
	(pass_waccess::check_block): ...to this.
	(pass_waccess::check_pointer_uses): New function.
	(pass_waccess::gimple_call_return_arg): New function.
	(pass_waccess::warn_invalid_pointer): New function.
	(pass_waccess::check_builtin): Handle free and realloc.
	(gimple_use_after_inval_p): New function.
	(get_realloc_lhs): New function.
	(maybe_warn_mismatched_realloc): New function.
	(pointers_related_p): New function.
	(pass_waccess::check_call): Call check_pointer_uses.
	(pass_waccess::execute): Compute and free dominance info.

libcpp/ChangeLog:

	* files.c (_cpp_find_file): Substitute a valid pointer for
	an invalid one to avoid -Wuse-after-free.

libiberty/ChangeLog:

	* regex.c: Suppress -Wuse-after-free.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free.
	* gcc.dg/Wmismatched-dealloc-3.c: Same.
	* gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free.
	* gcc.dg/attr-alloc_size-7.c: Same.
	* c-c++-common/Wuse-after-free-2.c: New test.
	* c-c++-common/Wuse-after-free-3.c: New test.
	* c-c++-common/Wuse-after-free-4.c: New test.
	* c-c++-common/Wuse-after-free-5.c: New test.
	* c-c++-common/Wuse-after-free-6.c: New test.
	* c-c++-common/Wuse-after-free-7.c: New test.
	* c-c++-common/Wuse-after-free.c: New test.
	* g++.dg/warn/Wmismatched-dealloc-3.C: New test.
	* g++.dg/warn/Wuse-after-free.C: New test.

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 4b8a094b206..fb1abc0de4c 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1362,6 +1362,14 @@  Wunused-const-variable=
 C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_unused_const_variable) Warning LangEnabledBy(C ObjC,Wunused-variable, 1, 0) IntegerRange(0, 2)
 Warn when a const variable is unused.
 
+Wuse-after-free
+C ObjC C++ LTO ObjC++ Alias(Wuse-after-free=, 2, 0) Warning
+Warn for uses of pointers to deallocated strorage.
+
+Wuse-after-free=
+C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_use_after_free) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall, 3, 0) IntegerRange(0, 3)
+Warn for uses of pointers to deallocated strorage.
+
 Wvariadic-macros
 C ObjC C++ ObjC++ CPP(warn_variadic_macros) CppReason(CPP_W_VARIADIC_MACROS) Var(cpp_warn_variadic_macros) Init(0) Warning LangEnabledBy(C ObjC C++ ObjC++,Wpedantic || Wtraditional)
 Warn about using variadic macros.
diff --git a/gcc/diagnostic-spec.c b/gcc/diagnostic-spec.c
index d1e563d19ba..921e7ab7423 100644
--- a/gcc/diagnostic-spec.c
+++ b/gcc/diagnostic-spec.c
@@ -99,6 +99,11 @@  nowarn_spec_t::nowarn_spec_t (opt_code opt)
 	m_bits = NW_UNINIT;
       break;
 
+    case OPT_Wreturn_local_addr:
+    case OPT_Wuse_after_free_:
+      m_bits = NW_DANGLING;
+      break;
+
     default:
       /* A catchall group for everything else.  */
       m_bits = NW_OTHER;
diff --git a/gcc/diagnostic-spec.h b/gcc/diagnostic-spec.h
index 368b75f3254..897afa1f3bd 100644
--- a/gcc/diagnostic-spec.h
+++ b/gcc/diagnostic-spec.h
@@ -41,11 +41,13 @@  public:
      NW_UNINIT = 1 << 3,
      /* Warnings about arithmetic overflow.  */
      NW_VFLOW = 1 << 4,
+     /* Warnings about dangling pointers.  */
+     NW_DANGLING = 1 << 5,
      /* All other unclassified warnings.  */
-     NW_OTHER = 1 << 5,
+     NW_OTHER = 1 << 6,
      /* All groups of warnings.  */
      NW_ALL = (NW_ACCESS | NW_LEXICAL | NW_NONNULL
-	       | NW_UNINIT | NW_VFLOW | NW_OTHER)
+	       | NW_UNINIT | NW_VFLOW | NW_DANGLING | NW_OTHER)
    };
 
   nowarn_spec_t (): m_bits () { }
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 3bddfbaae6a..46bc8046436 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4365,6 +4365,60 @@  annotations.
 Warn about overriding virtual functions that are not marked with the
 @code{override} keyword.
 
+@item -Wuse-after-free
+@itemx -Wuse-after-free=@var{n}
+@opindex Wuse-after-free
+@opindex Wno-use-after-free
+Warn about uses of pointers to dynamically allocated objects that have
+been rendered indeterminate by a call to a deallocation function.
+
+@table @gcctabopt
+@item -Wuse-after-free=1
+At level 1 the warning attempts to diagnose only unconditional uses of
+pointers made indeterminate by a deallocation call.  This includes
+double-@code{free} calls.  Although undefined, uses of indeterminate
+pointers in equality (or inequality) expressions are not diagnosed at
+this level.
+@item -Wuse-after-free=2
+At level 2, in addition to unconditional uses the warning also diagnoses
+conditional uses of pointers made indeterminate by a deallocation call.
+As at level 1, uses in (or inequality) equality expressions are not
+diagnosed.  For example, the second call to @code{free} in the following
+function is diagnosed at this level:
+@smallexample
+struct A @{ int refcount; void *data; @};
+
+void release (struct A *p)
+@{
+  int refcount = --p->refcount;
+  free (p);
+  if (refcount == 0)
+    free (p->data);   // warning: p may be used after free
+@}
+@end smallexample
+@item -Wuse-after-free=3
+At level 3, the warning also diagnoses uses of indeterminate pointers in
+equality expressions.  All uses of indeterminate pointers are undefined
+but equality tests sometimes appear after calls to @code{realloc} as
+an attempt to determine whether the call resulted in relocating the object
+to a different address.  They are diagnosed at a separate level to aid
+legacy code gradually transition to safe alternatives.  For example,
+the equality test in the function below is diagnosed at this level:
+@smallexample
+void adjust_pointers (int**, int);
+
+void grow (int **p, int n)
+@{
+  int **q = (int**)realloc (p, n *= 2);
+  if (q == p)
+    return;
+  adjust_pointers ((int**)q, n);
+@}
+@end smallexample
+@end table
+
+@option{-Wuse-after-free=3} is included in @option{-Wall}.
+
 @item -Wuseless-cast @r{(C++ and Objective-C++ only)}
 @opindex Wuseless-cast
 @opindex Wno-useless-cast
@@ -5685,6 +5739,7 @@  Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
 -Wunused-label     @gol
 -Wunused-value     @gol
 -Wunused-variable  @gol
+-Wuse-after-free=3  @gol
 -Wvla-parameter @r{(C and Objective-C only)} @gol
 -Wvolatile-register-var  @gol
 -Wzero-length-bounds}
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 48bf8aaff50..e396266088f 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -50,6 +50,7 @@ 
 #include "stringpool.h"
 #include "attribs.h"
 #include "demangle.h"
+#include "attr-fnspec.h"
 #include "pointer-query.h"
 
 /* Return true if tree node X has an associated location.  */
@@ -2068,6 +2069,7 @@  class pass_waccess : public gimple_opt_pass
   opt_pass *clone () { return new pass_waccess (m_ctxt); }
 
   virtual bool gate (function *);
+
   virtual unsigned int execute (function *);
 
 private:
@@ -2081,14 +2083,14 @@  private:
   /* Check a call to a built-in function.  */
   bool check_builtin (gcall *);
 
-  /* Check a call to an ordinary function.  */
-  bool check_call (gcall *);
+  /* Check a call to an ordinary function for invalid accesses.  */
+  bool check_call_access (gcall *);
 
   /* Check statements in a basic block.  */
-  void check (basic_block);
+  void check_block (basic_block);
 
   /* Check a call to a function.  */
-  void check (gcall *);
+  void check_call (gcall *);
 
   /* Check a call to the named built-in function.  */
   void check_alloca (gcall *);
@@ -2104,10 +2106,27 @@  private:
   void maybe_check_dealloc_call (gcall *);
   void maybe_check_access_sizes (rdwr_map *, tree, tree, gimple *);
 
+  /* Check for uses of indeterminate pointers.  */
+  void check_pointer_uses (gimple *, tree);
+
+  /* Return the argument that a call returns.  */
+  tree gimple_call_return_arg (gcall *);
+
+  void warn_invalid_pointer (tree, gimple *, gimple *, bool, bool = false);
+
+  /* Return true if use follows an invalidating statement.  */
+  bool use_after_inval_p (gimple *, gimple *);
+
   /* A pointer_query object and its cache to store information about
      pointers and their targets in.  */
   pointer_query m_ptr_qry;
   pointer_query::cache_type m_var_cache;
+
+  /* A bit is set for each basic block whose statements have been assigned
+     valid UIDs.  */
+  bitmap m_bb_uids_set;
+  /* The current function.  */
+  function *m_func;
 };
 
 /* Construct the pass.  */
@@ -2115,7 +2134,9 @@  private:
 pass_waccess::pass_waccess (gcc::context *ctxt)
   : gimple_opt_pass (pass_data_waccess, ctxt),
     m_ptr_qry (NULL, &m_var_cache),
-    m_var_cache ()
+    m_var_cache (),
+    m_bb_uids_set (),
+    m_func ()
 {
 }
 
@@ -2795,6 +2816,15 @@  pass_waccess::check_builtin (gcall *stmt)
       check_read_access (stmt, call_arg (stmt, 0));
       return true;
 
+    case BUILT_IN_FREE:
+    case BUILT_IN_REALLOC:
+      {
+	tree arg = call_arg (stmt, 0);
+	if (TREE_CODE (arg) == SSA_NAME)
+	  check_pointer_uses (stmt, arg);
+      }
+      return true;
+
     case BUILT_IN_GETTEXT:
     case BUILT_IN_PUTS:
     case BUILT_IN_PUTS_UNLOCKED:
@@ -2899,6 +2929,7 @@  pass_waccess::check_builtin (gcall *stmt)
 	return true;
       break;
     }
+
   return false;
 }
 
@@ -3224,7 +3255,7 @@  pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, tree fndecl, tree fntype,
    accesses.  Return true if a call has been handled.  */
 
 bool
-pass_waccess::check_call (gcall *stmt)
+pass_waccess::check_call_access (gcall *stmt)
 {
   tree fntype = gimple_call_fntype (stmt);
   if (!fntype)
@@ -3412,46 +3443,442 @@  pass_waccess::maybe_check_dealloc_call (gcall *call)
     }
 }
 
+/* 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,
+   or if they're in the same block, USE_STMT follows INVAL_STMT.  */
+
+bool
+pass_waccess::use_after_inval_p (gimple *inval_stmt, gimple *use_stmt)
+{
+  basic_block inval_bb = gimple_bb (inval_stmt);
+  basic_block use_bb = gimple_bb (use_stmt);
+
+  if (inval_bb != use_bb)
+    return dominated_by_p (CDI_DOMINATORS, use_bb, inval_bb);
+
+  if (bitmap_set_bit (m_bb_uids_set, inval_bb->index))
+    /* The first time this basic block is visited assign increasing ids
+       to consecutive statements in it.  Use the ids to determine which
+       precedes which.  This avoids the linear traversal on subsequent
+       visits to the same block.  */
+    for (auto si = gsi_start_bb (inval_bb); !gsi_end_p (si);
+	 gsi_next_nondebug (&si))
+      {
+	gimple *stmt = gsi_stmt (si);
+	unsigned uid = inc_gimple_stmt_max_uid (m_func);
+	gimple_set_uid (stmt, uid);
+      }
+
+  return gimple_uid (inval_stmt) < gimple_uid (use_stmt);
+}
+
+/* Issue a warning for the USE_STMT of pointer PTR rendered invalid
+   by INVAL_STMT.  PTR may be null when it's been optimized away.
+   MAYBE is true to issue the "maybe" kind of warning.  EQUALITY is
+   true when the pointer is used in an equality expression.  */
+
+void
+pass_waccess::warn_invalid_pointer (tree ptr, gimple *use_stmt,
+				    gimple *inval_stmt,
+				    bool maybe,
+				    bool equality /* = false */)
+{
+  /* Avoid printing the unhelpful "<unknown>" in the diagnostics.  */
+  if (ptr && TREE_CODE (ptr) == SSA_NAME
+      && (!SSA_NAME_VAR (ptr) || DECL_ARTIFICIAL (SSA_NAME_VAR (ptr))))
+    ptr = NULL_TREE;
+
+  location_t use_loc = gimple_location (use_stmt);
+  if (use_loc == UNKNOWN_LOCATION)
+    {
+      use_loc = cfun->function_end_locus;
+      if (!ptr)
+	/* Avoid issuing a warning with no context other than
+	   the function.  That would make it difficult to debug
+	   in any but very simple cases.  */
+	return;
+    }
+
+  if (is_gimple_call (inval_stmt))
+    {
+      if ((equality && warn_use_after_free < 3)
+	  || (maybe && warn_use_after_free < 2)
+	  || warning_suppressed_p (use_stmt, OPT_Wuse_after_free_))
+	return;
+
+      const tree inval_decl = gimple_call_fndecl (inval_stmt);
+
+      if ((ptr && warning_at (use_loc, OPT_Wuse_after_free_,
+			      (maybe
+			       ? G_("pointer %qE may be used after %qD")
+			       : G_("pointer %qE used after %qD")),
+			      ptr, inval_decl))
+	  || (!ptr && warning_at (use_loc, OPT_Wuse_after_free_,
+			      (maybe
+			       ? G_("pointer may be used after %qD")
+			       : G_("pointer used after %qD")),
+				  inval_decl)))
+	{
+	  location_t loc = gimple_location (inval_stmt);
+	  inform (loc, "call to %qD here", inval_decl);
+	  suppress_warning (use_stmt, OPT_Wuse_after_free_);
+	}
+      return;
+    }
+}
+
+/* If STMT is a call to either the standard realloc or to a user-defined
+   reallocation function returns its LHS and set *PTR to the reallocated
+   pointer.  Otherwise return null.  */
+
+static tree
+get_realloc_lhs (gimple *stmt, tree *ptr)
+{
+  if (gimple_call_builtin_p (stmt, BUILT_IN_REALLOC))
+    {
+      *ptr = gimple_call_arg (stmt, 0);
+      return gimple_call_lhs (stmt);
+    }
+
+  gcall *call = dyn_cast<gcall *>(stmt);
+  if (!call)
+    return NULL_TREE;
+
+  tree fnattr = NULL_TREE;
+  tree fndecl = gimple_call_fndecl (call);
+  if (fndecl)
+    fnattr = DECL_ATTRIBUTES (fndecl);
+  else
+    {
+      tree fntype = gimple_call_fntype (stmt);
+      if (!fntype)
+	return NULL_TREE;
+      fnattr = TYPE_ATTRIBUTES (fntype);
+    }
+
+  if (!fnattr)
+    return NULL_TREE;
+
+  for (tree ats = fnattr;  (ats = lookup_attribute ("*dealloc", ats));
+       ats = TREE_CHAIN (ats))
+    {
+      tree args = TREE_VALUE (ats);
+      if (!args)
+	continue;
+
+      tree alloc = TREE_VALUE (args);
+      if (!alloc)
+	continue;
+
+      if (alloc == DECL_NAME (fndecl))
+	{
+	  unsigned argno = 0;
+	  if (tree index = TREE_CHAIN (args))
+	    argno = TREE_INT_CST_LOW (TREE_VALUE (index)) - 1;
+	  *ptr = gimple_call_arg (stmt, argno);
+	  return gimple_call_lhs (stmt);
+	}
+    }
+
+  return NULL_TREE;
+}
+
+/* Warn if STMT is a call to a deallocation function that's not a match
+   for the REALLOC_STMT call.  Return true if warned.  */
+
+static bool
+maybe_warn_mismatched_realloc (tree ptr, gimple *realloc_stmt, gimple *stmt)
+{
+  if (!is_gimple_call (stmt))
+    return false;
+
+  tree fndecl = gimple_call_fndecl (stmt);
+  if (!fndecl)
+    return false;
+
+  unsigned argno = fndecl_dealloc_argno (fndecl);
+  if (call_nargs (stmt) <= argno)
+    return false;
+
+  if (matching_alloc_calls_p (realloc_stmt, fndecl))
+    return false;
+
+  /* Avoid printing the unhelpful "<unknown>" in the diagnostics.  */
+  if (ptr && TREE_CODE (ptr) == SSA_NAME
+      && (!SSA_NAME_VAR (ptr) || DECL_ARTIFICIAL (SSA_NAME_VAR (ptr))))
+    ptr = NULL_TREE;
+
+  location_t loc = gimple_location (stmt);
+  tree realloc_decl = gimple_call_fndecl (realloc_stmt);
+  tree dealloc_decl = gimple_call_fndecl (stmt);
+  if (ptr && !warning_at (loc, OPT_Wmismatched_dealloc,
+			  "%qD called on pointer %qE passed to mismatched "
+			  "allocation function %qD",
+			  dealloc_decl, ptr, realloc_decl))
+    return false;
+  if (!ptr && !warning_at (loc, OPT_Wmismatched_dealloc,
+			   "%qD called on a pointer passed to mismatched "
+			   "reallocation function %qD",
+			   dealloc_decl, realloc_decl))
+    return false;
+
+  inform (gimple_location (realloc_stmt),
+	  "call to %qD", realloc_decl);
+  return true;
+}
+
+/* Return true if P and Q point to the same object, and false if they
+   either don't or their relationship cannot be determined.  */
+
+static bool
+pointers_related_p (gimple *stmt, tree p, tree q, pointer_query &qry)
+{
+  if (!ptr_derefs_may_alias_p (p, q))
+    return false;
+
+  /* TODO: Work harder to rule out relatedness.  */
+  access_ref pref, qref;
+  if (!qry.get_ref (p, stmt, &pref, 0)
+      || !qry.get_ref (q, stmt, &qref, 0))
+    return true;
+
+  return pref.ref == qref.ref;
+}
+
+/* For a STMT either a call to a deallocation function or a clobber, warn
+   for uses of the pointer PTR it was called with (including its copies
+   or others derived from it by pointer arithmetic).  */
+
+void
+pass_waccess::check_pointer_uses (gimple *stmt, tree ptr)
+{
+  gcc_assert (TREE_CODE (ptr) == SSA_NAME);
+
+  const bool check_dangling = !is_gimple_call (stmt);
+  basic_block stmt_bb = gimple_bb (stmt);
+
+  /* If STMT is a reallocation function set to the reallocated pointer
+     and the LHS of the call, respectively.  */
+  tree realloc_ptr = NULL_TREE;
+  tree realloc_lhs = get_realloc_lhs (stmt, &realloc_ptr);
+
+  auto_bitmap visited;
+
+  auto_vec<tree> pointers;
+  pointers.safe_push (ptr);
+
+  /* Starting with PTR, iterate over POINTERS added by the loop, and
+     either warn for their uses in basic blocks dominated by the STMT
+     or in statements that follow it in the same basic block, or add
+     them to POINTERS if they point into the same object as PTR (i.e.,
+     are obtained by pointer arithmetic on PTR).  */
+  for (unsigned i = 0; i != pointers.length (); ++i)
+    {
+      tree ptr = pointers[i];
+      if (TREE_CODE (ptr) == SSA_NAME
+	  && !bitmap_set_bit (visited, SSA_NAME_VERSION (ptr)))
+	/* Avoid revisiting the same pointer.  */
+	continue;
+
+      use_operand_p use_p;
+      imm_use_iterator iter;
+      FOR_EACH_IMM_USE_FAST (use_p, iter, ptr)
+	{
+	  gimple *use_stmt = USE_STMT (use_p);
+	  if (use_stmt == stmt || is_gimple_debug (use_stmt))
+	    continue;
+
+	  if (realloc_lhs)
+	    {
+	      /* Check to see if USE_STMT is a mismatched deallocation
+		 call for the pointer passed to realloc.  That's a bug
+		 regardless of the pointer's value and so warn.  */
+	      if (maybe_warn_mismatched_realloc (*use_p->use, stmt, use_stmt))
+		continue;
+
+	      /* Pointers passed to realloc that are used in basic blocks
+		 where the realloc call is known to have failed are valid.
+		 Ignore pointers that nothing is known about.  Those could
+		 have escaped along with their nullness.  */
+	      value_range vr;
+	      if (m_ptr_qry.rvals->range_of_expr (vr, realloc_lhs, use_stmt))
+		{
+		  if (vr.zero_p ())
+		    continue;
+
+		  if (!pointers_related_p (stmt, ptr, realloc_ptr, m_ptr_qry))
+		    continue;
+		}
+	    }
+
+	  if (check_dangling
+	      && gimple_code (use_stmt) == GIMPLE_RETURN)
+	    /* Avoid interfering with -Wreturn-local-addr (which runs only
+	       with optimization enabled so it won't diagnose cases that
+	       would be caught here when optimization is disabled).  */
+	    continue;
+
+	  bool equality = false;
+	  if (is_gimple_assign (use_stmt))
+	    {
+	      tree_code code = gimple_assign_rhs_code (use_stmt);
+	      equality = code == EQ_EXPR || code == NE_EXPR;
+	    }
+	  else if (gcond *cond = dyn_cast<gcond *>(use_stmt))
+	    {
+	      tree_code code = gimple_cond_code (cond);
+	      equality = code == EQ_EXPR || code == NE_EXPR;
+	    }
+
+	  /* Warn if USE_STMT is dominated by the deallocation STMT.
+	     Otherwise, add the pointer to POINTERS so that the uses
+	     of any other pointers derived from it can be checked.  */
+	  if (use_after_inval_p (stmt, use_stmt))
+	    {
+	      /* TODO: Handle PHIs but careful of false positives.  */
+	      if (gimple_code (use_stmt) != GIMPLE_PHI)
+		{
+		  basic_block use_bb = gimple_bb (use_stmt);
+		  bool this_maybe
+		    = !dominated_by_p (CDI_POST_DOMINATORS, use_bb, stmt_bb);
+		  warn_invalid_pointer (*use_p->use, use_stmt, stmt,
+					this_maybe, equality);
+		  continue;
+		}
+	    }
+
+	  if (is_gimple_assign (use_stmt))
+	    {
+	      tree lhs = gimple_assign_lhs (use_stmt);
+	      if (TREE_CODE (lhs) == SSA_NAME)
+		{
+		  tree_code rhs_code = gimple_assign_rhs_code (use_stmt);
+		  if (rhs_code == POINTER_PLUS_EXPR || rhs_code == SSA_NAME)
+		    pointers.safe_push (lhs);
+		}
+	      continue;
+	    }
+
+	  if (gcall *call = dyn_cast <gcall *>(use_stmt))
+	    {
+	      if (gimple_call_return_arg (call))
+		if (tree lhs = gimple_call_lhs (call))
+		  if (TREE_CODE (lhs) == SSA_NAME)
+		    pointers.safe_push (lhs);
+	      continue;
+	    }
+	}
+    }
+}
+
 /* Check call STMT for invalid accesses.  */
 
 void
-pass_waccess::check (gcall *stmt)
+pass_waccess::check_call (gcall *stmt)
 {
   if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
     check_builtin (stmt);
 
-  if (is_gimple_call (stmt))
-    check_call (stmt);
+  if (tree callee = gimple_call_fndecl (stmt))
+    {
+      /* Check for uses of the pointer passed to either a standard
+	 or a user-defined deallocation function.  */
+      unsigned argno = fndecl_dealloc_argno (callee);
+      if (argno < (unsigned) call_nargs (stmt))
+	{
+	  tree arg = call_arg (stmt, argno);
+	  if (TREE_CODE (arg) == SSA_NAME)
+	    check_pointer_uses (stmt, arg);
+	}
+    }
 
-  maybe_check_dealloc_call (stmt);
+  check_call_access (stmt);
 
+  maybe_check_dealloc_call (stmt);
   check_nonstring_args (stmt);
 }
 
+
 /* Check basic block BB for invalid accesses.  */
 
 void
-pass_waccess::check (basic_block bb)
+pass_waccess::check_block (basic_block bb)
 {
   /* Iterate over statements, looking for function calls.  */
-  for (auto si = gsi_start_bb (bb); !gsi_end_p (si); gsi_next (&si))
+  for (auto si = gsi_start_bb (bb); !gsi_end_p (si);
+       gsi_next_nondebug (&si))
     {
-      if (gcall *call = dyn_cast <gcall *> (gsi_stmt (si)))
-	check (call);
+      gimple *stmt = gsi_stmt (si);
+      if (gcall *call = dyn_cast <gcall *> (stmt))
+	check_call (call);
     }
 }
 
+/* Return the argument that the call STMT to a built-in function returns
+   (including with an offset) or null if it doesn't.  */
+
+tree
+pass_waccess::gimple_call_return_arg (gcall *call)
+{
+  /* Check for attribute fn spec to see if the function returns one
+     of its arguments.  */
+  attr_fnspec fnspec = gimple_call_fnspec (call);
+  unsigned int argno;
+  if (!fnspec.returns_arg (&argno))
+    {
+      if (gimple_call_num_args (call) < 1)
+	return NULL_TREE;
+
+      if (!gimple_call_builtin_p (call, BUILT_IN_NORMAL))
+	return NULL_TREE;
+
+      tree fndecl = gimple_call_fndecl (call);
+      switch (DECL_FUNCTION_CODE (fndecl))
+	{
+	case BUILT_IN_MEMPCPY:
+	case BUILT_IN_MEMPCPY_CHK:
+	case BUILT_IN_MEMCHR:
+	case BUILT_IN_STRCHR:
+	case BUILT_IN_STRRCHR:
+	case BUILT_IN_STRSTR:
+	case BUILT_IN_STPCPY:
+	case BUILT_IN_STPCPY_CHK:
+	case BUILT_IN_STPNCPY:
+	case BUILT_IN_STPNCPY_CHK:
+	  argno = 0;
+	  break;
+
+	default:
+	  return NULL_TREE;
+	}
+    }
+
+  if (gimple_call_num_args (call) <= argno)
+    return NULL_TREE;
+
+  return gimple_call_arg (call, argno);
+}
+
 /* Check function FUN for invalid accesses.  */
 
 unsigned
 pass_waccess::execute (function *fun)
 {
+  calculate_dominance_info (CDI_DOMINATORS);
+  calculate_dominance_info (CDI_POST_DOMINATORS);
+
   /* Create a new ranger instance and associate it with FUN.  */
   m_ptr_qry.rvals = enable_ranger (fun);
+  m_func = fun;
+
+  auto_bitmap bb_uids_set (&bitmap_default_obstack);
+  m_bb_uids_set = bb_uids_set;
+
+  set_gimple_stmt_max_uid (m_func, 0);
 
   basic_block bb;
   FOR_EACH_BB_FN (bb, fun)
-    check (bb);
+    check_block (bb);
 
   if (dump_file)
     m_ptr_qry.dump (dump_file, (dump_flags & TDF_DETAILS) != 0);
@@ -3463,6 +3890,10 @@  pass_waccess::execute (function *fun)
   disable_ranger (fun);
   m_ptr_qry.rvals = NULL;
 
+  m_bb_uids_set = NULL;
+
+  free_dominance_info (CDI_POST_DOMINATORS);
+  free_dominance_info (CDI_DOMINATORS);
   return 0;
 }
 
diff --git a/gcc/testsuite/c-c++-common/Wuse-after-free-2.c b/gcc/testsuite/c-c++-common/Wuse-after-free-2.c
new file mode 100644
index 00000000000..598ecf595ea
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wuse-after-free-2.c
@@ -0,0 +1,137 @@ 
+/* Verify that accessing freed objects by built-in functions is diagnosed.
+   { dg-do compile }
+   { dg-options "-Wall" }  */
+
+typedef __SIZE_TYPE__ size_t;
+
+#if __cplusplus
+#  define EXTERN_C extern "C"
+#else
+#  define EXTERN_C extern
+#endif
+
+EXTERN_C void free (void*);
+EXTERN_C void* realloc (void*, size_t);
+
+EXTERN_C void* memcpy (void*, const void*, size_t);
+EXTERN_C char* strcpy (char*, const char*);
+EXTERN_C size_t strlen (const char*);
+
+
+void sink (void*, ...);
+
+struct Member { char *p; char a[4]; };
+
+int nowarn_strcpy_memptr (struct Member *p)
+{
+  char *q = strcpy (p->p, p->a);
+  free (p);
+  return *q;
+}
+
+int nowarn_strlen_memptr (struct Member *p)
+{
+  const char *q = p->p;
+
+  free (p);
+
+  return strlen (q);
+}
+
+int warn_strlen_memptr (struct Member *p)
+{
+  free (p);                   // { dg-message "call to '\(void \)?free\(\\(void\\*\\)\)?'" "note" }
+  return strlen (p->p);       // { dg-warning "-Wuse-after-free" }
+}
+
+int warn_strlen_memarray (struct Member *p)
+{
+  {
+    free (p);
+    return strlen (p->a);     // { dg-warning "-Wuse-after-free" }
+  }
+
+  {
+    char *q = p->a;
+
+    free (p);
+    return strlen (q);        // { dg-warning "-Wuse-after-free" "pr??????" { xfail *-*-* } }
+  }
+}
+
+void* nowarn_realloc_success (void *p)
+{
+  void *q = realloc (p, 7);
+  if (!q)
+    /* When realloc fails the original pointer remains valid.  */
+    return p;
+
+  return q;
+}
+
+void* warn_realloc_unchecked (void *p, int *moved)
+{
+  void *q = realloc (p, 7);   // { dg-message "call to '\(void\\* \)?realloc\(\\(void\\*, size_t\\)\)?'" "note" }
+  *moved = p != q;            // { dg-warning "-Wuse-after-free" }
+  return q;
+}
+
+void* nowarn_realloc_unchecked_copy (void *p1, void *p2, const void *s,
+				     int n, int *x)
+{
+  void *p3 = memcpy (p1, s, n);
+  void *p4 = realloc (p2, 7);
+  *x = p3 != p4;
+  return p4;
+}
+
+void* warn_realloc_unchecked_copy (void *p, const void *s, int n, int *moved)
+{
+  void *p2 = memcpy (p, s, n);
+  void *q = realloc (p, 7);   // { dg-message "call to '\(void\\* \)?realloc\(\\(void\\*, size_t\\)\)?'" "note" }
+  *moved = p2 != q;           // { dg-warning "-Wuse-after-free" }
+  return q;
+}
+
+void* warn_realloc_failed (void *p, int *moved)
+{
+  void *q = realloc (p, 7);   // { dg-message "call to '\(void\\* \)?realloc\(\\(void\\*, size_t\\)\)?'" "note" }
+  if (q)
+    {
+      /* When realloc succeeds the original pointer is invalid.  */
+      *moved = p != q;        // { dg-warning "-Wuse-after-free" }
+      return q;
+    }
+
+  return p;
+}
+
+extern void *evp;
+
+void* warn_realloc_extern (void *p, int *moved)
+{
+  evp = realloc (p, 7);
+  if (evp)
+    {
+      /* When realloc succeeds the original pointer is invalid.  */
+      *moved = p != evp;      // { dg-warning "-Wuse-after-free" "escaped" }
+      return evp;
+    }
+
+  return p;                   // { dg-bogus "-Wuse-after-free" "safe use after realloc failure" { xfail *-*-* } }
+}
+
+struct A { void *p, *q; int moved; };
+
+void* warn_realloc_arg (struct A *p)
+{
+  p->q = realloc (p->p, 7);
+  if (p->q)
+    {
+      /* When realloc succeeds the original pointer is invalid.  */
+      p->moved = p->p != p->q;  // { dg-warning "-Wuse-after-free" "escaped" { xfail *-*-* } }
+      return p->q;
+    }
+
+  return p->p;
+}
diff --git a/gcc/testsuite/c-c++-common/Wuse-after-free-3.c b/gcc/testsuite/c-c++-common/Wuse-after-free-3.c
new file mode 100644
index 00000000000..0a2db1a16c8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wuse-after-free-3.c
@@ -0,0 +1,83 @@ 
+/* Exercise -Wuse-after-free with user-defined deallocators.
+   { dg-do compile }
+   { dg-options "-O0 -Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+#if __cplusplus
+#  define EXTERN_C extern "C"
+#else
+#  define EXTERN_C extern
+#endif
+
+#define A(...) __attribute__ ((malloc (__VA_ARGS__)))
+
+EXTERN_C void free (void *);
+EXTERN_C void* realloc (void *, size_t);
+
+typedef struct List { struct List *next; } List;
+
+// User-defined allocator/deallocator just like like realloc and free.
+extern                     void  list_free (List *);
+extern                     List* list_realloc (size_t, List *);
+extern A (list_realloc, 2) List* list_realloc (size_t, List *);
+extern A (list_free, 1)    List* list_realloc (size_t, List *);
+
+
+void sink (void *);
+
+extern int ei;
+extern List *elp, *elpa[];
+
+void nowarn_list_free (struct List *lp)
+{
+  {
+    list_free (lp);
+    lp = 0;
+    sink (lp);
+  }
+  {
+    list_free (elp);
+    elp = 0;
+    sink (elp);
+  }
+  {
+    list_free (elpa[0]);
+    elpa[0] = 0;
+    sink (elpa[0]);
+  }
+  {
+    void *vp = elpa[0];
+    list_free (elpa[0]);
+    sink (vp);
+  }
+  {
+    List *p = elpa[1];
+    if (ei & 1)
+      list_free (p);
+    if (ei & 2)
+      sink (p);
+  }
+  {
+    struct List *next = lp->next;
+    list_free (lp);
+    list_free (next);
+  }
+}
+
+
+void nowarn_list_free_list (List *head)
+{
+  for (List *p = head, *q; p; p = q)
+    {
+      q = p->next;
+      list_free (p);
+    }
+}
+
+void warn_list_free_list (List *head)
+{
+  List *p = head;
+  for (; p; p = p->next)      // { dg-warning "\\\[-Wuse-after-free" }
+    list_free (p);            // { dg-message "call to '\(void \)?list_free\(\\(List\\*\\)\)?'" "note" }
+}
diff --git a/gcc/testsuite/c-c++-common/Wuse-after-free-4.c b/gcc/testsuite/c-c++-common/Wuse-after-free-4.c
new file mode 100644
index 00000000000..686ba7e256c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wuse-after-free-4.c
@@ -0,0 +1,102 @@ 
+/* Verify -Wuse-after-free=1 triggers only for unconditional uses and
+   not for equality expressions.
+   { dg-do compile }
+   { dg-options "-O0 -Wall -Wuse-after-free=1" } */
+
+#if __cplusplus
+#  define EXTERN_C extern "C"
+#else
+#  define EXTERN_C extern
+#endif
+
+EXTERN_C void free (void*);
+
+void sink (void*);
+
+
+void warn_double_free (void *p)
+{
+  free (p);
+  free (p);         // { dg-warning "pointer 'p' used" }
+}
+
+void nowarn_cond_double_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    free (p);
+}
+
+void warn_call_after_free (void *p)
+{
+  free (p);
+  sink (p);         // { dg-warning "pointer 'p' used" }
+}
+
+void nowarn_cond_call_after_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    sink (p);
+}
+
+void* warn_return_after_free (void *p)
+{
+  free (p);
+  return p;         // { dg-warning "pointer 'p' used" }
+}
+
+void* nowarn_cond_return_after_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    return p;
+  return 0;
+}
+
+void warn_relational_after_free (char *p, char *q[])
+{
+  free (p);
+
+  int a[] =
+    {
+     p < q[0],      // { dg-warning "pointer 'p' used" }
+     p <= q[1],     // { dg-warning "pointer 'p' used" }
+     p > q[2],      // { dg-warning "pointer 'p' used" }
+     p >= q[3],     // { dg-warning "pointer 'p' used" }
+     p == q[4],
+     p != q[5]
+    };
+
+  sink (a);
+}
+
+void nowarn_cond_relational_after_free (char *p, char *q[], int c)
+{
+  free (p);
+
+  int a[] =
+    {
+     c ? p < q[0] : q[0][0],
+     c ? p <= q[1] : q[1][1],
+     c ? p > q[2] : q[2][2],
+     c ? p >= q[3] : q[3][3],
+     c ? p == q[4] : q[4][4],
+     c ? p != q[5] : q[5][5],
+    };
+
+  sink (a);
+}
+
+
+// Verify no warning for the example in the manual.
+
+struct A { int refcount; void *data; };
+
+void release (struct A *p)
+{
+  int refcount = --p->refcount;
+  free (p);
+  if (refcount == 0)
+    free (p->data);   // no warning at level 1
+}
diff --git a/gcc/testsuite/c-c++-common/Wuse-after-free-5.c b/gcc/testsuite/c-c++-common/Wuse-after-free-5.c
new file mode 100644
index 00000000000..c6ff1f3fad2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wuse-after-free-5.c
@@ -0,0 +1,103 @@ 
+/* Verify -Wuse-after-free=2 triggers for conditional as well as
+   unconditional uses but not for equality expressions.
+   { dg-do compile }
+   { dg-options "-O0 -Wall -Wuse-after-free=2" } */
+
+
+#if __cplusplus
+#  define EXTERN_C extern "C"
+#else
+#  define EXTERN_C extern
+#endif
+
+EXTERN_C void free (void*);
+
+void sink (void*);
+
+
+void warn_double_free (void *p)
+{
+  free (p);
+  free (p);         // { dg-warning "pointer 'p' used" }
+}
+
+void warn_cond_double_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    free (p);       // { dg-warning "pointer 'p' may be used" }
+}
+
+void warn_call_after_free (void *p)
+{
+  free (p);
+  sink (p);         // { dg-warning "pointer 'p' used" }
+}
+
+void warn_cond_call_after_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    sink (p);       // { dg-warning "pointer 'p' may be used" }
+}
+
+void* warn_return_after_free (void *p)
+{
+  free (p);
+  return p;         // { dg-warning "pointer 'p' used" }
+}
+
+void* warn_cond_return_after_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    return p;       // { dg-warning "pointer 'p' may be used" }
+  return 0;
+}
+
+void warn_relational_after_free (char *p, char *q[])
+{
+  free (p);
+
+  int a[] =
+    {
+     p < q[0],      // { dg-warning "pointer 'p' used" }
+     p <= q[1],     // { dg-warning "pointer 'p' used" }
+     p > q[2],      // { dg-warning "pointer 'p' used" }
+     p >= q[3],     // { dg-warning "pointer 'p' used" }
+     p == q[4],
+     p != q[5]
+    };
+
+  sink (a);
+}
+
+void warn_cond_relational_after_free (char *p, char *q[], int c)
+{
+  free (p);
+
+  int a[] =
+    {
+     c ? p < q[0] : q[0][0],  // { dg-warning "pointer 'p' may be used" }
+     c ? p <= q[1] : q[1][1], // { dg-warning "pointer 'p' may be used" }
+     c ? p > q[2] : q[2][2],  // { dg-warning "pointer 'p' may be used" }
+     c ? p >= q[3] : q[3][3], // { dg-warning "pointer 'p' may be used" }
+     c ? p == q[4] : q[4][4],
+     c ? p != q[5] : q[5][5],
+    };
+
+  sink (a);
+}
+
+
+// Verify warning for the example in the manual.
+
+struct A { int refcount; void *data; };
+
+void release (struct A *p)
+{
+  int refcount = --p->refcount;
+  free (p);
+  if (refcount == 0)
+    free (p->data); // { dg-warning "pointer 'p' may be used" }
+}
diff --git a/gcc/testsuite/c-c++-common/Wuse-after-free-6.c b/gcc/testsuite/c-c++-common/Wuse-after-free-6.c
new file mode 100644
index 00000000000..581b1a0a024
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wuse-after-free-6.c
@@ -0,0 +1,105 @@ 
+/* Verify -Wuse-after-free=2 triggers for conditional as well as
+   unconditional uses but not for equality expressions.  Same as
+   -Wuse-after-free-5.c but with optimization.
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wuse-after-free=2" } */
+
+
+#if __cplusplus
+#  define EXTERN_C extern "C"
+#else
+#  define EXTERN_C extern
+#endif
+
+EXTERN_C void free (void*);
+
+void sink (void*);
+
+
+void warn_double_free (void *p)
+{
+  free (p);
+  free (p);         // { dg-warning "pointer 'p' used" }
+}
+
+void warn_cond_double_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    free (p);       // { dg-warning "pointer 'p' may be used" }
+}
+
+void warn_call_after_free (void *p)
+{
+  free (p);
+  sink (p);         // { dg-warning "pointer 'p' used" }
+}
+
+void warn_cond_call_after_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    sink (p);       // { dg-warning "pointer 'p' may be used" }
+}
+
+void* warn_return_after_free (void *p)
+{
+  free (p);
+  return p;         // { dg-warning "pointer 'p' used" }
+}
+
+void* warn_cond_return_after_free (void *p, int c)
+{
+  free (p);
+  // PHI handling not fully implemented.
+  if (c)
+    return p;       // { dg-warning "pointer 'p' may be used" "pr??????" { xfail *-*-* } }
+  return 0;
+}
+
+void warn_relational_after_free (char *p, char *q[])
+{
+  free (p);
+
+  int a[] =
+    {
+     p < q[0],      // { dg-warning "pointer 'p' used" }
+     p <= q[1],     // { dg-warning "pointer 'p' used" }
+     p > q[2],      // { dg-warning "pointer 'p' used" }
+     p >= q[3],     // { dg-warning "pointer 'p' used" }
+     p == q[4],
+     p != q[5]
+    };
+
+  sink (a);
+}
+
+void warn_cond_relational_after_free (char *p, char *q[], int c)
+{
+  free (p);
+
+  int a[] =
+    {
+     c ? p < q[0] : q[0][0],  // { dg-warning "pointer 'p' may be used" }
+     c ? p <= q[1] : q[1][1], // { dg-warning "pointer 'p' may be used" }
+     c ? p > q[2] : q[2][2],  // { dg-warning "pointer 'p' may be used" }
+     c ? p >= q[3] : q[3][3], // { dg-warning "pointer 'p' may be used" }
+     c ? p == q[4] : q[4][4],
+     c ? p != q[5] : q[5][5],
+    };
+
+  sink (a);
+}
+
+
+// Verify warning for the example in the manual.
+
+struct A { int refcount; void *data; };
+
+void release (struct A *p)
+{
+  int refcount = --p->refcount;
+  free (p);
+  if (refcount == 0)
+    free (p->data); // { dg-warning "pointer 'p' may be used" }
+}
diff --git a/gcc/testsuite/c-c++-common/Wuse-after-free-7.c b/gcc/testsuite/c-c++-common/Wuse-after-free-7.c
new file mode 100644
index 00000000000..12bb6f24ea5
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wuse-after-free-7.c
@@ -0,0 +1,103 @@ 
+/* Verify -Wuse-after-free=3 triggers for conditional and unconditional
+   uses in all expressions including equality.
+   { dg-do compile }
+   { dg-options "-O0 -Wall -Wuse-after-free=3" } */
+
+
+#if __cplusplus
+#  define EXTERN_C extern "C"
+#else
+#  define EXTERN_C extern
+#endif
+
+EXTERN_C void free (void*);
+
+void sink (void*);
+
+
+void warn_double_free (void *p)
+{
+  free (p);
+  free (p);         // { dg-warning "pointer 'p' used" }
+}
+
+void warn_cond_double_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    free (p);       // { dg-warning "pointer 'p' may be used" }
+}
+
+void warn_call_after_free (void *p)
+{
+  free (p);
+  sink (p);         // { dg-warning "pointer 'p' used" }
+}
+
+void warn_cond_call_after_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    sink (p);       // { dg-warning "pointer 'p' may be used" }
+}
+
+void* warn_return_after_free (void *p)
+{
+  free (p);
+  return p;         // { dg-warning "pointer 'p' used" }
+}
+
+void* warn_cond_return_after_free (void *p, int c)
+{
+  free (p);
+  if (c)
+    return p;       // { dg-warning "pointer 'p' may be used" }
+  return 0;
+}
+
+void warn_relational_after_free (char *p, char *q[])
+{
+  free (p);
+
+  int a[] =
+    {
+     p < q[0],      // { dg-warning "pointer 'p' used" }
+     p <= q[1],     // { dg-warning "pointer 'p' used" }
+     p > q[2],      // { dg-warning "pointer 'p' used" }
+     p >= q[3],     // { dg-warning "pointer 'p' used" }
+     p == q[4],     // { dg-warning "pointer 'p' used" }
+     p != q[5]      // { dg-warning "pointer 'p' used" }
+    };
+
+  sink (a);
+}
+
+void warn_cond_relational_after_free (char *p, char *q[], int c)
+{
+  free (p);
+
+  int a[] =
+    {
+     c ? p < q[0] : q[0][0],  // { dg-warning "pointer 'p' may be used" }
+     c ? p <= q[1] : q[1][1], // { dg-warning "pointer 'p' may be used" }
+     c ? p > q[2] : q[2][2],  // { dg-warning "pointer 'p' may be used" }
+     c ? p >= q[3] : q[3][3], // { dg-warning "pointer 'p' may be used" }
+     c ? p == q[4] : q[4][4], // { dg-warning "pointer 'p' may be used" }
+     c ? p != q[5] : q[5][5], // { dg-warning "pointer 'p' may be used" }
+    };
+
+  sink (a);
+}
+
+
+// Verify warning for the example in the manual.
+
+struct A { int refcount; void *data; };
+
+void release (struct A *p)
+{
+  int refcount = --p->refcount;
+  free (p);
+  if (refcount == 0)
+    free (p->data); // { dg-warning "pointer 'p' may be used" }
+}
diff --git a/gcc/testsuite/c-c++-common/Wuse-after-free.c b/gcc/testsuite/c-c++-common/Wuse-after-free.c
new file mode 100644
index 00000000000..81bb7ff3841
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wuse-after-free.c
@@ -0,0 +1,164 @@ 
+/* Exercise basic cases of -Wuse-after-free without optimization.
+   { dg-do compile }
+   { dg-options "-O0 -Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+#if __cplusplus
+#  define EXTERN_C extern "C"
+#else
+#  define EXTERN_C extern
+#endif
+
+EXTERN_C void* alloca (size_t);
+
+EXTERN_C void* calloc (size_t, size_t);
+EXTERN_C void* malloc (size_t);
+
+EXTERN_C void free (void*);
+
+
+void sink (void *);
+
+extern void* evp;
+extern void* evpa[];
+
+extern int ei;
+
+struct List { struct List *next; };
+
+void nowarn_free (void *vp, struct List *lp)
+{
+  {
+    free (vp);
+    vp = 0;
+    sink (vp);
+  }
+  {
+    free (evp);
+    evp = 0;
+    sink (evp);
+  }
+  {
+    free (evpa[0]);
+    evpa[0] = 0;
+    sink (evpa[0]);
+  }
+  {
+    void *vp = evpa[0];
+    free (evpa[1]);
+    sink (vp);
+  }
+  {
+    void *p = evpa[1];
+    if (ei & 1)
+      free (p);
+    if (ei & 2)
+      sink (p);
+  }
+  {
+    struct List *next = lp->next;
+    free (lp);
+    free (next);
+  }
+}
+
+void nowarn_free_arg (void *p, void *q)
+{
+  free (p);
+  if (q)
+    free (q);
+}
+
+void nowarn_free_extern (void)
+{
+  extern void *ep, *eq;
+  free (ep);
+  ep = eq;
+  free (ep);
+}
+
+void nowarn_free_assign (void)
+{
+  extern void *ep;
+  free (ep);
+  ep = 0;
+  free (ep);
+}
+
+#pragma GCC diagnostic push
+/* Verify that -Wuse-after-free works with #pragma diagnostic.  */
+#pragma GCC diagnostic ignored "-Wuse-after-free"
+
+void nowarn_double_free_suppressed (void *p)
+{
+  free (p);
+  free (p);
+}
+
+#pragma GCC diagnostic pop
+
+void warn_double_free_arg (void *p)
+{
+  free (p);                   // { dg-message "call to '\(void \)?free\(\\(void\\*\\)\)?'" "note" }
+  // Verify exactly one warning is issued.
+  free (p);                   // { dg-warning "\\\-Wuse-after-free" }
+                              // { dg-bogus "\\\-Wuse-after-free" "duplicate warning" { target *-*-* } .-1 }
+
+}
+
+void warn_double_free_extern (void)
+{
+  /* GCC assumes free() clobbers global memory and the warning is
+     too simplistic to see through that assumption.  */
+  extern void *ep, *eq;
+  {
+    eq = ep;
+    free (ep);                // { dg-message "call to 'free'" "pr??????" { xfail *-*-* } }
+    free (eq);                // { dg-warning "\\\-Wuse-after-free" "pr??????" { xfail *-*-* } }
+  }
+}
+
+void warn_deref_after_free (int *p, int i)
+{
+  int *q0 = p, *q1 = p + 1, *qi = p + i;
+  free (p);                   // { dg-message "call to '\(void \)?free\(\\(void\\*\\)\)?'" "note" }
+  *p = 0;                     // { dg-warning "\\\-Wuse-after-free" }
+
+  *q0 = 0;                    // { dg-warning "\\\-Wuse-after-free" }
+  *q1 = 0;                    // { dg-warning "\\\-Wuse-after-free" }
+  *qi = 0;                    // { dg-warning "\\\-Wuse-after-free" }
+}
+
+void warn_array_ref_after_free (int *p, int i)
+{
+  free (p);                   // { dg-message "call to '\(void \)?free\(\\(void\\*\\)\)?'" "note" }
+  p[i] = 0;                   // { dg-warning "\\\-Wuse-after-free" }
+}
+
+void nowarn_free_list (struct List *head)
+{
+  for (struct List *p = head, *q; p; p = q)
+    {
+      q = p->next;
+      free (p);
+    }
+}
+
+void warn_free_list (struct List *head)
+{
+  struct List *p = head;
+  for (; p; p = p->next)      // { dg-warning "\\\[-Wuse-after-free" }
+    free (p);                 // { dg-message "call to '\(void \)?free\(\\(void\\*\\)\)?'" "note" }
+}
+
+
+void warn_free (void *vp)
+{
+  {
+    free (vp);                // { dg-message "call to '\(void \)?free\(\\(void\\*\\)\)?'" "note" }
+    evp = vp;                 // { dg-warning "-Wuse-after-free" }
+    evpa[0] = vp;             // { dg-warning "-Wuse-after-free" }
+    evpa[1] = evp;
+  }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc-3.C b/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc-3.C
new file mode 100644
index 00000000000..05c7feef5c0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc-3.C
@@ -0,0 +1,70 @@ 
+/* Verify that passing a pointer to a deallocation function that was
+   previously passed to a mismatched reallocation function is diagnosed
+   by -Wmismatched-dealloc (and not by some other warning).
+   { dg-do compile }
+   { dg-options "-Wall" } */
+
+#define A(...) __attribute__ ((malloc (__VA_ARGS__)))
+
+typedef __SIZE_TYPE__ size_t;
+
+extern "C"
+{
+  void free (void *);
+  void* realloc (void *, size_t);
+}
+
+// User-defined allocator/deallocator just like like realloc.
+                   int* int_realloc (size_t, int *);
+A (int_realloc, 2) int* int_realloc (size_t, int *);
+
+
+void sink (void *);
+
+
+void* warn_realloc_op_delete (void *p)
+{
+  void *q = realloc (p, 5);   // { dg-message "call to 'void\\* realloc\\(void\\*, size_t\\)'" "note" }
+
+  operator delete (p);        // { dg-warning "'void operator delete\\(void\\*\\)' called on pointer 'p' passed to mismatched allocation function 'void\\* realloc\\(void\\*, size_t\\)' \\\[-Wmismatched-dealloc" }
+  return q;
+}
+
+void* warn_realloc_op_delete_cond (void *p)
+{
+  void *q = realloc (p, 5);      // { dg-message "call to 'void\\* realloc\\(void\\*, size_t\\)'" "note" }
+
+  if (!q)
+    operator delete (p);         // { dg-warning "'void operator delete\\(void\\*\\)' called on pointer 'p' passed to mismatched allocation function 'void\\* realloc\\(void\\*, size_t\\)'" }
+  return q;
+}
+
+void* warn_realloc_array_delete_char (char *p)
+{
+  char *q;
+  q = (char*)realloc (p, 7);  // { dg-message "call to 'void\\* realloc\\(void\\*, size_t\\)'" "note" }
+
+  if (!q)
+    delete[] (p);             // { dg-warning "'void operator delete \\\[]\\(void\\*\\)' called on pointer 'p' passed to mismatched allocation function 'void\\* realloc\\(void\\*, size_t\\)'" }
+  return q;
+}
+
+
+int* warn_int_realloc_op_delete (int *p)
+{
+  int *q;
+  q = int_realloc (5, p);     // { dg-message "call to 'int\\* int_realloc\\(size_t, int\\*\\)'" "note" }
+
+  operator delete (p);        // { dg-warning "'void operator delete\\(void\\*\\)' called on pointer 'p' passed to mismatched allocation function 'int\\* int_realloc\\(size_t, int\\*\\)' \\\[-Wmismatched-dealloc" }
+  return q;
+}
+
+
+int* warn_int_realloc_free (int *p)
+{
+  int *q;
+  q = int_realloc (5, p);    // { dg-message "call to 'int\\* int_realloc\\(size_t, int\\*\\)'" "note" }
+
+  free (p);                   // { dg-warning "'void free\\(void\\*\\)' called on pointer 'p' passed to mismatched allocation function 'int\\* int_realloc\\(size_t, int\\*\\)' \\\[-Wmismatched-dealloc" }
+  return q;
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wuse-after-free.C b/gcc/testsuite/g++.dg/warn/Wuse-after-free.C
new file mode 100644
index 00000000000..022bd8d39f9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wuse-after-free.C
@@ -0,0 +1,158 @@ 
+/* Exercise basic C++ only cases of -Wuse-after-free without optimization.
+   { dg-do compile }
+   { dg-options "-O0 -Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+extern "C" void free (void *);
+extern "C" void* realloc (void *, size_t);
+
+void sink (void *);
+
+extern void* evp;
+extern void* evpa[];
+
+extern int ei;
+
+struct List { struct List *next; };
+
+void nowarn_delete (void *vp, struct List *lp)
+{
+  {
+    operator delete (vp);
+    vp = 0;
+    sink (vp);
+  }
+  {
+    operator delete (evp);
+    evp = 0;
+    sink (evp);
+  }
+  {
+    operator delete (evpa[0]);
+    evpa[0] = 0;
+    sink (evpa[0]);
+  }
+  {
+    void *vp = evpa[0];
+    operator delete (evpa[0]);
+    sink (vp);
+  }
+  {
+    void *p = evpa[1];
+    if (ei & 1)
+      operator delete (p);
+    if (ei & 2)
+      sink (p);
+  }
+  {
+    struct List *next = lp->next;
+    operator delete (lp);
+    operator delete (next);
+  }
+}
+
+void nowarn_delete_arg (void *p, void *q)
+{
+  operator delete (p);
+  if (q)
+    operator delete (q);
+}
+
+void nowarn_delete_extern (void)
+{
+  extern void *ep, *eq;
+  operator delete (ep);
+  ep = eq;
+  operator delete (ep);
+}
+
+void nowarn_delete_assign (void)
+{
+  extern void *ep;
+  operator delete (ep);
+  ep = 0;
+  operator delete (ep);
+}
+
+void warn_double_delete_arg (void *p)
+{
+  operator delete (p);        // { dg-message "call to 'void operator delete\\(void\\*\\)'" "note" }
+  operator delete (p);        // { dg-warning "\\\-Wuse-after-free" }
+}
+
+void warn_delete_free_arg (void *p)
+{
+  operator delete (p);        // { dg-message "call to 'void operator delete\\(void\\*\\)'" "note" }
+  free (p);                   // { dg-warning "\\\-Wuse-after-free" }
+}
+
+void warn_free_delete_arg (void *p)
+{
+  free (p);                   // { dg-message "call to 'void free\\(void\\*\\)'" "note" }
+  operator delete (p);        // { dg-warning "\\\-Wuse-after-free" }
+}
+
+void warn_mismatched_double_delete_arg (void *p, void *q)
+{
+  operator delete (p);        // { dg-message "call to 'void operator delete\\(void\\*\\)'" "note" }
+  operator delete[] (p);      // { dg-warning "\\\-Wuse-after-free" }
+
+  operator delete[] (q);      // { dg-message "call to 'void operator delete \\\[]\\(void\\*\\)'" "note" }
+  operator delete (q);        // { dg-warning "\\\-Wuse-after-free" }
+}
+
+void warn_double_delete_extern (void)
+{
+  /* GCC assumes operator delete() clobbers global memory and the warning is
+     too simplistic to see through that assumption.  */
+  extern void *ep, *eq;
+  {
+    eq = ep;
+    operator delete (ep);     // { dg-message "call to 'operator delete'" "pr??????" { xfail *-*-* } }
+    operator delete (eq);     // { dg-warning "\\\-Wuse-after-free" "pr??????" { xfail *-*-* } }
+  }
+}
+
+void warn_deref_after_delete (int *p, int i)
+{
+  int *q0 = p, *q1 = p + 1, *qi = p + i;
+  operator delete (p);        // { dg-message "call to 'void operator delete\\(void\\*\\)'" "note" }
+  *p = 0;                     // { dg-warning "\\\-Wuse-after-free" }
+
+  *q0 = 0;                    // { dg-warning "\\\-Wuse-after-free" }
+  *q1 = 0;                    // { dg-warning "\\\-Wuse-after-free" }
+  *qi = 0;                    // { dg-warning "\\\-Wuse-after-free" }
+}
+
+void warn_array_ref_after_delete (int *p, int i)
+{
+  operator delete (p);        // { dg-message "call to 'void operator delete\\(void\\*\\)'" "note" }
+  p[i] = 0;                   // { dg-warning "\\\-Wuse-after-free" }
+}
+
+void nowarn_delete_list (struct List *head)
+{
+  for (struct List *p = head, *q; p; p = q)
+    {
+      q = p->next;
+      operator delete (p);
+    }
+}
+
+void warn_delete_list (struct List *head)
+{
+  struct List *p = head;
+  for (; p; p = p->next)      // { dg-warning "\\\[-Wuse-after-free" }
+    operator delete (p);      // { dg-message "call to 'void operator delete\\(void\\*\\)'" "note" }
+}
+
+void warn_delete (void *vp)
+{
+  {
+    operator delete (vp);     // { dg-message "call to 'void operator delete\\(void\\*\\)'" "note" }
+    evp = vp;                 // { dg-warning "-Wuse-after-free" }
+    evpa[0] = vp;             // { dg-warning "-Wuse-after-free" }
+    evpa[1] = evp;
+  }
+}
diff --git a/gcc/testsuite/gcc.dg/Wmismatched-dealloc-2.c b/gcc/testsuite/gcc.dg/Wmismatched-dealloc-2.c
index 21a5ea7c5da..c303d2f1775 100644
--- a/gcc/testsuite/gcc.dg/Wmismatched-dealloc-2.c
+++ b/gcc/testsuite/gcc.dg/Wmismatched-dealloc-2.c
@@ -26,6 +26,7 @@  void dealloc (void*);
 A (dealloc) void* alloc (int);
 
 void sink (void*);
+void* source (void);
 
 void test_alloc_A (void)
 {
@@ -107,35 +108,35 @@  void test_realloc_A (void *ptr)
 }
 
 
-void test_realloc (void *ptr)
+void test_realloc (void)
 {
   extern void free (void*);
   extern void* realloc (void*, size_t);
 
   {
-    void *p = realloc (ptr, 1);
+    void *p = realloc (source (), 1);
     p = realloc_A (p, 2);
     __builtin_free (p);
   }
 
   {
-    void *p = realloc (ptr, 2);
+    void *p = realloc (source (), 2);
     p = realloc_A (p, 2);
     free (p);
   }
 
   {
-    void *p = realloc (ptr, 3);
+    void *p = realloc (source (), 3);
     free (p);
   }
 
   {
-    void *p = realloc (ptr, 4);
+    void *p = realloc (source (), 4);
     __builtin_free (p);
   }
 
   {
-    void *p = realloc (ptr, 5);         // { dg-message "returned from 'realloc'" }
+    void *p = realloc (source (), 5);   // { dg-message "returned from 'realloc'" }
     dealloc (p);                        // { dg-warning "'dealloc' called on pointer returned from a mismatched allocation function" }
   }
 }
diff --git a/gcc/testsuite/gcc.dg/Wmismatched-dealloc-3.c b/gcc/testsuite/gcc.dg/Wmismatched-dealloc-3.c
index 5afcea39b5e..302900662ce 100644
--- a/gcc/testsuite/gcc.dg/Wmismatched-dealloc-3.c
+++ b/gcc/testsuite/gcc.dg/Wmismatched-dealloc-3.c
@@ -157,6 +157,7 @@  void test_reallocarray (void *p)
   }
 
   {
+    p = source ();
     void *q = realloc (p, 1);
     q = reallocarray (q, 2, 3);
     sink (q);
@@ -192,6 +193,7 @@  void test_reallocarray (void *p)
   }
 
   {
+    p = source ();
     void *q = reallocarray (p, 7, 8);
     q = __builtin_realloc (q, 9);
     sink (q);
@@ -199,6 +201,7 @@  void test_reallocarray (void *p)
   }
 
   {
+    p = source ();
     void *q = reallocarray (p, 7, 8);
     q = realloc (q, 9);
     sink (q);
@@ -206,6 +209,7 @@  void test_reallocarray (void *p)
   }
 
   {
+    p = source ();
     void *q = reallocarray (p, 8, 9);
     q = reallocarray (q, 3, 4);
     sink (q);
@@ -213,6 +217,7 @@  void test_reallocarray (void *p)
   }
 
   {
+    p = source ();
     void *q = reallocarray (p, 9, 10);
     q = reallocarray (q, 3, 4);
     sink (q);
diff --git a/gcc/testsuite/gcc.dg/attr-alloc_size-6.c b/gcc/testsuite/gcc.dg/attr-alloc_size-6.c
index bf010c53607..e28057f9007 100644
--- a/gcc/testsuite/gcc.dg/attr-alloc_size-6.c
+++ b/gcc/testsuite/gcc.dg/attr-alloc_size-6.c
@@ -5,7 +5,7 @@ 
    -Walloc-larger-than=maximum.  */
 /* { dg-do compile } */
 /* { dg-require-effective-target alloca } */
-/* { dg-options "-O0 -Wall -Walloc-size-larger-than=12345" } */
+/* { dg-options "-O0 -Wall -Walloc-size-larger-than=12345 -Wno-use-after-free" } */
 
 #define MAXOBJSZ  12345
 
diff --git a/gcc/testsuite/gcc.dg/attr-alloc_size-7.c b/gcc/testsuite/gcc.dg/attr-alloc_size-7.c
index 3adde5c2270..6c26935211a 100644
--- a/gcc/testsuite/gcc.dg/attr-alloc_size-7.c
+++ b/gcc/testsuite/gcc.dg/attr-alloc_size-7.c
@@ -4,7 +4,7 @@ 
    of the maximum specified by -Walloc-size-larger-than=maximum.  */
 /* { dg-do compile } */
 /* { dg-require-effective-target alloca } */
-/* { dg-options "-O1 -Wall -Walloc-size-larger-than=12345" } */
+/* { dg-options "-O1 -Wall -Walloc-size-larger-than=12345 -Wno-use-after-free" } */
 
 #define SIZE_MAX   __SIZE_MAX__
 #define MAXOBJSZ   12345
diff --git a/libcpp/files.c b/libcpp/files.c
index c93a03c69ef..64989219ce0 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -553,12 +553,11 @@  _cpp_find_file (cpp_reader *pfile, const char *fname, cpp_dir *start_dir,
 		  {
 		    /* If *hash_slot is NULL, the above
 		       htab_find_slot_with_hash call just created the
-		       slot, but we aren't going to store there
-		       anything, so need to remove the newly created
-		       entry.  htab_clear_slot requires that it is
-		       non-NULL, so store there some non-NULL pointer,
-		       htab_clear_slot will overwrite it
-		       immediately.  */
+		       slot, but we aren't going to store there anything
+		       of use, so need to remove the newly created entry.
+		       htab_clear_slot requires that it is non-NULL, so
+		       store some non-NULL but valid pointer there,
+		       htab_clear_slot will immediately overwrite it.  */
 		    *hash_slot = file;
 		    htab_clear_slot (pfile->file_hash, hash_slot);
 		  }
@@ -582,7 +581,7 @@  _cpp_find_file (cpp_reader *pfile, const char *fname, cpp_dir *start_dir,
 		if (*hash_slot == NULL)
 		  {
 		    /* See comment on the above htab_clear_slot call.  */
-		    *hash_slot = file;
+		    *hash_slot = &hash_slot;
 		    htab_clear_slot (pfile->file_hash, hash_slot);
 		  }
 		return NULL;
diff --git a/libiberty/regex.c b/libiberty/regex.c
index 5531d877f0b..b6cb2320b56 100644
--- a/libiberty/regex.c
+++ b/libiberty/regex.c
@@ -30,6 +30,10 @@ 
   #pragma alloca
 #endif
 
+#if __GNUC__ >= 12
+#  pragma GCC diagnostic ignored "-Wuse-after-free"
+#endif
+
 #undef	_GNU_SOURCE
 #define _GNU_SOURCE