[Bug analyzer/94355] New: support for C++ new expression

[Bug analyzer/94355] New: support for C++ new expression

[vanyacpp at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

            Bug ID: 94355
           Summary: support for C++ new expression
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: vanyacpp at gmail dot com
  Target Milestone: ---

At the moment static analyzer warns about leaked malloc. It would be great if
C++ new expression were also supported.

Example:

void f()
{
    char* p = new char;
}

Expected diagnostic:

warning: leak of 'p' [CWE-401] [-Wanalyzer-malloc-leak]

    3 |     char* p = new char;

[Bug analyzer/94355] support for C++ new expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This is already being actively discussed, and will probably be a GSoC project.

[Bug analyzer/94355] support for C++ new expression

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:1690a839cff2e0276017a013419d81d675bbf69d

commit r11-3090-g1690a839cff2e0276017a013419d81d675bbf69d
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Aug 28 13:43:56 2020 -0400

    analyzer: generalize sm-malloc to new/delete [PR94355]

    This patch generalizes the state machine in sm-malloc.c to support
    multiple allocator APIs, and adds just enough support for C++ new and
    delete to demonstrate the feature, allowing for detection of code
    paths where the result of new in C++ can leak - for some crude examples,
    at least (bearing in mind that the analyzer doesn't yet know about
    e.g. vfuncs, exceptions, inheritance, RTTI, etc)

    It also implements a new warning: -Wanalyzer-mismatching-deallocation.
    For example:

    demo.cc: In function 'void test()':
    demo.cc:8:8: warning: 'f' should have been deallocated with 'delete'
      but was deallocated with 'free' [CWE-762]
[-Wanalyzer-mismatching-deallocation]
        8 |   free (f);
          |   ~~~~~^~~
      'void test()': events 1-2
        |
        |    7 |   foo *f = new foo;
        |      |                ^~~
        |      |                |
        |      |                (1) allocated here (expects deallocation with
'delete')
        |    8 |   free (f);
        |      |   ~~~~~~~~
        |      |        |
        |      |        (2) deallocated with 'free' here; allocation at (1)
expects deallocation with 'delete'
        |

    The patch also adds just enough knowledge of exception-handling to
    suppress a false positive from -Wanalyzer-malloc-leak on
    g++.dg/analyzer/pr96723.C on the exception-handling CFG edge after
    operator new.  It does this by adding a constraint that the result is
    NULL if an exception was thrown from operator new, since the result from
    operator new is lost when following that exception-handling CFG edge.

    gcc/analyzer/ChangeLog:
            PR analyzer/94355
            * analyzer.opt (Wanalyzer-mismatching-deallocation): New warning.
            * region-model-impl-calls.cc
            (region_model::impl_call_operator_new): New.
            (region_model::impl_call_operator_delete): New.
            * region-model.cc (region_model::on_call_pre): Detect operator new
            and operator delete.
            (region_model::on_call_post): Likewise.
            (region_model::maybe_update_for_edge): Detect EH edges and call...
            (region_model::apply_constraints_for_exception): New function.
            * region-model.h (region_model::impl_call_operator_new): New decl.
            (region_model::impl_call_operator_delete): New decl.
            (region_model::apply_constraints_for_exception): New decl.
            * sm-malloc.cc (enum resource_state): New.
            (struct allocation_state): New state subclass.
            (enum wording): New.
            (struct api): New.
            (malloc_state_machine::custom_data_t): New typedef.
            (malloc_state_machine::add_state): New decl.
            (malloc_state_machine::m_unchecked)
            (malloc_state_machine::m_nonnull)
            (malloc_state_machine::m_freed): Delete these states in favor
            of...
            (malloc_state_machine::m_malloc)
            (malloc_state_machine::m_scalar_new)
            (malloc_state_machine::m_vector_new): ...this new api instances,
            which own their own versions of these states.
            (malloc_state_machine::on_allocator_call): New decl.
            (malloc_state_machine::on_deallocator_call): New decl.
            (api::api): New ctor.
            (dyn_cast_allocation_state): New.
            (as_a_allocation_state): New.
            (get_rs): New.
            (unchecked_p): New.
            (nonnull_p): New.
            (freed_p): New.
            (malloc_diagnostic::describe_state_change): Use unchecked_p and
            nonnull_p.
            (class mismatching_deallocation): New.
            (double_free::double_free): Add funcname param for initializing
            m_funcname.
            (double_free::emit): Use m_funcname in warning message rather
            than hardcoding "free".
            (double_free::describe_state_change): Likewise.  Use freed_p.
            (double_free::describe_call_with_state): Use freed_p.
            (double_free::describe_final_event): Use m_funcname in message
            rather than hardcoding "free".
            (double_free::m_funcname): New field.
            (possible_null::describe_state_change): Use unchecked_p.
            (possible_null::describe_return_of_state): Likewise.
            (use_after_free::use_after_free): Add param for initializing m_api.
            (use_after_free::emit): Use m_api->m_dealloc_funcname in message
            rather than hardcoding "free".
            (use_after_free::describe_state_change): Use freed_p.  Change the
            wording of the message based on the API.
            (use_after_free::describe_final_event): Use
            m_api->m_dealloc_funcname in message rather than hardcoding
            "free".  Change the wording of the message based on the API.
            (use_after_free::m_api): New field.
            (malloc_leak::describe_state_change): Use unchecked_p.  Update
            for renaming of m_malloc_event to m_alloc_event.
            (malloc_leak::describe_final_event): Update for renaming of
            m_malloc_event to m_alloc_event.
            (malloc_leak::m_malloc_event): Rename...
            (malloc_leak::m_alloc_event): ...to this.
            (free_of_non_heap::free_of_non_heap): Add param for initializing
            m_funcname.
            (free_of_non_heap::emit): Use m_funcname in message rather than
            hardcoding "free".
            (free_of_non_heap::describe_final_event): Likewise.
            (free_of_non_heap::m_funcname): New field.
            (allocation_state::dump_to_pp): New.
            (allocation_state::get_nonnull): New.
            (malloc_state_machine::malloc_state_machine): Update for changes
            to state fields and new api fields.
            (malloc_state_machine::add_state): New.
            (malloc_state_machine::on_stmt): Move malloc/calloc handling to
            on_allocator_call and call it, passing in the API pointer.
            Likewise for free, moving it to on_deallocator_call.  Handle calls
            to operator new and delete in an analogous way.  Use unchecked_p
            when testing for possibly-null-arg and possibly-null-deref, and
            transition to the non-null for the correct API.  Remove redundant
            node param from call to on_zero_assignment.  Use freed_p for
            use-after-free check, and pass in API.
            (malloc_state_machine::on_allocator_call): New, based on code in
            on_stmt.
            (malloc_state_machine::on_deallocator_call): Likewise.
            (malloc_state_machine::on_phi): Mark node param with
            ATTRIBUTE_UNUSED; don't pass it to on_zero_assignment.
            (malloc_state_machine::on_condition): Mark node param with
            ATTRIBUTE_UNUSED.  Replace on_transition calls with get_state and
            set_next_state pairs, transitioning to the non-null state for the
            appropriate API.
            (malloc_state_machine::can_purge_p): Port to new state approach.
            (malloc_state_machine::on_zero_assignment): Replace on_transition
            calls with get_state and set_next_state pairs.  Drop redundant
            node param.
            * sm.h (state_machine::add_custom_state): New.

    gcc/ChangeLog:
            PR analyzer/94355
            * doc/invoke.texi: Document -Wanalyzer-mismatching-deallocation.

    gcc/testsuite/ChangeLog:
            PR analyzer/94355
            * g++.dg/analyzer/new-1.C: New test.
            * g++.dg/analyzer/new-vs-malloc.C: New test.

[Bug analyzer/94355] support for C++ new expression

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-09-09

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
The above patch is an initial implementation of new/delete support; it works
for the simplest cases.

As noted in the commit message this patch was mostly about generalizing the
sm-malloc.cc code; the implementation of new/delete will no doubt need much
further refinement (and beyond that, the analyzer doesn't yet know about
vfuncs, exceptions, inheritance, RTTI, etc)

Specific things still to be investigated relating to new/delete (if only to add
test coverage):
* all of the various cases seen at
https://en.cppreference.com/w/cpp/memory/new/operator_new
* placement new
* global and class-specific replacements for operator new/new[]
* exceptions vs non-throwing allocators vs -fno-exceptions
* verify that ctors/dtors are simulated appropriately for both scalar and
vector new/delete

Anything else?

Note to self: see also https://en.cppreference.com/w/cpp/language/new

[Bug analyzer/94355] support for C++ new expression

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #4 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to Ivan Sorokin from comment #0)
> At the moment static analyzer warns about leaked malloc. It would be great
> if C++ new expression were also supported.
> 
> Example:
> 
> void f()
> {
>     char* p = new char;
> }
> 
> Expected diagnostic:
> 
> warning: leak of 'p' [CWE-401] [-Wanalyzer-malloc-leak]
> 
>     3 |     char* p = new char;

Actual diagnostic with commit
r11-3090-g1690a839cff2e0276017a013419d81d675bbf69d:

demo.cc: In function ‘void f()’:
demo.cc:4:1: warning: leak of ‘p’ [CWE-401] [-Wanalyzer-malloc-leak]
    4 | }
      | ^
  ‘void f()’: events 1-2
    |
    |    3 |     char* p = new char;
    |      |                   ^~~~
    |      |                   |
    |      |                   (1) allocated here
    |    4 | }
    |      | ~                  
    |      | |
    |      | (2) ‘p’ leaks here; was allocated at (1)
    |

