[Bug analyzer/107646] New: RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[Bug analyzer/107646] New: RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 107646
           Summary: RFE: can we reimplement gcc-python-plugin's cpychecker
                    as a -fanalyzer plugin?
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: dmalcolm at gcc dot gnu.org
  Target Milestone: ---

My gcc-python-plugin implements a crude static analysis of CPython code, using
symbolic execution in a manner similar to how -fanalyzer works, but it's been
bitrotting for years, and is slow (being written in Python itself):
  https://gcc-python-plugin.readthedocs.io/en/latest/cpychecker.html

I *think* this could now be reimplemented as a plugin to GCC's -fanalyzer (in
C++), where -fanalyzer provides most of the framework needed, and we'd mostly
just need to write the CPython-specific parts.

It would make an interesting GSoC project (albeit perhaps a challenging one).

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[dmalcolm at gcc dot gnu.org]

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

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
In particular, reference-count checking would probably be the most interesting
aspect of the project.

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[cvs-commit at gcc dot gnu.org]

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

--- 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:fafe2d18f791c6b97b49af7c84b1b5703681c3af

commit r14-2933-gfafe2d18f791c6b97b49af7c84b1b5703681c3af
Author: Eric Feng <ef2648@columbia.edu>
Date:   Wed Aug 2 16:54:55 2023 -0400

    analyzer: stash values for CPython plugin [PR107646]

    This patch adds a hook to the end of ana::on_finish_translation_unit
    which calls relevant stashing-related callbacks registered during plugin
    initialization. This feature is used to stash named types and global
    variables for a CPython analyzer plugin [PR107646].

    gcc/analyzer/ChangeLog:
            PR analyzer/107646
            * analyzer-language.cc (run_callbacks): New function.
            (on_finish_translation_unit): New function.
            * analyzer-language.h (GCC_ANALYZER_LANGUAGE_H): New include.
            (class translation_unit): New vfuncs.

    gcc/c/ChangeLog:
            PR analyzer/107646
            * c-parser.cc: New functions on stashing values for the
            analyzer.

    gcc/testsuite/ChangeLog:
            PR analyzer/107646
            * gcc.dg/plugin/plugin.exp: Add new plugin and test.
            * gcc.dg/plugin/analyzer_cpython_plugin.c: New plugin.
            * gcc.dg/plugin/cpython-plugin-test-1.c: New test.

    Signed-off-by: Eric Feng <ef2648@columbia.edu>

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Eric Feng <efric@gcc.gnu.org>:

https://gcc.gnu.org/g:38c00edd65c39b07166aa0913c79adb9bcac943c

commit r14-3162-g38c00edd65c39b07166aa0913c79adb9bcac943c
Author: Eric Feng <ef2648@columbia.edu>
Date:   Fri Aug 11 12:30:43 2023 -0400

    analyzer: More features for CPython analyzer plugin [PR107646]

    This patch adds known function subclasses for Python/C API functions
    PyList_New, PyLong_FromLong, and PyList_Append. It also adds new
    optional parameters for
    region_model::get_or_create_region_for_heap_alloc, allowing for the
    newly allocated region to immediately transition from the start state to
    the assumed non-null state in the malloc state machine if desired.
    Finally, it adds a new procedure, dg-require-python-h, intended as a
    directive in Python-related analyzer tests, to append necessary Python
    flags during the tests' build process.

    The main warnings we gain in this patch with respect to the known function
    subclasses mentioned are leak related. For example:

    rc3.c: In function âcreate_py_objectâ:
    â
    rc3.c:21:10: warning: leak of âitemâ [CWE-401] [-Wanalyzer-malloc-leak]
    â
       21 |   return list;
          â
          |          ^~~~
    â
      âcreate_py_objectâ: events 1-4
    â
        |
    â
        |    4 |   PyObject* item = PyLong_FromLong(10);
    â
        |      |                    ^~~~~~~~~~~~~~~~~~~
    â
        |      |                    |
    â
        |      |                    (1) allocated here
    â
        |      |                    (2) when âPyLong_FromLongâ succeeds
    â
        |    5 |   PyObject* list = PyList_New(2);
    â
        |      |                    ~~~~~~~~~~~~~
    â
        |      |                    |
    â
        |      |                    (3) when âPyList_Newâ fails
    â
        |......
    â
        |   21 |   return list;
    â
        |      |          ~~~~
    â
        |      |          |
    â
        |      |          (4) âitemâ leaks here; was allocated at (1)
    â

    Some concessions were made to
    simplify the analysis process when comparing kf_PyList_Append with the
    real implementation. In particular, PyList_Append performs some
    optimization internally to try and avoid calls to realloc if
    possible. For simplicity, we assume that realloc is called every time.
    Also, we grow the size by just 1 (to ensure enough space for adding a
    new element) rather than abide by the heuristics that the actual
