[Bug analyzer/96798] New: Analyzer failures on Darwin

[Bug analyzer/96798] New: Analyzer failures on Darwin

[dominiq at lps dot ens.fr]

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

            Bug ID: 96798
           Summary: Analyzer failures on Darwin
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: dominiq at lps dot ens.fr
                CC: iains at gcc dot gnu.org
  Target Milestone: ---
              Host: x86_64-apple-darwin19.6
            Target: x86_64-apple-darwin19.6
             Build: x86_64-apple-darwin19.6

On Darwin I get the following failures

FAIL: gcc.dg/analyzer/data-model-1.c  (test for warnings, line 519)
FAIL: gcc.dg/analyzer/data-model-1.c  (test for warnings, line 520)
FAIL: gcc.dg/analyzer/data-model-1.c (test for excess errors)
FAIL: gcc.dg/analyzer/memset-1.c  (test for warnings, line 67)
FAIL: gcc.dg/analyzer/memset-1.c  (test for warnings, line 68)
FAIL: gcc.dg/analyzer/memset-1.c (test for excess errors)

due to

/opt/gcc/work/gcc/testsuite/gcc.dg/analyzer/data-model-1.c: In function
'test_27':
/opt/gcc/work/gcc/testsuite/gcc.dg/analyzer/data-model-1.c:519:3: warning:
UNKNOWN
  519 |   __analyzer_eval (p->x == 0); /* { dg-warning "TRUE" } */
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/gcc/work/gcc/testsuite/gcc.dg/analyzer/data-model-1.c:520:3: warning:
UNKNOWN
  520 |   __analyzer_eval (p->y == 0); /* { dg-warning "TRUE" } */
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~

and

/opt/gcc/work/gcc/testsuite/gcc.dg/analyzer/memset-1.c:67:3: warning: TRUE
   67 |   __analyzer_eval (buf[42] == 'A'); /* { dg-warning "UNKNOWN" } */
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/gcc/work/gcc/testsuite/gcc.dg/analyzer/memset-1.c:68:3: warning: FALSE
   68 |   __analyzer_eval (buf[42] == '\0'); /* { dg-warning "UNKNOWN" } */
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[Bug analyzer/96798] Analyzer failures on Darwin

[dmalcolm at gcc dot gnu.org]

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

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Sorry about the failures.

I attempted to reproduce this via building a cross-compiler with
build==host==x86_64-pc-linux-gnu and --target=x86_64-apple-darwin19.6; I think
this comes down to the tests in question using the system's <string.h>.

What does the declaration of memset look like in your headers? (including
attributes)

[Bug analyzer/96798] Analyzer failures on Darwin

[dominiq at lps dot ens.fr]

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

--- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
> What does the declaration of memset look like in your headers?
> (including attributes)

void    *memset(void *__b, int __c, size_t __len);

[Bug analyzer/96798] Analyzer failures on Darwin

[dmalcolm at gcc dot gnu.org]

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

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks.  Unfortunately that didn't trigger the issue for me, sorry.

I'm wondering how best to debug this.

Could you attach the preprocessed source from the testcase(s) please?

Also, if you hack in a call to:

  __analyzer_dump ();

immediately *after* the call to memset in memset-1.c's test_5, what do you get?

I'm seeing:

rmodel:
stack depth: 1
  frame (index 0): frame: 'test_5'@1
clusters within frame: 'test_5'@1
  cluster for: buf
    key:   {kind: default, start: 0, size: 2048, next: 2048}
    value: 'char[256]' {UNKNOWN(char[256])}
m_called_unknown_fn: FALSE
constraint_manager:
  equiv classes:
    ec0: {(void *)0B == [m_constant]'0B'}
  constraints:

