[Bug analyzer/107573] New: RFE: analyzer handling of strtok

[Bug analyzer/107573] New: RFE: analyzer handling of strtok

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 107573
           Summary: RFE: analyzer handling of strtok
           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
                CC: jamie.bainbridge at gmail dot com
  Target Milestone: ---

Would be nice for -fanalyzer to check usage of strtok:

- complain about NULL passed as the string to the first call of strtok reached
from entry to "main" (I don't think there's a guarantee that strtok checks for
this).

- track the previous string passed to strtok, and complain about strtok (NULL,
delim) when the previous string is freed/out-of-scope, etc

- complain about passing a const buffer as the string, given that strtok will
write NUL terminators back to it if it finds a match

See:
  https://en.cppreference.com/w/c/string/byte/strtok
  https://man7.org/linux/man-pages/man3/strtok.3.html

[Bug analyzer/107573] RFE: analyzer handling of strtok

[jamie.bainbridge at gmail dot com]

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

--- Comment #1 from Jamie Bainbridge <jamie.bainbridge at gmail dot com> ---
Thanks for logging this!

(In reply to David Malcolm from comment #0)
> - complain about NULL passed as the string to the first call of strtok
> reached from entry to "main" (I don't think there's a guarantee that strtok
> checks for this).

UNIX libc (since BSD 4.3 or earlier) and musl (since always) check for this and
return NULL, but glibc intentionally crashes as a form of UB detection. Further
discussion of glibc behaviour at:

 https://sourceware.org/bugzilla/show_bug.cgi?id=16640
 https://sourceware.org/pipermail/libc-alpha/2022-November/143337.html

As we discussed via email, C99 and later have contained:

> The first call in the sequence has a non-null first argument;

[Bug analyzer/107573] RFE: analyzer handling of strtok

[cvs-commit at gcc dot gnu.org]

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

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

commit r14-5591-gf65f63c4d86a48be042a3ad242fffe5fe8347ff0
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Sat Nov 18 20:35:59 2023 -0500

    analyzer: new warning: -Wanalyzer-undefined-behavior-strtok [PR107573]

    This patch:
    - adds support to the analyzer for tracking API-private state
      or which we don't have a decl (such as strtok's internal state),
    - uses it to implement a new -Wanalyzer-undefined-behavior-strtok which
      warns when strtok (NULL, delim) is called as the first call to
      strtok after main.

    gcc/analyzer/ChangeLog:
            PR analyzer/107573
            * analyzer.h (register_known_functions): Add region_model_manager
            param.
            * analyzer.opt (Wanalyzer-undefined-behavior-strtok): New.
            * call-summary.cc
            (call_summary_replay::convert_region_from_summary_1): Handle
            RK_PRIVATE.
            * engine.cc (impl_run_checkers): Pass model manager to
            register_known_functions.
            * kf.cc (class undefined_function_behavior): New.
            (class kf_strtok): New.
            (register_known_functions): Add region_model_manager param.
            Use it to register "strtok".
            * region-model-manager.cc
            (region_model_manager::get_or_create_conjured_svalue): Add "idx"
            param.
            * region-model-manager.h
            (region_model_manager::get_or_create_conjured_svalue): Add "idx"
            param.
            (region_model_manager::get_root_region): New accessor.
            * region-model.cc (region_model::scan_for_null_terminator): Handle
            "expr" being null.
            (region_model::get_representative_path_var_1): Handle RK_PRIVATE.
            * region-model.h (region_model::called_from_main_p): Make public.
            * region.cc (region::get_memory_space): Handle RK_PRIVATE.
            (region::can_have_initial_svalue_p): Handle MEMSPACE_PRIVATE.
            (private_region::dump_to_pp): New.
            * region.h (MEMSPACE_PRIVATE): New.
            (RK_PRIVATE): New.
            (class private_region): New.
            (is_a_helper <const private_region *>::test): New.
            * store.cc (store::replay_call_summary_cluster): Handle
            RK_PRIVATE.
            * svalue.h (struct conjured_svalue::key_t): Add "idx" param to
            ctor and "m_idx" field.
            (class conjured_svalue::conjured_svalue): Likewise.

    gcc/ChangeLog:
            PR analyzer/107573
            * doc/invoke.texi: Add -Wanalyzer-undefined-behavior-strtok.

    gcc/testsuite/ChangeLog:
            PR analyzer/107573
            * c-c++-common/analyzer/strtok-1.c: New test.
            * c-c++-common/analyzer/strtok-2.c: New test.
            * c-c++-common/analyzer/strtok-3.c: New test.
            * c-c++-common/analyzer/strtok-4.c: New test.
            * c-c++-common/analyzer/strtok-cppreference.c: New test.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>