[Bug analyzer/103546] New: Analyzer reports null dereference in flex scanners

[Bug analyzer/103546] New: Analyzer reports null dereference in flex scanners

[marc@nieper-wisskirchen.de]

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

            Bug ID: 103546
           Summary: Analyzer reports null dereference in flex scanners
           Product: gcc
           Version: 11.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: marc@nieper-wisskirchen.de
  Target Milestone: ---

I am trying to compile a flex-generated lexical scanner with the analyzer
enabled.  However, the analyzer reports a NULL dereference in the
flex-generated file.

Is this really a bug in Flex? It looks like a false positive to me.

This is the MWE:

$ flex --version
flex 2.6.4
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/11/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none:amdgcn-amdhsa
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 11.2.0-7ubuntu2'
--with-bugurl=file:///usr/share/doc/gcc-11/README.Bugs
--enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++,m2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-11
--program-prefix=x86_64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/lib --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-vtable-verify --enable-plugin --enable-default-pie --with-system-zlib
--enable-libphobos-checking=release --with-target-system-zlib=auto
--enable-objc-gc=auto --enable-multiarch --disable-werror --enable-cet
--with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32
--enable-multilib --with-tune=generic
--enable-offload-targets=nvptx-none=/build/gcc-11-ZPT0kp/gcc-11-11.2.0/debian/tmp-nvptx/usr,amdgcn-amdhsa=/build/gcc-11-ZPT0kp/gcc-11-11.2.0/debian/tmp-gcn/usr
--without-cuda-driver --enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
--with-build-config=bootstrap-lto-lean --enable-link-serialization=2
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.2.0 (Ubuntu 11.2.0-7ubuntu2)
$ cat scan.l
%%
$ flex scan.l
$ gcc -fanalyzer -Werror -c lex.yy.c
In function ‘yy_init_buffer’:
lex.yy.c:1290:26: error: dereference of NULL ‘b’ [CWE-476]
[-Werror=analyzer-null-dereference]

[Bug analyzer/103546] Analyzer reports null dereference in flex scanners

[dmalcolm at gcc dot gnu.org]

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

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-01-05
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for filing this bug report; I too see the false positive (tested with
trunk).

It seems to be an issue with alias handling in the analyzer: if I'm debugging
things correctly, it seems the analyzer "thinks" that various writes through
unrelated pointers could clobber the value of yy_buffer_stack.  Hence we end up
with code paths in which yy_buffer_stack is non-NULL, then a write through a
pointer happens that the analyzer treats as possibly clobbered yy_buffer_stack,
and so it could theoretically be NULL at a subsequent test for NULL, and hence
we have false positives in which yy_buffer_stack is erroneously treated as
being NULL after yyensure_buffer_stack has returned.

It might be possible to fix this by "teaching" the analyzer about TBAA
(type-based alias analysis) so that it can reject some kinds of clobbering; I'm
not yet sure.

[Bug analyzer/103546] Analyzer reports null dereference in flex scanners

[cvs-commit at gcc dot gnu.org]

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

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

commit r12-6323-gd564a83d14252d7db01381f71900b7a68357803b
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu Jan 6 11:39:54 2022 -0500

    analyzer: make use of may_be_aliased in alias detection [PR103546]

    Whilst debugging PR analyzer/103546 (false +ve in flex-generated lexers)
    I noticed that the analyzer was considering that writes through symbolic
    pointers could be treated as clobbering static globals such as:

       static YY_BUFFER_STATE * yy_buffer_stack = NULL;

    even for such variables that never have their address taken.

    This patch fixes this issue at least, so that the analyzer can preserve
    knowledge of such globals on code paths with writes through symbolic
    pointers.

    It does not fix the false +ve in the lexer code.

    gcc/analyzer/ChangeLog:
            PR analyzer/103546
            * store.cc (store::eval_alias_1): Refactor handling of decl
            regions, adding a test for may_be_aliased, rejecting those for
            which it returns false.

    gcc/testsuite/ChangeLog:
            PR analyzer/103546
            * gcc.dg/analyzer/aliasing-3.c: New test.

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

