Statically Checking the Mozilla Codebase

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:

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.

Atom Feed for Comments 8 Responses to “Statically Checking the Mozilla Codebase”

  1. Gryph0n Says:

    Hey, the solution nsAutoString for seems so obvious that I’m sure you guys tried it and found flaws

    1>Make operator new as a private member of nsAutoString
    2>Make all constructors private to prevent member-of relationships like illustrated above

    What am I missing?

  2. Benjamin Smedberg Says:

    If all the constructors are private, then how do you use it properly on the stack?

  3. Simon Francis Says:

    Out of curiosity, why can’t nsAutoString be heap allocated?

  4. Ben C. Says:

    If all constructors are private and you still shouldn’t allocate it on the heap, presumably the only way to get one of these things would be to have a static method that creates instances in a static array on the stack and returns references? I don’t know, it seems ridiculous to me, but maybe I’m missing something.

    I would be interested in hearing what exactly is creating the stack-only restriction in the first place. Generally, that seems like a broken design to me, although I’m well aware that sometimes other concerns come above design purity.

  5. Benjamin Smedberg Says:

    nsAutoString allocates a 40-character buffer as part of the class: this avoids heap-allocating the string buffer for small strings. This is fine for temporaries on the stack because allocating stack is cheap but for heap allocations this is almost always a waste of space.

  6. jmdesp Says:

    Hum, for heap allocation they are some case where I know the size will almost always be around some fixed size, and a static buffer would be a great performance enhancement.

    Waht would be great would be to templatize nsAutoString to be able to use larger than 40-character size when you know it will be useful. For example for storing a email line.

  7. shaver » that’s when I shoot the water cannon Says:

    [...] by — the scale and…uniqueness of our codebase. One great milestone is the addition of build-time static checking to the mozilla2 build process. If the enforcement opportunities presented by static-checking.js are [...]

  8. AndersH Says:

    Have GCC definitively replaced Elsa?

Leave a Reply