where the diagnostic's location is at the point where "p" goes out of scope. 
There's an argument that it would be better emitting it at the "new".

[Bug analyzer/94355] support for C++ new expression

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:d4a906e7b51f3fc31f3328810f45ae4cf2e7bbc3

commit r11-3472-gd4a906e7b51f3fc31f3328810f45ae4cf2e7bbc3
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Sep 25 14:31:46 2020 -0400

    analyzer: add test for placement new

    gcc/testsuite/ChangeLog:
            PR analyzer/94355
            * g++.dg/analyzer/placement-new.C: New test.

[Bug analyzer/94355] support for C++ new expression

[vanyacpp at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #6 from Ivan Sorokin <vanyacpp at gmail dot com> ---
I played with -fanalyzer on godbolt (GCC trunk). I noticed that -fanalyzer
doesn't report double free in this (convoluted) case:

#include <cstdlib>

int main()
{
    int* p = new int;
    delete p;
    free(p);
}

[Bug analyzer/94355] support for C++ new expression

[vanyacpp at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #7 from Ivan Sorokin <vanyacpp at gmail dot com> ---
For me the support for operator new works well for trivially constructible
types. For a non-trivially constructible type I got a false positive:

struct foo { foo(); };

int main()
{
    delete new foo();
}

In function 'int main()':
cc1plus: warning: use of possibly-NULL 'operator new(1)' where non-null
expected [CWE-690] [-Wanalyzer-possible-null-argument]
  'int main()': event 1
    |
    |<source>:5:20:
    |    5 |     delete new foo();
    |      |                    ^
    |      |                    |
    |      |                    (1) this call could return NULL
    |
  'int main()': event 2
    |
    |cc1plus:
    | (2): argument 'this' ('operator new(1)') from (1) could be NULL where
non-null expected
    |
<source>:1:14: note: argument 'this' of 'foo::foo()' must be non-null
    1 | struct foo { foo(); };
      |              ^~~
Compiler returned: 0

https://godbolt.org/z/nPff9EGsY

Also the error location seems to be wrong. Removing "()" from "delete new
foo()" fixes the error location.

[Bug analyzer/94355] support for C++ new expression

[navarre.gcc.bugs at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

William Navarre <navarre.gcc.bugs at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |navarre.gcc.bugs at gmail dot com

--- Comment #8 from William Navarre <navarre.gcc.bugs at gmail dot com> ---
It seems that `operator new` is generally not supposed to return NULL --
std::bad_alloc() is supposed to be thrown instead. 

I made that change on my build (see below). I think that treating new's result
as never-null is probably the correct thing to do most of the time, but two
considerations: 

1) The case of allocating a zero-length array. 

2) The case that a project has replaced `operator new.` (See "global
replacements" at https://en.cppreference.com/w/cpp/memory/new/operator_new). 

Apparently projects can replace `operator new` (see "global replacements" at
https://en.cppreference.com/w/cpp/memory/new/operator_new). It's not clear 

--- a/gcc-11.1.0/gcc/analyzer/sm-malloc.cc
+++ b/gcc-11.1.0.navarre/gcc/analyzer/sm-malloc.cc
@@ -1527,9 +1527,9 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt,
          }

        if (is_named_call_p (callee_fndecl, "operator new", call, 1))
-         on_allocator_call (sm_ctxt, call, &m_scalar_delete);
+        on_allocator_call (sm_ctxt, call, &m_scalar_delete, true);
        else if (is_named_call_p (callee_fndecl, "operator new []", call, 1))
-         on_allocator_call (sm_ctxt, call, &m_vector_delete);
+        on_allocator_call (sm_ctxt, call, &m_vector_delete, true);
        else if (is_named_call_p (callee_fndecl, "operator delete", call, 1)
                 || is_named_call_p (callee_fndecl, "operator delete", call,
2))
          {

[Bug analyzer/94355] support for C++ new expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to William Navarre from comment #8)
> It seems that `operator new` is generally not supposed to return NULL --
> std::bad_alloc() is supposed to be thrown instead. 

If an operator new overload is declared noexcept, then it can return null on
failure. If it is not noexcept then it throws bad_alloc (or something derived
from it) and must never return null.


> 
> I made that change on my build (see below). I think that treating new's
> result as never-null is probably the correct thing to do most of the time,
> but two considerations: 
> 
> 1) The case of allocating a zero-length array. 

That still can't return null. It must return a valid non-null pointer, that
cannot be derefernced.

> 2) The case that a project has replaced `operator new.` (See "global
> replacements" at https://en.cppreference.com/w/cpp/memory/new/operator_new). 
> 
> Apparently projects can replace `operator new` (see "global replacements" at
> https://en.cppreference.com/w/cpp/memory/new/operator_new). It's not clear 

Exactly the same rules apply. If the operator new function is noexcept it
returns null to indicate allocation failure, otherwise it must throw and cannot
return null, ever.

[Bug analyzer/94355] support for C++ new expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
struct S {
    S() { p = new int(); }
    ~S() { delete p; }
    int* p = nullptr;
};

int main() {
    S s;
}


Everything the analyzer says is wrong :-)

dest.C: In function ‘int main()’:
dest.C:9:1: warning: leak of ‘s.S::p’ [CWE-401] [-Wanalyzer-malloc-leak]
    9 | }
      | ^
  ‘int main()’: events 1-2
    |
    |    7 | int main() {
    |      |     ^~~~
    |      |     |
    |      |     (1) entry to ‘main’
    |    8 |     S s;
    |      |       ~
    |      |       |
    |      |       (2) calling ‘S::S’ from ‘main’
    |
    +--> ‘S::S()’: events 3-5
           |
           |    2 |     S() { p = new int(); }
           |      |     ^     ~~~~~~~~~~~~~
           |      |     |       |         |
           |      |     |       |         (4) allocated here
           |      |     |       (5) assuming ‘operator new(4)’ is non-NULL
           |      |     (3) entry to ‘S::S’
           |
    <------+
    |
  ‘int main()’: events 6-7
    |
    |    8 |     S s;
    |      |       ^
    |      |       |
    |      |       (6) returning to ‘main’ from ‘S::S’
    |    9 | }
    |      | ~      
    |      | |
    |      | (7) ‘s.S::p’ leaks here; was allocated at (4)
    |
dest.C: In constructor ‘S::S()’:
dest.C:2:13: warning: dereference of possibly-NULL ‘operator new(4)’ [CWE-690]
[-Wanalyzer-possible-null-dereference]
    2 |     S() { p = new int(); }
      |           ~~^~~~~~~~~~~
  ‘S::S()’: events 1-2
    |
    |    2 |     S() { p = new int(); }
    |      |           ~~~~~~~~~~~~^
    |      |             |         |
    |      |             |         (1) this call could return NULL
    |      |             (2) ‘operator new(4)’ could be NULL: unchecked value
from (1)
    |

As already noted above, new can't return null here, and there is no dereference
anyway. And the pointer isn't leaked, but it seems maybe the analyzer doesn't
know about destructors?

[Bug analyzer/94355] support for C++ new expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
If we add a real bug (making a shallow copy of the object, which shares the
same heap pointer and then both destructors free the same one):

struct S {
    S() { p = new int(); }
    ~S() { delete p; }
    int* p = nullptr;
};

int main() {
    S s;
    S ss = s;
}

Then the analyzer still tells us the pointer is leaked:

dest.C: In function ‘int main()’:
dest.C:10:1: warning: leak of ‘s.S::p’ [CWE-401] [-Wanalyzer-malloc-leak]
   10 | }
      | ^
  ‘int main()’: events 1-2
    |
    |    7 | int main() {
    |      |     ^~~~
    |      |     |
    |      |     (1) entry to ‘main’
    |    8 |     S s;
    |      |       ~
    |      |       |
    |      |       (2) calling ‘S::S’ from ‘main’
    |
    +--> ‘S::S()’: events 3-5
           |
           |    2 |     S() { p = new int(); }
           |      |     ^     ~~~~~~~~~~~~~
           |      |     |       |         |
           |      |     |       |         (4) allocated here
           |      |     |       (5) assuming ‘operator new(4)’ is non-NULL
           |      |     (3) entry to ‘S::S’
           |
    <------+
    |
  ‘int main()’: events 6-7
    |
    |    8 |     S s;
    |      |       ^
    |      |       |
    |      |       (6) returning to ‘main’ from ‘S::S’
    |    9 |     S ss = s;
    |   10 | }
    |      | ~      
    |      | |
    |      | (7) ‘s.S::p’ leaks here; was allocated at (4)
    |

