[2.5] [Cool stuff] "checking" mode for kernel builds
Linus,
while eagerly waiting for the release of that tool, I have a few comments about the patch itself.
Quoted 13 lines
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/05/07 [email protected] 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!) # -------------------------------------------- # [...]
Review line 185
Quoted 4 lines
(quoted diff) +1
-0@@ -172,6 +182,7 @@ DEPMOD = /sbin/depmod KALLSYMS = scripts/kallsyms PERL = perl +CHECK = /home/torvalds/parser/check
Hardcoded path
Review line 69
Quoted 8 lines
(quoted diff) +1
-0MODFLAGS = -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
Review line 77
Quoted 8 lines
(quoted diff) +5
-0+ifneq ($(KBUILD_CHECKSRC),0) +quiet_cmd_checksrc = CHECK $< + cmd_checksrc = $(CHECK) $(CFLAGS) $< ; +endif + # Module versioning # ------------------------------------------------------
Regards,
Carl-Daniel
Quoted 5 lines
Quoted 2 lines
+# 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). ^_^
Quoted 3 lines · Carl-Daniel Hailfinger
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
[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 ]
Quoted 8 lines · Linus Torvalds
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.
Quoted 11 lines
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.
Quoted 4 lines
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
Quoted 4 lines · Carl-Daniel Hailfinger
Quoted 1 line
+# 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
Signature · 2 lines
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
Quoted 3 lines · Carl-Daniel Hailfinger
Quoted 1 line
+CHECK =
/home/torvalds/parser/checkHardcoded 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
};Quoted 8 lines · Aaron Lehmann
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
Quoted 6 lines · Linus Torvalds
Any takers? Some
Makefilemagic plus some hacky thing likegcc -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.
Quoted 5 lines · Aaron Lehmann
The output between "#include <...> search starts here:" and "End of search list." seems like the combination of what you want for
gcc_includepathandsys_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
Quoted 8 lines · Carl-Daniel Hailfinger
Quoted 4 lines
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
Quoted 11 lines · Linus Torvalds
Quoted 5 lines · Aaron Lehmann
The output between "#include <...> search starts here:" and "End of search list." seems like the combination of what you want for
gcc_includepathandsys_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 [email protected] 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
pre-process.c +2 -0
@@ -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 };
Signature · 2 lines
Ryan Anderson sometimes Pug Majere
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.
Signature · 4 lines
dep http://www.linuxandmain.com -- outside the box, barely within the envelope, and no animated paperclip anywhere.
Quoted 2 lines · Ryan Anderson
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