public inbox for gcc-bugs@sourceware.org help / color / mirror / Atom feed
From: "cvs-commit at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> To: gcc-bugs@gcc.gnu.org Subject: [Bug analyzer/105285] False positive with -Wanalyzer-null-dereference in git.git's reftable/reader.c Date: Wed, 27 Jul 2022 21:55:29 +0000 [thread overview] Message-ID: <bug-105285-4-XalhmJODoK@http.gcc.gnu.org/bugzilla/> (raw) In-Reply-To: <bug-105285-4@http.gcc.gnu.org/bugzilla/> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105285 --- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> --- The releases/gcc-12 branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>: https://gcc.gnu.org/g:05530fcea07a9ee5c7501867f3f11f0fbc504a06 commit r12-8632-g05530fcea07a9ee5c7501867f3f11f0fbc504a06 Author: David Malcolm <dmalcolm@redhat.com> Date: Wed Jul 27 17:38:53 2022 -0400 analyzer: handle repeated accesses after init of unknown size [PR105285] (cherry-picked from r13-7-g00c4405cd7f6a144d0a439e4d848d246920e6ff3) PR analyzer/105285 reports a false positive from -Wanalyzer-null-dereference on git.git's reftable/reader.c. A reduced version of the problem can be seen in test_1a of gcc.dg/analyzer/symbolic-12.c in the following: void test_1a (void *p, unsigned next_off) { struct st_1 *r = p; external_fn(); if (next_off >= r->size) return; if (next_off >= r->size) /* We should have already returned if this is the case. */ __analyzer_dump_path (); /* { dg-bogus "path" } */ } where the analyzer erroneously considers this path, where (next_off >= r->size) is both false and then true: symbolic-12.c: In function âtest_1aâ: symbolic-12.c:22:5: note: path 22 | __analyzer_dump_path (); /* { dg-bogus "path" } */ | ^~~~~~~~~~~~~~~~~~~~~~~ âtest_1aâ: events 1-5 | | 17 | if (next_off >= r->size) | | ^ | | | | | (1) following âfalseâ branch... |...... | 20 | if (next_off >= r->size) | | ~ ~~~~~~~ | | | | | | | (2) ...to here | | (3) following âtrueâ branch... | 21 | /* We should have already returned if this is the case. */ | 22 | __analyzer_dump_path (); /* { dg-bogus "path" } */ | | ~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (4) ...to here | | (5) here | The root cause is that, at the call to the external function, the analyzer considers the cluster for *p to have been touched, binding it to a conjured_svalue, but because p is void * no particular size is known for the write, and so the cluster is bound using a symbolic key covering the base region. Later, the accesses to r->size are handled by binding_cluster::get_any_binding, but binding_cluster::get_binding fails to find a match for the concrete field lookup, due to the key for the binding being symbolic, and reaching this code: 1522 /* If this cluster has been touched by a symbolic write, then the content 1523 of any subregion not currently specifically bound is "UNKNOWN". */ 1524 if (m_touched) 1525 { 1526 region_model_manager *rmm_mgr = mgr->get_svalue_manager (); 1527 return rmm_mgr->get_or_create_unknown_svalue (reg->get_type ()); 1528 } Hence each access to r->size is an unknown svalue, and thus the condition (next_off >= r->size) isn't tracked, leading to the path with contradictory conditions being treated as satisfiable. In the original reproducer in git's reftable/reader.c, the call to the external fn is: reftable_record_type(rec) which is considered to possibly write to *rec, which is *tab, where tab is the void * argument to reftable_reader_seek_void, and thus after the call to reftable_record_type some arbitrary amount of *rec could have been written to. This patch fixes things by detecting the "this cluster has been 'filled' with a conjured value of unknown size" case, and handling get_any_binding on it by returning a sub_svalue of the conjured_svalue, so that repeated accesses to r->size give the same symbolic value, so that the constraint manager rejects the bogus execution path, fixing the false positive. gcc/analyzer/ChangeLog: PR analyzer/105285 * store.cc (binding_cluster::get_any_binding): Handle accessing sub_svalues of clusters where the base region has a symbolic binding. gcc/testsuite/ChangeLog: PR analyzer/105285 * gcc.dg/analyzer/symbolic-12.c: New test. Signed-off-by: David Malcolm <dmalcolm@redhat.com>
next prev parent reply other threads:[~2022-07-27 21:55 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-15 9:39 [Bug analyzer/105285] New: " avarab at gmail dot com 2022-04-15 9:49 ` [Bug analyzer/105285] " avarab at gmail dot com 2022-04-15 9:54 ` avarab at gmail dot com 2022-04-15 15:11 ` dmalcolm at gcc dot gnu.org 2022-04-27 17:56 ` dmalcolm at gcc dot gnu.org 2022-04-27 18:07 ` dmalcolm at gcc dot gnu.org 2022-04-27 18:08 ` dmalcolm at gcc dot gnu.org 2022-04-27 18:08 ` dmalcolm at gcc dot gnu.org 2022-04-27 19:24 ` dmalcolm at gcc dot gnu.org 2022-04-28 17:48 ` cvs-commit at gcc dot gnu.org 2022-04-28 17:51 ` cvs-commit at gcc dot gnu.org 2022-04-28 17:57 ` dmalcolm at gcc dot gnu.org 2022-07-27 21:55 ` cvs-commit at gcc dot gnu.org 2022-07-27 21:55 ` cvs-commit at gcc dot gnu.org [this message] 2022-07-27 22:09 ` dmalcolm at gcc dot gnu.org
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=bug-105285-4-XalhmJODoK@http.gcc.gnu.org/bugzilla/ \ --to=gcc-bugzilla@gcc.gnu.org \ --cc=gcc-bugs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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).