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
