[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237 David Malcolm changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #13 from David Malcolm --- (In reply to David Malcolm from comment #8) > (In reply to David Binderman from comment #6) [...] > > Does the recent patch find the same or different resource leaks on gcc trunk > > ? > > I'll test that next. Let's track that in PR analyzer/93388. I think that covers the various specific items mentioned in this bug, so I'm going to close this one out.
[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237 --- Comment #12 from CVS Commits --- The master branch has been updated by David Malcolm : https://gcc.gnu.org/g:8397af8ed0db685312e44117fd52316b57c2c129 commit r10-5965-g8397af8ed0db685312e44117fd52316b57c2c129 Author: David Malcolm Date: Fri Dec 20 10:56:28 2019 -0500 analyzer: fix tests for UNKNOWN_LOCATION In the reproducer for PR analyzer/58237 I noticed that some events were missing locations (and text); for example event 3 here: | 15 | while (fgets(buf, 10, fp) != NULL) | | ~ | | | | | (2) following 'false' branch... | 'f1': event 3 | |cc1: | 'f1': event 4 | |:19:1: | 19 | } | | ^ | | | | | (4) 'fp' leaks here; was opened at (1) | The root cause is that various places in the analyzer compare locations against UNKNOWN_LOCATION, which fails to detect an unknown location for the case where an unknown_location has been wrapped into an ad-hoc location to record a block. This patch fixes the issue by using get_pure_location whenever testing against UNKNOWN_LOCATION to look through ad-hoc wrappers. For the case above, it thus picks a better location in supernode::get_start_location for event (3) above, improving it to: | 15 | while (fgets(buf, 10, fp) != NULL) | | ~ | | | | | (2) following 'false' branch... |.. | 19 | } | | ~ | | | | | (3) ...to here | | (4) 'fp' leaks here; was opened at (1) | gcc/analyzer/ChangeLog: PR analyzer/58237 * engine.cc (leak_stmt_finder::find_stmt): Use get_pure_location when comparing against UNKNOWN_LOCATION. (stmt_requires_new_enode_p): Likewise. (exploded_graph::dump_exploded_nodes): Likewise. * supergraph.cc (supernode::get_start_location): Likewise. (supernode::get_end_location): Likewise. gcc/testsuite/ChangeLog: PR analyzer/58237 * gcc.dg/analyzer/file-paths-1.c: New test.
[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237 --- Comment #11 from CVS Commits --- The master branch has been updated by David Malcolm : https://gcc.gnu.org/g:03dc3f26231cbf570028e14706f8ad77fd5a commit r10-5964-g03dc3f26231cbf570028e14706f8ad77fd5a Author: David Malcolm Date: Fri Dec 20 11:20:44 2019 -0500 tree-diagnostic-path.cc: properly handle ad-hoc wrappers of UNKNOWN_LOCATION In the reproducer for PR analyzer/58237 I noticed that some events that were missing locations were also missing text; for example event 3 here: | 15 | while (fgets(buf, 10, fp) != NULL) | | ~ | | | | | (2) following 'false' branch... | 'f1': event 3 | |cc1: | The root cause is that the path_summary-printing code doesn't consider ad-hoc locations when looking for reserved locations, and so fails to detect an unknown location for the case where an unknown location has been wrapped into an ad-hoc location to record a block. This patch fixes the issue by using get_pure_location, thus looking through ad-hoc wrappers, improving the result to: | 15 | while (fgets(buf, 10, fp) != NULL) | | ~ | | | | | (2) following 'false' branch... | 'f1': event 3 | |cc1: | (3): ...to here | gcc/ChangeLog: * tree-diagnostic-path.cc (path_summary::event_range::print): When testing for UNKNOWN_LOCATION, look through ad-hoc wrappers using get_pure_location.
[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237
--- Comment #10 from CVS Commits ---
The master branch has been updated by David Malcolm :
https://gcc.gnu.org/g:697251b7a1bb7c14d3805de22248e83a23b90d1a
commit r10-5963-g697251b7a1bb7c14d3805de22248e83a23b90d1a
Author: David Malcolm
Date: Thu Dec 19 15:59:04 2019 -0500
analyzer: add known stdio functions to sm-file.cc (PR analyzer/58237)
The analyzer ought to report various file leaks for the reproducer in
PR analyzer/58237, such as:
void f1(const char *str)
{
FILE * fp = fopen(str, "r");
char buf[10];
while (fgets(buf, 10, fp) != NULL)
{
/* Do something with buf */
}
/* Missing call to fclose. Need warning here for resource leak */
}
but fails to do so, due to not recognizing fgets, and thus
conservatively assuming that it could close "fp".
This patch adds a function_set to sm-file.cc of numerous stdio.h
functions that are known to not close the file (and which require a
valid FILE *, but that's a matter for a followup), fixing the issue.
gcc/analyzer/ChangeLog:
PR analyzer/58237
* analyzer-selftests.cc (selftest::run_analyzer_selftests): Call
selftest::analyzer_sm_file_cc_tests.
* analyzer-selftests.h (selftest::analyzer_sm_file_cc_tests): New
decl.
* sm-file.cc: Include "analyzer/function-set.h" and
"analyzer/analyzer-selftests.h".
(get_file_using_fns): New function.
(is_file_using_fn_p): New function.
(fileptr_state_machine::on_stmt): Return true for known functions.
(selftest::analyzer_sm_file_cc_tests): New function.
gcc/testsuite/ChangeLog:
PR analyzer/58237
* gcc.dg/analyzer/file-1.c (test_4): New.
* gcc.dg/analyzer/file-pr58237.c: New test.
[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237 --- Comment #9 from David Malcolm --- (In reply to David Malcolm from comment #7) [...] > (there are some issues with the paths, where some of the events aren't being > printed, presumably due to having UNKNOWN_LOCATION; will investigate) Fixed on branch by: * "[PATCH 1/2] (analyzer) tree-diagnostic-path.cc: properly handle ad-hoc wrappers of UNKNOWN_LOCATION" * https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01471.html * "[PATCH 2/2] analyzer: fix tests for UNKNOWN_LOCATION" * https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01472.html
[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237 --- Comment #8 from David Malcolm --- (In reply to David Binderman from comment #6) > I tried out recent cppcheck on recent gcc trunk and found these resource > leaks: > > trunk/libvtv/scripts/sum-vtv-counts.c:94:4: error: Resource leak: fp_in > [resourceLeak] > trunk/libvtv/scripts/sum-vtv-counts.c:105:7: error: Resource leak: fp_in > [resourceLeak] > trunk/zlib/contrib/minizip/mztools.c:290:3: error: Resource leak: fpOutCD > [resourceLeak] > > The first two are false positives, cppcheck doesn't notice the leak is > in main. My analyzer does the right thing here: it doesn't report the leaks; if I rename "main" to "not_main" it does report them. > Last one has a missing sanity check for fpOutCD != NULL, but since > it is in a /contrib/ directory, it won't get fixed here. Unfortunately the analyzer doesn't report this one; I've filed it for myself as PR analyzer/93032. > Does the recent patch find the same or different resource leaks on gcc trunk > ? I'll test that next. Thanks!
[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237 --- Comment #7 from David Malcolm --- (In reply to David Malcolm from comment #4) > Patch committed to dmalcolm/analyzer branch: > https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01425.html > Ought to be live on godbolt.org within the next 24 hours. The examples from comment #0: https://godbolt.org/z/KkEoNP (there are some issues with the paths, where some of the events aren't being printed, presumably due to having UNKNOWN_LOCATION; will investigate)
[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237 --- Comment #6 from David Binderman --- I tried out recent cppcheck on recent gcc trunk and found these resource leaks: trunk/libvtv/scripts/sum-vtv-counts.c:94:4: error: Resource leak: fp_in [resourceLeak] trunk/libvtv/scripts/sum-vtv-counts.c:105:7: error: Resource leak: fp_in [resourceLeak] trunk/zlib/contrib/minizip/mztools.c:290:3: error: Resource leak: fpOutCD [resourceLeak] The first two are false positives, cppcheck doesn't notice the leak is in main. Last one has a missing sanity check for fpOutCD != NULL, but since it is in a /contrib/ directory, it won't get fixed here. Does the recent patch find the same or different resource leaks on gcc trunk ?
[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237 --- Comment #5 from David Binderman --- Has this new code been tried out on gcc trunk or the linux kernel or a distribution like fedora or debian ? That might be the next phase of testing.
[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237 --- Comment #4 from David Malcolm --- Patch committed to dmalcolm/analyzer branch: https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01425.html Ought to be live on godbolt.org within the next 24 hours.
[Bug analyzer/58237] gcc fails to detect obvious resource leaks
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58237 David Malcolm changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2019-12-19 Component|plugins |analyzer Ever confirmed|0 |1 --- Comment #3 from David Malcolm --- I have a working implementation of this in my analyzer working copy (needs a patch before the branch will detect it).
