public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug analyzer/108475] New: -Wanalyzer-deref-before-check false positives seen in haproxy's tcpcheck.c
@ 2023-01-20  1:48 dmalcolm at gcc dot gnu.org
  2023-03-10 13:22 ` [Bug analyzer/108475] " cvs-commit at gcc dot gnu.org
  2023-03-10 13:57 ` dmalcolm at gcc dot gnu.org
  0 siblings, 2 replies; 3+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2023-01-20  1:48 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 108475
           Summary: -Wanalyzer-deref-before-check false positives seen in
                    haproxy's tcpcheck.c
           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
  Target Milestone: ---

Created attachment 54314
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54314&action=edit
Reproducer

I'm attaching a reduced reproducer for a false +ve from
-Wanalyzer-deref-before-check

https://godbolt.org/z/s9fvbT3fP

Trunk emits this, all of which appear to be a false positives:

<source>: In function 'proxy_parse_httpchk_req':
<source>:138:6: warning: check of 'meth' for NULL after already dereferencing
it [-Wanalyzer-deref-before-check]
  138 |   if (meth) {
      |      ^
  'proxy_parse_httpchk_req': events 1-14
    |
    |  110 |   if (hdrs || body) {
    |      |      ^
    |      |      |
    |      |      (1) following 'false' branch...
    |......
    |  115 |   chk = calloc(1, sizeof(*chk));
    |      |         ~~~~~~~~~~~~~~~~~~~~~~~
    |      |         |
    |      |         (2) ...to here
    |  116 |   if (!chk) {
    |      |      ~
    |      |      |
    |      |      (3) following 'false' branch (when 'chk' is non-NULL)...
    |......
    |  120 |   chk->action = TCPCHK_ACT_SEND;
    |      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |               |
    |      |               (4) ...to here
    |......
    |  127 |   if (*args[cur_arg]) {
    |      |      ~~~~~~~~~~~~~~~
    |      |      ||
    |      |      |(5) pointer 'meth' is dereferenced here
    |      |      (6) following 'true' branch...
    |  128 |     if (!*args[cur_arg + 1])
    |      |        ~      ~
    |      |        |      |
    |      |        |      (7) ...to here
    |      |        (8) following 'false' branch...
    |......
    |  131 |       meth = args[cur_arg];
    |      |                  ~
    |      |                  |
    |      |                  (9) ...to here
    |  132 |   }
    |  133 |   if (*args[cur_arg + 1])
    |      |      ~
    |      |      |
    |      |      (10) following 'true' branch...
    |  134 |     uri = args[cur_arg + 1];
    |      |               ~
    |      |               |
    |      |               (11) ...to here
    |  135 |   if (*args[cur_arg + 2])
    |      |      ~
    |      |      |
    |      |      (12) following 'false' branch...
    |......
    |  138 |   if (meth) {
    |      |      ~
    |      |      |
    |      |      (13) ...to here
    |      |      (14) pointer 'meth' is checked for NULL here but it was
already dereferenced at (5)
    |
<source>:147:6: warning: check of 'uri' for NULL after already dereferencing it
[-Wanalyzer-deref-before-check]
  147 |   if (uri) {
      |      ^
  'proxy_parse_httpchk_req': events 1-18
    |
    |  110 |   if (hdrs || body) {
    |      |      ^
    |      |      |
    |      |      (1) following 'false' branch...
    |......
    |  115 |   chk = calloc(1, sizeof(*chk));
    |      |         ~~~~~~~~~~~~~~~~~~~~~~~
    |      |         |
    |      |         (2) ...to here
    |  116 |   if (!chk) {
    |      |      ~
    |      |      |
    |      |      (3) following 'false' branch (when 'chk' is non-NULL)...
    |......
    |  120 |   chk->action = TCPCHK_ACT_SEND;
    |      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |               |
    |      |               (4) ...to here
    |......
    |  127 |   if (*args[cur_arg]) {
    |      |      ~
    |      |      |
    |      |      (5) following 'true' branch...
    |  128 |     if (!*args[cur_arg + 1])
    |      |        ~ ~~~~~~~~~~~~~~~~~~
    |      |        | |    |
    |      |        | |    (6) ...to here
    |      |        | (7) pointer 'uri' is dereferenced here
    |      |        (8) following 'false' branch...
    |......
    |  131 |       meth = args[cur_arg];
    |      |                  ~
    |      |                  |
    |      |                  (9) ...to here
    |  132 |   }
    |  133 |   if (*args[cur_arg + 1])
    |      |      ~
    |      |      |
    |      |      (10) following 'true' branch...
    |  134 |     uri = args[cur_arg + 1];
    |      |               ~
    |      |               |
    |      |               (11) ...to here
    |  135 |   if (*args[cur_arg + 2])
    |      |      ~
    |      |      |
    |      |      (12) following 'false' branch...
    |......
    |  138 |   if (meth) {
    |      |      ~
    |      |      |
    |      |      (13) ...to here
    |      |      (14) following 'true' branch (when 'meth' is non-NULL)...
    |  139 |     chk->send.http.meth.meth = find_http_meth(meth, strlen(meth));
    |      |                                                     ~~~~~~~~~~~~
    |      |                                                     |
    |      |                                                     (15) ...to
here
    |......
    |  142 |     if (!chk->send.http.meth.str.area) {
    |      |        ~
    |      |        |
    |      |        (16) following 'false' branch...
    |......
    |  147 |   if (uri) {
    |      |      ~
    |      |      |
    |      |      (17) ...to here
    |      |      (18) pointer 'uri' is checked for NULL here but it was
already dereferenced at (7)
    |
<source>:154:6: warning: check of 'vsn' for NULL after already dereferencing it
[-Wanalyzer-deref-before-check]
  154 |   if (vsn) {
      |      ^
  'proxy_parse_httpchk_req': events 1-12
    |
    |  108 |   hdrs = (*args[cur_arg + 2] ? strstr(args[cur_arg + 2], "\r\n") :
NULL);
    |      |           ^~~~~~~~~~~~~~~~~~
    |      |           |
    |      |           (1) pointer 'vsn' is dereferenced here
    |  109 |   body = (*args[cur_arg + 2] ? strstr(args[cur_arg + 2],
"\r\n\r\n") : NULL);
    |  110 |   if (hdrs || body) {
    |      |      ~     
    |      |      |
    |      |      (2) following 'false' branch...
    |......
    |  115 |   chk = calloc(1, sizeof(*chk));
    |      |         ~~~~~~~~~~~~~~~~~~~~~~~
    |      |         |
    |      |         (3) ...to here
    |  116 |   if (!chk) {
    |      |      ~     
    |      |      |
    |      |      (4) following 'false' branch (when 'chk' is non-NULL)...
    |......
    |  120 |   chk->action = TCPCHK_ACT_SEND;
    |      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |               |
    |      |               (5) ...to here
    |......
    |  135 |   if (*args[cur_arg + 2])
    |      |      ~     
    |      |      |
    |      |      (6) following 'true' branch...
    |  136 |     vsn = args[cur_arg + 2];
    |      |               ~
    |      |               |
    |      |               (7) ...to here
    |  137 | 
    |  138 |   if (meth) {
    |      |      ~     
    |      |      |
    |      |      (8) following 'false' branch (when 'meth' is NULL)...
    |......
    |  147 |   if (uri) {
    |      |      ~     
    |      |      |
    |      |      (9) ...to here
    |      |      (10) following 'false' branch (when 'uri' is NULL)...
    |......
    |  154 |   if (vsn) {
    |      |      ~     
    |      |      |
    |      |      (11) ...to here
    |      |      (12) pointer 'vsn' is checked for NULL here but it was
already dereferenced at (1)
    |

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Bug analyzer/108475] -Wanalyzer-deref-before-check false positives seen in haproxy's tcpcheck.c
  2023-01-20  1:48 [Bug analyzer/108475] New: -Wanalyzer-deref-before-check false positives seen in haproxy's tcpcheck.c dmalcolm at gcc dot gnu.org
@ 2023-03-10 13:22 ` cvs-commit at gcc dot gnu.org
  2023-03-10 13:57 ` dmalcolm at gcc dot gnu.org
  1 sibling, 0 replies; 3+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-03-10 13:22 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #1 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:c4fd232f9843bb800548a906653aeb0723cdb411

commit r13-6581-gc4fd232f9843bb800548a906653aeb0723cdb411
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Fri Mar 10 08:20:10 2023 -0500

    analyzer: fix deref-before-check false +ves seen in haproxy
[PR108475,PR109060]

    Integration testing showed various false positives from
    -Wanalyzer-deref-before-check where the expression that's dereferenced
    is different from the one that's checked, but the diagnostic is emitted
    because they both evaluate to the same symbolic value.

    This patch rejects such warnings, unless we have tree expressions for
    both and that both tree expressions are "spelled the same way" i.e.
    would be printed to the same user-facing string.

    gcc/analyzer/ChangeLog:
            PR analyzer/108475
            PR analyzer/109060
            * sm-malloc.cc (deref_before_check::deref_before_check):
            Initialize new field m_deref_expr.  Assert that arg is non-NULL.
            (deref_before_check::emit): Reject cases where the spelling of the
            thing that was dereferenced differs from that of what is checked,
            or if the dereference expression was not found.  Remove code to
            handle NULL m_arg.
            (deref_before_check::describe_state_change): Remove code to handle
            NULL m_arg.
            (deref_before_check::describe_final_event): Likewise.
            (deref_before_check::sufficiently_similar_p): New.
            (deref_before_check::m_deref_expr): New field.
            (malloc_state_machine::maybe_complain_about_deref_before_check):
            Don't warn if the diag_ptr is NULL.

    gcc/testsuite/ChangeLog:
            PR analyzer/108475
            PR analyzer/109060
            * gcc.dg/analyzer/deref-before-check-pr108475-1.c: New test.
            * gcc.dg/analyzer/deref-before-check-pr108475-haproxy-tcpcheck.c:
            New test.
            * gcc.dg/analyzer/deref-before-check-pr109060-haproxy-cfgparse.c:
            New test.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Bug analyzer/108475] -Wanalyzer-deref-before-check false positives seen in haproxy's tcpcheck.c
  2023-01-20  1:48 [Bug analyzer/108475] New: -Wanalyzer-deref-before-check false positives seen in haproxy's tcpcheck.c dmalcolm at gcc dot gnu.org
  2023-03-10 13:22 ` [Bug analyzer/108475] " cvs-commit at gcc dot gnu.org
@ 2023-03-10 13:57 ` dmalcolm at gcc dot gnu.org
  1 sibling, 0 replies; 3+ messages in thread
From: dmalcolm at gcc dot gnu.org @ 2023-03-10 13:57 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed by the above patch.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-03-10 13:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-20  1:48 [Bug analyzer/108475] New: -Wanalyzer-deref-before-check false positives seen in haproxy's tcpcheck.c dmalcolm at gcc dot gnu.org
2023-03-10 13:22 ` [Bug analyzer/108475] " cvs-commit at gcc dot gnu.org
2023-03-10 13:57 ` dmalcolm at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).