[Bug analyzer/103546] Analyzer reports null dereference in flex scanners

[dmalcolm at gcc dot gnu.org]

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

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
The -Wanalyzer-null-dereference false positive seems to be due to the analyzer
being overzealous about escaping, where it erroneously is treating
yy_buffer_stack as having escaped, and thus can be overwritten by external code
(such as isatty and __errno_location), thus leading to it considering
impossible paths where an external function sets yy_buffer_stack to NULL after
the test for NULL in yyensure_buffer_stack.

Am investigating.

[Bug analyzer/103546] Analyzer reports null dereference in flex scanners

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 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:c1b7d28a5987e74232b7f054849f8bd8ccc7e7de

commit r12-6376-gc1b7d28a5987e74232b7f054849f8bd8ccc7e7de
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Jan 7 13:49:28 2022 -0500

    analyzer: add region::is_named_decl_p

    This patch adds a debug function that I've found handy when debugging
    a problem with handling the decl "yy_buffer_stack" in PR analyzer/103546.

    gcc/analyzer/ChangeLog:
            * region.cc (region::is_named_decl_p): New.
            * region.h (region::is_named_decl_p): New decl.

    gcc/ChangeLog:
            * doc/analyzer.texi (Other Debugging Techniques): Document
            region::is_named_decl_p.

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

[Bug analyzer/103546] Analyzer reports null dereference in flex scanners

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 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:4409152a4acaec5b58a93996088d0df9aaa779b8

commit r12-6377-g4409152a4acaec5b58a93996088d0df9aaa779b8
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Jan 7 13:36:00 2022 -0500

    analyzer: implement __analyzer_dump_escaped

    PR analyzer/103546 seems to involve an issue in how the analyzer
    tracks which decls have escaped, so this patch adds a way to directly
    test this from DejaGnu.

    gcc/analyzer/ChangeLog:
            * region-model-impl-calls.cc (cmp_decls): New.
            (cmp_decls_ptr_ptr): New.
            (region_model::impl_call_analyzer_dump_escaped): New.
            * region-model.cc (region_model::on_stmt_pre): Handle
            __analyzer_dump_escaped.
            * region-model.h (region_model::impl_call_analyzer_dump_escaped):
            New decl.
            * store.h (binding_cluster::get_base_region): New accessor.

    gcc/ChangeLog:
            * doc/analyzer.texi
            (Special Functions for Debugging the Analyzer): Document
            __analyzer_dump_escaped.

    gcc/testsuite/ChangeLog:
            * gcc.dg/analyzer/analyzer-decls.h (__analyzer_dump_escaped): New
            decl.
            * gcc.dg/analyzer/escaping-1.c: New test.

[Bug analyzer/103546] Analyzer reports null dereference in flex scanners

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 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:78a17f4452db9514da7cc8706c654cb98ba0a8e6

commit r13-4399-g78a17f4452db9514da7cc8706c654cb98ba0a8e6
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Tue Nov 29 19:56:27 2022 -0500

    analyzer work on issues with flex-generated lexers [PR103546]

    PR analyzer/103546 tracks various false positives seen on
    flex-generated lexers.

    Whilst investigating them, I noticed an ICE with
    -fanalyzer-call-summaries due to attempting to store sm-state
    for an UNKNOWN svalue, which this patch fixes.

    This patch also provides known_function implementations of all of the
    external functions called by the lexer, reducing the number of false
    positives.

    The patch doesn't eliminate all false positives, but adds integration
    tests to try to establish a baseline from which the remaining false
    positives can be fixed.

    gcc/analyzer/ChangeLog:
            PR analyzer/103546
            * analyzer.h (register_known_file_functions): New decl.
            * program-state.cc (sm_state_map::replay_call_summary): Rejct
            attempts to store sm-state for caller_sval that can't have
            associated state.
            * region-model-impl-calls.cc (register_known_functions): Call
            register_known_file_functions.
            * sm-fd.cc (class kf_isatty): New.
            (register_known_fd_functions): Register it.
            * sm-file.cc (class kf_ferror): New.
            (class kf_fileno): New.
            (class kf_getc): New.
            (register_known_file_functions): New.

    gcc/ChangeLog:
            PR analyzer/103546
            * doc/invoke.texi (Static Analyzer Options): Add isatty, ferror,
            fileno, and getc to the list of functions known to the analyzer.

    gcc/testsuite/ChangeLog:
            PR analyzer/103546
            * gcc.dg/analyzer/ferror-1.c: New test.
            * gcc.dg/analyzer/fileno-1.c: New test.
            * gcc.dg/analyzer/flex-with-call-summaries.c: New test.
            * gcc.dg/analyzer/flex-without-call-summaries.c: New test.
            * gcc.dg/analyzer/getc-1.c: New test.
            * gcc.dg/analyzer/isatty-1.c: New test.

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

