[Bug ipa/114408] New: Crash when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[Bug ipa/114408] New: Crash when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[gabravier at gmail dot com]

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

            Bug ID: 114408
           Summary: Crash when invoking strcmp multiple times with
                    -fsanitize=undefined -O1 -fanalyzer -flto
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gabravier at gmail dot com
  Target Milestone: ---

int main(){}

int HMAP_unset_copy(const char *key) {
    return __builtin_strcmp("a", key) + __builtin_strcmp("a", key);
}

Compiling this program with `-fsanitize=undefined -O1 -fanalyzer -flto` results
in the following:

<source>: In function 'HMAP_unset_copy':
<source>:4:41: warning: check of 'key_4(D)' for NULL after already
dereferencing it [-Wanalyzer-deref-before-check]
    4 |     return __builtin_strcmp("a", key) + __builtin_strcmp("a", key);
      |                                         ^
  'HMAP_unset_copy': events 1-2
    |
    |    4 |     return __builtin_strcmp("a", key) + __builtin_strcmp("a",
key);
    |      |            ^                            ~
    |      |            |                            |
    |      |            |                            (2) pointer 'key_4(D)' is
checked for NULL here but it was already dereferenced at (1)
    |      |            (1) pointer 'key_4(D)' is dereferenced here
    |
during IPA pass: whole-program
At top level:
lto1: internal compiler error: in release_function_body, at cgraph.cc:1813
0x221519c internal_error(char const*, ...)
        ???:0
0x926a67 fancy_abort(char const*, int, char const*)
        ???:0
0xa1a687 cgraph_node::release_body(bool)
        ???:0
0xa1c2d2 cgraph_node::remove()
        ???:0
0xcea661 symbol_table::remove_unreachable_nodes(_IO_FILE*)
        ???:0
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
lto-wrapper: fatal error: /opt/compiler-explorer/gcc-snapshot/bin/gcc returned
1 exit status
compilation terminated.
/opt/compiler-explorer/gcc-trunk-20240320/bin/../lib/gcc/x86_64-linux-gnu/14.0.1/../../../../x86_64-linux-gnu/bin/ld:
error: lto-wrapper failed
collect2: error: ld returned 1 exit status
Compiler returned: 1

[Bug analyzer/114408] [13/14 Regression] ICE when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |12.3.0
   Target Milestone|---                         |13.3
           Assignee|unassigned at gcc dot gnu.org      |dmalcolm at gcc dot gnu.org
            Summary|ICE when invoking strcmp    |[13/14 Regression] ICE when
                   |multiple times with         |invoking strcmp multiple
                   |-fsanitize=undefined -O1    |times with
                   |-fanalyzer -flto            |-fsanitize=undefined -O1
                   |                            |-fanalyzer -flto
                 CC|                            |dmalcolm at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
      Known to fail|                            |13.2.1
          Component|ipa                         |analyzer
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-03-21

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed also with GCC 13, works with GCC 12.

The diagnostic is because of UBSAN instrumentation, it's already visible
without -flto.

The issue with the ICE is likely that the sanitizer computes dominance info at
IPA time but fails to release it?

I see in sm-malloc.cc:

    /* Reject the warning if the deref's BB doesn't dominate that
       of the check, so that we don't warn e.g. for shared cleanup
       code that checks a pointer for NULL, when that code is sometimes
       used before a deref and sometimes after.
       Using the dominance code requires setting cfun.  */
    auto_cfun sentinel (m_deref_enode->get_function ());
    calculate_dominance_info (CDI_DOMINATORS);
    if (!dominated_by_p (CDI_DOMINATORS,
                         m_check_enode->get_supernode ()->m_bb,
                         m_deref_enode->get_supernode ()->m_bb))
      return false;

    return ctxt.warn ("check of %qE for NULL after already"
                      " dereferencing it",
                      m_arg);

but no free_dominance_info anywhere.  It would of course be quite expensive
to re-compute all dominance info every time here, so analyzer needs to
loop over all functions releasing dominance info when done.

[Bug analyzer/114408] [13/14 Regression] ICE when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[law at gcc dot gnu.org]

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

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at gcc dot gnu.org
           Priority|P3                          |P1

[Bug analyzer/114408] [13/14 Regression] ICE when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[dmalcolm at gcc dot gnu.org]

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

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Created attachment 57781
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57781&action=edit
WIP patch for the the ICE

The attached patch seems to fix the ICE.  AIUI I'm lazily creating dominance
info as it's needed; calculate_dominance_info has this early exit:

  if (dom_computed[dir_index] == DOM_OK)
    {
      checking_verify_dominators (dir);
      return;
    }

