| Age | Commit message (Collapse) | Author | Files | Lines |
|
Currently, -Wno-universal-initializer is simply implemented
by simply replacing '{ 0 }' by '{ }'.
However, this is a bit too simple when it concerns scalars
initialized with '{ 0 }' because:
* sparse & GCC issued warnings for empty scalar initializers
* initializing a pointer with '{ }' is extra bad.
So, restore the old behaviour for scalar initializers.
This is done by leaving '{ 0 }' as-is at parse time and changing
it as '{ }' only at evaluation time for compound initializers.
Fixes: 537e3e2daebd37d69447e65535fc94e82b38fc18
Thanks-to: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
* conditionally accept { 0 } without warnings
|
|
* warn when jumping into statement expressions
* warn when using undefined labels
* warn on defined but unused labels
It's not allowed to do a goto into an expression statement.
For example, it's not well defined what should happen if such
an expression is not otherwise reachable and/or can be optimized
away. For such situations GCC issues an error, clang doesn't
and produce a valid IR but Spare produce an invalid IR with
branches to unexisting BBs.
The goals of the patches in this series are:
*) to detect such gotos at evaluation time;
*) issue a sensible error message;
*) avoid the linearization of functions with invalid gotos.
The implementation principle behind these is to add a new kind
of scope (label_scope), one for the usual function scope of
labels one for each statement expressions. This new scope,
instead of being used as a real scope for the visibility of
labels, is used to mark where labels are defined and where
they're used.
Using this label scope as a real scope controling the
visibility of labels was quite appealing and was the initial
drive for this implementation but has the problem of inner
scope shadowing earlier occurence of labels identically
named. This is of course desired for 'normal' symbols but for
labels (which are normally visible in the whole function
and which may be used before being declared/defined)
it has the disadvantage of:
*) inhibiting the detecting of misuses once an inner scope
is closed
*) allowing several distinct labels with the same name
in a single function (this can be regarded as a feature
but __label__ at block scope should be used for this)
*) create diffrences about what is permssble or not between
sparse and GCC or clang.
|
|
In standard C '{ 0 }' is valid to initialize any compound object.
OTOH, Sparse allows '{ }' for the same purpose but:
1) '{ }' is not standard
2) Sparse warns when using '0' to initialize pointers.
Some projects (git) legitimately like to be able to use the
standard '{ 0 }' without the null-pointer warnings
So, add a new warning flag (-Wno-universal-initializer) to
handle '{ 0 }' as '{ }', suppressing the warnings.
Reference: https://lore.kernel.org/git/1df91aa4-dda5-64da-6ae3-5d65e50a55c5@ramsayjones.plus.com/
Reference: https://lore.kernel.org/git/e6796c60-a870-e761-3b07-b680f934c537@ramsayjones.plus.com/
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Currently, attributes on labels were simply ignored. This was fine
since nothing was done wth them anyway.
But now that Sparse can give a warning for unused labels it would
be nice to also support the attribute 'unused' not to issues the
warning when not desired.
So, add a small helper around handle_attributes() and use this
instead of skipping the attributes.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
In most cases symbols are automatically marked as being used via
a successfull call to lookup_symbols(), the idea being that the
symbol will be created at its declaration and then any (successfull)
lookup will correspond to an use.
For labels, things are slightly different because labels are
created on-demand via label_symbol() and their use can precede their
'declaration'. And of, course, label_symbol() has no ways to know
if it is used for a definition or an use.
So, fix this by adding an argument to label_symbol(), explictly
telling if the call correspond to an use or not.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
It's invalid to jump inside a statement expression.
So, detect such jumps, issue an error message and mark the
function as useless for linearization since the resulting IR
would be invalid.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
One way of detecting gotos inside an statement expression
is to use a new kind of scope for the gotos & labels.
Since gotos don't need to have their label predeclared,
nothing can be checked at parsing time but later it can
be checked that a goto doesn't jump inside one of the
label scope created by statement expressions.
So, add additional scope information to gotos and labels
to allow such check to be done.
Note: the label's symbols are still created in the function
scope since they belong to a single namespace.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The functions {start,end}_symbol_scope() haven't been renamed
when separated scope have been introduced for functions & blocks.
But these functions only start & end a block scope.
So, rename them to their more direct name: {start,end}_block_scope().
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Labels declared wth __label__ are special because they must follow
the block scope normally used for variables instad of using the
scope used for labels.
So, use bind_symbol_with_scope() instead of first using bind_symbol()
and then changing the namespace.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
A compound statement starts and ends a block scope, so
it's better to start & end this scope inside the function
parsing the statement: compound_statement.
The only exception is for the body of a function where
the scope also enclose the parameter declaration but that
is fine since the function is special anyway.
Move the calls to start & close the block scope inside
compound_statement() and directly call statement_list()
for the function body.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Now that the distinction is made between type modifiers and
'declaration' modifiers, there is no more reasons to parse
this attribute differently than other attributes/modifiers.
Even more so because this special casing made this attribute
to be ignored when placed after the declarator.
So, use the the generic code for 'declaration modifiers'
to parse this attribute.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Now that the distinction is made between type modifiers and
'declaration' modifiers, there is no more reasons to parse
this attribute differently than other attributes/modifiers.
So, use the the generic code for 'declaration modifiers'
to parse this attribute.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Now that the distinction is made between type modifiers and
'declaration' modifiers, there is no more reasons to parse
this attribute differently than other attributes/modifiers.
So, use the the generic code for 'declaration modifiers'
to parse this attribute.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
But for the moment do nothing special with it.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
When parsing a declaration, type specifiers, qualifiers and other
modifiers are handled by declaration_specifiers(). Some of these
are part of the type being declared but some others only concern
the object in itself. For example, the storage specifiers pertains
to the objects being declared but not their type. Because of this
the storage specifiers need to be processed separately in order to
be correctly applied to the object node. This is done via the helper:
storage_modifier().
However, some attributes are exactly in the same situation (an
obvious example is something like the section attribute).
These attributes should also be moved to the declaration and it's
only because they are currently ignored/without effect that they're
not causing problem in the type.
So generalize storage_modifiers() into decl_modifiers() to extract
all modifiers not pertaining to the type of the declared object.
The modifiers currently concerned are the attributes:
- unused
- pure
- noreturn
- externally_visible
Note: currently this change shouldn't have any effects other
than not showing anymore the "[unused]" when displaying
the type differences in diagnostics.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Add support for the attribute 'unused' (and its double underscore
variant. There is no semantic attached to it but it's now at least
parsed and added to the modifiers.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
A warning is issued when a qualifier or another modifier
is present more than once in a declaration.
This is fine but in the kernel some attributes, for example
'unused' & 'gnu_inline', are sometimes present multiple times
in the same declaration (mainly they are added in the define
used for 'inline'). This then creates a lot of useless noise.
So, use a (now empty) white list to not warn when these attributes
are present multiple times in the same declaration.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
to avoid duplicated code checking for ... duplicated modifiers!
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Despite the similarity with typeof, the approach taken here
is relatively different. A specific symbol type (SYM_TYPEOF)
is not used, instead a new flag is added to decl_state, another
one in the declared symbol and a new internal type is used:
'autotype_ctype'. It's this new internal type that will be
resolved to the definitive type at evalution time.
It seems to be working pretty well, maybe because it
hasn't been tested well enough.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Currently it is not possible to figure out the scope of the private
struct/union/enum type, its ->scope is NULL because bind_symbol() is
not called.
Change struct_union_enum_specifier() to set sym->scope = block_scope
in this case, this is what bind_symbol() does when type has a name.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Diagnostics related to a bitfield and issued after parsing
didn't display the bitfield name because it was not available.
Now that that the name is available, use it in error messages
since it helps to find the origin of the problem.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
While issuing a diagnostic related to a bitfield, it's useful
to display the bitfield's name. Unfortunately, this name is not
stored in the symbol and thus is only available during parsing.
Fix this by adding the ident to the symbol initialization.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Sparse knows about __int128_t, __uint128_t & __int128.
However, internally, these types are treated as a kind of 128-bit
'long long long' type. It's mainly a question of variable naming,
but these types are also displayed by show_typename() as
'long long long' which can't be parsed back, neither by GCC,
nor even by sparse itself.
So, rename the variables to use 'int128' and let show_typename()
display these types as '[signed|unsigned] __int128'.
Reported-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
|
|
Modifiers like MOD_CHAR, MOD_LONG, ... are used to keep track
of the size/rank of integers and floats. But this is problematic:
* they use 5 of the precious modifiers bits
* they're not really modifiers but the base of the primitive types
* a 'long double' has MOD_LONGLONG set
Change this by using instead the explicit notion of 'rank'.
The advanatges are:
* using only 3 bits (in struct symbol)
* free 5 modifier bits
* follow closely the definition & rules of the standard
* things like integer promotion are slightly simpler.
Note: this is somewhat experimental as I'm not yet convinced
that everything is correct (but the testsuite is OK
and gives the same result for a defconfig + allyesconfig
build of the kernel).
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Function attributes need to be parsed differently
than the usual specifiers: For example, in code like:
#define __noreturn __attribute__((noreturn))
__noreturn void foo(int a);
the __noreturn attribute should apply to the function type
while a specifier like 'const' would apply to its return type.
The situation is quite similar to how storage specifiers
must not be handled by alloc_indirect_symbol().
However, the solution used for storage specifiers (apply the
modifier bits only after the declarator is reached: cfr.commit
233d4e17c ("function attributes apply to the function declaration"))
can't be used here (because the storage modifiers can be applied
to the outermost declarator and function attributes may be
applied more deeply if function pointers are present).
Fix this by:
1) reverting the previous storage-specifier-like solution
2) collect function specifiers MODs in a new separate
field in the declaration context (f_modifiers)
3) apply these modifiers when the declarator for the
function type is reached (note: it must not be
applied to the SYM_FN itself since this correspond
to the function's return type; it must be applied to
the parent node which can be a SYM_NODE or a SYM_PTR).
4) also apply these modifiers to the declared symbol,
if this symbol is a function declaration, to take
into account attributes which are placed at the end
of the declaration and not in front.
Reported-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Fixes: 233d4e17c544e1de252aed8f409630599104dbc7
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Function attributes relate to the function declaration they
appear in. Sparse ignore most these attributes but a few ones
have a semantic value: 'pure', 'noreturn' & 'externally_visible'.
Due to how Sparse parse attributes and how these attributes
are stored for functions, the attributes 'pure' & 'noreturn'
are applied not to the function itself but its return type
if the function returns a pointer.
Fix this by extracting these attributes from the declaration
context and ensure they're applied to the declarator.
Reported-by: John Levon <john.levon@joyent.com>
Reported-by: Alex Kogan <alex.kogan@oracle.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Some older codebases hit this warning all the time, so it's useful
to be able to disable it.
Signed-off-by: John Levon <john.levon@joyent.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
In declaration_specifiers() the variable 'size' is used to
make the distinction between char/short/int/long/long long/...
but this correspond more closely to the notion of 'rank' since
some of these types can have the same bit-size.
Rename the variable 'size' to 'rank'.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
To determine the rank of shorts & floats, the keyword type
KW_LONG is used but there is no need for a specific keyword
since testing for 'Set_Long' has the same effect.
So, remove this keyword and test for 'Set_Long' instead
as this somehow clarify the processing of specifiers.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
To determine the rank of shorts & floats, the keyword type
KW_SHORT is used but there is no need for a specific keyword
since testing for 'Set_Short' or 'Set_Float' has the same
effect.
So, remove this keyword and test the Set_... flags instead
as this somehow clarify the processing of specifiers.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
__int128 is processed as-if 'long' is applied to a
'long long'-like type. But this is not necessary or
desirable: better to be more direct and process it
as a kind of 'long long long' type.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Add it for the specifier that implies an integer type.
Since Cint is (and must be) zero, it changes nothing
but it helps a little for as documentation.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The array types[] is initialized without using 'designators'
but the entries are in fact indexed by the 'class'.
Make this clear by using the 'class' constant (CInt, CUInt, CSInt
& CReal) as designator in the table initializer.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
When parsing specifiers, Sparse process chars independently
of other integers. But this is not necessary (as long as the
proper see/test/set flags are correct).
Change this by processing chars the same as other integers
but 'below' shorts.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
To avoid macro name collisions and improve portability
the kernel want to use a double underscore prefix and
suffix on all attributes.
However, for some of them (mainly sparse's extensions),
Sparse know only about the plain name.
Teach Sparse about the double underscore for all
attributes.
Link: https://lore.kernel.org/r/7a15bc8ad7437dc3a044a4f9cd283500bd0b5f36.camel@perches.com
Link: https://lore.kernel.org/r/19fd23e98bab65a1ee624445193bd2ed86108881.camel@perches.com
Originally-by: Joe Perches <joe@perches.com>
Reported-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The "graph" binary segfaults on this input:
asm("");
with gdb saying (edited for clarity):
Program received signal SIGSEGV, Segmentation fault.
in graph_ep (ep=0x7ffff7f62010) at graph.c:52
(gdb) p ep->entry
$1 = (struct instruction *) 0x0
Sadly, the commit that introduced this crash:
15fa4d60e ("topasm: top-level asm is special")
was (part of a bigger series) meant to fix crashes because
of such toplevel asm statements.
Toplevel ASM statements are quite abnormal:
* they are toplevel but anonymous symbols
* they should be limited to basic ASM syntax but are not
* they are given the type SYM_FN but are not functions
* there is nothing to evaluate or expand about it.
These cause quite a few problems including crashes, even
before the above commit.
So, before handling them more correctly and instead of
adding a bunch of special cases here and there, temporarily
take the more radical approach of stopping to add them to
the list of toplevel symbols.
Fixes: 15fa4d60ebba3025495bb34f0718764336d3dfe0
Reported-by: Vegard Nossum <vegard.nossum@gmail.com>
Analyzed-by: Vegard Nossum <vegard.nossum@gmail.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Currently, ASM operands aren't expanded or even evaluated.
This causes Sparse to emit warnings about 'unknown expression'
during the linearization of these operands if they contains,
for example, calls to __builtin_compatible_types_p().
Note: the correct handling of ASM operands needs to make
the distinction between 'memory' operands and 'normal'
operands. For this, it is needed to look at the constraints
and these are architecture specific. The patches in this
series only consider the constraints m, v, o & Q as
being for memory operands and, happily, these seems
to cover most usage for the most common architectures.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The function cast_value() needs the exact type of the old expression
but when called via cast_enum_list() this type is incorrect because:
- the same struct is used for the new and the old expression
- the type of the new expression is adjusted before cast_value()
is called.
Fix this by adjusting the type of the new expression only after
cast_value() has been called.
Fixes: 604a148a73af ("enum: fix cast_enum_list()")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The function cast_enum_list() is used to give the same type to
all elements of an enum declaration. The base case for doing this
is to call cast_value() on the element, but this call is not done
is the size of the element already match the size of the common type.
This special case is an optimization but not an interesting one since
cast_value() is not a costly function. OTOH, it somehow complicates
the flow inside cast_enum_list().
So, remove the optimisation by letting cast_value() to handle
all cases.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Before commit 756731e9 ("use a specific struct for asm operands")
ASM operands where stored as a list of n times 3 expressions.
After this commit, the triplets where stored inside a single
expression of type EXPR_ASM_OPERAND.
However, while this improved the parsing and use of ASM operands
it needlessly reuse 'struct expression' for something that is not
an expression at all.
Fix this by really using a specific struct for ASM operands.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The syntax of extended ASM statements requires that the
bodies & constraints are given via a literal string.
However, at parsing time more general expressions are accepted
and it's checked only at evaluation time if these are effectively
string literals. This has at least two drawbacks:
*) evaluate_asm_statement() is slightly more complicated than
needed, mixing these checks with the real evaluation code
*) in case of error, the diagnostic is issued later than
other syntaxic warnings.
Fix this by checking at parse-time that ASM bodies & constraints
are string literals and not some arbitrary expressions.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The error handling during the parsing of _Static_assert()'s
message string is relatively complex.
Simplify this by using the new helper string_expression().
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
sparse warns for non-static functions that don't have a separate
declaration. The kernel contains several such functions that are marked
as __attribute__((externally_visible)) to mark that they are called from
assembly code. Assembly code doesn't need a header with a declaration to
call a function. Therefore, suppress the warning for functions with
__attribute__((externally_visible)).
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The parsing of enums contains a self-assignment to base_type.
But this self-assignment doesn't show very clearly that the
variable doesn't need to change and that some compilers complain.
So, replace that self-assignment by a null statement.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
GCC's trunk now allows to specifiy 'inline' with asm statements.
This feature has been asked by kernel devs and will most probably
by used for the kernel.
So, teach sparse about this syntax too.
Note: for sparse, there is no semantic associated to this inline
because sparse doesn't make any size-based inlining decisions.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Warn on non-sensical declarations like:
int __user __iomem *ptr;
These can be easily be checked at parsing time.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Currently, address space 1 is displayed as '<asn:1>' and so on.
Now that address spaces can be displayed by name, the address space
number should just be an implementation detail and it would make
more sense the be able to 'declare' these address space directly
by name, like:
#define __user attribute((noderef, address_space(__user)))
Since directly using the name instead of an number creates some
problems internally, allow this syntax but for the moment keep
the address space number and use a table to lookup the number
from the name.
References: https://marc.info/?l=linux-sparse&m=153627490128505
Idea-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Currently, address space are identified by an number and
displayed as '<asn:%d>'. It would be more useful to display
a name like the one used in the code: '__user', '__iomem', ....
Prepare this by using an identifier instead of the AS number.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Sparse unconditionally issues warnings about non-ANSI function
declarations & definitions.
However, some environments have large amounts of legacy headers
that are pre-ANSI, and can't easily be changed. These generate
a lot of useless warnings.
Fix this by using the options flags -Wstrict-prototypes &
-Wold-style-definition to conditionalize these warnings.
Signed-off-by: John Levon <levon@movementarian.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The declaration of a function without prototype is currently
silently accepted by sparse but a warning is issued for
'old-style' declarations:
... warning: non-ANSI function declaration ...
However, the difference between these two cases is made by
checking if a ';' directly follow the parentheses. So:
int foo();
is silently accepted, while a warning is issued for:
int foo(a) int a;
but also for:
int foo(), bar();
This last case, while unusual, is not less ANSI than a simple
'int foo();'. It's just detected so because there is no ';'
directly after the first '()'.
Fix this by also using ',' to detect the end of function
declarations and their ANSIness.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
In legacy environment, a lot of warnings can be issued about
arguments without an explicit type.
Fix this by contitionalizing such warnings with the flag
-Wimplicit-int, reducing the level of noise in such environment.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
In an old-style function definition, if not explicitly specified,
the type of an argument defaults to 'int'.
Sparse issues an error for such arguments and leaves the type
as 'incomplete'. This can then create a cascade of other warnings.
Fix this by effectively giving the type 'int' to such arguments.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Currently, the error message issued for an empty enum is
"bad enum definition". This is exactly the same message
used when one of the enumerator is invalid.
Fix this by using a specific error message.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
GCC uses an unsigned type for enum's basetype unless one of
the enumerators is negative.
Using 'int' for plain simple enumerators and then using the same
rule as for integer constants (int -> unsigned int -> long -> ...)
should be more natural but doing so creates useless warnings when
using sparse on the kernel code.
So, do the same as GCC:
* uses the smaller type that fits all enumerators,
* uses at least int or unsigned int,
* uses an signed type only if one of the enumerators is negative.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
In Standard C, enumerators have type 'int' and an unspecified
base type. OTOH, GCC (and thus sparse) llows any integer type
(and sparse also more or less allows bitwise types).
After the enum's decalration is parsed, the enumerators are
converted to the underlying type.
Also, GCC (and thus sparse) uses an unsigned type unless one of
the enumerators have a negative value.
This is a problem, though, because when comparing simple integers
with simple enumerators like:
enum e { OK, ONE, TWO };
the integers will unexpectedly be promoted to unsigned.
GCC avoid these promotions by not converting the enumerators that
fit in an int.
For GCC compatibility, do the same: do not convert enumerators
that fit in an int.
Note: this is somehow hackish but without this some enum usages
in the kernel give useless warnings with sparse.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
To determine the base type of enums it is needed to keep
track of the range that the enumerators can take.
However, this tracking seems to be more complex than needed.
It's now simplified like this:
-) a single 'struct range' keep track of the biggest positive
value and the smallest negative one (if any)
-) the bound checking in itself is then quite similar to
what was already done:
*) adjust the bit size if the type is negative
*) check that the positive bound is in range
*) if the type is unsigned
-> check that the negative bound is 0
*) if the type is signed
-> check that the negative bound is in range
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
During the parsing of enum definitions, if some invalid
type combination is reached, the base type is forced to
'bad_ctype'. Good.
However, this is done without a warning and it's only when
the enum is used that some sign of a problem may appear,
with no hint toward the true cause.
Fix this by issuing a warning when the base type becomes invalid
but only if the type of the enumerator is itself not already
set to 'bad_ctype' (since it this case a more specific warning
has already been issued).
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Sparse supports enum initializers with bitwise types but
this makes sense only if they are all the same type.
Add a check and issue a warning if an enum is initialized
with different restricted types.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
As an extension to the standard C types, parse supports bitwise
types (also called 'restricted') which should in no circonstances
mix with other types.
In the kernel, some enums are defined with such bitwise types
as initializers; the goal being to have slightly more strict enums.
While the semantic of such enums is not very clear, using a mix
of bitwise and not-bitwise initializers completely defeats the
desired stricter typing.
Attract some attention to such mixed initialization by issuing
a single warning for each such declarations.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The C standard requires that the type of enum constants is 'int'.
So a constant not representable as an int can't be used as the
initializer of an enum. GCC extend this by using, instead of 'int',
the smallest type that can represent all the values of the enum:
first int, then unsigned int, long, ...
For sparse, we need to take in account the bitwise integers.
However, currently sparse doesn't do this based on the values
but on the type, so if one of the initializer is, for example,
1L, the base type is forced to a size as least as wide as 'long'.
Fix this by removing the call to bigger_enum_type().
Note that this is essentially a revert of commit
"51d3e7239: Make sure we keep enum values in a sufficiently large type for parsing"
which had the remark: "Make sure that the intermediate stages
keep the intermediate types big enough to cover the full range."
But this is not needed as during parsing, the values are kept at
full width and with their original type & value.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The C standard requires that the type of enum constants is 'int'
and let the enum base/compatible type be implementation defined.
For this base type, instead of 'int', GCC uses the smallest type
that can represent all the values of the enum (int, unsigned int,
long, ...)
Sparse has the same logic as GCC but if all the initializers
have the same type, this type is used instead.
This is a sensible choice but often gives differents
result than GCC.
To stay more compatible with GCC, always use the same logic
and thus only keep the common type as base type for restricted
types.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Sparse want that an enum's enumerators have all the same type.
This is done by first determining the common type and then
calling cast_enum_list() which use cast_value() on each member
to cast them to the common type.
However, cast_value() doesn't create a new expression and doesn't
change the ctype of the target: the target expression is supposed
to have already the right type and it's just the value that is
transfered from the source expression and size adjusted.
It's seems that in cast_enum_list() this has been overlooked
with the result that the value is correctly adjusted but keep
it's original type.
Fix this by updating, for each member, the desired type.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Shifting by an amount greater or equal than the width
of the type is Undefined Behaviour. In the present case,
when type_is_ok() is called with a type as wide as an
ullong (64 bits here), the bounds are shifted by 64
which is UB and at execution (on x86) the value is simply
unchanged (since the shift is done with the amount modulo 63).
This, of course, doesn't give the expected result and
as consequence valid enums can have an invalid base type
(bad_ctype).
Fix this by doing the shift with a small helper which
return 0 if the amount is equal to the maximum width.
NB. Doing the shift in two steps could also be a solution,
as maybe some clever trick, but since this code is in
no way critical performance-wise, the solution here
has the merit to be very explicit.
Fixes: b598c1d75a9c455c85a894172329941300fcfb9f
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
* add support for __has_attribute()
|
|
Attributes can be used with the plain keyword or squeezed
between a pair of double underscrore.
For some reasons, 'designated_init' was not allowed with its
underscores and '__transparent_union__' wasn't without them.
So, allow '__designated_init__' & 'transparent_union'.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
In the list of keywords '__mode__' was just before the entries for
modes but 'mode' was lost in the middle of some other attributes.
Move 'mode' justbefore '__mode__'.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The error message 'unknown mode attribute' is displayed with an
additional newline at the end.
Remove this superfluous newline.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
sparse support GCC's modes like __SI__, __DI__, as well as
__word__ & __pointer__ but GCC also supports a mode __byte__.
For completeness, add this mode as an alias for mode __QI__.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
sparse support GCC's modes like __SI__, __DI__, and __word__
but GCC also supports a mode __pointer__ which is used by Xen.
Add support for this missing __pointer__ mode.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The keywords for modes (SI, DI, ...) don't need a length modifier
like MOD_CHAR or MOD_LONG as the corresponding type (and thus its
length) is given by the the '.to_mode' method.
Remove these modifiers from the keyword definitions, their presence
while unneeded is confusing.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
'fix-builtin-expect' and 'int-const-expr' into tip
|
|
If a label is defined several times, an error is issued about it.
Nevertheless, the label is used as is and once the code is linearized
several BB are created for the same label and this create
inconsistencies. For example, some code will trigger assertion failures
in rewrite_parent_branch().
Avoid the consistencies by ignoring redefined labels.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Statements with an empty expression, like:
__context__();
or
__context__(x,);
are silently accepted. Worse, since NULL expressions are usually
ignored because it is assumed they have already been properly
diagnosticated, no warnings of any kind are given at some later
stage.
Fix this by explicitly checking after empty expressions and
emit an error message if needed.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The expected syntax for the __context__ statement is:
__context__(<expression>);
or
__context__(<context>, <expression>);
but originally it was just:
__context__ <expression>
In other words the parenthesis were not needed and are
still not needed when no context is given.
One problem with the current way to parse these statements is
that very few validation is done. For example, code like:
__context__;
is silently accepted, as is:
__context__ a, b;
which is of course not the same as:
__context__(a,b);
And code like:
__context__(,1);
results in a confusing error message:
error: an expression is expected before ')'
error: Expected ) in expression
error: got ,
So, given that:
* the kernel has always used the syntax with parenthesis,
* the two arguments form requires the parenthesis and thus
a function-like syntax
use a more direct, robust and simpl parsing which enforce
the function-like syntax for both forms.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The expected syntax for the __context__ statement is:
__context__(<inc/dec value>);
or
__context__(<context>, <inc/dec value>);
The distinction between the two formats is made by checking if
the expression is a PREOP with '(' as op and with an comma
expression as inner expression.
However, code like:
__context__;
or
__context__(;
crashes while trying to test the non-existing expression
(after PREOP or after the comma expression).
Fix this by testing if the expression is non-null before
dereferencing it.
Note: this fix has the merit to directly address the problem
but doesn't let a diagnostic to be issued for the case
__context__;
which is considered as perfectly valid.
The next patch will take care of this.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Currently the parsing of the attribute 'context' is rather
complex and uses a loop which allows 1, 2, 3 or more arguments.
But the the real syntax is only correct for 2 or 3 arguments.
Furthermore the parsing mixes calls to expect() with its own
error reporting. This is a problem because if the error has first
been reported by expect(), the returned token is 'bad_token'
which has no position so you can have error logs like:
test.c:1:43: error: Expected ( after context attribute
test.c:1:43: error: got )
builtin:0:0: error: expected context input/output values
But the 'builtin:0.0' should really be 'test.c:1.43' or, even better,
there shouldn't be a double error reporting.
Fix this by simplifying the parsing and only support 2 or 3 args.
Also, make the error messages slightly more explicit about the
nature of the error.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
'fix-redef-typedef' and 'fixes' into tip
|
|
One of sparse's extension to the C language is an operator
to check ranges. This operator takes 3 operands: the expression
to be checked and the bounds.
The syntax for this operator is such that the operands need to
be a 3-items comma separated expression. This is a bit weird
and doesn't play along very well with macros, for example.
Change the syntax to a 3-arguments function-like operator.
NB. Of course, this will break all existing uses of this
extension not using parenthesis around the comma
expression but there doesn't seems to be any.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
It seems that some system headers (Debian x32's glibc) use
these types based on GCC's version without checking one of
the '__STDC_IEC_60559_<something>' macro. This, of course,
creates warnings and errors when using sparse on them.
Avoid these errors & warnings by letting sparse know about
these types.
Note: Full support would require changes in the parsing to
recognize the suffixes for constants ([fF]32, ...),
the conversion rules between these types and the
standard ones and probably and most others things,
all outside the scope of this patch.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Acked-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
|
|
check_duplicates() verifies that symbols are not redefined and
warns if they are.
However, this function is only called at evaluation-time and
then only for symbols corresponding to objects and functions.
So, typedefs can be redefined without any kind of diagnostic.
Fix this by calling check_duplicates() at parsing time on typedefs.
Note: this is C11's semantic or GCC's C89/C99 in non-pedantic mode.
Reported-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
The warning message for unknown attributes is a bit longish and
uses the word 'attribute' twice.
Change the message for something more direct, shorter and without
repetition.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
'fix-volatile-simplification', 'struct-asm-ops', 'restricted-pointers', 'fix-f2i-casts', 'symaddr-description', 'flush-stdout' and 'diet-simple' into tip
|
|
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
This only add the parsing and checks as a type qualifier;
there is no operational semantic associated with it.
Note: this only support _Atomic as *type qualifier*, not
as a *type specifier* (partly because there an
ambiguity on how to parse '_Atomic' when followed
by an open parenthesis (can be valid as qualifier
and as specifier)).
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Note: there is still no semantic associated with 'restrict'
but this is a preparatory step.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
This is slightly shorter (and thus may avoid long lines) and
facilitate the introduction of MOD_RETRICT in a later patch.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
MOD_TOPLEVEL & MOD_INLINE are already include in MOD_STORAGE
so there is no need to repeat them in MOD_IGNORE & elsewhere
where MOD_STORAGE is used.
Change this by removing the redundant MOD_TOPLEVEL & MOD_INLINE.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
but is used to track which inline functions are
effectively used. So better remove it from the MOD_...
and implement the same functionality via a flag
in struct symbol.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
ASM operands have the following syntax:
[<ident>] "<constraint>" '(' <expr> ')'
For some reasons, during parsing this is stored
as a sequence of 3 expressions. This has some serious
disadvantages though:
- <ident> has not the type of an expression
- it complicates processing when compared to having a specific
struct for it (need to loop & maintain some state).
- <ident> is optional and stored as a null pointer when not present
which is annoying, for example, if null pointers are used internally
in ptr-lists to mark removed pointers.
Fix this by using a specific structure to store the 3 elements
of ASM operands.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
This will greatly reduce the "unknown attribute" warning.
Signed-off-By: Christopher Li <sparse@chrisli.org>
|
|
Avoid create a new one if same symbol exists.
Signed-off-By: Christopher Li <sparse@chrisli.org>
|
|
It was used in kvm/vmx.c
Signed-of-By: Christopher Li <sparse@chrisli.org>
|
|
Currently, no warning is given for symbols for which no
type is explicitely given. But for functions we received
a pointless warning like here under if the returned type
is effectively an int:
warning: incorrect type in return expression (invalid types)
expected incomplete type
got int
Fix this by issuing the warning.
Also give an implicit type of int, as required by C89, to
avoid pointless warning about the expected incomplete type.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
There is two ways a keyword can be marked as reserved:
- explicitly, by using IDENT_RESERVED() in "ident-list.h"
- implicitly, by using NS_TYPEDEF as namespace in parse.c::keyword_table
Since the implicit way is not obvious, help to make it more clear
by adding a small comment on top of keyword_table[].
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
This patch introduces support for the C11 _Static_assert() construct.
Per the N1539 draft standard, the syntax changes for this construct
include:
declaration:
<declaration-specifiers> <init-declarator-list>[opt] ;
<static_assert-declaration>
struct-declaration:
<specifier-qualifier-list> <struct-declarator-list>[opt] ;
<static_assert-declaration>
static_assert-declaration:
_Static_assert ( <constant-expression> , <string-literal> ) ;
Signed-off-by: Lance Richardson <lrichard@redhat.com>
Signed-off-by: Chris Li <sparse@chrisli.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
|
|
Before the call to the method external_decl::validate_decl()
there is another validation done which check if the declaration
linkage is not external, otherwise an error is issued and the
'extern' is removed from the declaration.
While also valid for C99 for-loop initializer, this is less
desirable because in this context, 'extern' is invalid anyway
and removing it from the declaration make it imposible to issue
a diagnostic about it.
Fix this by moving the 'extern with initializer' check after the
call to validate_decl() method, where it is always pertinent and
so allowing process_for_loop_decl() to make its own diagnostic.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
In C99, it is valid to declare a variable inside a
for-loop initializer but only when the storage is local
(automatic or register). Until now this was not enforced.
Fix this, when parsing declarations in a for-loop context,
by calling external_decl() with a validate method doing the
appropriate check of the storage.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
After parsing and validation, the symbols in the declaration
are added to the list given in argument, *if* they are not extern
symbols. The symbols that are extern are them not added to the list.
This is what is needed for usual declarations but ignoring extern
symbols make it impossible to emit a diagnostic in less usual
situation.
This is motivated by the validation of variable declaration inside
a for-loop initializer, which is valid in C99 but only for variable
with local storage.
The change consists in adding to external_declaration() an optional
callback 'validate_decl()' which, if present (non-null), is called
just before adding the declaration to the list.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
The flag -Wbitwise have no effect since patch 02a886bfa
("Introduce keyword driven attribute parsing"): the corresponding
checks are now always done.
Fix that by reintroducing it in the same way as it was:
ignore the bitwise attribute if the flag is not set.
It's less invasive that checking the flag at each place
an corresponding warning is emitted.
Also, to not perturb the current situation the flag is now
enabled by default.
Reported-by: Edward Cree <ecree@solarflare.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
So far, builtin functions which had some evaluate/expand method
couldn't also have a prototype because the declaration of the
prototype and the definition of the builtin with its method would
each have their own symbol and only one of them will be seen, the
last one, the one for the prototype.
This also meant that the evaluate/expand methods had to take care
to set the correct types for they argumenst & results, which is fine
for some generic builtins like __builtin_constant_p() but much less
practical for the ones like __builtin_bswap{16,32,64}().
Fix this by letting symbols with same name share their methods.
Originally-by: Christopher Li <sparse@chrisli.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This is quite similar to GCC's 'attribute(aligned(..))' but defined
as a new specifier and also accepting a typename as argument.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This is mainly a new name for GCC's 'noreturn' attribute but defined
as a new function specifier.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This is simply a new name for GCC's '__thread' which was already
supported.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
There is already support for __int128_t & __uint128_t but not yet
for GCC's __int128.
This patch add support for it and a couple of test cases.
Note: it's slightly more tricky that it look because contrary to
'__int128_t', '__int128' is not an exact type (it can still receive
the 'unsigned' or 'signed' specifier).
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
In the code doing the parsing of type declaration there is a few
arrays for the each integer size (short, int, long, long long, ...)
The array for the unsigned and explicitely unsigned integer have
entry for 'slllong' & 'ulllong' last element but the one for plain
integer is missing its 'lllong' entry which make sparse crash when
trying to use this type.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
GCC creates new attributes quite often, generaly for specific
usages irrelevant to what sparse is used for.
Throwing errors on these create needless noise and annoyance
which is better to avoid.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
And so quiets a warning from sparse about an undeclared symbol
when running on its own code.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
The patch used the wrong enum for the maximum size of the array.
I just noticed now, while doing some merging, that what have been commited
on the master tree is not the patch discussed here.
What have been commited is the same change but with 'CMax' (integer class)
while it was supposed to be 'SMax' (for the array of specifier).
The version on sparse-next is correct though.
Here is a patch fixing this:
Fixes: 1db3b627 ("Handle SForced in storage_modifiers")
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
We have been seeing errors like this for a while now in the sparse
Fedora package, when doing kernel builds:
./include/linux/err.h:53:25: warning: dereference of noderef expression
./include/linux/err.h:35:16: warning: dereference of noderef expression
This spews all over the build because this comes from IS_ERR(), which
is called everywhere. Even odder, it turns out that if we build the
package with -fpic turned off, then it works fine.
With some brute-force debugging, I think I've finally found the cause.
This array is missing the SForced element. When this is added then the
problem goes away.
As to why this goes away when -fpic is removed, I can only assume that
we get lucky with the memory layout and have a zeroed out region just
beyond the end of the array.
Fixes: 3829c4d8b097776e6b3472290a9fae08a705ab7a
Cc: Al Viro <viro@ftp.linux.org.uk>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Add attribute "no_sanitize_address" or "__no_sanitize_address__" as an ignored
attribute. Fixes this sparse warning:
include/linux/compiler.h:232:8: error: attribute 'no_sanitize_address': unknown attribute
Also add test case for 'no_sanitize_address': validation/attr-no_sanitize_address.c.
'make check' says for this test case:
TEST attribute no_sanitize_address (attr-no_sanitize_address.c)
Signed-off-by: Rui Teng <rui.teng@linux.vnet.ibm.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
The __assume_aligned__ attribute can be safely ignored, add it
to the list of ignored attributes and add a test to verify that
this attribute is ignored.
Signed-off-by: Lance Richardson <lrichard@redhat.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Missing enum members in case statements in c2xml.c and parse.c were
causing compile time complaints by gcc 5.1.1. Adding a default case
satisfies the compiler and notifies the reviewer that there are
cases not explicitly mentioned being handled by the default case.
Signed-off-by: Tony Camuso <tcamuso@redhat.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
gcc knows about a new "hotpatch" attribute which sparse can safely ignore,
since it modifies only which code will be generated just like the
"no_instrument_function" attribute.
The gcc hotpatch feature patch:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=11762b8363737591bfb9c66093bc2edf289b917f
Currently the Linux kernel makes use of this attribute:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=61f552141c9c0e88b3fdc7046265781ffd8fa68a
Without this patch sparse will emit warnings like
"error: attribute 'hotpatch': unknown attribute"
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Add some more ignored attributes which are used in glibc header files,
along with a simple test case which includes all three inline attributes
(__gnu_inline__, __always_inline__ and __noinline__).
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Consider the following case, extern inline declare after
extern declare of the same function.
extern int g(int);
extern __inline__ int g(int x)
{
return x;
}
Sparse will give the first function global scope and
the second one file scope. Also the first one will get the
function body from the second one. That cause the failure
of the validation/extern-inlien.c
This change rebind the scope of the same_symbol chain to
the new scope. It will pass the extern-inline.c test case.
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
The n1570 specifies (in 6.7.6.2.3) that either type-qualifiers
(ie: "restrict") come first and are followed by "static" or the
opposite ("static" then type-qualifiers).
Also add a test.
Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This stops warnings in code using socket operations with a modern glibc,
which otherwise result in warnings of the form:
warning: incorrect type in argument 2 (invalid types)
expected union __CONST_SOCKADDR_ARG [usertype] __addr
got struct sockaddr *<noident>
Since transparent unions are only applicable to function arguments, we
create a new function to check that the types are compatible
specifically in this context.
Also change the wording of the existing warning slightly since sparse
does now support them. The warning is left in case people want to avoid
using transparent unions.
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
The old code was relicensed by Novafora Corporation, successor in interest to
Transmeta Corporation, in 2009. Other authors were also asked about the change
of their contributions to the MIT license and all with copyrightable changes
agreed to it.
Signed-off-by: Franz Schrober <franzschrober@yahoo.de>
Acked-by: Adam DiCarlo <adam@bikko.org>
Acked-by: Al Viro <viro@ZenIV.linux.org.uk>
Acked-by: Alberto Bertogli <albertito@blitiri.com.ar>
Acked-by: Alecs King <alecs@perlchina.org>
Acked-by: Alexander Shishkin <alexander.shishckin@gmail.com>
Acked-by: Alexey Dobriyan <adobriyan@gmail.com>
Acked-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
Acked-by: Andries E. Brouwer <Andries.Brouwer@cwi.nl>
Acked-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Acked-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Acked-by: Ben Pfaff <blp@nicira.com>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Acked-by: Bernd Petrovitsch <bernd@petrovitsch.priv.at>
Acked-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Acked-by: Blue Swirl <blauwirbel@gmail.com>
Acked-by: Chris Forbes <chrisf@ijw.co.nz>
Acked-by: Chris Wedgwood <cw@f00f.org>
Acked-by: Christopher Li <sparse@chrisli.org>
Acked-by: Damien Lespiau <damien.lespiau@gmail.com>
Acked-by: Dan Carpenter <error27@gmail.com>
Acked-by: Dan McGee <dan@archlinux.org>
Acked-by: Daniel De Graaf <danieldegraaf@gmail.com>
Acked-by: Daniel Sheridan <dan.sheridan@postman.org.uk>
Acked-by: Dave Jones <davej@redhat.com>
Acked-by: David Given <dg@cowlark.com>
Acked-by: David Miller <davem@redhat.com>
Acked-by: David Mosberger-Tang <dmosberger@gmail.com>
Acked-by: David Olien <David.Olien@lsi.com>
Acked-by: Diego Elio Pettenò <flameeyes@flameeyes.eu>
Acked-by: Emil Medve <Emilian.Medve@Freescale.com>
Acked-by: Ethan Jackson <jacksone@nicira.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Frank Zago <fzago@systemfabricworks.com>
Acked-by: Frederic Crozat <fcrozat@suse.com>
Acked-by: Geoff Johnstone <geoff.johnstone@gmail.com>
Acked-by: Hannes Eder <hannes@hanneseder.net>
Acked-by: Jan Pokorný <pokorny_jan@seznam.cz>
Acked-by: Jeff Garzik <jgarzik@redhat.com>
Acked-by: Jiri Slaby <jslaby@suse.cz>
Acked-by: Joe Perches <joe@perches.com>
Acked-by: Joel Soete <rubisher@scarlet.be>
Acked-by: Johannes Berg <johannes@sipsolutions.net>
Acked-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Acked-by: Josh Triplett <josh@kernel.org>
Acked-by: Kamil Dudka <kdudka@redhat.com>
Acked-by: Kim Phillips <kim.phillips@linaro.org>
Acked-by: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Acked-by: Kovarththanan Rajaratnam <kovarththanan.rajaratnam@gmail.com>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Acked-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Acked-by: Martin Nagy <nagy.martin@gmail.com>
Acked-by: Masatake YAMATO <yamato@redhat.com>
Acked-by: Mauro Dreissig <mukadr@gmail.com>
Acked-by: Michael Büsch <m@bues.ch>
Acked-by: Michael Stefaniuc <mstefani@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Mika Kukkonen <mikukkon@iki.fi>
Acked-by: Mike Frysinger <vapier@gentoo.org>
Acked-by: Mitesh Shah <Mitesh.Shah@synopsys.com>
Acked-by: Morten Welinder <mortenw@gnome.org>
Acked-by: Namhyung Kim <namhyung@gmail.com>
Acked-by: Nicolas Kaiser <nikai@nikai.net>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Pavel Roskin <proski@gnu.org>
Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>
Acked-by: Peter Jones <pjones@redhat.com>
Acked-by: Peter A Jonsson <pj@sics.se>
Acked-by: Ralf Wildenhues <Ralf.Wildenhues@gmx.de>
Acked-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Acked-by: Randy Dunlap <rdunlap@xenotime.net>
Acked-by: Reinhard Tartler <siretart@tauware.de>
Ached-by: Richard Knutsson <richard.knutsson@gmail.com>
Acked-by: Rob Taylor <rob.taylor@codethink.co.uk>
Acked-by: Rui Saraiva <rmpsaraiva@gmail.com>
Acked-by: Ryan Anderson <ryan@michonline.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Acked-by: Samuel Bronson <naesten@gmail.com>
Acked-by: Santtu Hyrkkö <santtu.hyrkko@gmail.com>
Acked-by: Shakthi Kannan <shakthimaan@gmail.com>
Acked-by: Stephen Hemminger <shemminger@linux-foundation.org>
Acked-by: Thomas Schmid <Thomas.Schmid@br-automation.com>
Acked-by: Tilman Sauerbeck <tilman@code-monkey.de>
Acked-by: Vegard Nossum <vegardno@ifi.uio.no>
Acked-by: Xi Wang <xi.wang@gmail.com>
Acked-by: Yura Pakhuchiy <pakhuchiy@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
It will indicate this argument will skip the type compatible check.
It allow PTR_ERR() to accept __iomem pointer without complaining.
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Add attribute "noclone" or "__noclone" or "__noclone__" as an
ignored attribute. Fixes this sparse warning:
arch/x86/kvm/vmx.c:6268:13: error: attribute '__noclone__': unknown attribute
Also add test case for 'noclone': validation/attr-noclone.c.
'make check' says for this test case:
TEST attribute noclone (attr-noclone.c)
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Add some more ignored attributes which are used in glibc header files. It
silences the warnings such as the following:
/usr/include/bits/fcntl2.h:36:1: error: attribute '__error__': unknown attribute
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
We already had "vector_size" but we also need __vector_size__ to silence
some warnings in glibc:
/usr/include/bits/link.h:67:45: error: attribute '__vector_size__': unknown attribute
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This patch sets ->stmt of a SYM_LABEL to the corresponding label
statement. If ->stmt was already set, it is a duplicate label.
On the other hand, if ->stmt of a goto label is not set during
evaluation, the label was never declared.
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This patch adds the 'leaf' GCC attribute to the list of ignored
attributes. Glibc uses this attribute causing the following
warnings in userspace projects:
/usr/include/stdlib.h:514:26: error: attribute '__leaf__': unknown attribute
Signed-off-by: Ethan Jackson <ethan@nicira.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This patch fixes __builtin_safe_p() to work properly for calls to pure
functions.
Cc: Christopher Li <sparse@chrisli.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Skip the expression instead of adding a null one.
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Michael Stefaniuc <mstefani@redhat.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This patch adds the 'artifical' GCC attribute to list of ignore attributes.
It's an attribute that's used by glibc which causes the following bogus sparse
warnings when using it for userspace projects:
/usr/include/bits/stdlib.h:37:1: error: attribute '__artificial__': unknown attribute
/usr/include/bits/stdlib.h:64:1: error: attribute '__artificial__': unknown attribute
Signed-off-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Report by Randy Dunlap, attribute vector_size is causing
error. Ignore it for now.
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Jan Pokorny <pokorny_jan@seznam.cz>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
The Blackfin port uses some custom attributes to control memory placement,
and it has some custom builtins. So add the ones that the kernel actually
utilizes to avoid massive build errors with sparse.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Morten Welinder <terra@gnome.org>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
1) We now handle only "asm (volatile|goto)?", whereas
"asm volatile? goto?" is correct.
2) We need to match only goto_ident, so do it explicitly against
token->ident without match_idents.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Christopher <sparse@chrisli.org>
|
|
As of gcc 4.5, asm goto("jmp %l[label]" : OUT : IN : CLOB : LABELS) is
supported. Add this support to the parser so that it won't choke on
the newest Linux kernel when compiling with gcc 4.5.
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
may_alias is used in the wild (glib) and makes sparse spew a lot of
unhelpful warning messages. Ignore it (for now?).
Signed-off-by: Damien Lespiau <damien.lespiau@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
The GCC "naked" attribute is used on certain architectures to generate
functions without a prologue/epilogue.
Ignore it in sparse.
Signed-off-by: Michael Buesch <mb@bu3sch.de>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This adds more ignored gcc-style attributes.
externally_visible is a standard gcc attribute.
signal is an AVR8 attribute used to define interrupt service routines.
Ignore these attributes, as they are currently not useful for sparse checking.
Signed-off-by: Michael Buesch <mb@bu3sch.de>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
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>
|
|
For Win64 compiles Wine does
#ifndef __ms_va_list
# if defined(__x86_64__) && defined (__GNUC__)
# define __ms_va_list __builtin_ms_va_list
# define __ms_va_start(list,arg) __builtin_ms_va_start(list,arg)
# define __ms_va_end(list) __builtin_ms_va_end(list)
# else
Wouldn't be as bad if sparse cannot handle those but it trips over
WINBASEAPI DWORD WINAPI FormatMessageA(DWORD,LPCVOID,DWORD,DWORD,LPSTR,DWORD,__ms_va_list*);
WINBASEAPI DWORD WINAPI FormatMessageW(DWORD,LPCVOID,DWORD,DWORD,LPWSTR,DWORD,__ms_va_list*);
producing this errors for basically every file:
wine/include/winbase.h:1546:96: error: Expected ) in function declarator
wine/include/winbase.h:1546:96: error: got *
wine/include/winbase.h:1547:97: error: Expected ) in function declarator
wine/include/winbase.h:1547:97: error: got *
Signed-off-by: Michael Stefaniuc <mstefaniuc@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Wine has annotated the Win32 alloc functions with the alloc_size
attribute. This cuts down the noise a lot when running sparse on the
Wine source code.
Signed-off-by: Michael Stefaniuc <mstefaniuc@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This is needed for getting a meaningful sparse run on a Wine 64-bit
compile. Else the basic Win32 headers will produce tons of
error: attribute 'ms_abi': unknown attribute
which end in
error: too many errors.
The sysv_abi attribute was just added for symmetry.
Signed-off-by: Michael Stefaniuc <mstefaniuc@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Adding ignored attributes is much easier now.
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Hello,
enclosed is a simple patch adding support for attribute 'noreturn' to the
parser. The enhancement makes it possible to optimize walk through CFG and
thus help us to fight with the state explosion. The benefit is demonstrated
on a simple real-world example.
Generated CFG before patch:
http://dudka.cz/devel/html/slsparse-before/slplug.c-handle_stmt_assign.svg
Generated CFG after patch:
http://dudka.cz/devel/html/slsparse-after/slplug.c-handle_stmt_assign.svg
It's one of the key features I am currently missing in SPARSE in contrast
to gcc used as parser. Thanks in advance for considering it!
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
GCC provides a 128 bit type called internally as TImode (__int128_t)on 64 bit
platforms (at least x86_64 and Sparc64). These types are used by OpenBIOS.
Add support for types "long long long", __mode__(TI) and __(u)int128_t.
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
This avoids getting annoying warnings from <curl/typecheck-gcc.h> and from
<bits/string3.h>, which use the "__attribute__((__warning__ (msg)))" gcc
attribute.
[ The attribute causes gcc to print out the supplied warning message if
the function is used. We should some day support it, but this patch just
makes us ignore it. ]
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Otherwise sparse is very unhappy about the current glibc header files
(aio.h, netdb.h. regex.h and spawn.h at a minimum).
It's a hack, and not a proper parsing with saving the information. It just
ignores any "restrict" keyword at the start of an abstract array
declaration, but it's better than what we have now.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
GCC supports a __thread storage class, used to indicate thread-local
storage. It may be used alone, or with extern or static.
This patch makes sparse aware of it, and check those restrictions.
Signed-off-by: Alberto Bertogli <albertito@blitiri.com.ar>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
We weren't checking if the initializer isn't NULL, which caused sparse
to segfault later on when performing lazy evaluation in classify_type().
Signed-off-by: Martin Nagy <nagy.martin@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Remove all previous checks for Waddress_space and add one centralized to
the address_space attribute handler. If user passes the
-Wno-address-space option, we behave as if every pointer had no address
space.
Signed-off-by: Martin Nagy <nagy.martin@gmail.com>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
On Thu, Mar 19, 2009 at 09:52:50PM +0000, Al Viro wrote:
Yeah... It's an old b0rken handling of calls for K&R + changes that exposed
that even worse.
Status quo is restored by the patch below, but it's a stopgap - e.g.
void f();
void g(void)
{
f(0, 0);
}
will warn about extra arguments as if we had void f(void); as sparse had
been doing all along. B0rken.
Testcase for the segfault is
void f(x, y);
void g(void)
{
f(0, 0);
}
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
There's no need to concat the context list into (empty) one of new node,
only to free the original one. Moving the pointer to list instead works
fine...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <chrisl@hera.kernel.org>
|
|
Keep storage class (and "is it inline") explicitly in decl_state;
translate to modifiers only when we are done with parsing. That
avoids the need to separate MOD_STORAGE bits while constructing
the type (e.g. in alloc_indirect_symbol(), etc.). It also allows
to get rid of MOD_FORCE for good - instead of passing it to typename()
we pass an int * and let typename() tell whether we'd got a force-cast.
Indication of force-cast never makes it into the modifier bits at all.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <chrisl@hera.kernel.org>
|
|
There's no point whatsoever in constructing modifiers for chosen
type when decoding integer constant only to have them picked
apart by ctype_integer(). Seeing that the former is the only
caller of the latter these days, we can bloody well just pass
the rank and signedness explicitly and save a lot of efforts.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <chrisl@hera.kernel.org>
|
|
a) __label__ in gcc is not a type, it's a statement. Accepted in the beginning
of compound-statement, has form __label__ ident-list;
b) instead of crapping into NS_SYMBOL namespace (and consequent shadowing
issues), reassign the namespace to NS_LABEL after we'd bound it. We'll get
block scope and label namespace, i.e. what we get in gcc.
c) MOD_LABEL can be dropped now.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Doing lookup_symbol() with NS_TYPEDEF will happily skip the redeclarations
of the same identifier with NS_SYMBOL. We need to check that we are not
dealing with something like
typedef int T;
void f(int T)
{
static T a; /* not a valid declaration - T is not a typedef name */
or similar (e.g. enum member shadowing a typedef, etc.).
While we are at it, microoptimize similar code in lookup_type() - instead
of sym->namespace == NS_TYPEDEF we can do sym->namespace & NS_TYPEDEF;
the former will turn into "fetch 32bit value, mask all but 9 bits, compare
with NS_TYPEDEF", the latter - "check that one bit in 32bit value is set".
We never mix NS_TYPEDEF with anything in whatever->namespace, so the
tests are equivalent.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
It starts after the end of enumerator; i.e. if we have
enum {
...
Foo = expression,
...
};
the scope of Foo starts only after the end of expression.
Rationale: 6.2.1p7.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
... at least to the extent we used to do it. It still does _not_
cover the perversions gcc can do with that, but at least it deals
with regressions. Full solution will have to wait for full-blown
imitation of what gcc people call __attribute__ semantics, the
bastards...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Other than for attributes of labels (completely ignored, and we can
simply use skip_attributes() there), all callers of handle_attributes
actually get ctype == &ctx->ctype for some ctx. Ditto for ->declarator().
Switch both to passing ctx instead (has to be done at the same time,
since we have handle_attributes() called from ->declarator() for struct).
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
At this point there's not much in common between qualifiers-only
and full cases; easier to split the sucker in two and lose the
qual argument. Clean it up, while we are at it...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
... and don't do full-blown apply_ctype() every damn time.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Make sure that we accept the right set; kill ad-hackery around checks
for banned combinations. Instead of that we keep a bitmap describing
what we'd already seen (with several extra bits for 'long long' and
for keeping track of can't-combine-with-anything stuff), check and
update it using the values in ..._op and keep track of size modifiers
more or less explicitly.
Testcases added. A _lot_ of that used to be done wrong.
Note that __attribute__((mode(...))) got more broken by this one;
the next several changesets will take care of that.
One more thing: we are -><- close to getting rid of MOD_SPECIFIER bits
for good.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
... and yes, right now it's ucking fugly. Will get sanitized shortly.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Take typedef handling in declaration_specifiers() into separate
branch; kill useless check for qual in case the type we've got
has non-NULL base_type (we'd have already buggered off in that
situation before we get to the check in question).
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
We set MOD_USERTYPE only on the first one, so declaration_specifiers
didn't recognize something like
unsigned P;
where P had been such a typedef as redefinition (see the testcase).
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
const and friends are reserved words, TYVM...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Introduce struct decl_state that will hold the declaration parser
state. Pass it to declarator() and direct_declarator(). Note
that at this stage we are putting more stuff on stack *and* get
extra copying; that's temporary and we'll deal with that in subsequent
patches.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
a) handling of nested declarator does not belong in the loop, for fsck sake
b) functions and arrays don't mix (and functions don't mix with functions)
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
There's no reason to pass symbol to declarator/direct_declarator;
we only use &sym->ctype, so passing ctype * is fine.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Don't mess with applying attributes in which_kind(); leave that until
we know whether that's function declarator or a nested one. We'll need
that to deal with scopes properly. Clean parameter_type_list() and
parameter_declaration(), get rid of a leak.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
In int f(__user int *p) __user applies to p, not to f...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
* new helper function - which_kind(). Finds how to parse what
follows ( in declarator.
* parameter-type-list and identifier-list handling separated
and cleaned up.
* saner recovery from errors
* int f(__attribute__((....)) x) is prohibited by gcc syntax;
attributes are not allowed in K&R identifier list, obviously,
and gcc refused to treat that as "int is implicit if missing"
kind of case.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
a) asm aliases are accepted by gcc *only* directly after the top-level
declarator within external declaration (and they make no sense whatsoever
elsewhere - field names, function parameter names, casts, etc. simply
do not reach the assembler). Anyway, gcc parser will reject those
anywhere else.
b) post-declarator attributes are *not* accepted in nested declarators.
In bitfield declarations they must follow the entire thing, _including_
:<width>. They are not accepted in typenames either. Basically, gcc
has distinction between the attributes of type and attributes of
declaration; "after the entire declaration" is for the latter and they
get applied to type only as a fallback. So that's deliberate on part
of gcc...
c) no attributes in direct-declarator in front of [.....] or (.....).
Again, gcc behaviour.
d) in external declarations only attributes are accepted after commas.
IOW, you can't write int x, const *y, z; and get away with that.
Replacing const with __attribute__((...)) will get it accepted by
gcc, so we need to accept that variant. However, any qualifiers
in that position will get rejected by anything, including gcc.
Note that there's a damn good reason why such syntax is a bad idea and
that reason applies to attributes as well. Think what happens if later
we remove 'x' from the example above; 'z' will suddenly become const int.
So I think we want eventually to warn about the use of attributes in
that position as well. For now I've got it in sync with gcc behaviour.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
no nested declarators after [...] or (parameters)
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
Don't mix it with parameter-type-list, add saner checks.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
The rule for ident-less declaration is
declaration -> declaration-specifiers ;
not
declaration -> declaration-specifiers abstract-declarator;
IOW, struct foo; is OK and so's struct foo {int x; int y;} (and even
simply int; is allowed by syntax - it's rejected by constraints, but
that's a separate story), but not struct foo (void); and its ilk.
See C99 6.7p1 for syntax; C90 is the same in that area and gcc also
behaves the same way. Unlike gcc I've made it a warning (gcc produces
a hard error for those).
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
On Thu, Feb 05, 2009 at 09:19:21PM +0000, Al Viro wrote:
> IOW, direct_declarator() (which doubles for direct-abstract-declarator) should
> have more than one-bit indication of which case we've got. Right now it's
> done by "have we passed a non-NULL ident ** to store the identifier being
> declared"; that's not enough. What we need is explicit 'is that a part of
> parameter declaration' flag; then the rule turns into
> if (p && *p)
> fn = 1; /* we'd seen identifier already, can't be nested */
> else if match_op(next, ')')
> fn = 1; /* empty list can't be direct-declarator or
> * direct-abstract-declarator */
> else
> fn = (in_parameter && lookup_type(next));
Umm... It's a bit more subtle (p goes NULL after the nested one is
handled), so we need to keep track of "don't allow nested declarator from
that point on" explicitly. Patch follows:
Subject: [PATCH] Handle nested declarators vs. parameter lists correctly
Seeing a typedef name after ( means that we have a parameter-type-list
only if we are parsing a parameter declaration or a typename; otherwise
it might very well be a redeclaration (e.g. int (T); when T had been a
typedef in outer scope).
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
|
|
There are several interesting problems caused by the fact that
we create a separate symbol for each declaration of given function.
1)
static inline int f(void);
static int g(void)
{
return f();
}
static inline int f(void)
{
return 0;
}
gives an error, since the instance of f in g is not associated with anything
useful. Needless to say, this is a perfectly valid C. Moreover,
static inline int f(void)
{
return 0;
}
static inline int f(void);
static int g(void)
{
return f();
}
will step on the same thing. Currently we get the former case all over the
place in the kernel, thanks to the way DEFINE_SYSCALLx() is done.
I have a kinda-sorta fix for that (basically, add a reference to external
definition to struct symbol and update it correctly - it's not hard).
However, that doesn't cover *another* weirdness in the same area -
gccisms around extern inline. There we can have inline and external
definitions in the same translation unit (and they can be different,
to make the things even more interesting). Anyway, that's a separate
story - as it is, we don't even have a way to tell 'extern inline ...'
from 'inline ...'
2) More fun in the same area: checks for SYM_FN in external_declaration()
do not take into account the possibility of
void f(int);
typeof(f) g;
Ergo, we get linkage-less function declarations. Fun, innit? No patch.
3) Better yet, sparse does _NOT_ reject
typeof(f) g
{
...
}
which is obviously a Bloody Bad Idea(tm) (just think what that does to
argument list). Similar crap is triggerable with typedef. IMO, we really
ought to reject _that_ - not only 6.9.1(2) explicitly requires that, but
there's no even remotely sane way to deal with arguments.
4)
static void f(void);
...
void f(void);
triggers "warning: symbol 'f' was not declared. Should it be static?"
which is at least very confusing - it *is* declared and it *is* static.
IOW, we do not collect the linkage information sanely. (2) will make
fixing that one very interesting.
Anyway, proposed patch for (1) follows:
Subject: [PATCH] Handle mix of declarations and definitions
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
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: Christopher Li <sparse@chrisli.org>
Acked-by: Thomas Schmid <Thomas.Schmid@br-automation.com>
|
|
An error would be issued if such union is actually used.
Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
|
|
They describe how likely the function is to be executed, which can
affect optimization. Also ignore the attributes with underscores.
Signed-off-by: Pavel Roskin <proski@gnu.org>
|
|
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>
|
|
* don't crap the type->ident for unsigned int just because somebody did
typedef unsigned int x;
only structs, unions, enums and restricted types need it.
* generate saner warnings for restricted, include type name(s) into them.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
|
|
This avoids error messages like this:
error: attribute 'malloc': unknown attribute
Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
|