https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106428
Bug ID: 106428 Summary: -Wanalyzer-file-leak false positive with if ((ptr = fopen(...)) == NULL) ... Product: gcc Version: 12.1.1 Status: UNCONFIRMED Severity: normal Priority: P3 Component: analyzer Assignee: dmalcolm at gcc dot gnu.org Reporter: eggert at cs dot ucla.edu Target Milestone: --- Created attachment 53342 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53342&action=edit Compile with '-O2 -S -fanalyzer' to demonstrate the bug This is GCC 12.1.1 20220507 (Red Hat 12.1.1-1) on x86-64. Compile the attached program t.i with 'gcc -fanalyzer -O2 -S t.i'. It outputs a false alarm as shown at the end of this comment. If I replace lines 12046-12049: if ((ent->v.file.fp = fopen (ent->v.file.name, "r")) == ((void *)0) ) open_fatal (ent->v.file.name); with the following (which is equivalent, since ent->v.file.fp is NULL before the code is executed): FILE *fp = fopen (ent->v.file.name, "r"); if (!fp) open_fatal (ent->v.file.name); ent->v.file.fp = fp; then the first diagnostic, which is a false positive, goes away. Here is the diagnostic output, the first of which is the false positive. t.i: In function ‘read_next_name’: t.i:12046:7: warning: leak of FILE ‘fopen(*ent.v.file.name, "r")’ [CWE-775] [-Wanalyzer-file-leak] 12046 | if ((ent->v.file.fp = fopen (ent->v.file.name, "r")) == | ^ ‘name_scan’: events 1-2 | |13033 | name_scan (const char *file_name) | | ^~~~~~~~~ | | | | | (1) entry to ‘name_scan’ |...... |13039 | struct name *cursor = namelist_match (file_name, length); | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (2) calling ‘namelist_match’ from ‘name_scan’ | +--> ‘namelist_match’: events 3-5 | |12316 | namelist_match (char const *file_name, size_t length) | | ^~~~~~~~~~~~~~ | | | | | (3) entry to ‘namelist_match’ |...... |12320 | for (p = namelist; p; p = p->next) | | ~ | | | | | (4) following ‘true’ branch (when ‘p’ is non-NULL)... |12321 | { |12322 | if (p->name[0] | | ~~~~~~~ | | | | | (5) ...to here | <------+ | ‘name_scan’: events 6-11 | |13039 | struct name *cursor = namelist_match (file_name, length); | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (6) returning to ‘name_scan’ from ‘namelist_match’ |13040 | if (cursor) | | ~ | | | | | (7) following ‘false’ branch (when ‘cursor’ is NULL)... |...... |13048 | if (same_order_option && namelist && namelist->found_count) | | ~~~~~~~~~~~~~~~~~~ | | || | | |(8) ...to here | | (9) following ‘true’ branch... |13049 | { |13050 | name_gather (); | | ~~~~~~~~~~~~~~ | | | | | (10) ...to here | | (11) calling ‘name_gather’ from ‘name_scan’ | +--> ‘name_gather’: events 12-13 | |12170 | name_gather (void) | | ^~~~~~~~~~~ | | | | | (12) entry to ‘name_gather’ |...... |12179 | if (same_order_option) | | ~ | | | | | (13) following ‘true’ branch... | ‘name_gather’: event 14 | |cc1: | (14): ...to here | ‘name_gather’: event 15 | |12183 | while ((ep = name_next_elt (0)) && ep->type == NELT_CHDIR) | | ^~~~~~~~~~~~~~~~~ | | | | | (15) calling ‘name_next_elt’ from ‘name_gather’ | +--> ‘name_next_elt’: events 16-19 | |12110 | name_next_elt (int change_dirs) | | ^~~~~~~~~~~~~ | | | | | (16) entry to ‘name_next_elt’ |...... |12115 | while ((ep = name_head) != | | ~~~~~~~~~~~~~~~~~~~ | | | | | (17) following ‘true’ branch (when ‘ep’ is non-NULL)... |12116 | ((void *)0) | | ~~~~~~~~~~~ |...... |12119 | switch (ep->type) | | ~~~~~~~~ | | | | | (18) ...to here |...... |12122 | name_list_advance (); | | ~~~~~~~~~~~~~~~~~~~~ | | | | | (19) calling ‘name_list_advance’ from ‘name_next_elt’ | +--> ‘name_list_advance’: events 20-24 | |11759 | name_list_advance (void) | | ^~~~~~~~~~~~~~~~~ | | | | | (20) entry to ‘name_list_advance’ |...... |11767 | if (elt->type == NELT_OPTION || elt->type == NELT_CHDIR) | | ~ | | | | | (21) following ‘false’ branch... |...... |11775 | if (elt->type != NELT_NOOP) | | ~ | | | | | (22) ...to here | | (23) following ‘false’ branch... |11776 | unconsumed_option_free (); |11777 | free (elt); | | ~~~~~~~~~~ | | | | | (24) ...to here | <------+ | ‘name_next_elt’: events 25-28 | |12115 | while ((ep = name_head) != | | ~~~~~~~~~~~~~~~~~~~ | | | | | (26) following ‘true’ branch (when ‘ep’ is non-NULL)... |12116 | ((void *)0) | | ~~~~~~~~~~~ |...... |12119 | switch (ep->type) | | ~~~~~~~~ | | | | | (27) ...to here |...... |12122 | name_list_advance (); | | ^~~~~~~~~~~~~~~~~~~~ | | | | | (25) returning to ‘name_next_elt’ from ‘name_list_advance’ |...... |12126 | if (read_next_name (ep, &entry) == 0) | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (28) calling ‘read_next_name’ from ‘name_next_elt’ | +--> ‘read_next_name’: events 29-34 | |12028 | read_next_name (struct name_elt *ent, struct name_elt *ret) | | ^~~~~~~~~~~~~~ | | | | | (29) entry to ‘read_next_name’ |12029 | { |12030 | if (!ent->v.file.fp) | | ~ | | | | | (30) following ‘true’ branch... |12031 | { |12032 | if (!strcmp (ent->v.file.name, "-")) | | ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | | | (31) ...to here | | (32) following ‘false’ branch (when the strings are non-equal)... |...... |12041 | if (add_file_id (ent->v.file.name)) | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (33) ...to here | | (34) calling ‘add_file_id’ from ‘read_next_name’ | +--> ‘add_file_id’: events 35-37 | |11891 | add_file_id (const char *filename) | | ^~~~~~~~~~~ | | | | | (35) entry to ‘add_file_id’ |...... |11897 | if (stat (filename, &st)) | | ~ | | | | | (36) following ‘false’ branch... |11898 | stat_fatal (filename); |11899 | reading_from = file_list_name (); | | ~~~~~~~~~~~~~~~~~ | | | | | (37) ...to here | <------+ | ‘read_next_name’: events 38-42 | |12041 | if (add_file_id (ent->v.file.name)) | | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | || | | |(38) returning to ‘read_next_name’ from ‘add_file_id’ | | (39) following ‘false’ branch... |...... |12046 | if ((ent->v.file.fp = fopen (ent->v.file.name, "r")) == | | ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | | | (40) ...to here | | | (41) opened here | | (42) ‘fopen(*ent.v.file.name, "r")’ leaks here; was opened at (41) | t.i:12046:7: warning: leak of ‘fopen(*ent.v.file.name, "r")’ [CWE-401] [-Wanalyzer-malloc-leak] 12046 | if ((ent->v.file.fp = fopen (ent->v.file.name, "r")) == | ^ ‘name_scan’: events 1-2 | |13033 | name_scan (const char *file_name) | | ^~~~~~~~~ | | | | | (1) entry to ‘name_scan’ |...... |13039 | struct name *cursor = namelist_match (file_name, length); | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (2) calling ‘namelist_match’ from ‘name_scan’ | +--> ‘namelist_match’: events 3-5 | |12316 | namelist_match (char const *file_name, size_t length) | | ^~~~~~~~~~~~~~ | | | | | (3) entry to ‘namelist_match’ |...... |12320 | for (p = namelist; p; p = p->next) | | ~ | | | | | (4) following ‘true’ branch (when ‘p’ is non-NULL)... |12321 | { |12322 | if (p->name[0] | | ~~~~~~~ | | | | | (5) ...to here | <------+ | ‘name_scan’: events 6-11 | |13039 | struct name *cursor = namelist_match (file_name, length); | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (6) returning to ‘name_scan’ from ‘namelist_match’ |13040 | if (cursor) | | ~ | | | | | (7) following ‘false’ branch (when ‘cursor’ is NULL)... |...... |13048 | if (same_order_option && namelist && namelist->found_count) | | ~~~~~~~~~~~~~~~~~~ | | || | | |(8) ...to here | | (9) following ‘true’ branch... |13049 | { |13050 | name_gather (); | | ~~~~~~~~~~~~~~ | | | | | (10) ...to here | | (11) calling ‘name_gather’ from ‘name_scan’ | +--> ‘name_gather’: events 12-13 | |12170 | name_gather (void) | | ^~~~~~~~~~~ | | | | | (12) entry to ‘name_gather’ |...... |12179 | if (same_order_option) | | ~ | | | | | (13) following ‘true’ branch... | ‘name_gather’: event 14 | |cc1: | (14): ...to here | ‘name_gather’: event 15 | |12183 | while ((ep = name_next_elt (0)) && ep->type == NELT_CHDIR) | | ^~~~~~~~~~~~~~~~~ | | | | | (15) calling ‘name_next_elt’ from ‘name_gather’ | +--> ‘name_next_elt’: events 16-19 | |12110 | name_next_elt (int change_dirs) | | ^~~~~~~~~~~~~ | | | | | (16) entry to ‘name_next_elt’ |...... |12115 | while ((ep = name_head) != | | ~~~~~~~~~~~~~~~~~~~ | | | | | (17) following ‘true’ branch (when ‘ep’ is non-NULL)... |12116 | ((void *)0) | | ~~~~~~~~~~~ |...... |12119 | switch (ep->type) | | ~~~~~~~~ | | | | | (18) ...to here |...... |12122 | name_list_advance (); | | ~~~~~~~~~~~~~~~~~~~~ | | | | | (19) calling ‘name_list_advance’ from ‘name_next_elt’ | +--> ‘name_list_advance’: events 20-24 | |11759 | name_list_advance (void) | | ^~~~~~~~~~~~~~~~~ | | | | | (20) entry to ‘name_list_advance’ |...... |11767 | if (elt->type == NELT_OPTION || elt->type == NELT_CHDIR) | | ~ | | | | | (21) following ‘false’ branch... |...... |11775 | if (elt->type != NELT_NOOP) | | ~ | | | | | (22) ...to here | | (23) following ‘false’ branch... |11776 | unconsumed_option_free (); |11777 | free (elt); | | ~~~~~~~~~~ | | | | | (24) ...to here | <------+ | ‘name_next_elt’: events 25-28 | |12115 | while ((ep = name_head) != | | ~~~~~~~~~~~~~~~~~~~ | | | | | (26) following ‘true’ branch (when ‘ep’ is non-NULL)... |12116 | ((void *)0) | | ~~~~~~~~~~~ |...... |12119 | switch (ep->type) | | ~~~~~~~~ | | | | | (27) ...to here |...... |12122 | name_list_advance (); | | ^~~~~~~~~~~~~~~~~~~~ | | | | | (25) returning to ‘name_next_elt’ from ‘name_list_advance’ |...... |12126 | if (read_next_name (ep, &entry) == 0) | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (28) calling ‘read_next_name’ from ‘name_next_elt’ | +--> ‘read_next_name’: events 29-34 | |12028 | read_next_name (struct name_elt *ent, struct name_elt *ret) | | ^~~~~~~~~~~~~~ | | | | | (29) entry to ‘read_next_name’ |12029 | { |12030 | if (!ent->v.file.fp) | | ~ | | | | | (30) following ‘true’ branch... |12031 | { |12032 | if (!strcmp (ent->v.file.name, "-")) | | ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | | | (31) ...to here | | (32) following ‘false’ branch (when the strings are non-equal)... |...... |12041 | if (add_file_id (ent->v.file.name)) | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (33) ...to here | | (34) calling ‘add_file_id’ from ‘read_next_name’ | +--> ‘add_file_id’: events 35-37 | |11891 | add_file_id (const char *filename) | | ^~~~~~~~~~~ | | | | | (35) entry to ‘add_file_id’ |...... |11897 | if (stat (filename, &st)) | | ~ | | | | | (36) following ‘false’ branch... |11898 | stat_fatal (filename); |11899 | reading_from = file_list_name (); | | ~~~~~~~~~~~~~~~~~ | | | | | (37) ...to here | <------+ | ‘read_next_name’: events 38-42 | |12041 | if (add_file_id (ent->v.file.name)) | | ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | || | | |(38) returning to ‘read_next_name’ from ‘add_file_id’ | | (39) following ‘false’ branch... |...... |12046 | if ((ent->v.file.fp = fopen (ent->v.file.name, "r")) == | | ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | | | (40) ...to here | | | (41) allocated here | | (42) ‘fopen(*ent.v.file.name, "r")’ leaks here; was allocated at (41) | t.i: In function ‘name_gather’: t.i:12184:13: warning: leak of ‘xstrdup(*ep.v.name)’ [CWE-401] [-Wanalyzer-malloc-leak] 12184 | change_dir = chdir_arg (xstrdup (ep->v.name)); | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ‘name_scan’: events 1-2 | |13033 | name_scan (const char *file_name) | | ^~~~~~~~~ | | | | | (1) entry to ‘name_scan’ |...... |13039 | struct name *cursor = namelist_match (file_name, length); | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (2) calling ‘namelist_match’ from ‘name_scan’ | +--> ‘namelist_match’: events 3-5 | |12316 | namelist_match (char const *file_name, size_t length) | | ^~~~~~~~~~~~~~ | | | | | (3) entry to ‘namelist_match’ |...... |12320 | for (p = namelist; p; p = p->next) | | ~ | | | | | (4) following ‘true’ branch (when ‘p’ is non-NULL)... |12321 | { |12322 | if (p->name[0] | | ~~~~~~~ | | | | | (5) ...to here | <------+ | ‘name_scan’: events 6-11 | |13039 | struct name *cursor = namelist_match (file_name, length); | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (6) returning to ‘name_scan’ from ‘namelist_match’ |13040 | if (cursor) | | ~ | | | | | (7) following ‘false’ branch (when ‘cursor’ is NULL)... |...... |13048 | if (same_order_option && namelist && namelist->found_count) | | ~~~~~~~~~~~~~~~~~~ | | || | | |(8) ...to here | | (9) following ‘true’ branch... |13049 | { |13050 | name_gather (); | | ~~~~~~~~~~~~~~ | | | | | (10) ...to here | | (11) calling ‘name_gather’ from ‘name_scan’ | +--> ‘name_gather’: events 12-13 | |12170 | name_gather (void) | | ^~~~~~~~~~~ | | | | | (12) entry to ‘name_gather’ |...... |12179 | if (same_order_option) | | ~ | | | | | (13) following ‘true’ branch... | ‘name_gather’: event 14 | |cc1: | (14): ...to here | ‘name_gather’: event 15 | |12183 | while ((ep = name_next_elt (0)) && ep->type == NELT_CHDIR) | | ^~~~~~~~~~~~~~~~~ | | | | | (15) calling ‘name_next_elt’ from ‘name_gather’ | +--> ‘name_next_elt’: events 16-20 | |12110 | name_next_elt (int change_dirs) | | ^~~~~~~~~~~~~ | | | | | (16) entry to ‘name_next_elt’ |...... |12115 | while ((ep = name_head) != | | ~~~~~~~~~~~~~~~~~~~ | | | | | (17) following ‘true’ branch (when ‘ep’ is non-NULL)... |12116 | ((void *)0) | | ~~~~~~~~~~~ |...... |12119 | switch (ep->type) | | ~~~~~~~~ | | | | | (18) ...to here |...... |12131 | if (change_dirs) | | ~ | | | | | (19) following ‘false’ branch (when ‘change_dirs == 0’)... |...... |12139 | copy_name (ep); | | ~~~~~~~~~~~~~~ | | | | | (20) ...to here | <------+ | ‘name_gather’: events 21-25 | |12183 | while ((ep = name_next_elt (0)) && ep->type == NELT_CHDIR) | | ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | | | (22) following ‘true’ branch... | | (21) returning to ‘name_gather’ from ‘name_next_elt’ |12184 | change_dir = chdir_arg (xstrdup (ep->v.name)); | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | | | (23) ...to here | | | (24) allocated here | | (25) ‘xstrdup(*ep.v.name)’ leaks here; was allocated at (24) |