But strangely it also tells use there's a use-after-free, which is odd if it
thinks the pointer was nver freed:

dest.C: In destructor ‘S::~S()’:
dest.C:3:22: warning: use after ‘delete’ of ‘s.S::p’ [CWE-416]
[-Wanalyzer-use-after-free]
    3 |     ~S() { delete p; }
      |                      ^
  ‘int main()’: events 1-2
    |
    |    7 | int main() {
    |      |     ^~~~
    |      |     |
    |      |     (1) entry to ‘main’
    |    8 |     S s;
    |      |       ~
    |      |       |
    |      |       (2) calling ‘S::S’ from ‘main’
    |
    +--> ‘S::S()’: events 3-5
           |
           |    2 |     S() { p = new int(); }
           |      |     ^     ~~~~~~~~~~~~~
           |      |     |       |         |
           |      |     |       |         (4) allocated here
           |      |     |       (5) assuming ‘operator new(4)’ is non-NULL
           |      |     (3) entry to ‘S::S’
           |
    <------+
    |
  ‘int main()’: events 6-7
    |
    |    8 |     S s;
    |      |       ^
    |      |       |
    |      |       (6) returning to ‘main’ from ‘S::S’
    |    9 |     S ss = s;
    |   10 | }
    |      | ~      
    |      | |
    |      | (7) calling ‘S::~S’ from ‘main’
    |
    +--> ‘S::~S()’: events 8-11
           |
           |    3 |     ~S() { delete p; }
           |      |     ^      ~~~~~~~~
           |      |     |      |      |
           |      |     |      |      (10) ...to here
           |      |     |      |      (11) deleted here
           |      |     |      (9) following ‘true’ branch...
           |      |     (8) entry to ‘S::~S’
           |
    <------+
    |
  ‘int main()’: events 12-13
    |
    |   10 | }
    |      | ^
    |      | |
    |      | (12) returning to ‘main’ from ‘S::~S’
    |      | (13) calling ‘S::~S’ from ‘main’
    |
    +--> ‘S::~S()’: events 14-17
           |
           |    3 |     ~S() { delete p; }
           |      |     ^      ~~~~~~~~  ~
           |      |     |      |      |  |
           |      |     |      |      |  (17) use after ‘delete’ of
