[2.5] [Cool stuff] "checking" mode for kernel builds

[2.5] [Cool stuff] "checking" mode for kernel builds

[Carl-Daniel Hailfinger]

Linus,

while eagerly waiting for the release of that tool, I have a few
comments about the patch itself.

> # The following is the BitKeeper ChangeSet Log
> # --------------------------------------------
> # 03/05/07	torvalds@home.transmeta.com	1.1063.14.3
> # Support a "checking" mode for kernel builds, that runs a
> # user-supplied source checker on all C files before compiling
> # them.
> # 
> # I'll release the actual checker once I've cleaned it up a
> # bit more (yay, all the copyright paperwork completed!)
> # --------------------------------------------
> #
> [...]
> @@ -172,6 +182,7 @@
>  DEPMOD		= /sbin/depmod
>  KALLSYMS	= scripts/kallsyms
>  PERL		= perl
> +CHECK		= /home/torvalds/parser/check

Hardcoded path

>  MODFLAGS	= -DMODULE
>  CFLAGS_MODULE   = $(MODFLAGS)
>  AFLAGS_MODULE   = $(MODFLAGS)
> @@ -66,6 +66,12 @@
>  	 $(subdir-ym) $(always)
>  	@:
>  
> +# Linus's kernel sanity checking tool

IIRC my english lessons it should be
+# Linus' kernel sanity checking tool

> +ifneq ($(KBUILD_CHECKSRC),0)
> +quiet_cmd_checksrc = CHECK   $<
> +      cmd_checksrc = $(CHECK) $(CFLAGS) $< ;
> +endif
> + 
>  # Module versioning
>  # ------------------------------------------------------
>  

Regards,
Carl-Daniel

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[John Anthony Kazos Jr.]

> > +# Linus's kernel sanity checking tool
>
>IIRC my english lessons it should be
>+# Linus' kernel sanity checking tool

It's just like if you want to quote a command like "chroot," which is 
confusing and nasty and ugly, whereas using "chroot", while not really 
correct, is The Right Thing To Do. We computer scientists have our own, 
*more precise* form(s) of our language(s). ^_^ 

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[Linus Torvalds]

On Tue, 27 May 2003, Carl-Daniel Hailfinger wrote:
> 
> while eagerly waiting for the release of that tool, I have a few
> comments about the patch itself.

Well, I guess I can just tell people about it. It's unfinished enough that
I'm a bit embarrassed about some of it, but I've gotten the permission
from Transmeta to make it open source, and it's actually been available
for some time on "bk://kernel.bkbits.net/torvalds/sparse". I just didn't 
tell about it in public (although a few people have known about it).

Be gentle with it. It does many things wrong, including (but limited 
to) enums, but it does a lot of things right too.

The biggest problem (apart from the things it does wrong) is that some
parts of the kenel re-use the same structure to hold both user- and kernel
pointers. That's a big no-no for the static semantic parser, since it's 
literally a _static_ type-checker, and doesn't know about dynamic 
differences. 

The main offender is the networking layer that sometimes keeps user
pointers and sometimes kenrel pointers in a "struct iovec". David has
promised to look into this, and seemed confident that he can fix it 
easily.

Most people who have used the tool actually like how it forces you to make 
it very _explicit_ whether you're using a user pointer or not. But I have 
to admit that I've grown tired of trying to look at all the uses and 
making sure which sparse warnings are valid and which aren't.

				Linus

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[Carl-Daniel Hailfinger]

[Linus: Sorry that your e-mail to me bounced. Was a provider screwup,
should be fixed now.]
[CC:ed kerneljanitors, who might be interested using it.
 CC:ed Dan Carpenter, who mantains a set of tools similar to sparse, but
as a gcc patch at http://kbugs.org ]

Linus Torvalds wrote:
> Well, I guess I can just tell people about it. It's unfinished enough that
> I'm a bit embarrassed about some of it, but I've gotten the permission
> from Transmeta to make it open source, and it's actually been available
> for some time on "bk://kernel.bkbits.net/torvalds/sparse". I just didn't 
> tell about it in public (although a few people have known about it).
> 
> Be gentle with it. It does many things wrong, including (but limited 
> to) enums, but it does a lot of things right too.

Playing with it right now.

> The biggest problem (apart from the things it does wrong) is that some
> parts of the kenel re-use the same structure to hold both user- and kernel
> pointers. That's a big no-no for the static semantic parser, since it's 
> literally a _static_ type-checker, and doesn't know about dynamic 
> differences. 
> 
> The main offender is the networking layer that sometimes keeps user
> pointers and sometimes kernel pointers in a "struct iovec". David has
> promised to look into this, and seemed confident that he can fix it 
> easily.
> 
> Most people who have used the tool actually like how it forces you to make 
> it very _explicit_ whether you're using a user pointer or not. But I have 
> to admit that I've grown tired of trying to look at all the uses and 
> making sure which sparse warnings are valid and which aren't.

Dan? IIRC you have tools to tackle this issue in a distributed manner.


Regards,
Carl-Daniel

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[dan carpenter]

On Tuesday 27 May 2003 03:24 am, Carl-Daniel Hailfinger wrote:

> > Most people who have used the tool actually like how it forces you to
> > make it very _explicit_ whether you're using a user pointer or not. But I
> > have to admit that I've grown tired of trying to look at all the uses and
> > making sure which sparse warnings are valid and which aren't.
>
> Dan? IIRC you have tools to tackle this issue in a distributed manner.
>
>

Tracking bugs from one version to the next works pretty well for kbugs.org.  
Anyone can moderate the bugs as real or not.  I haven't used sparse script 
yet, but I'll do that tomorrow.  I'd be happy to post the results on 
kbugs.org.  :)

