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. 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. 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. 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 > >
0001-analyzer-add-known-function-handling-for-atoi-atol-a.patch
Description: Binary data
