On Mon, Apr 27, 2015 at 11:50:55AM -0400, Oleg Drokin wrote:
>
> On Apr 27, 2015, at 4:38 AM, Dan Carpenter wrote:
>
> > On Sat, Apr 25, 2015 at 12:54:24PM -0400, Oleg Drokin wrote:
> >> But if I change vmalloc to kmalloc then I get the warnings about both leaked memory and wrong size.
> >> So it appears like the only functions tracked for allocation are the ones explicitly listed in
> >> check_allocation_funcs.c?
> >> But why are the long lists of other allocations generated in the smatch_data then (Which is pretty cool,
> >> btw, I needed to populate those lists in the old smatch unfree.pl which was quite tedious).
> >
> > Of course, there isn't a good reason for this…
>
> Is there some documentation anywhere on how things are actually working for new test writing and such?
Not really... :( [Here is where I deleted all my promises to do better].
> I dug a bit and got some basic understanding with registered callbacks for various functions and stuff.
> Now, I wonder what extra state from common framework I can glance.
The "extra" stuff is trying to record the value of every variable. The
value is stored an "estate" which holds a range_list of sval_t. An
sval_t holds the type int, long, etc and the value. A range list is
like (-1),3-5.
Generally you should just use the if (get_implied_rl(variable, &res))
type functions to find if Smatch knows the value of a variable.
>
> For example - the annoying false positives from functions that do:
> do_stuff(int lock)
> {
> if (lock)
> spin_lock(somelock);
>
> stuff;
>
> if (lock)
> spin_unlock(somelock);
> }
> I hope there's a way to get an idea of the preceding branch conditions and compare notes on lock/unlock
> for example?
This example code shouldn't cause a false positive. smatch_implied.c
knows that the "if (lock)" condition implies that lock is set.
I also recently made it so smatch_comparisons.c can set implications.
smatch_comparison.c is used for when we are comparing:
if (unknown_variable <= other_unknown)
lock();
What doesn't work is function calls:
if (needs_lock(foo))
That's tricky to add because of side effects. (Not impossibly tricky).
Also "if (foo & NEEDS_LOCK)" doesn't work but it's similar to
smatch_comparison.c so I should be able to add that.
>
> Any way to get the current function name to avoid false positives with aliased variable names?
"cur_func" has the function name a string.
"cur_func_sym" has the function a symbol struct pointer.
I'm not sure what "aliased variable names" means.
> Is there a way to tell local variable from a global?
I'm actually trying to figure that out myself. I think,
if (sym->ctype.modifiers & (MOD_TOPLEVEL | MOD_STATIC)) {
Will tell you if variable is top level and or local.
> The "expression" structure looks somewhat menacing at this point.
Read __split() smatch_flow.c to see how it works.
The symbol struct is probably more difficult. Every variable has a
symbol pointer... C types are also symbol pointers.
>
> Additionally the current failure to print filename where the error is if we are dealing with
> a macro expansion from a header (it prints the correct line number, but wrong file).
I'm not certain what you mean here. If it sees a macro() then it should
use the file and line of the .c file. I think you are talking about
macros which generate functions?
Ah. I think you are still looking at --info output and that is not
meant for human consumption. The filename for static inline functions
needs to be the .c file for --info output but you shouldn't be looking
at that. For normal output it will use the .h filename as you would
expect.
>
> Oh, also the SQL statements - what are those for? just debug?
If you cat all the SQL statements into a 17 GB warns.txt file and then
run `~/smatch/smatch_data/db/create_db.sh -p=kernel warns.txt` then it
creates a smatch_db.sqlite with call and return path information so you
can do cross function analysis.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe smatch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
