More Fun with Compiler Warnings

Friday, December 19th, 2008

My build machine for compiler warnings has been up for a while and reporting “warn-new” to the tinderbox. Since all this data is in a sqlite database, I wanted to present it in a better form than the tinderbox waterfall. So I’ve written up a quick frontend for the data.

Take a tour now!

I really love cherrypy+genshi+sqlite3 for this sort of work. It’s painlessly easy to get a database frontend up without worrying much about SQL or HTML injection.

The front-end graphs the count of unique warnings for each build, lists new warnings and fixed warnings for each build, and allows users to query warnings by directory, committer, and warning message. If you’d like to add additional queries, get the code here and contact me: I’m happy to upload the warning database for other people to experiment on.

As often happens, I’m hosting this on a machine in my home office, so it may disappear at any time. If it seems generally useful, I’ll talk to the Mozilla release team about getting a supported version running on the Mozilla network.

Laying Blame

Tuesday, December 9th, 2008

As I mentioned last week, I’ve been resurrecting a project to report on compiler warnings. A basic form of this buildbot is now operational on the Firefox tinderbox tree (look to the far right for the static-analysis-bsmedberg column). It prints a summary of the total number of warnings on the summary page: in the full tinderbox log, it lists each warning and who can be “blamed” for that warning:

/builds/static-analysis-buildbot/slave/full/build/memory/jemalloc/jemalloc.c:177:1: warning: C++ style comments are not allowed in ISO C90 blamed on Taras Glek  in revision hg:36156fbf817d8a0e2d54a271cf0bff94a1c41c13:memory/jemalloc/jemalloc.c
/builds/static-analysis-buildbot/slave/full/build/js/src/jsdbgapi.cpp:712: warning: ISO C++ forbids casting between pointer-to-function and pointer-to-object blamed on in revision cvs:3.36:js/src/jsdbgapi.c

Assigning blame can be a tricky process. In order to figure out the blame for a warning, the code uses the following steps:

  • Resolve relative paths against the current working directory, using GNU make “Entering/Leaving directory” markers as a guide.
  • Dereference symlinks to find the source tree location of an error. For instance, Mozilla headers which produce warnings often do so via paths in dist/include. We have to resolve these to their original source tree location in order to find blame.
  • Using mercurial APIs (through python), find the mercurial changeset which introduced the line in question.
  • If the code dates back to Mercurial revision 9b2a99adc05e, which is the original import of CVS code to Mercurial, use a database of CVS blame to find the original CVS checking which was responsible for introducing that line of code.

If you’re interested, take a look at the build log parsing code, or see the scripts which save CVS blame to a database (thanks Ted!).

The current reporting system for warnings is very primitive. I’m currently working on a second version of this code which will provide additional features:

  • Compare warnings with the previous build and highlight “new” warnings. I do this by recording the error text and the blamed location of the warning. As lines are added and removed from the code, the reported location of the warning changes, but the location of Hg/CVS blame doesn’t. This means it is a stable location which can be used for comparisons across runs. It even works across file renames/moves!
  • Web frontend to the warning database to allow users to query warnings by user or directory.
  • Classify warnings by “type”. This is not a simple process, because GCC mixes distinctive error text, such as “may be used uninitialized in this function” with variable names; and the granularity of -fdiagnostic-show-option is low enough that it’s not very useful by itself. Oh, I wish GCC had error codes like MSVC does: C1234 is easy to recognize!

At one point, I thought I could implement all of the warning mechanism on the buildbot server by parsing the BuildStep logs. It quickly became clear that I couldn’t do that, because I couldn’t resolve symlinks, and getting Mercurial blame was difficult or impossible. My new version actually uses a hybrid mechanism where the build log is parsed on the buildbot slave: this parses out warnings, resolves symlinks, and looks up blame. It then sends the results back via stdout to the master. A custom build step on the master parses this log, saves the information to a database, and does the checking for new warnings and prints various results to custom build logs.

Static-Checking Tinderbox Online

Monday, June 30th, 2008

Today I set up a buildbot/tinderbox for the mozilla-central codebase built with static checking. This allows us to enforce annotations such as NS_FINAL_CLASS and NS_STACK_CLASS throughout our codebase. See the static-analysis-bsmedberg tree on the mozilla-central tinderbox.

Stack-only Classes

As an example, I today annotated nsAutoString as a stack class. If someone commits code which allocates an nsAutoString on the heap, the static-checking tinderbox will turn red.

I have a set of similar patches in the works which mark various helper classes as stack-only. These patches are needed because the XPCOMGC rewrites treat stack-only classes differently from regular heap-allocated types.

Help Wanted

A while back Dave Mandelin wrote an analysis of outparam usage. This is now running and producing warnings. I would like to find some help to go through the fairly large number of warnings this analysis produces and find the real bugs and fix the bogus warnings in the analysis.

The most important warning to check is

warning: outparam not written on NS_SUCCEEDED

This indicates a condition where the analyzer can’t prove that an outparam was written, but the method returned NS_SUCCEEDED anyway. This can lead to uninitialized memory errors and odd latent bugs. If you’d like to help, please hop over to the #mmgc IRC channel, and dmandelin or I can help walk you through the analysis/fixing process.

Local Machine

Because the dehydra/treehydra codebase is still in flux prior to the 1.0 release, I am currently maintaining this on one of my local machines, so if it goes down please don’t pester Mozilla’s IT or release teams. Once dehydra 1.0 is released, we will get turn on static checking on the main unit-test tinderboxes maintained by the Mozilla release team.

Things I Learned From Writing a Mercurial Extension

Thursday, March 6th, 2008

I wrote my first mercurial extension today. It will print a log of all the commits you need to get from a changeset A which used to be the only head of a repository to a changeset B which is now the only head of the repository.It’s part of a project to get our buildbots hooked up to mercurial in a sane way. I need to make the same logic available via HTTP. It wasn’t too hard, but there are some tricky/undocumented things in the mercurial API that made life difficult.

  • The wire protocol is a pretty elegant way to ask a server for changesets when you don’t know what the graph looks like yet with relatively few queries.
  • The HTTP version of the protocol returns all the results in a plain-text format, except for the actual changesets which come across as a relatively opaque “bundle”.
  • The between command in the wire protocol doesn’t do what you think it does, or what the documentation seems to say it does. Instead of returning all the changesets between point A and point B, it walks backward through the list returning a selection of changesets along the line. The farther you go back the less often it will pick a changeset to return.
  • There is currently no way to query hgweb for metadata about a particular commit, such as the author, date, or commit message. This is trivial to add, and I will be submitting a patch shortly.
  • With a few extra functions, it should be relatively trivial to write a web repository browser which allows you to nagivate the revision graph graphically and dynamically load information about history as you need it.
  • cmdutil.show_changeset can be used to show changesets the same way hg log does. I haven’t yet figured out how to expose all of the formatting options it accepts as options accepted by my extension.