Skip to end of metadata
Go to start of metadata

With the integration of:

    2933 compiler warning gags need better granularity

The warning coverage of the illumos source base is vastly greater.

If you modify illumos at all, you should read the rest of this email. If you don't, feel free not to.

If you are downstream of illumos, you should certainly read this in its entirety. It is very unlikely your source tree will build without further work after merging these changes

What

Rather than disabling a fairly large set of GCC warnings implicitly in the compiler wrapper, most warnings are now disabled on a per-component basis.  Any component clean of a given warning at the present time thus has that warning enabled, such that it stays clean.

Warnings are disabled via appending to the CERRWARN variable in your Makefiles, most Makefiles in the system will provide examples.

Why

Because several people mentioned that they were missing bugs that the compiler would have told them about if we were not disabling them over-broadly, because it allows us to keep the code generally cleaner, and it provides somewhat of a step toward a world without the need for lint.

How this affects you

If you are downstream of illumos and have made any source changes there is a really good chance that this will break your build. That can't be helped, really. In the systems I have used as experiments, the breakage has been minor and probably is pointing out actual bugs.

What you should do

When you encounter a compiler warning, you should fix it!

This does not mean that you should take whatever action is the easiest to make the compiler shut up, it means that you look at what the compiler is telling you and determine if an actual bug is present.

For example:

If warned that a variable may be used uninitialised, you should not merely add '= NULL' to the declaration, you should find the potential uninitialised use, and determine what should happen instead. The = NULL will shut the compiler up, but you'll still crash.

When you're fixing warnings, you should do so within a 'bldenv' environment, rather than with successive nightly builds. Due to the multi-pass nature of things, and the fact warnings may be emitted from several passes, it is quite possible that fixing or disabling one warning will cause others to be reported instead. It's really frustrating to do this under nightly, as you can imagine.

If you end up having to disable a warning, you do so in the manner done in this change, via appending to CERRWARN:

CERRWARN += -_gcc=-Wno-uninitialized

for example; you will find the -W flag that enables the warning mentioned at the end of the warning message itself. You should expect to explain to any code reviewers why you had to disable the warning.

REMEMBER that when disabling a warning in the kernel there are generally both intel and sparc Makefiles that will need changing, gagging it on only one platform would be a really silly way to break the build.

GCC also provides a mechanism to disable warnings across certain lines of code, though it is not attractive. You will find it in the GCC manual, and should consider wrapping it up in macros (put them in sys/ccompile.h), if you want to use it extensively.

What might go wrong that isn't your fault

Again, due to the nature of the compiler, and warnings being emitted from several passes it is possible, though fairly unlikely, for your code to produce warning messages on some platforms but not others (SPARC v. x86, for eg.) That will of course be really frustrating, but you should expect people to not mind too much if this happens, though it will still need to be fixed.

False positives

There are likely to be false positives, this is sad but hard to avoid. If false positives for a given class of warning are sufficiently prevalent, we can disable that warning globally again. If code exists such that the warning cannot be fixed, the same applies.

"clobbered by longjmp or vfork"

Unfortunately, GCC's -Wclobbered warning calls out "longjmp or vfork". It doesn't mean this, any function which returns in the same manner as longjmp or vfork have the same issue. You're just as likely to see this if you use on_trap, on_fault, etc, you're just then going to be confused about what the message is saying.

You want to fix some stuff, what seems worth fixing

I'd fix areas of the source you cared about, so that you got more warnings enabled to stop anything being introduced.

If I were just looking for things that seemed worth the effort to pro-actively fix I would look at:

  • implicit function declarations 
    some of these hide real bugs (because we can't type check function calls if we don't have prototypes).
  • uninitialized use
    We're running into these more and more, might as well nail them all, but note as I said above that you should actually think about what the code is doing.
  • variable clobbering by longjmp and friends
Labels: