Hi David, Thank you so much! I am thrilled to have my first patch queued for GCC 17.
Apologies for the trailing whitespace. I will configure my editor to highlight visual whitespace and ensure I run git diff --check before formatting all future patches. Regarding the trivial C extension: I actually started writing one yesterday (buggy_module.c) with an intentional Py_INCREF(Py_None) memory leak without a corresponding Py_DECREF, just to see how the standard analyzer reacted versus the new analyzer_cpython_plugin. I am going to build it out using setuptools today to observe its behavior at runtime. Noted on the CPython versions! Based on the PEP 683 structural changes to ob_refcnt that you addressed in PR 112520, I will explicitly target Python 3.11+ as the baseline for my GSoC proposal scope to ensure compatibility with the modern struct layouts. Thanks again for all your guidance on this first patch! Best regards, Saksham Gupta On Thu, 12 Mar 2026 at 04:31, David Malcolm <[email protected]> wrote: > On Wed, 2026-03-11 at 12:15 +0530, Saksham Gupta wrote: > > Hi David, > > > > Thank you for the detailed review! It is great to hear that the core > > logic > > is on the right track. > > > > I have incorporated all of your feedback into a v2 patch (attached). > > Specifically: > > > > 1. Renamed the class to kf_atoi_family. > > 2. Added support for atoll in both namespaces. > > 3. Removed the unnecessary -Wno-analyzer-too-complex pragma from > > the > > test. > > 4. Added test coverage for atoi without an LHS, atoi with an > > unterminated buffer, and valid calls to atol and atoll. > > Thanks - the updated patch looks great. I've added it to my queue of > patches for GCC 17. > > One minor nit: git is complaining (correctly) about whitespace in a > couple of places in the patch, within class kf_atoi_family: > > Applying: analyzer: add known function handling for atoi, atol, and atoll > .git/rebase-apply/patch:27: trailing whitespace. > > .git/rebase-apply/patch:28: trailing whitespace. > /* atoi returns an integer, but we don't know what it is statically. > warning: 2 lines add whitespace errors. > > Ideally please turn on visual whitespace (or whatever such a mode is > called) in your editor to make it easier to keep things clean in new > code. > > > > > Regarding the bootstrap build: I haven't done a full bootstrap yet, > > but I > > will kick one off locally on my machine today. I would also love to > > get > > access to the Compile Farm to test across different architectures > > like > > Linux/x86_64, so I will submit an account request there. > > Great. > > > > > Understood entirely on the Stage 4 feature freeze! I am perfectly > > happy for > > this to sit in the queue for GCC 17. Right now, my primary focus is > > diving > > deep into the CPython API checking for the GSoC proposal. > > If you haven't tried writing a trivial CPython extension module in C by > hand yet, that would be a good exercise [1]. IIRC there's a link to > the tutorial for this on the project wiki page. > > One other thing to note is that the CPython API changes somewhat from > release to release; see e.g. bug 112520 where the plugin broke due to > changes in CPython 3.11. It's best to pick one version of CPython 3 > and focus on it, at least initially. > > Thanks > Dave > > [1] FWIW I recommend that people shouldn't write .c code for real > CPython extensions, and should instead use libffi, or write a .pyx file > and use Cython to translate it to .c - Cython tends to avoid the > mistakes that humans make when hand-authoring .c code. But doing it as > a learning exercise ought to be useful. > > > > > Regards, > > Saksham Gupta > > > > > > On Tue, 10 Mar 2026 at 06:14, David Malcolm <[email protected]> > > wrote: > > > > > On Mon, 2026-03-09 at 12:09 +0530, Saksham Gupta wrote: > > > > Hi David, > > > > > > > > Following your suggestion to look into kf.cc, I noticed that the > > > > analyzer > > > > didn't have known function handlers for atoi or atol. > > > > > > > > I have implemented a kf_atoi class that verifies the incoming > > > > argument is a > > > > valid, null-terminated string and sets the LHS to a generic > > > > unknown > > > > value. > > > > I also added a regression test (atoi-1.c) which successfully > > > > flags > > > > uninitialized buffers passed to atoi. > > > > > > > > I've attached the patch (with the Signed-off-by tag and > > > > ChangeLog). > > > > I've > > > > run the DejaGnu test suite on this (make check-gcc > > > > RUNTESTFLAGS="analyzer.exp=atoi-1.c") and it passes with zero > > > > regressions. > > > > > > Thanks for the patch! The kf stuff is a major part of the CPython > > > API > > > project, so it's good to see you getting familiar with it. > > > > > > Overall, the patch looks great, but I have a few comments: > > > > > > > diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc > > > > index b1ccbd6584a..bc625682717 100644 > > > > --- a/gcc/analyzer/kf.cc > > > > +++ b/gcc/analyzer/kf.cc > > > > @@ -833,6 +833,25 @@ private: > > > > tree m_var_decl; // could be NULL > > > > }; > > > > > > > > +class kf_atoi: public known_function > > > > > > How about naming this "kf_atoi_family" since it handles various > > > functions, not just "atoi" specifically. > > > > > > > +{ > > > > +public: > > > > + bool matches_call_types_p (const call_details &cd) const final > > > override > > > > + { > > > > + return (cd.num_args () == 1 && cd.arg_is_pointer_p (0)); > > > > + } > > > > + > > > > + void impl_call_pre (const call_details &cd) const final > > > > override > > > > + { > > > > + /* atoi expects a valid, null-terminated string. */ > > > > + cd.check_for_null_terminated_string_arg (0, false, nullptr); > > > > + > > > > + /* atoi returns an integer, but we don't know what it is > > > statically. > > > > + Tell the analyzer to assume it returns a generic, unknown > > > > value. > > > */ > > > > + cd.set_any_lhs_with_defaults (); > > > > + } > > > > +}; > > > > > > Looks correct. > > > > > > > In theory we could try to model the state of the environment > > > variables > > > > @@ -2289,6 +2308,9 @@ register_known_functions > > > > (known_function_manager > > > &kfm, > > > > /* Known builtins and C standard library functions > > > > the analyzer has known functions for. */ > > > > { > > > > + kfm.add ("atoi", std::make_unique<kf_atoi> ()); > > > > + kfm.add ("atol", std::make_unique<kf_atoi> ()); > > > > > > We could do "atoll" here for good measure as well. > > > > > > > > > > @@ -2388,6 +2410,9 @@ register_known_functions > > > > (known_function_manager > > > &kfm, > > > > from <cstdlib> etc for the C spellings of these headers > > > > (e.g. > > > <stdlib.h>), > > > > so we must match against these too. */ > > > > { > > > > + kfm.add_std_ns ("atoi", std::make_unique<kf_atoi> ()); > > > > + kfm.add_std_ns ("atol", std::make_unique<kf_atoi> ()); > > > > > > Likewise here. > > > > > > > diff --git a/gcc/testsuite/gcc.dg/analyzer/atoi-1.c > > > b/gcc/testsuite/gcc.dg/analyzer/atoi-1.c > > > > new file mode 100644 > > > > index 00000000000..67d7b103457 > > > > --- /dev/null > > > > +++ b/gcc/testsuite/gcc.dg/analyzer/atoi-1.c > > > > @@ -0,0 +1,11 @@ > > > > +/* { dg-additional-options "-Wno-analyzer-too-complex" } */ > > > > > > Do we need -Wno-analyzer-too-complex here, or was this due to > > > copy&pasting from another example? > > > > > > > +#include <stdlib.h> > > > > + > > > > +void test_valid_atoi(void) { > > > > + int val = atoi("123"); /* Should be fine. */ > > > > +} > > > > + > > > > +void test_uninitialized_atoi(void) { > > > > + char buf[10]; > > > > + int val = atoi(buf); /* { dg-warning "use of uninitialized > > > > value" } > > > */ > > > > +} > > > > > > This is good, but please add more test coverage: > > > > > > * a call to atoi for which the return value isn't taken > > > (set_any_lhs_with_defaults handles this, but in the past I've had > > > crashes in kf handlers due to assuming that there is an LHS to > > > write > > > the result to). > > > > > > * a call to atoi with an unterminated buffer > > > > > > * calls to atol and atoll to check that they're wired up. > > > > > > You should probably get familiar with how to do a bootstrap build > > > of > > > GCC. Have you tried this yet? Let us know if you need a more > > > powerful > > > machine; see e.g. https://portal.cfarm.net/ or want some pointers > > > on > > > how to do this. > > > > > > One other thing to mention - right now we're in feature freeze for > > > GCC > > > 16, trying to focus on stabilizing for the release - we call this > > > "stage 4" of the release cycle. Hence I plan to wait before > > > actually > > > pushing the patch. Hopefully we'll transition to stage 1 of the > > > GCC 17 > > > release cycle sometime next month (when we're open to new > > > features). > > > > > > Hope this all makes sense; thanks again for the patch > > > Dave > > > > > > > >
