dead code / nullptr issue in elfchk

[Jeff Johnson]

There are two instances of an interesting pointer handling situation. This
one is from misc-test/elfchk/sections.c:checkElfSection(), pasted from
Coverity report.

319void
320checkElfsection(int index, ElfFile *file1, struct tetj_handle *journal)
321{
322 Elf_Shdr *hdr1;
323 int i;
324 int fail = 0;
325#define TMP_STRING_SIZE (200)
326 char tmp_string[TMP_STRING_SIZE+1];
327 int optional_flags = 0;
328
329 if ( index == 0 ) return; /* A dummy section */
330

assignment: Assigning: "hdr1" = "file1->saddr + index".
331 hdr1 = &(file1->saddr[index]);

notnull: At condition "hdr1", the value of "hdr1" cannot be NULL.

dead_error_condition: The condition "!hdr1" cannot be true.

CID 1099442 (#1 of 1): Logically dead code (DEADCODE)dead_error_line: Execution
cannot reach this statement "return;".
332 if (!hdr1) return; /* moved above VERBOSE block to avoid nullptr deref
*/
333
334#ifdef VERBOSE
335 fprintf( stderr, "checkElfsection[%d]: %s\n", index,
336 ElfGetString(file1, hdr1->sh_name));
337#endif /* VERBOSE */

Almost the same sequence exists in
misc-test/elfchk/proghdr.c:checkElfproghdr():

660void
661checkElfproghdr(int index, ElfFile * file, struct tetj_handle *journal)
662{
663 int i;
664 Elf_Phdr *hdr;
665

assignment: Assigning: "hdr" = "file->paddr + index".
666 hdr = &(file->paddr[index]);
667

notnull: At condition "hdr", the value of "hdr" cannot be NULL.

dead_error_condition: The condition "!hdr" cannot be true.
668 if (!hdr)

CID 1099441 (#1 of 1): Logically dead code (DEADCODE)dead_error_line: Execution
cannot reach this statement "return;".
669 return;
670
671 if (elfchk_debug & DEBUG_PROGRAM_HEADERS)
672 fprintf(stderr, "Header[%2d] type %x\n", index, hdr->p_type);

======================
Commentary:

The relevant bits of struct ElfFile are:

        Elf_Shdr *saddr; /* Start address of next section of file */
        Elf_Phdr *paddr; /* address of the program header of file */

So these two are pointers which we index as arrays. The assignment in
both cases takes address-of (line 331 in first snip, like 666 in second),
which means even if the value at "index" is null, hdr1/hdr will not
be null. But the comment on the first snip (line 332) indicates that
the reference hdr1->sh_name can be a null defererence.

All this tells me something's dicey about this code!
[reply] [-] Comment 1