Bug 80532 - warning on pointer access after free
Summary: warning on pointer access after free
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 7.0
: P3 normal
Target Milestone: 12.0
Assignee: Martin Sebor
URL:
Keywords: diagnostic, missed-optimization, patch
: 85055 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-04-26 18:12 UTC by Martin Sebor
Modified: 2023-12-30 02:25 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-08-29 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2017-04-26 18:12:23 UTC
Using the value of a pointer that points to an object whose lifetime has ended is undefined, and a common source of bugs.  In the following test case, function free_list shows an example of one such bug (courtesy of Kernighan & Ritchie) where a freed pointer is dereferenced.

Function foobar shows another example.  That example is interesting because it shows that GCC is smart enough to eliminate the first call to memset (because the lifetime of the object the pointer points ends with the subsequent call to free) but it doesn't eliminate the second call to free or the second call to memset, even though that operate on an object whose lifetime has ended and will almost certainly corrupt the heap.  GCC also doesn't warn on those calls.  It would be a useful feature if GCC did both: warn and eliminate the undefined calls/accesses.


$ cat c.c && gcc -O2 -S -Wall -Wextra -Wpedantic -fdump-tree-optimized=/dev/stdout c.c
#include <stdlib.h>
#include <string.h>

struct node { struct node *next; int value; };

void free_list (struct node *head)
{
  struct node *p;

  for (p = head; p != 0; p = p->next)
    free (p);
}

void foobar (void *p, unsigned n)
{
  memset (p, 0, n);
  free (p);
  free (p);
  memset (p, 0, n);
}

;; Function free_list (free_list, funcdef_no=14, decl_uid=2614, cgraph_uid=14, symbol_order=14)

Removing basic block 5
Removing basic block 6
Removing basic block 7
Removing basic block 8
free_list (struct node * head)
{
  struct node * p;

  <bb 2> [15.00%]:
  if (head_3(D) != 0B)
    goto <bb 3>; [85.00%]
  else
    goto <bb 4>; [15.00%]

  <bb 3> [85.00%]:
  # p_9 = PHI <p_6(3), head_3(D)(2)>
  free (p_9);
  p_6 = p_9->next;
  if (p_6 != 0B)
    goto <bb 3>; [85.00%]
  else
    goto <bb 4>; [15.00%]

  <bb 4> [15.00%]:
  return;

}



;; Function foobar (foobar, funcdef_no=15, decl_uid=2622, cgraph_uid=15, symbol_order=15)

foobar (void * p, unsigned int n)
{
  long unsigned int _1;

  <bb 2> [100.00%]:
  _1 = (long unsigned int) n_2(D);
  free (p_4(D));
  free (p_4(D));
  memset (p_4(D), 0, _1); [tail call]
  return;

}


