 So this is probably what you're expecting instead of the previous slides. Well, I have my regular audience. They know what to expect. They know what to expect. There's always going to be a travel log to start my talk out. I'm Frank Rowand. I have a couple of roles that relate to this talk. I work for Sony and one of those roles is I teach people inside of Sony how to work in open source, including how to work with the kernel. And some of the things I teach in a class are what motivated me to give this talk. I was trying to explain to them how do you figure it out warnings when you modify source code. And I'm also a kernel maintainer. And in that role, I face the same problem. And I have my own set of scripts historically to deal with that issue. But when I started this talk, I realized I could do much better. And I came up with a much better set of scripts, which we'll see late in the talk. So you probably are all familiar with static analysis tools. This goes to the title of the talk. Just some examples of them are GCC, DTC, SMATCH, SPARS. And we have Julia here. She has her tool, which I can never say the name of correctly. There are a whole bunch of different static tools that will show you possible problems or typically warnings like from GCC. So what is the problem that led to this talk? And I have a premise to talk about the problem. Three parts to the premise. First, there are many warnings reported by the existing static analysis tools against the code that's already in the kernel. The second premise is it can be difficult to determine when you get new warnings when you added a change to the code. Because there are so many warnings already in the code. The third premise is because of that, there are many developers and maintainers who don't even bother to check to see if there are new warnings when they get a new submission of code. So let's break that apart and see, is this true or is this false? And I'll look for your opinion also to see what you think. So here's a simple example. Look at the bolded code. That's really the part that matters. All the other stuff are steps that I just have to go through to do this sort of a check when I'm doing it by hand. First of all, I check out some version of the code that I'm going to modify. And then you see the bottom of that block of instructions, I do a make with w equals 3. So w equals 1, 2, or 3 invokes a different set of checks for different types of warnings. And I'm doing this make against a single object file. And I'm redirecting the output of this into just a normal file. So now I have a file full of warnings. Then the second block, I've checked out a new version of the code. You'll see there's one commit made there. I've modified one file. And again, I go through these black magic steps and I do the make w equals 3. And I put the warnings into a new file called new warnings. So I have old warnings with original warnings, file new warnings with a new set of warnings. So here's my first premise. Are there many warnings pre-existing? And taking a simplistic look at this, we just simply do a word count of the number of lines in those two files. 1,318 lines of warnings in the old, 1,320 after I made my modification. So let me start claiming my first premise. Yes, there are many warnings in existing code. It varies by file, but this was just a random file I just picked. Second premise, is it hard to find these new warnings when they occur? So I showed you how many lines there were in each of these warning files. Now if I just diff those two files and again look at how many lines there are, this is a nice simplistic approach. You should be able to just diff them, right? But no, I have 127 lines of difference. The naive way of looking at this, the very first thing I'm saying, I have two new lines of warnings. So why do I have 127 lines in my diff? And we'll see that. And here's that diff. Sorry, can anyone in the back read that? This is page one of the diff and page two of the diff. See, you can get a sense of what you're facing if you're trying to find where the heck is my two new lines of warning. So I'm going to claim that yes, it's hard to find new warnings with existing infrastructure. Okay, and in here just for reference, this is that one change I made. I simply added one empty function with one argument and exported it. And that's what led to the new warning or warnings, whatever it is. And we'll come back to this later. Third premise, do people actually check for warnings given this existing, okay, so we have a few people, let me start with narrowing this down a little bit. So when you just build the kernel, the default is w equals zero. So I'd expect all of you would check for new warnings for w equals zero. So the question then becomes, for w equals one, two and three, do you check? We had a handful of hands, like five hands that went up said that you do check for warnings. Do you always check for warnings? Do you sometimes check for warnings? How consistent? Kind of if the earned builds a thousand ran configs, we're going to have some, one of the things that I thought about when I started approaching this problem for myself was there must be a tool out there and I'll come back because we have actually someone told me about a tool yesterday. And occasionally you see reports on the mail list, your submission led to a new warning. But I wasn't able to figure out what these tools are and how they work. And so my premise was they're probably talking about w equals zero. Hopefully there is someone out there who's automated this and is checking one, two and three as well. But I didn't find those. So I wrote my own. Now I'm making this claim that it's really difficult. How about maybe I'm wrong? So actually did I not didn't ask. So how many of you think that my three premises are true? Kind of true. Sad but true. How many people think that it's not a problem. It's easy. You don't have any issues with it. Okay, no hands. Okay. So here's a counter example saying maybe I'm wrong. Here's another commit an entirely different source change as a single warning. And here I do the checkout and here's my get log showing I have a single patch. And I do a nice simple make with w equals one and my output is nice and simple. There's my warning. It's obvious. It just jumps right out. No problem. So sometimes it's easy. It's no big deal. And I have reformatted that warning message. So it's nice and pretty and easy to see. If we look at the actual raw, let me step back. So there are a lot of subsystems. If you say w equals one, they will build cleanly. And that's really, really good. A lot of subsystems is not to you get to w equals two or three. You'll start seeing a lot of warnings. But then there are subsystems where even if w equals one, you will get plenty of warnings. Okay. So like I said, it's really easy to see that warning. This is not reformatted. This is what it actually looks like. It wraps around. So it's not quite as pretty, but still no big deal. Really easy to read. Really easy to see. And I don't build in the source tree. I build in an external directory. So the world looks different in my world. Instead of just having warning messages have filenames starting at the root of the kernel directory or the kernel tree, mine start at slash and show the full path up to my git tree and then the path within the git tree. So my paths and my files are much longer, as you can see in this example. And just comparing those two examples, there's the warning message originally if you're building in tree versus building out of tree. So it's a little bit uglier. It's a little bit more noise. It's a little bit harder to find error messages and interpret them, especially if you have a lot of them. Okay. So am I whining? You know, if I'm complaining about something as trivial as that, am I smart enough to be a kernel developer? Can I even tie shoes? Look, I do have lace-up shoes. I do tie my shoes. Tim claims I did a code of conduct violation on myself. Do you want to report me or should I? So what are the implications of this? It really turns out that there are many, many sources of these little bits of noise. And each and everyone on its own may not be a big issue. But when they add up, it starts to become really, really hard sometimes to find your new warnings. So as trying to get rid of lots of different types of noise. So we have some examples, different types of noise. If you do a make even quiet, make is going to tell you what it's doing. It's going to tell you what script it's calling. It's going to tell you what files it's compiling, things like that. It's going to tell you when it's leaving directories. So I like to get rid of all that noise. That's useless to me. I don't need that. So with a simple pipeline, I can do a grep and get rid of that. Make dash s. I was looking for that. You would think there would be a silent. Okay, so Aaron tells us to make dash s. I'll have to do it the easy way then. The way that I did it here was simply redirecting standard out. So it goes away and I'm only getting standard error, which is where the warnings are. And then I pipe it through said to remove that long prefix on my path. So that said is substituting get curder into blank. Get curder is a script that I wrote. And just a real quick digression. Why don't you just use PWD? It works for most people. I have a corner case. There's always a corner case. And I get to my kernel tree through a soft link. And PWD gives a different result when you get in through a soft link. And it doesn't match what makes says. So I wrote get curder to report what makes thinks the path is trivial. But you know, it's all these little details that add up into making this a more difficult task for the average person who really doesn't want to spend time on fixing these things. Another gotcha is if you do make with W equals one, and then you do make W equals two of the same exact file, there's extra work that goes on. And so you have to do make W equals one, do whatever you want to do, make W equals two, throw that away, do make W equals two again. And here's an example. So I did the make W equals one. And you can see that there's a lot of CCs going on, a lot of compiling host CC, host LD, the linkers going on. There's even a CC, a whole bunch of stuff going on there. The second time I do make W equals one, all that stuff disappears. So pardon? Dependencies. I didn't look into why those dependencies exist. Make W equals two will change the compiler flags. And the kernel remembers how you compiled every single file that you have compiled before with what flags. And the flags are different from the last time it will compile it again. Yeah. In this example, these other compiles had no warnings. I do have cases that I've encountered where these other incidental compiles generate warnings also. So you don't want to mix those into your warnings output file and think here's a new warning because of the change I made. It's not. It's a preexisting warning. Another issue is I'm sure you're used to GCC warnings. The line number is in there. If a line number of where a warning is changes, here's an example where it moved by three lines, the warning message is different as far as diff is concerned, simply because the line number has changed. So it's easy enough to filter that out. You can use said, there are a lot of tools to get rid of those line numbers. But it's just annoying. And if you do remove the line numbers and look at the result with no line numbers and then try and go and find out what line number is that warning on, that line number is now gone. You have to go back to the original warning diff that had the line numbers. It's just real pain, real annoying. Another source of noise. This is kind of a misnomer when I say it's a macro expansion. This is really an error check within something that a macro calls. But for shorthand, this is essentially a macro expansion leads to something that has a line number incorporated into it. So it's the underscore underscore compile time assert underscore line number. So if I have this macro moved from one line to another, the warning message changes that number. This was a surprise to find. It was interesting chasing it down to see quite what was being invoked. So I have my own personal script as maintainer. I teach my class inside of Sony and as giving them line by line examples of how to fix all these things and build pipelines and what commands to use. And I was going to essentially give that as my talk. And then I got into it and I thought this is crazy. I really should just fix up the scripts and make it easy. None of this do it by hand all the time. So I have a new script called check worn and some programs and here's an example of using it. This is the same commit for my very, very first example, simply adding one new function with one parameter and exporting the function. And here's an example of running the script. I simply check out my original version. I run my script. Warn check. And I got carried away here. V verbose. Rf2 is how what filters is going to apply. Dash s was save my temporary warning files so I can look at them afterwards. And then I'm just saying here's the source file that I want to analyze. So I could have ignored all those other flags and just said drivers of overlay. And the script will create warning files for the original checked out version. Then it prompts me and says somehow get to your new version of the file. So you could do a get check out. You could do a quilt push. You could apply a patch by hand whatever your workflow is to get to a new version of the file and then just continue from that point. So in my case I did a get check out to get to my newer version. And here's the output from that script. There are going to be several sections starting kind of at a rough get an idea how things changed. This is not a very precise accurate report at this level. There could be false positives, false negatives. So for W equals zero, the old version had zero warnings, zero lines of warnings in the output and the new one still has zero. When I went to W equals one we went from two lines to three lines of output. W equals two we went from 606 to 609. So there are three more lines of warning. Not necessarily three more warnings, but three more lines of warning message. So this is kind of a rough, but it gives you a good quick characterization of did things maybe change and how aggressively did they change. Then the next report coming out of this is for each type of warning, how many were there before and how many after. And so far the tool knows about GCC warnings and DTC warnings. I haven't yet added in SMATCH and SPARS. So it categorizes by type of warning and you can see it highlights W redundant decals incremented by one. So it's a real good way again of getting a quick overview what type of errors happened or warnings did they change. If you added a new warning of one type and removed one of a different type that would jump out here whereas that might not jump out in the number of lines in a warning report or number of warnings. So there are definitely false negatives and false positives still available in these various outputs. And this is the W equals one, two and three. So I was showing the whole list of, so for W equals three that was redundant decals which I just showed. W equals two, one of the warnings they checked for were shadows. And W equals one, it caught a missing prototype. So it's good to check all those W levels. And this is a real good example of why that applies. Then there's the detail. This is the one that is much harder to get a false negative. I said that when you just do a diff with no line numbers it's hard to figure out where the problem really is. So the very first chunk, the filtered chunk, is the diff with no line numbers. And it did pull out there's only one new warning. We see the two new lines. We can see what the warning was. It was that redundant decal. And then I show both the old warnings and the new warnings with the line numbers in them. So now you have the source, the reference to see where to go look in the source. So in the old coding of course there was no warning in this case. In the new code we see the three lines of new warnings with the line numbers involved. So it's really easy to go find that new warning out in the source code. That's really convenient. So where do you find Warren Check? Eventually I want it to end up in the kernel source tree. I hope we get it there. There's been talk recently about creating an area for maintainers tools to live in the kernel source tree. And it's a gray area. Is it a maintainer tool? Is it a submitter tool? Is it both? I think they'll all fit in the same place. And hopefully we do create that location. And hopefully something like this will get accepted in there. Right now I have it on GitHub so you can pull it down and use it for yourself. It is totally useful at this point. It's better than what I've been using myself. So I'm happy to have a better tool for myself. And as I continue to work on it, I will mirror it on GitHub. So as it gets enhanced it will continue to live there. Once it's in kernel.org it will probably disappear from GitHub eventually. So there's still some things to do. There's still some rough edges. I'm a bash person so I wrote it in bash. I probably should make it work for just plain old shell, right? Otherwise the kernel people won't like it so much. I love three space tabs. Eight space tabs to get it in the kernel. So a lot of dumb little things like that. Little trivia things. Like I said I support C files, assembly files, and DTS files. And I like to add support for some of the other static tools. I showed you the example of running command and I specified a single file name. It's going to be a very simple change to be able to specify a whole list of files and run the report for all of them. And the natural outflow of that is if you have two git commits you should be able to feed the script one git commit old, one git commit new. And the script should be able to check out the old version, do the checks, check out the new version, do the checks. And it should be able to know what were the files modified between those commits. So it should be able to figure that out itself or let you manually choose which of the files modified you want to look at specifically. And again I like it to go to kernel.org. So what I'm asking for is I found just through stumbling over them some of these things that lead to difficulty filtering out changes that really aren't changes but just things like the lines moved. So if you encounter any other types of patterns that the tool doesn't take care of please let me know and I'd be glad to filter those out too. And I meant to sidetrack here. So I mentioned that, okay, you can never say your name. I want to use the g but it's here. Oops, you got to get you so you can see. Just told me yesterday he has a tool that does a lot of the similar things for removing some of that noise, things like lag numbers. He does some other things to move some other noise. And the place that he uses it is he takes reports of entire kernel builds from Michael Ellerman and he runs his tool against those. So it's a slightly different environment. My tool would not work in that very well. My tool is focused on a single file or a small number of files. And his tool is in his Git repo, the Linux script on GitHub. GitHub G E G E E R T U. Another thing on my second slide or first slide I talked about static tools and gave just a few examples. Mr. Arndt gave a talk at ELC 2016 about static tools. And I recommend going and looking at this for a lot more information on some of the static tools. But it just was a distraction for today to try and go into any detail of static tools. So at this point does this seem like a good solution? Do you think life will be easier? Will people start being able to check for warnings? Will people be more likely to check for warnings if these tools are in the kernel tree? Where's the microphone? I'd give it a spin. Okay, got one. I would be happy to sacrifice, I don't know, what a 20, 30K of my Git repositories going forward. Nice. I have some other plans, but I could probably use those tools to help with those. So some of the other plans I have are reorganizing the warning levels so that a lot more of these things in W equals one that have very few warnings get enabled by default and then we fix them all. And I also have some ideas for how you can more easily turn on and off a particular warning locally in the code when there's a known warning that we don't have a good solution for, but we want to shut it up. And I have ideas for turning on extra warnings per directory or per file. So you don't have to pass it on the command line, you just annotate the make file saying all of this directory we always want with W equals one for things like drivers media and drivers MFD. They already test that way, which means there are no false positives. And then they get everybody else to do the same. Yeah. You'll also find that in different subsystems, there are different practices for the different warning levels. Like I said, some subsystems are really clean at W equals one. And so you can expect that if you inject a new error at that level, you'll get called out on it. There are other subsystems with so many that maybe no one will notice if you add another one. But overall, I think we would like to push the entire kernel toward fewer and fewer warnings. More questions? Comments? Question from the way in the back. Have you considered using LLVM for compiling? Yeah. So there's LLVM is, I always get confused. Is Clang and LLVM, are they related or separate and different? Same thing. Same. Okay. Yeah. I was hoping that the format of the warnings coming out of LLVM will be the same. And that's one of my questions to follow up on to make sure it really is. So if that's the case, then it will just work. So what I'm searching for is the dash W, essentially. There's a little bit more to it. But if the same format of warning comes out, it should be really easy. Yeah. So I don't know the answer to that question. But a few years ago, I looked at like the error messages produced by GCC and the error messages produced by LLVM. And it seemed like LLVM was more intelligent about removing irrelevant things. GCC was tend to get confused and then spew out a lot of messages that didn't mean anything. Yeah. So you might get more precise or at least different information from LLVM. Yeah. That'd be good to get rid of some of the false negative or false positives. And again, if there are other static tools that people are interested in, I'd love to hear about them. So GCC, I don't know which version it was, but they just redid a bunch of their error message formats. So they tried to make them more user friendly, I think because of competition from Clang, because Clang had better error messages. That was the general consensus. Right. So you may have to adapt to that. So the only part of the format that matters to this tool is the dash W, some string, because it reports that table of how many were there before, how many are there after. But if you look in the diff, you'll see the full string of the warning message. So as long as just dash W, some string, I'm good. Okay. So I have a couple of questions. Firstly, do maintainers actually take warning fixes seriously? I'm sorry, is that it again? Do maintainers take warning fixes seriously? Some of them seem to. Yeah. I've unfortunately been forced to do pre-sales work. So I've been trying to do something useful by hunting sparse warnings in the kernel. I'm up to 147 patches now. Yeah. And what do you feel about people adding more warnings? Yeah. If it's a warning that's showing a real potential code problem, then to me that's a good warning. If it's just a preference warning, like, well, I'll use the check patch example, you know, 80 line limit. Then sometimes that's a nuisance warning. Sometimes it actually is a very valuable warning because sometimes when I've hit it, it makes me stop and think, is my code structured well? Do I have horrendously long variable names, which are more difficult to read and understand and scan through. And so actually that 80 column warning is sometimes helped me clean up my code. Sometimes it's just a lot easier to have clean code when it's long lines. So again, it's the type of warning. If it's useful, yes, more warnings are good. Just to go back to my example, of those 147 patches, there are actual four bugs found in that where people have either passed the wrong argument or have missed some sort of user space annotation or in one case missed an engine cast. Yeah. And I actually saw those messages in the mail list and I was really happy to see real bugs being catch caught. And in my own examples, in device tree, when I started running these different levels of W equals, I have caught real bugs also just with the existing warnings. So it definitely is valuable when I can look at the ones that are real versus versus not. And my last comment is one of the things I have been recently doing is why I'm running sparse a lot is I'm currently teaching sparse how to deal with printer formatting strings, which apparently is a lot of fun and they are incredibly difficult things to deal with. So hopefully that will get merged at some point. Yeah, that'd be great. I'm glad you've been working on this stuff. Originally, in my early years, I wasn't using smash and sparse. There are so many levels of things to do as you become more involved as a maintainer or submitter. And once I started using those tools, I really became hooked on them and they become part of my automated process. Just makes it a whole lot easier when you automate things. Question over or comment? Hi. I'm not sure it's related, but I have observed that some warnings are specific to configuration. It means that for the same code base, you change the .config file and then some warnings pop up. Yes. So how is it related to your work basically? Yeah, this tool can't deal with that at all. It doesn't know, but that is one of the things I teach about in my class. One of the things that the tool does is check whether the .o, the corresponding .o exists. If it doesn't exist, it gives you a warning. It'll try and make it. It checks again. If it still doesn't exist, that's a very good clue that your config is off. You need to change your config. But there are other config types. There's a lot of, if a certain config do real stuff, else essentially stub things out because you still want the builds to build and try and detect warnings and errors. And you have to know whether you're checking the real code or stubs and be sure that, in the example of running my tool, you'd actually want to run it with both configs, the one with the real code and the one with the stubs, and make sure that they're both clean. And that becomes a very complex, ugly issue with the commentorial config options, which may impact your code. That may not be obvious. It may be more generic config options that aren't specific to your code. Okay. And what configuration do you use for your experiments? My experiment is on a dragon board. And I'm using the MSM 8970, it's arch arm dev config. MSM 8974. Yeah. Just that dev config. And then given that I do device tree, I like to turn on my unit testing. So I tend to turn that on. And I actually had to, in this case, to get the overlay file, because that only comes into play once you start doing dynamic. And I turn on dynamic and unit tests kind of as a whole. So that's a real good example. If I didn't turn it on, I couldn't even check for any warnings in this file that I'd modified. Thank you. So we have two minutes, question from Julia and question up here. So just in the interest of endless self-promotion, a couple years ago, I'm presented here a tool which is called jmake, which lets you, it informs you if the configuration you chose doesn't actually compile the lines that you changed. That would be really good to tie into this. I have a last question for people. Is anybody actually checking the warning tools to make sure they are actually producing correct warnings? Answer from Arndt. Where's my compiler guy? I was talking to him earlier. I've just spent a year building kernels with Clang, and when I submitted the last patches for the current merge window, Linus pointed out that I had introduced a very trivial warning that Clang had apparently never missed and that Jesus had always warned about and was very embarrassing. So yes, a lot of the tools do miss important warnings. Yeah, that's an example of where it's good to have multiple tool chains building the same project. You have 10 seconds. I'll be around till tomorrow morning. I'll definitely be at the closing game session if you don't find me anywhere else, and I'll hang around here after the talk. I have no need to rush off right now. Pardon? Oh, I've in half an hour after rush off, but until then, I'm available here. Thank you all for coming.