(signifying among other things that "buf" now has unknown content, that the
memset could have overwritten the earlier write to buf[42], given that "n"
isn't known).

Alternatively, is there a public test box available somewhere?  I didn't see
one in the GCC compile farm.

[Bug analyzer/96798] Analyzer failures on Darwin

[dominiq at lps dot ens.fr]

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

--- Comment #4 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Would it be possible to have a reduced test?

[Bug analyzer/96798] Analyzer failures on Darwin

[dominiq at lps dot ens.fr]

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

--- Comment #5 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
Created attachment 49152
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49152&action=edit
Preprocessed file for the test_5 in memset-1.c

[Bug analyzer/96798] Analyzer failures on Darwin

[dmalcolm at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-08-29
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

--- Comment #6 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks!  The "memset" call has become a call to "__builtin___memset_chk"
(perhaps due to _FORTIFY_SOURCE, or something similar in Darwin's libc?), and
the analyzer doesn't (yet) know about that builtin.

I can reproduce the issue by hacking this into the test:

#define memset(DST, SRC, LEN) \
  __builtin___memset_chk ((DST), (SRC), (LEN), \
                          __builtin_object_size((DST), 0))

There are at least two issues here:
(a) looks like region_model::on_call_pre is erroneously treating a builtin I
haven't coded yet as a no-op; it should instead conservatively assume that any
escaped/reachable regions are affected
(b) the analyzer should handle that builtin (and probably others)

[Bug analyzer/96798] Analyzer failures on Darwin

[iains at gcc dot gnu.org]

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

--- Comment #7 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to David Malcolm from comment #6)
> Thanks!  The "memset" call has become a call to "__builtin___memset_chk"
> (perhaps due to _FORTIFY_SOURCE, or something similar in Darwin's libc?),

(transitive include of strings.h, for macOS >= 10.5)
usr/include/_types.h:#    define _FORTIFY_SOURCE 2      /* on by default */

usr/include/strings.h:

#if defined (__GNUC__) && _FORTIFY_SOURCE > 0 && !defined (__cplusplus)
/* Security checking functions.  */
#include <secure/_strings.h>
#endif


secure/_strings.h:

#if _USE_FORTIFY_LEVEL > 0

....

#if __has_builtin(__builtin___memset_chk) || defined(__GNUC__)
#undef bzero
/* void bzero(void *s, size_t n) */
#define bzero(dest, ...) \
                __builtin___memset_chk (dest, 0, __VA_ARGS__, __darwin_obsz0
(dest))
#endif

(AFAIR, fort

> and the analyzer doesn't (yet) know about that builtin.
> 
> I can reproduce the issue by hacking this into the test:
> 
> #define memset(DST, SRC, LEN) \
>   __builtin___memset_chk ((DST), (SRC), (LEN), \
> 			  __builtin_object_size((DST), 0))
> 
> There are at least two issues here:
> (a) looks like region_model::on_call_pre is erroneously treating a builtin I
> haven't coded yet as a no-op; it should instead conservatively assume that
> any escaped/reachable regions are affected
> (b) the analyzer should handle that builtin (and probably others)

[Bug analyzer/96798] Analyzer failures on Darwin

[iains at gcc dot gnu.org]

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

--- Comment #8 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #7)
> (In reply to David Malcolm from comment #6)
> > Thanks!  The "memset" call has become a call to "__builtin___memset_chk"
> > (perhaps due to _FORTIFY_SOURCE, or something similar in Darwin's libc?),
> 
> (transitive include of strings.h, for macOS >= 10.5)
> usr/include/_types.h:#    define _FORTIFY_SOURCE 2	/* on by default */
> 
> usr/include/strings.h:
> 
> #if defined (__GNUC__) && _FORTIFY_SOURCE > 0 && !defined (__cplusplus)
> /* Security checking functions.  */
> #include <secure/_strings.h>
> #endif
> 
> 
> secure/_strings.h:
> 
> #if _USE_FORTIFY_LEVEL > 0
> 
> ....
> 
> #if __has_builtin(__builtin___memset_chk) || defined(__GNUC__)
> #undef bzero
> /* void bzero(void *s, size_t n) */
> #define bzero(dest, ...) \
>                 __builtin___memset_chk (dest, 0, __VA_ARGS__, __darwin_obsz0
> (dest))
> #endif
> 

Oops hit send too soon.

string.h is a transitive include of strings.h and has:
#if defined (__GNUC__) && _FORTIFY_SOURCE > 0 && !defined (__cplusplus)
/* Security checking functions.  */
#include <secure/_string.h>
#endif

_strings.h:

#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 70000 ||
__MAC_OS_X_VERSION_MIN_REQUIRED >= 1090 || \
                defined(__DRIVERKIT_VERSION_MIN_REQUIRED)
#if __has_builtin(__builtin___memccpy_chk) && __HAS_FIXED_CHK_PROTOTYPES
#undef memccpy
/* void *memccpy(void *dst, const void *src, int c, size_t n) */
#define memccpy(dest, ...) \
                __builtin___memccpy_chk (dest, __VA_ARGS__, __darwin_obsz0
(dest))
#endif
#endif

So.. essentially, the checked builtins are going to be used everywhere by
default on modern Darwin (and some are going to be used even on venerable
Darwin).

[Bug analyzer/96798] Analyzer failures on Darwin

[iains at gcc dot gnu.org]

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

--- Comment #9 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #8)
> (In reply to Iain Sandoe from comment #7)
> > (In reply to David Malcolm from comment #6)
> > > Thanks!  The "memset" call has become a call to "__builtin___memset_chk"
> > > (perhaps due to _FORTIFY_SOURCE, or something similar in Darwin's libc?),
> > 
> > (transitive include of strings.h, for macOS >= 10.5)
> > usr/include/_types.h:#    define _FORTIFY_SOURCE 2	/* on by default */
> > 
> > usr/include/strings.h:
> > 
> > #if defined (__GNUC__) && _FORTIFY_SOURCE > 0 && !defined (__cplusplus)
> > /* Security checking functions.  */
> > #include <secure/_strings.h>
> > #endif
> > 
> > 
> > secure/_strings.h:
> > 
> > #if _USE_FORTIFY_LEVEL > 0
> > 
> > ....
> > 
> > #if __has_builtin(__builtin___memset_chk) || defined(__GNUC__)
> > #undef bzero
> > /* void bzero(void *s, size_t n) */
> > #define bzero(dest, ...) \
> >                 __builtin___memset_chk (dest, 0, __VA_ARGS__, __darwin_obsz0
> > (dest))
> > #endif
> > 
> 
> Oops hit send too soon.
> 
> string.h is a transitive include of strings.h and has:
> #if defined (__GNUC__) && _FORTIFY_SOURCE > 0 && !defined (__cplusplus)
> /* Security checking functions.  */
> #include <secure/_string.h>
> #endif
> 
> _strings.h:
^^ typo -- secure/_string.h:
> 
> #if __IPHONE_OS_VERSION_MIN_REQUIRED >= 70000 ||
> __MAC_OS_X_VERSION_MIN_REQUIRED >= 1090 || \
>                 defined(__DRIVERKIT_VERSION_MIN_REQUIRED)
> #if __has_builtin(__builtin___memccpy_chk) && __HAS_FIXED_CHK_PROTOTYPES
> #undef memccpy
> /* void *memccpy(void *dst, const void *src, int c, size_t n) */
> #define memccpy(dest, ...) \
>                 __builtin___memccpy_chk (dest, __VA_ARGS__, __darwin_obsz0
> (dest))
> #endif
> #endif
> 
> So.. essentially, the checked builtins are going to be used everywhere by
> default on modern Darwin (and some are going to be used even on venerable
> Darwin).

[Bug analyzer/96798] Analyzer failures on Darwin

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 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:ee7bfbe5eb70a23bbf3a2cedfdcbd2ea1a20c3f2

commit r11-2956-gee7bfbe5eb70a23bbf3a2cedfdcbd2ea1a20c3f2
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Aug 31 16:20:55 2020 -0400

    analyzer: gather builtin/internal fn handling into switch statements

    Clean up this code in preparation for fixing PR analyzer/96798.

    gcc/analyzer/ChangeLog:
            * region-model.cc (region_model::on_call_pre): Gather handling of
            builtins and of internal fns into switch statements.  Handle
            "alloca" and BUILT_IN_ALLOCA_WITH_ALIGN.

[Bug analyzer/96798] Analyzer failures on Darwin

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 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:bc62bfb0f43eeada02cb924e3cb5457a399b01c0

commit r11-2957-gbc62bfb0f43eeada02cb924e3cb5457a399b01c0
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Aug 31 11:55:34 2020 -0400

    analyzer: handle __builtin___memset_chk [PR96798]

    gcc/analyzer/ChangeLog:
            PR analyzer/96798
            * region-model.cc (region_model::on_call_pre): Handle
            BUILT_IN_MEMSET_CHK.

    gcc/testsuite/ChangeLog:
            PR analyzer/96798
            * gcc.dg/analyzer/memset-1.c (test_5a): New.

[Bug analyzer/96798] Analyzer failures on Darwin

[dmalcolm at gcc dot gnu.org]

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

--- Comment #12 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Does r11-2957-gbc62bfb0f43eeada02cb924e3cb5457a399b01c0 fix the failing tests
seen on Darwin?

In any case, I should probably also fix:
> (a) looks like region_model::on_call_pre is erroneously treating a builtin I
> haven't coded yet as a no-op; it should instead conservatively assume that
> any escaped/reachable regions are affected
(replying to myself from comment #6)

[Bug analyzer/96798] Analyzer failures on Darwin

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 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:b7028f060c6760b336b416897412e327ded12ab5

commit r11-3130-gb7028f060c6760b336b416897412e327ded12ab5
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Aug 31 15:55:45 2020 -0400

    analyzer: stricter handling of non-pure builtins [PR96798]

    Amongst other things PR analyzer/96798 notes that
    region_model::on_call_pre treats any builtin that hasn't been coded
    yet as a no-op (albeit with an unknown return value), which is wrong
    for non-pure builtins.

    This patch updates that function's handling of such builtins so that it
    instead conservatively assumes that any escaped/reachable regions can
    be affected by the call, and implements enough handling of specific
    builtins to avoid regressing the testsuite (I hope).

    gcc/analyzer/ChangeLog:
            PR analyzer/96798
            * region-model-impl-calls.cc (region_model::impl_call_memcpy):
            New.
            (region_model::impl_call_strcpy): New.
            * region-model.cc (region_model::on_call_pre): Flag unhandled
            builtins that are non-pure as having unknown side-effects.
            Implement BUILT_IN_MEMCPY, BUILT_IN_MEMCPY_CHK, BUILT_IN_STRCPY,
            BUILT_IN_STRCPY_CHK, BUILT_IN_FPRINTF, BUILT_IN_FPRINTF_UNLOCKED,
            BUILT_IN_PUTC, BUILT_IN_PUTC_UNLOCKED, BUILT_IN_FPUTC,
            BUILT_IN_FPUTC_UNLOCKED, BUILT_IN_FPUTS, BUILT_IN_FPUTS_UNLOCKED,
            BUILT_IN_FWRITE, BUILT_IN_FWRITE_UNLOCKED, BUILT_IN_PRINTF,
            BUILT_IN_PRINTF_UNLOCKED, BUILT_IN_PUTCHAR,
            BUILT_IN_PUTCHAR_UNLOCKED, BUILT_IN_PUTS, BUILT_IN_PUTS_UNLOCKED,
            BUILT_IN_VFPRINTF, BUILT_IN_VPRINTF.
            * region-model.h (region_model::impl_call_memcpy): New decl.
            (region_model::impl_call_strcpy): New decl.

    gcc/testsuite/ChangeLog:
            PR analyzer/96798
            * gcc.dg/analyzer/memcpy-1.c: New test.
            * gcc.dg/analyzer/strcpy-1.c: New test.

[Bug analyzer/96798] Analyzer failures on Darwin

[dmalcolm at gcc dot gnu.org]

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

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

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

--- Comment #14 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Hopefully the above commits fix this for you; marking as resolved.  Please
reopen if I've messed up.