From: Dawson Engler <engler@csl.Stanford.EDU>
To: adilger@clusterfs.com (Andreas Dilger)
Cc: linux-kernel@vger.kernel.org, mc@cs.Stanford.EDU
Subject: Re: [CHECKER] 54 missing null pointer checks in 2.4.17
Date: Mon, 10 Jun 2002 00:05:48 -0700 (PDT) [thread overview]
Message-ID: <200206100705.AAA19208@csl.Stanford.EDU> (raw)
In-Reply-To: <20020610063510.GG20388@turbolinux.com> from "Andreas Dilger" at Jun 10, 2002 12:35:10 AM
> Ah, but the checker is still (subtly) wrong in this case. The difference
> is that "jbd_kmalloc()" (a macro calling __jbd_kmalloc in the 5 functions
> which check the return code) depends on the "journal_oom_retry" variable
> to determine whether or not it is "allowed" to return NULL. In contrast,
> the one call to "jbd_rep_kmalloc()" flagged above is a macro which
> calls __jbd_kmalloc() with "retry = 1" so it is never allowed to fail
> and return NULL.
Ah. Got it. Yeah, we're not doing much inter-procedural false path
pruning. Hopefully within a month or so --- Andy Chou and Yichen Xie
are building an analysis pass that uses a SAT solver to suppress such
things. It discovers some pretty crazy relationships and is actually
pretty fast.
> in most cases the checker is correct.
To be fair, it's the checker + our inspection that is mostly correct
;-) Though the uninspected false pos rate is pretty low.
> Have you thought about supporting
> "checker meta comments" (like lint did) to allow one to flag a piece of
> code as being "correct" for a certain check so that it doesn't always
> show up on your test runs?
I wasn't that optimistic that people would be willing to annotate their
code. It is pretty easy to add such annotations with distinguished
function calls. E.g.,
/* shut up checker null pointer warnings */
mc_no_null_bug(p);
where p is a pointer var --- it can be #define'd to nothing when the
checker isn't being used. Also, the checker can turn the annot into
a sort of checkable comment by warning when the annotation is not
needed.
Instead we use a history-based approach: both false positives and bugs
are stuffed into a file which subsequent runs use to relabel messages
as old false positives or unfixed bugs. The messages are canonicalized
so that most source edits don't make them invalid. E.g., we keep file,
function, variable names and such but strip line numbers and other
things. The advantage is that you don't have to modify your source for
checkers to go over it. Which is good, given the current patch
process.
If you're interested, there are a bunch of papers on this and other things
at
www.stanford.edu/~engler
Thanks for your feedback!
Dawson
next prev parent reply other threads:[~2002-06-10 7:05 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2002-06-10 3:55 [CHECKER] 54 missing null pointer checks in 2.4.17 Dawson Engler
2002-06-10 5:22 ` Kasper Dupont
2002-06-10 5:56 ` Dawson Engler
2002-06-10 5:28 ` Andreas Dilger
2002-06-10 6:07 ` Dawson Engler
2002-06-10 6:35 ` Andreas Dilger
2002-06-10 7:05 ` Dawson Engler [this message]
2002-06-10 7:19 ` Giuliano Pochini
2002-06-10 12:40 ` john slee
2002-06-10 7:03 ` Brad Hards
2002-06-10 7:08 ` Dawson Engler
2002-06-11 0:30 ` Greg KH
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=200206100705.AAA19208@csl.Stanford.EDU \
--to=engler@csl.stanford.edu \
--cc=adilger@clusterfs.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mc@cs.Stanford.EDU \
/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: link
Be 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