aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohannes Berg <johannes@sipsolutions.net>2008-04-10 15:25:20 +0200
committerJosh Triplett <josh@freedesktop.org>2008-04-21 10:59:37 -0700
commitc5b808c9964d62fc026d9398a4a62c3ce7bacac8 (patch)
tree63dcb9d451ce5062c51e70a10e59aef019b6f8d6 /sparse.1
parentcgcc: handle ppc arch (diff)
downloadsparse-c5b808c9964d62fc026d9398a4a62c3ce7bacac8.tar.gz
sparse-c5b808c9964d62fc026d9398a4a62c3ce7bacac8.tar.bz2
sparse-c5b808c9964d62fc026d9398a4a62c3ce7bacac8.zip
make sparse keep its promise about context tracking
The sparse man page promises that it will check this: Functions with the extended attribute __attribute__((context(expression,in_context,out_context)) require the context expression (for instance, a lock) to have the value in_context (a constant nonnegative integer) when called, and return with the value out_context (a constant nonnegative integer). It doesn't keep that promise though, nor can it, especially with contexts that can be acquired recursively (like RCU in the kernel.) This patch makes sparse track different contexts, and also follows up on that promise, but with slightly different semantics: * the "require the context to have the value" is changed to require it to have /at least/ the value if 'in_context', * an exact_context(...) attribute is introduced with the previously described semantics (to be used for non-recursive contexts), * the __context__ statement is extended to also include a required context argument (same at least semantics), Unfortunately, I wasn't able to keep the same output, so now you'll see different messages from sparse, especially when trying to unlock a lock that isn't locked you'll see a message pointing to the unlock function rather than complaining about the basic block, you can see that in the test suite changes. This patch also contains test updates and a lot of new tests for the new functionality. Except for the changed messages, old functionality should not be affected. However, the kernel use of __attribute__((context(...)) is actually wrong, the kernel often does things like: static void *dev_mc_seq_start(struct seq_file *seq, loff_t * pos) __acquires(dev_base_lock) { [...] read_lock(&dev_base_lock); [...] } rather than static void *dev_mc_seq_start(struct seq_file *seq, loff_t * pos) __acquires(dev_base_lock) { [...] __acquire__(dev_base_lock); read_lock(&dev_base_lock); [...] } (and possibly more when read_lock() is annotated appropriately, such as dropping whatever context read_lock() returns to convert the context to the dev_base_lock one.) Currently, sparse doesn't care, but if it's going to check the context of functions contained within another function then we need to put the actual __acquire__ together with acquiring the context. The great benefit of this patch is that you can now document at least some locking assumptions in a machine-readable way: before: /* requires mylock held */ static void myfunc(void) {...} after: static void myfunc(void) __requires(mylock) {...} where, for sparse, #define __requires(x) __attribute__((context(x,1,1))) Doing so may result in lots of other functions that need to be annoated along with it because they also have the same locking requirements, but ultimately sparse can check a lot of locking assumptions that way. I have already used this patch and identify a number of kernel bugs by marking things to require certain locks or RCU-protection and checking sparse output. To do that, you need a few kernel patches which I'll send separately. Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Diffstat (limited to 'sparse.1')
-rw-r--r--sparse.134
1 files changed, 24 insertions, 10 deletions
diff --git a/sparse.1 b/sparse.1
index 7a39d85..3de183c 100644
--- a/sparse.1
+++ b/sparse.1
@@ -73,20 +73,34 @@ Warn about potential errors in synchronization or other delimited contexts.
Sparse supports several means of designating functions or statements that
delimit contexts, such as synchronization. Functions with the extended
attribute
-.BI __attribute__((context( expression , in_context , out_context ))
-require the context \fIexpression\fR (for instance, a lock) to have the value
+.BI __attribute__((context( [expression ,] in_context , out_context ))
+require the context \fIexpression\fR (for instance, a lock) to have at least the value
\fIin_context\fR (a constant nonnegative integer) when called, and return with
-the value \fIout_context\fR (a constant nonnegative integer). For APIs
-defined via macros, use the statement form
-.BI __context__( expression , in_value , out_value )
-in the body of the macro.
-
-With \fB-Wcontext\fR Sparse will warn when it sees a function change the
-context without indicating this with a \fBcontext\fR attribute, either by
+the value adjusted by \fIout_context - in_context\fR (where
+\fIout_context\fR is a constant nonnegative integer). To change the value
+of a context (for example in macros), use the statement
+.BI __context__( [expression , ]adjust_value[ , required] )
+where \fIadjust_value\fR is a constant integer and \fIrequired\fR is a
+constant nonnegative integer. Not giving \fIrequired\fR is equivalent to
+giving zero and means that the statement does not need the context as a
+precondition, when given it means that the context must at least have the
+value of \fIrequired\fR.
+
+To indicate that a function requires
+.BI exactly
+a certain lock context (not "at least" as above), use the form
+.BI __attribute__((exact_context( [expression ,] in_context , out_context ))
+There currently is no corresponding
+.BI __exact_context__( [expression , ]adjust_value[ , required] )
+statement.
+
+Sparse will warn when it sees a function change a
+context without indicating this with a \fBcontext\fR or \fBexact_context\fR attribute, either by
decreasing a context below zero (such as by releasing a lock without acquiring
it), or returning with a changed context (such as by acquiring a lock without
releasing it). Sparse will also warn about blocks of code which may
-potentially execute with different contexts.
+potentially execute with different contexts and about functions that are
+executed without a lock they require.
Sparse issues these warnings by default. To turn them off, use
\fB\-Wno\-context\fR.