regards,
dan carpenter

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[Miles Bader]

Carl-Daniel Hailfinger <c-d.hailfinger.kernel.2003@gmx.net> writes:
> > +# Linus's kernel sanity checking tool
> 
> IIRC my english lessons it should be
> +# Linus' kernel sanity checking tool

BTW, it just follows the sound, and at least I say something sounding
like `Linusez kernel' -- which would be written "Linus's kernel."

[in some cases, e.g. many plurals, you _do_ merge the two uses of S,
e.g., "The Smiths' car"]

-Miles
-- 
I'm beginning to think that life is just one long Yoko Ono album; no rhyme
or reason, just a lot of incoherent shrieks and then it's over.  --Ian Wolff

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[Aaron Lehmann]

On Tue, May 27, 2003 at 02:17:45AM +0200, Carl-Daniel Hailfinger wrote:
> > +CHECK		= /home/torvalds/parser/check
> 
> Hardcoded path

Well, the checker its share of those problems:

const char *gcc_includepath[] = {
    "/usr/lib/gcc-lib/i386-redhat-linux/3.2.1/include",
    "/usr/lib/gcc-lib/i386-redhat-linux/3.2.2/include",
    NULL
};

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[Linus Torvalds]

On Mon, 26 May 2003, Aaron Lehmann wrote:
> 
> Well, the checker its share of those problems:
> 
> const char *gcc_includepath[] = {
>     "/usr/lib/gcc-lib/i386-redhat-linux/3.2.1/include",
>     "/usr/lib/gcc-lib/i386-redhat-linux/3.2.2/include",
>     NULL
> };

Any takers? Some Makefile magic plus some hacky thing like

	gcc -print-file-name=include

(Yeah, that's not righ either, it just happens to work. I don't know what
the proper way of making gcc expose its local paths is).

So I'm doing what works for me, and open source (in this case the OSL) 
means that you can fix it and send me patches if you want to ;)

Btw, taling about it embarrassed me so much that I just fixed the enum
confusion, so now a few less of the checker warnings are bogus. It still
misparses some assembly (notably anything that is at the top level, not
inside a function, and the kinds of asms used to rename variables).

And it doesn't handle pragmas, in particular "pragma pack" is hard to do
right (well, from a _kernel_ checking standpoint it doesn't matter, but I
do want it to actually generate a good parse tree too, which means that I
try to get things like structure member offsets etc _right_. And as I
currently just ignore - and warn about - "pragma pack", my type evaluation
doesn't get the offsets/alignments right).

But _most_ of the warnings are because of type differences. Even those are
sometimes bogus, though, so don't assume it's right. 

Oh, before I forget: it also refuses to parse some gcc constructs that I
personally don't like, like the gcc extension to make certain things
lvalues even though they really aren't (ie casts and the ?: operator).

Even though I myself have been known to use those gcc extensions, I think
they are _wrong_, since they don't actually buy you anything except for a
dubious syntactic shorthand. It's the wrong kind of language extension.

Other - more worthwhile - extensions I do actually support.

		Linus

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[Aaron Lehmann]

On Mon, May 26, 2003 at 08:23:37PM -0700, Linus Torvalds wrote:
> Any takers? Some Makefile magic plus some hacky thing like
> 
> 	gcc -print-file-name=include
> 
> (Yeah, that's not righ either, it just happens to work. I don't know what
> the proper way of making gcc expose its local paths is).

gcc -v will tell you the final include path, but only when you
actually compile something. I'd probably make the makefile hackery parse
the ouput of echo | gcc -v -E -. Yeah, it's ugly.

The output between "#include <...> search starts here:" and "End of
search list." seems like the combination of what you want for
gcc_includepath and sys_includepath. I assume the output is ordered. I
might send a patch if I'm bored tonight.

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[Linus Torvalds]

On Mon, 26 May 2003, Aaron Lehmann wrote:
> 
> The output between "#include <...> search starts here:" and "End of
> search list." seems like the combination of what you want for
> gcc_includepath and sys_includepath. I assume the output is ordered. I
> might send a patch if I'm bored tonight.

If it comes to parsing "gcc -v" output, I have to say that I personally
find that to be just crazy, and I'd much rather just see the (incorrect
but working) '-print-file-name=include' hack.

Note that to some degree the _better_ approach is to just make 
sparse-specific header files available to sparse itself, instead of making 
the checker try to look too much like gcc including using gcc's internal 
header files.

The kernel in particular only needs a very few files for the compiler,
notably <stdarg.h>.

Of course, for debugging I especially initially used a lot of non-kernel
files, and while my own personal interest is purely in the front-end, I'm
actually hoping that some crack-smoking crazy person would actually write
a back-end for it too, just for fun.

My own "test-parse.c" has it's own little back-end that outputs a very
strange kind of pseudo-assembler, and that was absolutely _critical_ to 
finding a lot of parsing and evaluation bugs. 

I guess I'm just a hopeless retard, but I initially tried to print out the
parse tree with all the type information, and I couldn't make sense of it
visually. Making a stupid back-end that outputs something that looks
almost like real assembly language was a huge advantage for me, because it
meant that I could mentally parse the output much better, and I found an
incredible number of type evaluation bugs that way.

But that's clearly all my back-end is useful for. 

			Linus

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[Ryan Anderson]

On Mon, May 26, 2003 at 10:07:28PM -0700, Linus Torvalds wrote:
> 
> On Mon, 26 May 2003, Aaron Lehmann wrote:
> > 
> > The output between "#include <...> search starts here:" and "End of
> > search list." seems like the combination of what you want for
> > gcc_includepath and sys_includepath. I assume the output is ordered. I
> > might send a patch if I'm bored tonight.
> 
> If it comes to parsing "gcc -v" output, I have to say that I personally
> find that to be just crazy, and I'd much rather just see the (incorrect
> but working) '-print-file-name=include' hack.

How's this for a hack that makes it work a bit better?

# This is a BitKeeper generated patch for the following project:
# Project Name: TSCT - The Silly C Tokenizer
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.344   -> 1.345  
#	       pre-process.c	1.62    -> 1.63   
#	            Makefile	1.19    -> 1.20   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/05/27	ryan@michonline.com	1.345
# This update (hopefully) will allow 'check' to find the appropriate, gcc-local 
# include headers (dependent, of course, upon the system it was built on)
# 
# It's still a hack, but at least it will work in a few more places.
# --------------------------------------------
#
diff -Nru a/Makefile b/Makefile
--- a/Makefile	Tue May 27 05:10:56 2003
+++ b/Makefile	Tue May 27 05:10:56 2003
@@ -32,7 +32,7 @@
 expression.o: $(LIB_H)
 lib.o: $(LIB_H)
 parse.o: $(LIB_H)
-pre-process.o: $(LIB_H)
+pre-process.o: $(LIB_H) pre-process.h
 scope.o: $(LIB_H)
 show-parse.o: $(LIB_H)
 symbol.o: $(LIB_H)
@@ -40,5 +40,8 @@
 test-parsing.o: $(LIB_H)
 tokenize.o: $(LIB_H)
 
+pre-process.h:
+	echo "#define GCC_INTERNAL_INCLUDE \"`gcc -print-file-name=include`\"" > pre-process.h
+
 clean:
-	rm -f *.[oasi] core core.[0-9]* $(PROGRAMS)
+	rm -f *.[oasi] core core.[0-9]* $(PROGRAMS) pre-process.h
diff -Nru a/pre-process.c b/pre-process.c
--- a/pre-process.c	Tue May 27 05:10:56 2003
+++ b/pre-process.c	Tue May 27 05:10:56 2003
@@ -18,6 +18,7 @@
 #include <fcntl.h>
 #include <limits.h>
 
+#include "pre-process.h"
 #include "lib.h"
 #include "parse.h"
 #include "token.h"
@@ -47,6 +48,7 @@
 const char *gcc_includepath[] = {
 	"/usr/lib/gcc-lib/i386-redhat-linux/3.2.1/include",
 	"/usr/lib/gcc-lib/i386-redhat-linux/3.2.2/include",
+	GCC_INTERNAL_INCLUDE,
 	NULL
 };
 

-- 

Ryan Anderson
  sometimes Pug Majere

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[Linus Torvalds]

On Tue, 27 May 2003, Ryan Anderson wrote:
> 
> How's this for a hack that makes it work a bit better?

Well, since it clearly isn't any worse than what I have now, I'll just say 
"hell yes!", and apply it.

In fact, I'll just make the old gcc paths go away entirely, as this should 
make them be non-issues.

		Linus

Re: [2.5] [Cool stuff] "checking" mode for kernel builds

[dep]

Carl-Daniel Hailfinger's  quote:

| > +# Linus's kernel sanity checking tool
|
| IIRC my english lessons it should be
| +# Linus' kernel sanity checking tool

only if there is more than one linus. which we know is not only not 
the case but indeed not possible.
-- 
dep

http://www.linuxandmain.com -- outside the box, barely within
the envelope, and no animated paperclip anywhere.