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)
