Things I’ve Learned

Wednesday, May 27th, 2009

Things I’ve learned recently:

  • Using hg log on a file that was removed will not list the revision in which the file was removed. You want hg log --removed.
  • Waist Watcher sodas are sweetened with Splenda, but don’t have the metallic taste that some other diet sodas do. I especially like the Citrus Frost (grapefruit) flavor. It’s like Fresca without the hidden Aspartame. (I have bad digestive reactions to Apartame.)
  • Linking libxul on my Linux machine takes between 3 and 10 seconds, but apparently this is unusual. Several other people have reported link times that can range into the minutes. I recommend buying as much RAM as you can: if your entire object directory fits in the filesystem memory cache, build speed is much faster.
  • When Microsoft Visual C++ mangles function symbols, the return type is encoded in the mangled name. When GCC mangles names, the return type is not encoded:
    GCC

    MSVC

    int testfunc(int)

    _Z8testfunci

    ?testfunc@@YAHH@Z

    void testfunc(int)

    _Z8testfunci

    ?testfunc@@YAXH@Z

    This means that separate functions in different translation units may not differ only by return type. We found this trying to combine the Chromium IPC code with Mozilla: const std::string& EmptyString() and const nsAFlatString& EmptyString() differ only by return type. On Windows this links correctly, but on Linux this causes multiple-definition errors.

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 brendan@mozilla.org 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.

Things I’ve Learned

Friday, May 2nd, 2008

Has GCC Dehydra Replaced Elsa?

Sunday, March 9th, 2008

No.

GCC Dehydra allows us to do analysis passes on our existing code. In the far future it may also allow us to do optimization passes. But it does not have the ability to do code rewriting, and probably won’t gain that ability any time soon. In order to be able to do C++->C++ transformations, you have to have correct end positions for all statements/tokens, not just beginning positions. GCC does not track this information, and making it do so would be a massive undertaking.

Mozilla2 still lives in a dual world where automatic code refactoring is done using the elsa/oink toolchain, while static analysis is taking place using the GCC toolchain.

Statically Checking the Mozilla Codebase

Wednesday, March 5th, 2008

In the header file that declares class nsAutoString, there is an important comment: “Do not allocate this class on the heap”. This rule, buried deep in a header file that almost nobody reads, is a small example of a problem that plagues the Mozilla codebase: it’s easy to write incorrect code.

Mozilla, and XPCOM in particular, uses a meta-language on top of C++. This meta-language of helper classes and typesafe templates allows experienced XPCOM coders to avoid some of the complexities of XPCOM refcounting and memory management. Unfortunately, it is possible, even easy, to use this meta-language incorrectly or inefficiently. The following code, while correct C++, is incorrect “Mozilla C++”:

class Foo
{
private:
  nsAutoString mStr;
  ...
};

void Bad()
{
  Foo *foo = new Foo(); // allocated nsAutoString on the heap!
}

Errors such as this are especially insidious because the code works correctly; it is merely an inefficient use of memory which can add up quickly.

Taras and David have been working on a solution which will allow application-specific rules such as “only allocate nsAutoString on the stack” to be enforced at compile-time. It is called Dehydra GCC. It is a tool which translates the internal GCC representation of C++ code into a JavaScript object model, and allows application authors to write analysis passes as scripts.

This past week I hooked up Dehydra to the Mozilla build system. It is now possible to configure --with-static-checking=/path/to/gcc_dehydra.so and Dehydra will enforce a few basic rules: classes annotated with NS_STACK_CLASS may only be allocated on the stack, and classes annotated with NS_FINAL_CLASS may not be subclassed. For an example of a more complicated analysis, see bug 420933, ensuring that XPCOM methods handle outparams correctly.

Dehydra currently only works on Linux, and building it is a bit complicated: a custom patched GCC is required, as well as a spidermonkey package. Complete directions are available on the Mozilla Developer Center.

Dehydra is still a work in progress. We are working to complete the following tasks:

We are actively looking for hackers to help out with this project. There are many different kinds of tasks people can help with:

  • Write analysis passes to check for common bad-code patterns in Mozilla or other projects
  • Implement a webtool that allows users to browse code exactly
  • Help package up GCC-dehydra in RPMs and .deb packages for easy installation into Linux distros

I am very excited about the prospect of using static checking tools such as this one to improve our code quality and development cycle, and I’m looking forward to new and unexpected uses for this code! In future posts I will cover basics of writing a dehydra script.