and free_dominance_info has this early exit:

  if (!dom_info_available_p (fn, dir))
    return;

So iterating through all funs with gimple bodies at the end of analyzer calling
free_dominance_info on them ought to clean things up - and seems to fix the
ICE.

However I'm having trouble writing a regression test for this, with the
combination of ubsan and lto: I get:

output is /usr/bin/ld: cannot find -lubsan
collect2: error: ld returned 1 exit status

Ideas on fixing welcome.

[Bug analyzer/114408] [13/14 Regression] ICE when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #2)
> However I'm having trouble writing a regression test for this, with the
> combination of ubsan and lto: I get:
> 
> output is /usr/bin/ld: cannot find -lubsan
> collect2: error: ld returned 1 exit status

The test would need to go into gcc.dg/ubsan/ directory (or g++.dg/ubsan/ for
C++), that is where the *.exp files arrange for -lubsan to be found.
The test would of course then need to use analyzer effective target in it and
add whatever dg-options are needed.

[Bug analyzer/114408] [13/14 Regression] ICE when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Or the option option would be try if it also ICEs without your patch with
-fsanitize=undefined -fsanitize-trap=undefined -O1 -fanalyzer -flto , then you
could put it into gcc.dg/analyzer/ and just use fsanitize_undefined effective
target there.

[Bug analyzer/114408] [13/14 Regression] ICE when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks; I have it reproducing in DejaGnu now (and the ICE fix).

Am looking at fixing the false postive.

[Bug analyzer/114408] [13/14 Regression] ICE when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 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:80a0cb37456c49dbc25cca7cd554f78bc504373e

commit r14-9646-g80a0cb37456c49dbc25cca7cd554f78bc504373e
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Sat Mar 23 09:52:38 2024 -0400

    analyzer: fix ICE and false positive with -Wanalyzer-deref-before-check
[PR114408]

    gcc/analyzer/ChangeLog:
            PR analyzer/114408
            * engine.cc (impl_run_checkers): Free up any dominance info that
            we may have created.
            * kf.cc (class kf_ubsan_handler): New.
            (register_sanitizer_builtins): New.
            (register_known_functions): Call register_sanitizer_builtins.

    gcc/testsuite/ChangeLog:
            PR analyzer/114408
            * c-c++-common/analyzer/deref-before-check-pr114408.c: New test.
            * c-c++-common/ubsan/analyzer-ice-pr114408.c: New test.

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

[Bug analyzer/114408] [13 Regression] ICE when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[13/14 Regression] ICE when |[13 Regression] ICE when
                   |invoking strcmp multiple    |invoking strcmp multiple
                   |times with                  |times with
                   |-fsanitize=undefined -O1    |-fsanitize=undefined -O1
                   |-fanalyzer -flto            |-fanalyzer -flto

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

The ICE was introduced by r13-5261-g0d6f7b1dd62e9c9dccb0b9b673f9cc3238b7ea6d
when fixing bug 108455.  Keeping open to track backporting to GCC 13.

[Bug analyzer/114408] [13 Regression] ICE when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by David Malcolm
<dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:96f7a3694c3e4c72af6258cc9b38bce30e609bee

commit r13-8758-g96f7a3694c3e4c72af6258cc9b38bce30e609bee
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu May 9 13:09:32 2024 -0400

    analyzer: fix ICE and false positive with -Wanalyzer-deref-before-check
[PR114408]

    Backported from commit r14-9646-g80a0cb37456c49 (moving testcase to gcc.dg
    and handling conflict in kf.cc)

    gcc/analyzer/ChangeLog:
            PR analyzer/114408
            * engine.cc (impl_run_checkers): Free up any dominance info that
            we may have created.
            * kf.cc (class kf_ubsan_handler): New.
            (register_sanitizer_builtins): New.
            (register_known_functions): Call register_sanitizer_builtins.

    gcc/testsuite/ChangeLog:
            PR analyzer/114408
            * gcc.dg/analyzer/deref-before-check-pr114408.c: New test.
            * c-c++-common/ubsan/analyzer-ice-pr114408.c: New test.

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

[Bug analyzer/114408] ICE when invoking strcmp multiple times with -fsanitize=undefined -O1 -fanalyzer -flto

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
            Summary|[13 Regression] ICE when    |ICE when invoking strcmp
                   |invoking strcmp multiple    |multiple times with
                   |times with                  |-fsanitize=undefined -O1
                   |-fsanitize=undefined -O1    |-fanalyzer -flto
                   |-fanalyzer -flto            |

--- Comment #9 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed for GCC 13 (for the upcoming GCC 13.3) by the above patch.