implementation
    follows.

    gcc/analyzer/ChangeLog:
            PR analyzer/107646
            * call-details.h: New function.
            * region-model.cc
(region_model::get_or_create_region_for_heap_alloc):
            New optional parameters.
            * region-model.h (class region_model): New optional parameters.
            * sm-malloc.cc (on_realloc_with_move): New function.
            (region_model::transition_ptr_sval_non_null): New function.

    gcc/testsuite/ChangeLog:
            PR analyzer/107646
            * gcc.dg/plugin/analyzer_cpython_plugin.c: Analyzer support for
            PyList_New, PyList_Append, PyLong_FromLong
            * gcc.dg/plugin/plugin.exp: New test.
            * lib/target-supports.exp: New procedure.
            * gcc.dg/plugin/cpython-plugin-test-2.c: New test.

    Signed-off-by: Eric Feng <ef2648@columbia.edu>

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[dmalcolm at gcc dot gnu.org]

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

--- Comment #4 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Some ideas of projects we could analyze:
- minimal Cython-generated C file
- https://pypi.org/project/psycopg2/
- https://pypi.org/project/numpy

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[dmalcolm at gcc dot gnu.org]

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

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
How precisely to track behavior of API entrypoints?  We can’t implement
known_functions that precisely model every entrypoint.

Consider:
https://docs.python.org/3/c-api/dict.html#c.PyDict_SetItem
which has:
Insert val into the dictionary p with a key of key. 
key must be hashable; if it isn’t, TypeError will be raised. 
Return 0 on success or -1 on failure. 
This function does not steal a reference to val.

How much of this could be expressed via function attributes, rather than
known_function subclasses?

List of 20 most commonly used CPython API entrypoints from my 2013 survey:
https://fedorapeople.org/~dmalcolm/presentations/PyCon-US-2013/PyCon-US-2013-dmalcolm-StaticAnalysis.html#(25)

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[dmalcolm at gcc dot gnu.org]

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

--- Comment #6 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #5)
> How precisely to track behavior of API entrypoints?  We can’t implement
> known_functions that precisely model every entrypoint.
> 
> Consider:
> https://docs.python.org/3/c-api/dict.html#c.PyDict_SetItem

Some attribute ideas:

extern int PyDict_SetItem(PyObject *p, PyObject *key, PyObject *val)
  __attribute__((cpython_param_must_be_hashable (key))
  __attribute__((return_val_on_success (0))
  __attribute__((return_val_on_failure (-1))
  __attribute__((cpython_param_incremented_on_success (key))
  __attribute__((cpython_param_untouched_on_success (val))
;

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[dmalcolm at gcc dot gnu.org]

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

--- Comment #7 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #6)
> (In reply to David Malcolm from comment #5)
> Some attribute ideas:
> 
> extern int PyDict_SetItem(PyObject *p, PyObject *key, PyObject *val)
>   __attribute__((cpython_param_must_be_hashable (key))

...though this implies supporting parameter names in attributes (is there a BZ
for this?)

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[dmalcolm at gcc dot gnu.org]

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

--- Comment #8 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #4)
> Some ideas of projects we could analyze:

* https://pypi.org/project/mercurial/ ; see:
     https://repo.mercurial-scm.org/hg-stable/file/tip/mercurial/cext

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-08-17
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[dmalcolm at gcc dot gnu.org]

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

--- Comment #9 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #4)
> Some ideas of projects we could analyze:

https://github.com/fedora-python/python-ethtool
(Although deprecated, it's relatively small and has been ported to Python 3)

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Eric Feng <efric@gcc.gnu.org>:

https://gcc.gnu.org/g:597b9ec69bca8acb7a3d65641c0a730de8b27ed4

commit r14-3580-g597b9ec69bca8acb7a3d65641c0a730de8b27ed4
Author: Eric Feng <ef2648@columbia.edu>
Date:   Wed Aug 30 17:52:24 2023 -0400

    analyzer: implement reference count checking for CPython plugin [PR107646]

    This patch introduces initial support for reference count checking of
    PyObjects in relation to the Python/C API for the CPython plugin.
    Additionally, the core analyzer underwent several modifications to
    accommodate this feature. These include:

    - Introducing support for callbacks at the end of
      region_model::pop_frame. This is our current point of validation for
      the reference count of PyObjects.
    - An added optional custom stmt_finder parameter to
      region_model_context::warn. This aids in emitting a diagnostic
      concerning the reference count, especially when the stmt_finder is
      NULL, which is currently the case during region_model::pop_frame.

    The current diagnostic we emit relating to the reference count
    appears as follows:

    rc3.c:23:10: warning: expected âitemâ to have reference count: â1â
but
    ob_refcnt field is: â2â
       23 |   return list;
          |          ^~~~
      âcreate_py_objectâ: events 1-4
        |
        |    4 |   PyObject* item = PyLong_FromLong(3);
        |      |                    ^~~~~~~~~~~~~~~~~~
        |      |                    |
        |      |                    (1) when âPyLong_FromLongâ succeeds
        |    5 |   PyObject* list = PyList_New(1);
        |      |                    ~~~~~~~~~~~~~
        |      |                    |
        |      |                    (2) when âPyList_Newâ succeeds
        |......
        |   14 |   PyList_Append(list, item);
        |      |   ~~~~~~~~~~~~~~~~~~~~~~~~~
        |      |   |
        |      |   (3) when âPyList_Appendâ succeeds, moving buffer
        |......
        |   23 |   return list;
        |      |          ~~~~
        |      |          |
        |      |          (4) here
        |

    This is a WIP in several ways:
    - Currently, functions returning PyObject * are assumed to always
      produce a new reference.
    - The validation of reference count is only for PyObjects created within
      a function body. Verifying reference counts for PyObjects passed as
      parameters is not supported in this patch.

    gcc/analyzer/ChangeLog:
            PR analyzer/107646
            * engine.cc (impl_region_model_context::warn): New optional
            parameter.
            * exploded-graph.h (class impl_region_model_context): Likewise.
            * region-model.cc (region_model::pop_frame): New callback
            feature for region_model::pop_frame.
            * region-model.h (struct append_regions_cb_data): Likewise.
            (class region_model): Likewise.
            (class region_model_context): New optional parameter.
            (class region_model_context_decorator): Likewise.

    gcc/testsuite/ChangeLog:
            PR analyzer/107646
            * gcc.dg/plugin/analyzer_cpython_plugin.c: Implements reference
            count checking for PyObjects.
            * gcc.dg/plugin/cpython-plugin-test-2.c: Moved to...
            * gcc.dg/plugin/cpython-plugin-test-PyList_Append.c: ...here
            (and added more tests).
            * gcc.dg/plugin/cpython-plugin-test-1.c: Moved to...
            * gcc.dg/plugin/cpython-plugin-test-no-Python-h.c: ...here (and
            added more tests).
            * gcc.dg/plugin/plugin.exp: New tests.
            * gcc.dg/plugin/cpython-plugin-test-PyList_New.c: New test.
            * gcc.dg/plugin/cpython-plugin-test-PyLong_FromLong.c: New test.

    Signed-off-by: Eric Feng <ef2648@columbia.edu>

[Bug analyzer/107646] RFE: can we reimplement gcc-python-plugin's cpychecker as a -fanalyzer plugin?

[dmalcolm at gcc dot gnu.org]

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

--- Comment #11 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
I've created a wiki page to track this project:
  https://gcc.gnu.org/wiki/StaticAnalyzer/CPython