[Bug analyzer/103546] Analyzer reports null dereference in flex scanners

[dmalcolm at gcc dot gnu.org]

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

--- Comment #7 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
As it notes, the above patch reduces the number of false positives on
flex-generated scanners, but doesn't fix them all.  Keeping this bug open to
track fixing them.

[Bug analyzer/103546] -Wanalyzer-null-dereference false positives reported on flex-generated scanners

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from GCC 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:08b7462d3ad8e5acd941b7c777c5b26b4064d686

commit r14-6239-g08b7462d3ad8e5acd941b7c777c5b26b4064d686
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Dec 6 19:25:26 2023 -0500

    analyzer: fix taint false positives with UNKNOWN [PR112850]

    PR analyzer/112850 reports a false positive from
    -Wanalyzer-tainted-allocation-size on the Linux kernel [1] where
    -fanalyzer complains that an allocation size is attacker-controlled
    despite the value being correctly sanitized against upper and lower
    limits.

    The root cause is that the expression is sufficiently complex
    to exceed the -param=analyzer-max-svalue-depth= threshold,
    currently at 12, with depth 13, and so it is treated as UNKNOWN.
    Hence the sanitizations are seen as comparisons of an UNKNOWN
    symbolic value against constants, and these were being ignored
    by the taint state machine.

    The expression in question is relatively typical for those seen in
    Linux kernel ioctl handlers, and I was surprised that it had exceeded
    the analyzer's default expression complexity limit.

    This patch addresses this problem in three ways:
    (a) the default value of the threshold parameter is increased, from 12
    to 18, so that such expressions are precisely handled
    (b) adding a new -Wanalyzer-symbol-too-complex to warn when the symbol
    complexity limit is reached.  This is off by default for users, and
    on by default in the test suite.
    (c) the taint state machine handles comparisons against UNKNOWN svalues
    by dropping all taint information on that execution path, so that if
    the complexity limit has been exceeded we don't generate false positives

    As well as fixing the taint false positive (PR analyzer/112850), the
    patch also fixes a couple of leak false positives seen on flex-generated
    scanners (PR analyzer/103546).

    [1] specifically, in sound/core/rawmidi.c's handler for
    SNDRV_RAWMIDI_STREAM_OUTPUT.

    gcc/ChangeLog:
            PR analyzer/103546
            PR analyzer/112850
            * doc/invoke.texi: Add -Wanalyzer-symbol-too-complex.

    gcc/analyzer/ChangeLog:
            PR analyzer/103546
            PR analyzer/112850
            * analyzer.opt (-param=analyzer-max-svalue-depth=): Increase from
            12 to 18.
            (Wanalyzer-symbol-too-complex): New.
            * diagnostic-manager.cc
            (null_assignment_sm_context::clear_all_per_svalue_state): New.
            * engine.cc (impl_sm_context::clear_all_per_svalue_state): New.
            * program-state.cc (sm_state_map::clear_all_per_svalue_state):
            New.
            * program-state.h (sm_state_map::clear_all_per_svalue_state): New
            decl.
            * region-model-manager.cc
            (region_model_manager::reject_if_too_complex): Add
            -Wanalyzer-symbol-too-complex.
            * sm-taint.cc (taint_state_machine::on_condition): Handle
            comparisons against UNKNOWN.
            * sm.h (sm_context::clear_all_per_svalue_state): New.

    gcc/testsuite/ChangeLog:
            PR analyzer/103546
            PR analyzer/112850
            * c-c++-common/analyzer/call-summaries-pr107158-2.c: Add
            -Wno-analyzer-symbol-too-complex.
            * c-c++-common/analyzer/call-summaries-pr107158.c: Likewise.
            *
