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

[Eric Feng <ef2648@columbia.edu>]

On Tue, Aug 29, 2023 at 5:14 PM David Malcolm <dmalcolm@redhat.com> wrote:
>
> On Tue, 2023-08-29 at 13:28 -0400, Eric Feng wrote:
> > Additionally, by using the old model and the pointer per your
> > suggestion,
> > we are able to find the representative tree and emit a more accurate
> > diagnostic!
> >
> > 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
> >     |
>
> Excellent, that's a big improvement.
>
> >
> > If a representative tree is not found, I decided we should just bail
> > out
> > of emitting a diagnostic for now, to avoid confusing the user on what
> > the problem is.
>
> Fair enough.
>
> >
> > I've attached the patch for this (on top of the previous one) below.
> > If
> > it also looks good, I can merge it with the last patch and push it in
> > at
> > the same time.
>
> I don't mind either way, but please can you update the tests so that we
> have some automated test coverage that the correct name is being
> printed in the warning.
>
> Thanks
> Dave
>

Sorry — forgot to hit 'reply all' in the previous e-mail. Resending to
preserve our chain on the list:

---

Thanks; pushed to trunk with nits fixed:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=597b9ec69bca8acb7a3d65641c0a730de8b27ed4.

Incidentally, I updated my formatting settings in VSCode, which I've
previously mentioned in passing. In case anyone is interested:

"C_Cpp.clang_format_style": "{ BasedOnStyle: GNU, UseTab: Always,
TabWidth: 8, IndentWidth: 2, BinPackParameters: false,
AlignAfterOpenBracket: Align,
AllowAllParametersOfDeclarationOnNextLine: true }",

This fixes some issues with the indent width and also ensures function
parameters of appropriate length are aligned properly and on a new
line each (like the rest of the analyzer code).