‘*this.S::p’; deleted at (11)
           |      |     |      |      (16) ...to here
           |      |     |      (15) following ‘true’ branch...
           |      |     (14) entry to ‘S::~S’
           |

I think it's (correctly) telling us that the destructor ss.~S() uses the value
after s.~S() already freed it. But it would be nice if it mentioned that the
second destructor is on ss, not s.

The earlier warning tells us the object: "warning: use after ‘delete’ of
‘s.S::p’" 

So it would be nice if the use-after-free was identified as being ss.S::p
(which has the same value as s.S::p).

And it's not actually a use-after-free (and certainly not at the closing brace
of the destructor where the location points to), it's a double-free.

Ideally it would have complained when we copied the pointer value in the
trivial copy constructor and didn't zero out the original. That shallow copy
was the actual bug, the double-free is a symptom of it.


dest.C: In constructor ‘S::S()’:
dest.C:2:13: warning: dereference of possibly-NULL ‘operator new(4)’ [CWE-690]
[-Wanalyzer-possible-null-dereference]
    2 |     S() { p = new int(); }
      |           ~~^~~~~~~~~~~
  ‘S::S()’: events 1-2
    |
    |    2 |     S() { p = new int(); }
    |      |           ~~~~~~~~~~~~^
    |      |             |         |
    |      |             |         (1) this call could return NULL
    |      |             (2) ‘operator new(4)’ could be NULL: unchecked value
