public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug analyzer/94976] New: Oddities with -fanalyzer and -flto (SSA names leaking through)
@ 2020-05-06 16:05 mark at gcc dot gnu.org
  0 siblings, 0 replies; only message in thread
From: mark at gcc dot gnu.org @ 2020-05-06 16:05 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 94976
           Summary: Oddities with -fanalyzer and -flto (SSA names leaking
                    through)
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: mark at gcc dot gnu.org
  Target Milestone: ---

$ gcc --version
gcc (GCC) 10.0.1 20200430 (Red Hat 10.0.1-0.13)

This is the build of elfutils libelf with both -flto and -fanalyzer.
The report is correct. __elf64_getshdr_rdlock () can return NULL and we are
directly dereferencing the result without a NULL check.

The report could be a bit better though:

In function ‘elf_strptr’:
elf_strptr.c:148:11: warning: dereference of NULL ‘_67’ [CWE-690]
[-Wanalyzer-null-dereference]
  148 |       if (unlikely (shdr->sh_type != SHT_STRTAB))
      |           ^
  ‘elf_strptr’: events 1-14
    |
    |   73 | elf_strptr (Elf *elf, size_t idx, size_t offset)
    |      | ^
    |      | |
    |      | (1) entry to ‘elf_strptr’
    |   74 | {
    |   75 |   if (elf == NULL)
    |      |      ~
    |      |      |
    |      |      (2) following ‘false’ branch (when ‘elf_77(D)’ is
non-NULL)...
    |......
    |   78 |   if (elf->kind != ELF_K_ELF)
    |      |   ~  ~
    |      |   |  |
    |      |   |  (4) following ‘false’ branch...
    |      |   (3) ...to here
    |......
    |   84 |   rwlock_rdlock (elf->lock);
    |      |   ~
    |      |   |
    |      |   (5) ...to here
    |......
    |   96 |       if (idx < runp->max)
    |      |          ~
    |      |          |
    |      |          (6) following ‘true’ branch...
    |   97 |  {
    |   98 |    if (idx < runp->cnt)
    |      |    ~  ~
    |      |    |  |
    |      |    |  (8) following ‘true’ branch...
    |      |    (7) ...to here
    |   99 |      strscn = &runp->data[idx];
    |      |      ~
    |      |      |
    |      |      (9) ...to here
    |......
    |  119 |   if (elf->class == ELFCLASS32)
    |      |      ~
    |      |      |
    |      |      (10) following ‘false’ branch...
    |......
    |  147 |       Elf64_Shdr *shdr = strscn->shdr.e64 ?:
__elf64_getshdr_rdlock (strscn);
    |      |       ~                                    ~ ~
    |      |       |                                    | |
    |      |       |                                    | (13) ...to here
    |      |       |                                    | (14) calling
‘__elf64_getshdr_rdlock’ from ‘elf_strptr’
    |      |       (11) ...to here                      (12) following ‘false’
branch...
    |
    +--> ‘__elf64_getshdr_rdlock’: events 15-16
           |
           |elf32_getshdr.c:250:1:
           |  250 | __elfw2(LIBELFBITS,getshdr_rdlock) (Elf_Scn *scn)
           |      | ^
           |      | |
           |      | (15) entry to ‘__elf64_getshdr_rdlock’
           |......
           |  254 |   if (!scn_valid (scn))
           |      |        ~
           |      |        |
           |      |        (16) calling ‘scn_valid’ from
‘__elf64_getshdr_rdlock’
           |
           +--> ‘scn_valid’: events 17-22
                  |
                  |  228 | scn_valid (Elf_Scn *scn)
                  |      | ^
                  |      | |
                  |      | (17) entry to ‘scn_valid’
                  |  229 | {
                  |  230 |   if (scn == NULL)
                  |      |      ~
                  |      |      |
                  |      |      (18) following ‘false’ branch (when ‘scn_12(D)’
is non-NULL)...
                  |......
                  |  233 |   if (unlikely (scn->elf->state.elf.ehdr == NULL))
                  |      |   ~  ~
                  |      |   |  |
                  |      |   |  (20) following ‘true’ branch...
                  |      |   (19) ...to here
                  |  234 |     {
                  |  235 |       __libelf_seterrno (ELF_E_WRONG_ORDER_EHDR);
                  |      |       ~
                  |      |       |
                  |      |       (21) ...to here
                  |      |       (22) calling ‘__libelf_seterrno’ from
‘scn_valid’
                  |
                  +--> ‘__libelf_seterrno’: events 23-25
                         |
                         |elf_error.c:334:1:
                         |  334 | __libelf_seterrno (int value)
                         |      | ^
                         |      | |
                         |      | (23) entry to ‘__libelf_seterrno’
                         |  335 | {
                         |  336 |   global_error = value >= 0 && value <
nmsgidx ? value : ELF_E_UNKNOWN_ERROR;
                         |      |                ~                             
         ~
                         |      |                |                             
         |
                         |      |                (25) ...to here               
         (24) following ‘true’ branch...
                         |
                  <------+
                  |
                ‘scn_valid’: event 26
                  |
                  |elf32_getshdr.c:235:7:
                  |  235 |       __libelf_seterrno (ELF_E_WRONG_ORDER_EHDR);
                  |      |       ^
                  |      |       |
                  |      |       (26) returning to ‘scn_valid’ from
‘__libelf_seterrno’
                  |
           <------+
           |
         ‘__elf64_getshdr_rdlock’: events 27-28
           |
           |  254 |   if (!scn_valid (scn))
           |      |      ~ ^
           |      |      | |
           |      |      | (27) returning to ‘__elf64_getshdr_rdlock’ from
‘scn_valid’
           |      |      (28) following ‘false’ branch...
           |
         ‘__elf64_getshdr_rdlock’: event 29
           |
           |lto1:
           | (29): ...to here
           |
         ‘__elf64_getshdr_rdlock’: event 30
           |
           |lto1:
           | (30): ‘<anonymous>’ is NULL
           |
    <------+
    |
  ‘elf_strptr’: events 31-32
    |
    |elf_strptr.c:147:46:
    |  147 |       Elf64_Shdr *shdr = strscn->shdr.e64 ?:
__elf64_getshdr_rdlock (strscn);
    |      |                                              ^
    |      |                                              |
    |      |                                              (31) return of NULL
to ‘elf_strptr’ from ‘__elf64_getshdr_rdlock’
    |  148 |       if (unlikely (shdr->sh_type != SHT_STRTAB))
    |      |           ~                                   
    |      |           |
    |      |           (32) dereference of NULL ‘_67’
    |

First it is very long. I kind of trust the analyzer correctly analyzing that
__elf64_getshdr_rdlock could return NULL. It would be nice if there was some
option to show the condensed version (just the last 10 lines).

Second there are several mentions of "SSA names" in the analyzer messages that
aren't really useful. e.g (2) following ‘false’ branch (when ‘elf_77(D)’ is
non-NULL) or (32) dereference of NULL ‘_67’, etc.

Last, the ~ for (32) dereference of NULL should really point at the ->, not the
unlikely macro: # define unlikely(expr) __builtin_expect (!!(expr), 0)

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2020-05-06 16:05 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06 16:05 [Bug analyzer/94976] New: Oddities with -fanalyzer and -flto (SSA names leaking through) mark 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).