tmp$
Comment 1 Eric Gallager 2017-08-29 15:22:35 UTC
Confirmed; I'm surprised there wasn't already a bug for detecting use-after-free at compile-time... (or maybe I just didn't find it)
Comment 2 Eric Gallager 2018-03-24 02:03:46 UTC
*** Bug 85055 has been marked as a duplicate of this bug. ***
Comment 3 David Malcolm 2019-12-07 01:37:43 UTC
My analyzer finds these:

./xgcc -B. -fanalyzer -c ../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c -ftime-report
../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c: In function ‘free_list’:
../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c:14:28: warning: use after ‘free’ of ‘p’ [CWE-416] [-Wanalyzer-use-after-free]
   14 |   for (p = head; p != 0; p = p->next) /* { dg-warning "use after 'free' of 'p'" } */
      |                          ~~^~~~~~~~~
  ‘free_list’: events 1-4
    |
    |   14 |   for (p = head; p != 0; p = p->next) /* { dg-warning "use after 'free' of 'p'" } */
    |      |   ^~~                    ~~~~~~~~~~~
    |      |   |                        |
    |      |   |                        (4) use after ‘free’ of ‘p’; freed at (3)
    |      |   (1) following ‘true’ branch (when ‘p’ is non-NULL)...
    |   15 |     free (p); /* { dg-message "freed here" } */
    |      |     ~~~~~~~~
    |      |     |
    |      |     (2) ...to here
    |      |     (3) freed here
    |
../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c:14:28: note: 8 duplicates
   14 |   for (p = head; p != 0; p = p->next) /* { dg-warning "use after 'free' of 'p'" } */
      |                          ~~^~~~~~~~~
../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c: In function ‘foobar’:
../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c:24:3: warning: double-‘free’ of ‘p’ [CWE-415] [-Wanalyzer-double-free]
   24 |   free (p); /* { dg-warning "double-'free' of 'p'" } */
      |   ^~~~~~~~
  ‘foobar’: events 1-2
    |
    |   22 |   memset (p, 0, n);
    |      |   ^~~~~~~~~~~~~~~~
    |      |   |
    |      |   (1) first ‘free’ here
    |   23 |   free (p); /* { dg-message "first 'free' here" } */
    |   24 |   free (p); /* { dg-warning "double-'free' of 'p'" } */
    |      |   ~~~~~~~~
    |      |   |
    |      |   (2) second ‘free’ here; first ‘free’ was at (1)
    |
Comment 4 Eric Gallager 2020-11-03 04:11:16 UTC
(In reply to David Malcolm from comment #3)
> My analyzer finds these:
> 
> ./xgcc -B. -fanalyzer -c ../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c
> -ftime-report
> ../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c: In function ‘free_list’:
> ../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c:14:28: warning: use after
> ‘free’ of ‘p’ [CWE-416] [-Wanalyzer-use-after-free]
>    14 |   for (p = head; p != 0; p = p->next) /* { dg-warning "use after
> 'free' of 'p'" } */
>       |                          ~~^~~~~~~~~
>   ‘free_list’: events 1-4
>     |
>     |   14 |   for (p = head; p != 0; p = p->next) /* { dg-warning "use
> after 'free' of 'p'" } */
>     |      |   ^~~                    ~~~~~~~~~~~
>     |      |   |                        |
>     |      |   |                        (4) use after ‘free’ of ‘p’; freed
> at (3)
>     |      |   (1) following ‘true’ branch (when ‘p’ is non-NULL)...
>     |   15 |     free (p); /* { dg-message "freed here" } */
>     |      |     ~~~~~~~~
>     |      |     |
>     |      |     (2) ...to here
>     |      |     (3) freed here
>     |
> ../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c:14:28: note: 8 duplicates
>    14 |   for (p = head; p != 0; p = p->next) /* { dg-warning "use after
> 'free' of 'p'" } */
>       |                          ~~^~~~~~~~~
> ../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c: In function ‘foobar’:
> ../../src/gcc/testsuite/gcc.dg/analyzer/pr80532.c:24:3: warning:
> double-‘free’ of ‘p’ [CWE-415] [-Wanalyzer-double-free]
>    24 |   free (p); /* { dg-warning "double-'free' of 'p'" } */
>       |   ^~~~~~~~
>   ‘foobar’: events 1-2
>     |
>     |   22 |   memset (p, 0, n);
>     |      |   ^~~~~~~~~~~~~~~~
>     |      |   |
>     |      |   (1) first ‘free’ here
>     |   23 |   free (p); /* { dg-message "first 'free' here" } */
>     |   24 |   free (p); /* { dg-warning "double-'free' of 'p'" } */
>     |      |   ~~~~~~~~
>     |      |   |
>     |      |   (2) second ‘free’ here; first ‘free’ was at (1)
>     |

So... since the analyzer has been merged now... ok to close as FIXED?
Comment 5 Martin Sebor 2020-12-01 17:35:08 UTC
My hope is to implement the warning in the middle end (I actually have a prototype  but it's not ready for GCC 11).
Comment 6 Eric Gallager 2021-05-04 16:44:32 UTC
(In reply to Martin Sebor from comment #5)
> My hope is to implement the warning in the middle end (I actually have a
> prototype  but it's not ready for GCC 11).

So... do you want to take over the "assignee" role from David, then?
Comment 7 Martin Sebor 2021-05-04 19:40:59 UTC
Since David's already implemented this in the analyzer and I have work in progress to add this to the middle end core let me assign this to myself.
Comment 8 Martin Sebor 2021-12-15 17:41:48 UTC
Patch submitted for GCC 12:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583044.html
Comment 9 GCC Commits 2022-01-15 23:46:27 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:671a283636de75f7ed638ee6b01ed2d44361b8b6

commit r12-6605-g671a283636de75f7ed638ee6b01ed2d44361b8b6
Author: Martin Sebor <msebor@redhat.com>
Date:   Sat Jan 15 16:37:54 2022 -0700

    Add -Wuse-after-free [PR80532].
    
    gcc/c-family/ChangeLog
    
            PR tree-optimization/80532
            * c.opt (-Wuse-after-free): New options.
    
    gcc/ChangeLog:
    
            PR tree-optimization/80532
            * common.opt (-Wuse-after-free): New options.
            * 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:
    
            PR tree-optimization/80532
            * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free.
            * gcc.dg/Wmismatched-dealloc-3.c: Same.
            * gcc.dg/analyzer/file-1.c: Prune expected warning.
            * gcc.dg/analyzer/file-2.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.
Comment 10 Martin Sebor 2022-01-16 00:04:49 UTC
Implemented in GCC 12 as -Wuse-after-free.
Comment 11 Eric Gallager 2023-10-21 10:46:28 UTC
(In reply to CVS Commits from comment #9)
> The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:
> 
> https://gcc.gnu.org/g:671a283636de75f7ed638ee6b01ed2d44361b8b6
> 
> commit r12-6605-g671a283636de75f7ed638ee6b01ed2d44361b8b6
> Author: Martin Sebor <msebor@redhat.com>
> Date:   Sat Jan 15 16:37:54 2022 -0700
> 
[...snip...]
>     libiberty/ChangeLog:
>     
>             * regex.c: Suppress -Wuse-after-free.

Was this part necessary? I'm wondering if it might be covering up an actual error that I'm seeing on CheriBSD on cfarm240...