diff options
| author | Johannes Berg <johannes@sipsolutions.net> | 2008-04-10 15:25:20 +0200 |
|---|---|---|
| committer | Josh Triplett <josh@freedesktop.org> | 2008-04-21 10:59:37 -0700 |
| commit | c5b808c9964d62fc026d9398a4a62c3ce7bacac8 (patch) | |
| tree | 63dcb9d451ce5062c51e70a10e59aef019b6f8d6 /validation | |
| parent | f93bc9bade1f2db9320ad65ffa174ff3f684849f (diff) | |
| download | sparse-dev-c5b808c9964d62fc026d9398a4a62c3ce7bacac8.tar.gz | |
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 'validation')
| -rw-r--r-- | validation/context-named.c | 496 | ||||
| -rw-r--r-- | validation/context-statement.c | 58 | ||||
| -rw-r--r-- | validation/context.c | 40 |
3 files changed, 580 insertions, 14 deletions
diff --git a/validation/context-named.c b/validation/context-named.c new file mode 100644 index 00000000..c65b2022 --- /dev/null +++ b/validation/context-named.c @@ -0,0 +1,496 @@ +static void a(void) __attribute__((context(TEST,0,1))) +{ + __context__(TEST,1); +} + +static void r(void) __attribute__((context(TEST,1,0))) +{ + __context__(TEST,-1,1); +} + +static void a2(void) __attribute__((context(TEST2,0,1))) +{ + __context__(TEST2,1); +} + +static void r2(void) __attribute__((context(TEST2,1,0))) +{ + __context__(TEST2,-1,1); +} + +#define check_test2() __context__(TEST2,0,1) + +static void good_paired1(void) +{ + a(); + a2(); + r(); + r2(); +} + +static void good_paired2(void) +{ + a(); + r(); + a(); + r(); + a2(); + r2(); +} + +static void good_paired3(void) +{ + a(); + a(); + r(); + r(); + a2(); + a2(); + r2(); + r2(); +} + +static void good_lock1(void) __attribute__((context(TEST,0,1))) +{ + a(); +} + +static void good_lock2(void) __attribute__((context(TEST,0,1))) +{ + a(); + r(); + a(); +} + +static void good_lock3(void) __attribute__((context(TEST,0,1))) +{ + a(); + a(); + r(); +} + +static void good_unlock1(void) __attribute__((context(TEST,1,0))) +{ + r(); +} + +static void good_unlock2(void) __attribute__((context(TEST,1,0))) +{ + a(); + r(); + r(); +} + +static void warn_lock1(void) +{ + a(); +} + +static void warn_lock2(void) +{ + a(); + r(); + a(); +} + +static void warn_lock3(void) +{ + a(); + a(); + r(); +} + +static void warn_unlock1(void) +{ + r(); +} + +static void warn_unlock2(void) +{ + a(); + r(); + r(); +} + +extern int condition, condition2; + +static int good_if1(void) +{ + a(); + if(condition) { + r(); + return -1; + } + r(); + return 0; +} + +static void good_if2(void) +{ + if(condition) { + a(); + r(); + } +} + +static void good_if3(void) +{ + a(); + if(condition) { + a(); + r(); + } + r(); +} + +static int warn_if1(void) +{ + a(); + if(condition) + return -1; + r(); + return 0; +} + +static int warn_if2(void) +{ + a(); + if(condition) { + r(); + return -1; + } + return 0; +} + +static void good_while1(void) +{ + a(); + while(condition) + ; + r(); +} + +static void good_while2(void) +{ + while(condition) { + a(); + r(); + } +} + +static void good_while3(void) +{ + while(condition) { + a(); + r(); + if(condition2) + break; + a(); + r(); + } +} + +static void good_while4(void) +{ + a(); + while(1) { + if(condition2) { + r(); + break; + } + } +} + +static void good_while5(void) +{ + a(); + while(1) { + r(); + if(condition2) + break; + a(); + } +} + +static void warn_while1(void) +{ + while(condition) { + a(); + } +} + +static void warn_while2(void) +{ + while(condition) { + r(); + } +} + +static void warn_while3(void) +{ + while(condition) { + a(); + if(condition2) + break; + r(); + } +} + +static void good_goto1(void) +{ + a(); + goto label; +label: + r(); +} + +static void good_goto2(void) +{ + a(); + goto label; + a(); + r(); +label: + r(); +} + +static void good_goto3(void) +{ + a(); + if(condition) + goto label; + a(); + r(); +label: + r(); +} + +static void good_goto4(void) +{ + if(condition) + goto label; + a(); + r(); +label: + ; +} + +static void good_goto5(void) +{ + a(); + if(condition) + goto label; + r(); + return; +label: + r(); +} + +static void warn_goto1(void) +{ + a(); + goto label; + r(); +label: + ; +} + +static void warn_goto2(void) +{ + a(); + goto label; + r(); +label: + a(); + r(); +} + +static void warn_goto3(void) +{ + a(); + if(condition) + goto label; + r(); +label: + r(); +} + +static void warn_multiple1(void) +{ + a(); + a2(); +} + +static void warn_multiple2(void) +{ + a2(); + a(); +} + +static void warn_mixed1(void) +{ + a2(); + r(); +} + +static void warn_mixed2(void) +{ + a2(); + if (condition) { + a(); + r2(); + } + r(); +} + +static void warn_mixed3(void) +{ + a2(); + if (condition) { + r2(); + return; + } + r(); +} + +static void warn_mixed4(void) +{ + a2(); + if (condition) { + a(); + r(); + return; + } + r(); +} + +static void good_mixed1(void) +{ + if (condition) { + a(); + r(); + } else { + a2(); + r2(); + } +} + +static void good_mixed2(void) +{ + if (condition) { + a(); + r(); + } + a2(); + r2(); +} + +static int need_lock(void) __attribute__((context(TEST,1,1))) +{ +} + +static void need_lock_exact(void) __attribute__((exact_context(TEST,1,1))) +{ +} + +static void need_lock2(void) __attribute__((context(TEST,1,1))) +{ + need_lock(); +} + +static void good_fn(void) +{ + a(); + need_lock(); + r(); +} + +static void good_fn2(void) +{ + a(); + a(); + need_lock(); + r(); + r(); +} + +static void good_fn2(void) +{ + a(); + if (condition) + need_lock(); + r(); +} + +static void good_fn3(void) __attribute__((context(TEST,1,1))) +{ + if (condition) + need_lock2(); +} + +static void warn_fn(void) +{ + a2(); + need_lock(); + r2(); +} + +static void warn_fn2(void) +{ + a2(); + need_lock2(); + r2(); +} + +static void good_exact_fn(void) +{ + a(); + need_lock_exact(); + r(); +} + +static void warn_exact_fn1(void) +{ + a(); + a(); + need_lock_exact(); + r(); + r(); +} + +static void warn_exact_fn2(void) +{ + a2(); + need_lock_exact(); + r2(); +} + +/* + * check-name: Check -Wcontext with lock names + * + * check-error-start +context-named.c:86:3: warning: context imbalance in 'warn_lock1' - wrong count at exit (TEST) +context-named.c:93:3: warning: context imbalance in 'warn_lock2' - wrong count at exit (TEST) +context-named.c:100:3: warning: context imbalance in 'warn_lock3' - wrong count at exit (TEST) +context-named.c:105:3: warning: context problem in 'warn_unlock1' - function 'r' expected different context +context-named.c:112:3: warning: context problem in 'warn_unlock2' - function 'r' expected different context +context-named.c:152:9: warning: context imbalance in 'warn_if1' - wrong count at exit (TEST) +context-named.c:162:9: warning: context imbalance in 'warn_if2' - wrong count at exit (TEST) +context-named.c:218:4: warning: context imbalance in 'warn_while1' - wrong count at exit (TEST) +context-named.c:225:4: warning: context problem in 'warn_while2' - function 'r' expected different context +context-named.c:235:4: warning: context imbalance in 'warn_while3' - wrong count at exit (TEST) +context-named.c:295:5: warning: context imbalance in 'warn_goto1' - wrong count at exit (TEST) +context-named.c:305:6: warning: context imbalance in 'warn_goto2' - wrong count at exit (TEST) +context-named.c:315:6: warning: context problem in 'warn_goto3' - function 'r' expected different context +context-named.c:321:7: warning: context imbalance in 'warn_multiple1' - wrong count at exit (TEST) +context-named.c:327:6: warning: context imbalance in 'warn_multiple2' - wrong count at exit (TEST2) +context-named.c:333:6: warning: context problem in 'warn_mixed1' - function 'r' expected different context +context-named.c:343:6: warning: context problem in 'warn_mixed2' - function 'r' expected different context +context-named.c:353:6: warning: context problem in 'warn_mixed3' - function 'r' expected different context +context-named.c:364:6: warning: context imbalance in 'warn_mixed4' - wrong count at exit (TEST2) +context-named.c:434:14: warning: context problem in 'warn_fn' - function 'need_lock' expected different context +context-named.c:441:15: warning: context problem in 'warn_fn2' - function 'need_lock2' expected different context +context-named.c:456:20: warning: context problem in 'warn_exact_fn1' - function 'need_lock_exact' expected different context +context-named.c:464:20: warning: context problem in 'warn_exact_fn2' - function 'need_lock_exact' expected different context + * check-error-end + */ diff --git a/validation/context-statement.c b/validation/context-statement.c new file mode 100644 index 00000000..ec26fefe --- /dev/null +++ b/validation/context-statement.c @@ -0,0 +1,58 @@ +#define a() __context__(LOCK, 1) +#define r() __context__(LOCK, -1) +#define m() __context__(LOCK, 0, 1) +#define m2() __context__(LOCK, 0, 2) + +static void good_ar(void) +{ + a(); + r(); +} + +static void bad_arr(void) +{ + a(); + r(); + r(); +} + +static void good_macro1(void) +{ + a(); + m(); + r(); +} + +static void good_macro2(void) +{ + a(); + a(); + m(); + m2(); + r(); + r(); +} + +static void bad_macro1(void) +{ + m(); + a(); + r(); +} + +static void bad_macro2(void) +{ + a(); + r(); + m(); +} + +/* + * check-name: Check __context__ statement with required context + * + * check-error-start +context-statement.c:16:8: warning: context imbalance in 'bad_arr' - unexpected unlock (LOCK) +context-statement.c:38:5: warning: context imbalance in 'bad_macro1' - __context__ statement expected different lock context (LOCK) +context-statement.c:47:5: warning: context imbalance in 'bad_macro2' - __context__ statement expected different lock context (LOCK) + * check-error-end + */ diff --git a/validation/context.c b/validation/context.c index 4b15e757..df337e5c 100644 --- a/validation/context.c +++ b/validation/context.c @@ -314,23 +314,35 @@ static void warn_cond_lock1(void) condition2 = 1; /* do stuff */ r(); } + +static void warn_odd_looping(void) +{ + int i; + + for (i = 0; i < 2; i++) + a(); + for (i = 0; i < 2; i++) + r(); +} + /* * check-name: Check -Wcontext * * check-error-start -context.c:69:13: warning: context imbalance in 'warn_lock1' - wrong count at exit -context.c:74:13: warning: context imbalance in 'warn_lock2' - wrong count at exit -context.c:81:13: warning: context imbalance in 'warn_lock3' - wrong count at exit -context.c:88:13: warning: context imbalance in 'warn_unlock1' - unexpected unlock -context.c:93:13: warning: context imbalance in 'warn_unlock2' - unexpected unlock -context.c:131:12: warning: context imbalance in 'warn_if1' - wrong count at exit -context.c:140:12: warning: context imbalance in 'warn_if2' - different lock contexts for basic block -context.c:202:2: warning: context imbalance in 'warn_while1' - different lock contexts for basic block -context.c:210:3: warning: context imbalance in 'warn_while2' - unexpected unlock -context.c:216:2: warning: context imbalance in 'warn_while3' - wrong count at exit -context.c:274:13: warning: context imbalance in 'warn_goto1' - wrong count at exit -context.c:283:13: warning: context imbalance in 'warn_goto2' - wrong count at exit -context.c:300:5: warning: context imbalance in 'warn_goto3' - different lock contexts for basic block -context.c:315:5: warning: context imbalance in 'warn_cond_lock1' - different lock contexts for basic block +context.c:71:3: warning: context imbalance in 'warn_lock1' - wrong count at exit +context.c:78:3: warning: context imbalance in 'warn_lock2' - wrong count at exit +context.c:85:3: warning: context imbalance in 'warn_lock3' - wrong count at exit +context.c:90:3: warning: context problem in 'warn_unlock1' - function 'r' expected different context +context.c:97:3: warning: context problem in 'warn_unlock2' - function 'r' expected different context +context.c:137:9: warning: context imbalance in 'warn_if1' - wrong count at exit +context.c:147:9: warning: context imbalance in 'warn_if2' - wrong count at exit +context.c:203:4: warning: context imbalance in 'warn_while1' - wrong count at exit +context.c:210:4: warning: context problem in 'warn_while2' - function 'r' expected different context +context.c:220:4: warning: context imbalance in 'warn_while3' - wrong count at exit +context.c:280:5: warning: context imbalance in 'warn_goto1' - wrong count at exit +context.c:290:6: warning: context imbalance in 'warn_goto2' - wrong count at exit +context.c:300:6: warning: context problem in 'warn_goto3' - function 'r' expected different context +context.c:315:6: warning: context problem in 'warn_cond_lock1' - function 'r' expected different context +context.c:325:10: warning: context problem in 'warn_odd_looping' - function 'r' expected different context * check-error-end */ |