c-c++-common/analyzer/deref-before-check-pr109060-haproxy-cfgparse.c:
            Likewise.
            * c-c++-common/analyzer/feasibility-3.c: Add
            -Wno-analyzer-too-complex and -Wno-analyzer-symbol-too-complex.
            * c-c++-common/analyzer/flex-with-call-summaries.c: Add
            -Wno-analyzer-symbol-too-complex.  Remove fail for
            PR analyzer/103546 leak false positive.
            * c-c++-common/analyzer/flex-without-call-summaries.c: Remove
            xfail for PR analyzer/103546 leak false positive.
            * c-c++-common/analyzer/infinite-recursion-3.c: Add
            -Wno-analyzer-symbol-too-complex.
            *
c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:
            Likewise.
            *
c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c:
            Likewise.
            * c-c++-common/analyzer/null-deref-pr108400-SoftEtherVPN-WebUi.c:
            Likewise.
            * c-c++-common/analyzer/null-deref-pr108806-qemu.c: Likewise.
            * c-c++-common/analyzer/null-deref-pr108830.c: Likewise.
            * c-c++-common/analyzer/pr94596.c: Likewise.
            * c-c++-common/analyzer/strtok-2.c: Likewise.
            * c-c++-common/analyzer/strtok-4.c: Add -Wno-analyzer-too-complex
            and -Wno-analyzer-symbol-too-complex.
            * c-c++-common/analyzer/strtok-cppreference.c: Likewise.
            * gcc.dg/analyzer/analyzer.exp: Add -Wanalyzer-symbol-too-complex
            to DEFAULT_CFLAGS.
            * gcc.dg/analyzer/attr-const-3.c: Add
            -Wno-analyzer-symbol-too-complex.
            * gcc.dg/analyzer/call-summaries-pr107072.c: Likewise.
            * gcc.dg/analyzer/doom-s_sound-pr108867.c: Likewise.
            * gcc.dg/analyzer/explode-4.c: Likewise.
            * gcc.dg/analyzer/null-deref-pr102671-1.c: Likewise.
            * gcc.dg/analyzer/null-deref-pr105755.c: Likewise.
            * gcc.dg/analyzer/out-of-bounds-curl.c: Likewise.
            * gcc.dg/analyzer/pr101503.c: Likewise.
            * gcc.dg/analyzer/pr103892.c: Add -Wno-analyzer-too-complex and
            -Wno-analyzer-symbol-too-complex.
            * gcc.dg/analyzer/pr94851-4.c: Add
            -Wno-analyzer-symbol-too-complex.
            * gcc.dg/analyzer/pr96860-1.c: Likewise.
            * gcc.dg/analyzer/pr96860-2.c: Likewise.
            * gcc.dg/analyzer/pr98918.c: Likewise.
            * gcc.dg/analyzer/pr99044-2.c: Likewise.
            * gcc.dg/analyzer/uninit-pr108806-qemu.c: Likewise.
            * gcc.dg/analyzer/use-after-free.c: Add -Wno-analyzer-too-complex
            and -Wno-analyzer-symbol-too-complex.
            * gcc.dg/plugin/plugin.exp: Add new tests for
            analyzer_kernel_plugin.c.
            * gcc.dg/plugin/taint-CVE-2011-0521-4.c: Update expected results.
            * gcc.dg/plugin/taint-CVE-2011-0521-5.c: Likewise.
            * gcc.dg/plugin/taint-CVE-2011-0521-6.c: Likewise.
            * gcc.dg/plugin/taint-CVE-2011-0521-5-fixed.c: Remove xfail.
            * gcc.dg/plugin/taint-pr112850-precise.c: New test.
            * gcc.dg/plugin/taint-pr112850-too-complex.c: New test.
            * gcc.dg/plugin/taint-pr112850-unsanitized.c: New test.
            * gcc.dg/plugin/taint-pr112850.c: New test.

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