Re: smatch update / 2.5.64 / kbugs.org

[Oleg Drokin <green@namesys.com>]

Hello!

On Thu, Mar 06, 2003 at 02:37:27AM -0500, dan carpenter wrote:

> The smatch bugs for kernel 2.5.64 are up.  The 
> new url for the smatch bug database is http://kbugs.org.  

Unfortunatelly the bug database does not work. I mean I cannot connect to it.

> One new script from Monday was "UnFree."  This check 
> looks for variables that aren't freed on the error paths.
> http://kbugs.org/cgi-bin/index.py?page=bug_list&script=UnFree&kernel=2.5.64

This is very interesting project, thanks a lot.
I spent some time on this today.
This script can produce a lot less false positives with even more custom merge rules.
Here's the diff that if run on fs/ext3/super.c from current bk tree, produces
only one true bug. (your version from cvs produces one real bug and two false positives)
(8 less hits on my default build).
Still there is a lot room for improvement of course (And probably I will spend some more time
on it), there are still valid cases left catched:
pointer assigned to some externally visible variable/structure before exit (fs/reiserfs/fix_node.c)
kmalloc only done conditionally (fs/reiserfs/namei.c)
probably more ;)

Also I think with this script it is somewhat easy to catch these cases (well, that needs to be coded of course):
loosing allocated pointer by assigning other value to variable and not storing old value anywhere.

Thank you.

Bye,
    Oleg
--- unfree.pl	Thu Mar  6 13:58:33 2003
+++ unfree-new.pl	Thu Mar  6 18:16:51 2003
@@ -12,6 +12,15 @@
     print get_filename(), " ", get_start_line($var), " ", get_lineno(), " ", get_func_pos(), " $msg\n";
 }
 
+
+sub merge_free_and_null($$$){
+    my ($name, $one, $two) = @_;
+
+    if ( ($one eq "free" && $two eq "null") || ($one eq "null" && $two eq "free") ) {
+	return "free";
+    }
+    return merge_rules($name, $one, $two);
+}
 sub merge_rules($$$){
     my ($name, $one, $two) = @_;
 
@@ -36,7 +45,7 @@
 
     return "undefined";
 }
-add_merge_function("merge_rules");
+add_merge_function("merge_free_and_null");
 
 while ($data = get_data()){
     my $tmp;
@@ -88,6 +97,11 @@
 		    set_true_path($name,"non_null");
 		}
 	    }
+	    if ( ! ($cond =~ /_expr/) && ! ($cond =~ /^compound_cond/) ) {
+		if (get_state ($cond)){
+		    set_true_path($cond,"non_null");
+		}
+	    }
 	}
 	for my $cond (@{$conds{falsepath}}){
 	    if ($cond =~ /eq_expr\(\((.*)\)\(integer_cst\(0\)\)\)/){
@@ -102,6 +116,11 @@
 		    set_false_path($name,"null");
 		}
 	    }
+	    if ( ! ($cond =~ /_expr/) && ! ($cond =~ /^compound_cond/) ) {
+		if (get_state ($cond)){
+		    set_false_path($cond,"null");
+		}
+	    }
 	}
     }elsif ($data =~ /^(do_cond) (.*)/){            # check do while() for thouroughness
 	$conds = split_conds($1);
@@ -119,11 +138,23 @@
 		}
 	    }
 	}
+	if ( ! ($cond =~ /_expr/) && ! ($cond =~ /^compound_cond/) ) {
+	    if (get_state ($cond)){
+		set_state($cond,"null");
+	    }
+	}
     }
     
     if ($data =~ /call_expr\(\(addr_expr function_decl\(kfree\)\)\(tree_list: (.*)\)\)/){
 	my $var = $1;
-	if (get_state($var)){
+	my $state = get_state($var);
+	if ( !($state =~ /non_null/) && $state =~ /null/ ) {
+	    error_msg("Freeing null pointer", $state);
+	}
+	if ( $state =~ /free/ ) {
+	    error_msg("Freeing already freed", $state);
+	}
+	if ($state){
 	    set_state($var, "free");
 	}
     }