Static code analysis

[Synon]

1. Do you use it?
2. How important do you think it is?
3. What software do you use?
4. How would you rate that software out of 10?
5. How applicable to/important to OSDev would you say static code analysis is?

For me,
1. Rarely
2. Quite, I just never remember to use it
3. I can't remember (I think it was clang/LLVM)
4. It was pretty good
5. I don't know

[Combuster]

Static code analysis happens on many levels...

I generally use GCC with -Werror and a lot of additional options to enforce a coding standard that is significantly better than most code you find online. I'm also using XCode as part of paid work which comes with an language/API specific static analyser that usually fixes quite a few bugs before you encounter them as well.

Considering that both static analysers are part of the compiler package, I consider it really poor practice for people not using them - I could post work examples on just how much effect those tools can have but I would violate NDA with that.

Suffice to say, my OS is warning-free and my build system enforces that property before it proceeds to testing.

[Owen]

XCode analyzer _is_ the llvm staticanalyzer, just properly integrated into the IDE.

Plus API specific knowledge

[Solar]

1. Yes.
2. Elementary.
3. Most of my work is C++ maintenance, and usually I can be happy if the code I am given compiles "without too many warnings". Any dedicated static analysis tool would probably simply throw up. I.e., I am on the neverending quest of getting another compiler warning flag enabled without drowning in warnings.
4. Any software project should begin with finding the most stringent compiler warnings possible, enabling them by default, and refusing any sources throwing compiler warnings from being committed. Any source not qualifying in this way (including any code I work on at the office) rates 0 on my personal scale. (I know that's not what you asked... )
5. See 4. If your language of choice is supported by other static code analysis tools, employ them.

Retrofitting stuff like this is next to impossible. Setting it up right at the beginning is dead easy. Draw your own conclusions.

[FlashBurn]

What is with warnings like that the size parameter of new (C++) isn´t used (in General unused parameters for an api)? I´m disabling this warning with the help of GCC pragmas. This are warnings you can´t really avoid.

[FlashBurn]

Well, there is -Wno-unused first

This is no option, because I want this warning for cases were I really missed something.

and second you can omit the names of the parameters you do not really use.

I don´t know if I got you right, but e.g. "new" has to be defined as "void* new(size_t size)" also if I´m not using the size parameter. The next thing is that I have a HAL where not every hardware needs every parameter so I can´t omit the parameters.

[FlashBurn]

Thx, I will try that, didn´t know that this will work.

[Owen]

Also, you should be using the size parameter... how else are you supposed to know how big the object you're allocating memory for is?

[FlashBurn]

Also, you should be using the size parameter... how else are you supposed to know how big the object you're allocating memory for is?

SlabAllocator.

I´m using new because it hides the specific allocator, the code is easier to read and I can easily use a constructor.

[Synon]

@berkus,
That's a good idea, I will do that. I'll have it analyse each file before compilation.

@Combuster,
I compile with -Wall -Wextra -pedantic -ansi -std=C99 when I'm adding a feature, but I add -Werror and change -pedantic to -pedantic-errors when I get it working.

[Combuster]

Actually, -Wall -Wextra -pedantic is somewhere about half of what I use:

Code: Select all

CCPARMS= -c -I ./include/common -I ./include/ia-pc -I ./include/arch-x86 -march=i486 
   -Wall -Wextra -pedantic -O2 -Wshadow -Wpointer-arith -Wcast-align -Wwrite-strings 
   -Wmissing-prototypes -Wmissing-declarations -Wredundant-decls -std=c99 
   -Wnested-externs -Winline -Wstrict-prototypes -Wno-attributes -Wc++-compat 
   -Wchar-subscripts -Wcomment -Wno-deprecated-declarations -Wno-div-by-zero 
   -Wno-endif-labels -Wfloat-equal -Wformat=2 -Wno-format-extra-args -Wformat-nonliteral 
   -Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration -Wimplicit-int 
   -Winit-self -Winline -Wno-int-to-pointer-cast -Winvalid-pch -Wmain -Wmissing-braces
   -Wmissing-field-initializers -Wmissing-format-attribute -Wmissing-include-dirs 
   -Wmissing-noreturn -Wno-multichar -Wnonnull -Wpacked -Wparentheses -Wpointer-arith
   -Wno-pointer-to-int-cast -Wredundant-decls -Wreturn-type -Wsequence-point 
   -Wshadow -Wno-sign-compare -Wstack-protector -Wstrict-aliasing -Wswitch 
   -Wswitch-default -Wswitch-enum -Wsystem-headers -Wtrigraphs -Wundef -Wuninitialized
   -Wunknown-pragmas -Wno-pragmas -Wno-unused -Wunused-function -Wunused-label 
   -Wunused-parameter -Wunused-value -Wunused-variable -Wvariadic-macros 
   -Wvolatile-register-var -Wwrite-strings -Wdisabled-optimization -Werror

[Love4Boobies]

I compile with -Wall -Wextra -pedantic -ansi -std=C99 when I'm adding a feature, but I add -Werror and change -pedantic to -pedantic-errors when I get it working.

-ansi is C89, -std=C99 is C99. It's either one or the other.

[Love4Boobies]

Actually, -Wall -Wextra -pedantic is somewhere about half of what I use:

Code: Select all

CCPARMS= -c -I ./include/common -I ./include/ia-pc -I ./include/arch-x86 -march=i486 
   -Wall -Wextra -pedantic -O2 -Wshadow -Wpointer-arith -Wcast-align -Wwrite-strings 
   -Wmissing-prototypes -Wmissing-declarations -Wredundant-decls -std=c99 
   -Wnested-externs -Winline -Wstrict-prototypes -Wno-attributes -Wc++-compat 
   -Wchar-subscripts -Wcomment -Wno-deprecated-declarations -Wno-div-by-zero 
   -Wno-endif-labels -Wfloat-equal -Wformat=2 -Wno-format-extra-args -Wformat-nonliteral 
   -Wformat-security -Wformat-y2k -Wimplicit -Wimplicit-function-declaration -Wimplicit-int 
   -Winit-self -Winline -Wno-int-to-pointer-cast -Winvalid-pch -Wmain -Wmissing-braces
   -Wmissing-field-initializers -Wmissing-format-attribute -Wmissing-include-dirs 
   -Wmissing-noreturn -Wno-multichar -Wnonnull -Wpacked -Wparentheses -Wpointer-arith
   -Wno-pointer-to-int-cast -Wredundant-decls -Wreturn-type -Wsequence-point 
   -Wshadow -Wno-sign-compare -Wstack-protector -Wstrict-aliasing -Wswitch 
   -Wswitch-default -Wswitch-enum -Wsystem-headers -Wtrigraphs -Wundef -Wuninitialized
   -Wunknown-pragmas -Wno-pragmas -Wno-unused -Wunused-function -Wunused-label 
   -Wunused-parameter -Wunused-value -Wunused-variable -Wvariadic-macros 
   -Wvolatile-register-var -Wwrite-strings -Wdisabled-optimization -Werror

Err, -Winline and -Wwrite-strings appear twice, -Wmissing-noreturn doesn't exist, -Wstack-protector only works with -fstack-protector, you can use De Morgan's laws to get fewer -Wno-unused-* flags, and many of the other flags are either turned on by default or implied by -Wall -Wextra -pedantic -O2 -switch (the latter instead of two other switches). Here's the more compact equivalent (i.e., the same warnings are explicitly disabled and I haven't enabled any of the warnings you haven't):

Code: Select all

    CCPARMS= -c -I ./include/common -I ./include/ia-pc -I ./include/arch-x86 -march=i486
       -Wall -Wextra -pedantic -O2 -Wshadow -Wcast-align -Wwrite-strings
       -Wmissing-prototypes -Wmissing-declarations -Wredundant-decls -std=c99
       -Wnested-externs -Winline -Wstrict-prototypes -Wno-attributes -Wc++-compat
       -Wno-deprecated-declarations -Wno-div-by-zero
       -Wno-endif-labels -Wfloat-equal -Wformat=2 -Wno-format-extra-args
       -Winit-self -Winvalid-pch
       -Wmissing-format-attribute -Wmissing-include-dirs
       -Wno-multichar -Wpacked
       -Wno-pointer-to-int-cast -Wredundant-decls
       -Wshadow -Wno-sign-compare
       -Wswitch -Wsystem-headers -Wundef
       -Wno-pragmas -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-unused-result
       -Wwrite-strings -Wdisabled-optimization -Werror

PS: I was doing some work on my build system while writing this.

[Rusky]

Clang, in addition to a static analyzer, has a -Weverything flag that might make that sort of thing easier.

[Synon]

I compile with -Wall -Wextra -pedantic -ansi -std=C99 when I'm adding a feature, but I add -Werror and change -pedantic to -pedantic-errors when I get it working.

-ansi is C89, -std=C99 is C99. It's either one or the other.

I put both, I thought -ansi meant it would only use standard features. I'll take the -ansi out.

@Combuster,
Lol, surely -Wall and -Wextra account for all of those? I mean, -Wall is "enable all warnings" so I would've thought it would, you know, enable all warnings...