from (1)
    |

[Bug analyzer/94355] support for C++ new expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #11)
> Ideally it would have complained when we copied the pointer value in the
> trivial copy constructor and didn't zero out the original. That shallow copy
> was the actual bug, the double-free is a symptom of it.

I've created PR 107534 for that.

[Bug analyzer/94355] support for C++ new expression

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #13 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #10)

[...snip...]

> As already noted above, new can't return null here, and there is no
> dereference anyway. And the pointer isn't leaked, but it seems maybe the
> analyzer doesn't know about destructors?

FWIW the analyzer (presumably incorrectly) considers the case where operator
new returns NULL, and then attempts to write 0 to it.

The leak is due to the analyzer (presumably incorrectly) following the EH cfg
edge from BB 2 to BB 5 below.

int main ()
{
  struct S s;
  int D.2800;
  int _5;

  <bb 2> :
  S::S (&s);

  <bb 3> :
  S::~S (&s);
  s ={v} {CLOBBER(eol)};
  _5 = 0;

  <bb 4> :
<L0>:
  return _5;

  <bb 5> :
<L1>:
  s ={v} {CLOBBER(eol)};
  resx 1

}

[Bug analyzer/94355] support for C++ new expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #13)
> (In reply to Jonathan Wakely from comment #10)
> 
> [...snip...]
> 
> > As already noted above, new can't return null here, and there is no
> > dereference anyway. And the pointer isn't leaked, but it seems maybe the
> > analyzer doesn't know about destructors?
> 
> FWIW the analyzer (presumably incorrectly) considers the case where operator
> new returns NULL, and then attempts to write 0 to it.

Yeah, that's incorrect, the write is guaranteed to be OK because if new fails
it throws an exception before the write ever happens.

If a non-throwing form of new is used, e.g. p = new(std::nothrow) int(), then
the compiler *does* insert a null check before writing to it. So in the case
where a null check is necessary, the compiler inserts one.

See the -fcheck-new option:

  Check that the pointer returned by "operator new" is non-null before
attempting to modify the storage allocated.  This check is normally unnecessary
because the C++ standard specifies that "operator new" only returns 0 if it is
declared "throw()", in which case the compiler always checks the return value
even without this option.  In all other cases, when "operator new" has a
non-empty exception specification, memory exhaustion is signalled by throwing
"std::bad_alloc".  See also new (nothrow).

[Bug analyzer/94355] analyzer support for C++ new expression

[vultkayn at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #15 from Benjamin Priour <vultkayn at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #14)
> [...snip...]
> 
> See the -fcheck-new option:
> 
>   Check that the pointer returned by "operator new" is non-null before
> attempting to modify the storage allocated.  This check is normally
> unnecessary because the C++ standard specifies that "operator new" only
> returns 0 if it is declared "throw()", in which case the compiler always
> checks the return value even without this option.  In all other cases, when
> "operator new" has a non-empty exception specification, memory exhaustion is
> signalled by throwing "std::bad_alloc".  See also new (nothrow).


Should we use the above flag's value to also optionally disable the analyzer
warnings on operator new possibly returning NULL?

Or maybe there could be an additional flag -fanalyzer-new-returns-null, turned
'on' by default.

Having such capability would be useful to run the analyzer against itself, as
GCC is built without exceptions (thus every operator new possibly returns
NULL).

[Bug analyzer/94355] analyzer support for C++ new expression

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94355

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Benjamin Priour <vultkayn@gcc.gnu.org>:

https://gcc.gnu.org/g:e7b267444045c507654a2a3f758efee5d5b550df

commit r14-3632-ge7b267444045c507654a2a3f758efee5d5b550df
Author: benjamin priour <priour.be@gmail.com>
Date:   Fri Sep 1 00:01:29 2023 +0200

    analyzer: Add support of placement new and improved operator new
[PR105948,PR94355]

    Fixed spurious possibly-NULL warning always tagging along throwing
    operator new despite it never returning NULL.
    Now operator new is correctly recognized as possibly returning NULL
    if and only if it is non-throwing or exceptions have been disabled.
    Different standard signatures of operator new are now properly
    recognized.

    Added support of placement new, so that it is now properly recognized,
    and a 'heap_allocated' region is no longer created for it.
    Placement new size is also checked and a 'Wanalyzer-allocation-size'
    is emitted when relevant, as well as always a 'Wanalyzer-out-of-bounds'.

    'operator new' non-throwing variants are detected y checking the types
    of the parameters.
    Indeed, in a call to new (std::nothrow) () the chosen overload
    has signature 'operator new (void*, std::nothrow_t&)', where the second
    parameter is a reference. In a placement new, the second parameter will
    always be a void pointer.

    Prior to this patch, some buffers first allocated with 'new', then deleted
    an thereafter used would result in a 'Wanalyzer-user-after-free'
    warning. However the wording was "use after 'free'" instead of the
    expected "use after 'delete'".
    This patch fixes this by introducing a new kind of poisoned value,
    namely POISON_KIND_DELETED.

    Due to how the analyzer sees calls to non-throwing variants of
    operator new, dereferencing a pointer freshly allocated in this fashion
    caused both a 'Wanalyzer-use-of-uninitialized-value' and a
    'Wanalyzer-null-dereference' to be emitted, while only the latter was
    relevant. As a result, 'null-dereference' now supersedes
    'use-of-uninitialized'.

    Signed-off-by: benjamin priour <vultkayn@gcc.gnu.org>

    gcc/analyzer/ChangeLog:

            PR analyzer/105948
            PR analyzer/94355
            * analyzer.h (is_placement_new_p): New declaration.
            * call-details.cc
            (call_details::deref_ptr_arg): New function.
            Dereference the argument at given index if possible.
            * call-details.h: Declaration of the above function.
            * kf-lang-cp.cc (is_placement_new_p): Returns true if the gcall
            is recognized as a placement new.
            (kf_operator_delete::impl_call_post): Unbinding a region and its
            descendents now poisons with POISON_KIND_DELETED.
            (register_known_functions_lang_cp): Known function "operator
            delete" is now registered only once independently of its number of
            arguments.
            * region-model.cc (region_model::eval_condition): Now
            recursively calls itself if any of the operand is wrapped in a
            cast.
            * sm-malloc.cc (malloc_state_machine::on_stmt):
            Add placement new recognition.
            * svalue.cc (poison_kind_to_str): Wording for the new PK.
            * svalue.h (enum poison_kind): Add value POISON_KIND_DELETED.

    gcc/testsuite/ChangeLog:

            PR analyzer/105948
            PR analyzer/94355
            * g++.dg/analyzer/out-of-bounds-placement-new.C: Added a directive.
            * g++.dg/analyzer/placement-new.C: Added tests.
            * g++.dg/analyzer/new-2.C: New test.
            * g++.dg/analyzer/noexcept-new.C: New test.
            * g++.dg/analyzer/placement-new-size.C: New test.