| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Some structure types provide a set of fields of which most users will
only initialize the subset they care about. Users of these types should
always use designated initializers, to avoid relying on the specific
structure layout. Examples of this type of structure include the many
*_operations structures in Linux, which contain a set of function
pointers; these structures occasionally gain a new field, lose an
obsolete field, or change the function signature for a field.
Add a new attribute designated_init; when used on a struct, it tells
Sparse to warn on any positional initialization of a field in that
struct.
The new flag -Wdesignated-init controls these warnings. Since these
warnings only fire for structures explicitly tagged with the attribute,
enable the warning by default.
Includes documentation and test case.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
sparse's -Wall option turns on all sparse warnings, including those that
many projects will not want; for instance, warnings that enforce
particular stylistic choices, or behavior allowed by a standard but
considered questionable or error-prone. Furthermore, using -Wall means
accepting all future warnings sparse may start issuing, not just those
intentionally turned on by default.
Other compilers like GCC also use -Wall, and interpret it to mean "turn
on a sensible set of warnings". Since sparse exists to emit warnings,
it already defaults to emitting a sensible set of warnings. Many
projects pass the same options to both sparse and the C compiler,
including warning options like -Wall; this results in turning on
excessive amounts of sparse warnings.
cgcc already filtered out -Wall, but many projects invoke sparse
directly rather than using cgcc. Remove that filter, now that -Wall
does not change sparse's behavior.
Projects almost certainly don't want to use the new -Wsparse-all option;
they should choose the specific set of warnings they want, or just go
with sparse's defaults.
Also update cgcc to know about Wsparse-all and not pass it to GCC, and
update a test case that unnecessarily used -Wall.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Make tokenizer aware of tabstops and add the commandline option:
-ftabstop=WIDTH
Set the distance between tab stops. This helps sparse report correct
column numbers in warnings or errors. If the value is less than 1 or
greater than 100, the option is ignored. The default is 8.
With simplifications suggested by Christopher Li and Junio C Hamano.
Signed-off-by: Hannes Eder <hannes@hanneseder.net>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
> Do you want to resend your change which revert the context changes?
> Make it base on Josh's git's tree and I will merge your changes in my
> branch.
Below. Or I can give it to you in git if you prefer. I still think we
should redo this in some form so that annotations with different
contexts can work properly, but I don't have time to take care of it
right now.
johannes
>From ca95b62edf1600a2b55ed9ca0515d049807a84fc Mon Sep 17 00:00:00 2001
From: Johannes Berg <johannes@sipsolutions.net>
Date: Tue, 23 Dec 2008 10:53:19 +0100
Subject: [PATCH] Revert context tracking code
|
|
|
|
| |
Signed-off-by: Alexey zaytsev <alexey.zaytsev@gmail.com>
|
|
|
|
|
|
|
|
|
| |
This adds -W[no-]declaration-after-statement, which makes warnings about
declarations after statements a command-line option. (The code to implement
the warning was already in there via a #define; the patch just exposes it
at runtime.) Rationale: C99 allows them, C89 doesn't.
Signed-off-by: Geoff Johnstone <geoff.johnstone@googlemail.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This patch enables a very simple form of conditional context tracking,
namely something like
if (spin_trylock(...)) {
[...]
spin_unlock(...);
}
Note that
__ret = spin_trylock(...);
if (__ret) {
[...]
spin_unlock(...);
}
does /not/ work since that would require tracking the variable and doing
extra checks to ensure the variable isn't globally accessible or similar
which could lead to race conditions.
To declare a trylock, one uses:
int spin_trylock(...) __attribute__((conditional_context(spinlock,0,1,0)))
{...}
Note that doing this currently excludes that function itself from context
checking completely.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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>
|
|
|
|
| |
Signed-off-by: Josh Triplett <josh@freedesktop.org>
|
|
This manpage documents every Sparse warning option and the corresponding
attributes, except that it does not document -Wuninitialized, which only seems
to work with -v, and which references internal Sparse register numbers.
Signed-off-by: Josh Triplett <josh@freedesktop.org>
|