On Thu, Nov 27, 2014 at 9:04 PM, Dan Carpenter dan.carpenter@oracle.com wrote:
On Thu, Nov 27, 2014 at 06:11:30PM +0100, Daniel Vetter wrote:
btw not sure whether checker should just look through WARN_ON, we have lots of places where we've historically screwed up and added a WARN_ON + early return to make sure we'll in the future somewhat recover. This is really important for gfx since at boot-up (due to fbcon locking bonghits) the entire intial modeset is run with console_lock held. And that's a few 10k lines of code depending upon platform :(
So we absolutely have to handle failures robustely, but if checkers assume that it's ok to pass crap caught by WARN_ONs around then that's might reduce checker usefulness quite a bit.
If you do:
if (WARN_ON(xxx)) return -ESOMETHING;
Then that's important because it affects code flow and Smatch does the right thing, but if it's:
WARN_ON(xxx);
then Smatch ignores that. I guess I could hack it so WARN_ON() was treated like BUG_ON()...
I think even the if above should be treated like a BUG_ON for analysis, since something definitely went wrong that should have been. If the checker treats it as normal control flow it might not see bugs which are there (since it probably assumes that the worst-case recovery code is normal flow when it's just to make sure we can get useable bug reports instead of "machine hung"). -Daniel