mwu's blog
A tale of a fearless coder
The Twelve Booleans of Mozilla
July 28th, 2011 by mwu
In order to switch Mozilla away from PRBools and to real boolean types, it’s necessary to first fix any code that relies on PRBool being a plain integer. This can be accomplished by ensuring every assignment to a PRBool or return from a function returning PRBool is in fact a boolean.
I wrote a clang plugin to do this called BoolCheck. It’s like Taras’ Prcheck, except based on clang instead of elsa. BoolCheck borrows Prcheck’s tests but everything else was written from scratch.
BoolCheck can check code like this:
typedef void (*prboolFuncPointerType)(PRBool i, double j, PRBool k);
prboolFuncPointerType* f;
void foo() {
int foo = 12;
PRBool zoo = 13;
(*f)(foo, 44, 99);
}
and output errors like this:
% clang++ -c -Xclang -load -Xclang ../BoolCheck.so -Xclang -add-plugin -Xclang boolcheck -Xclang -plugin-arg-boolcheck -Xclang -load-type-list -Xclang -plugin-arg-boolcheck -Xclang test-bools func_pointer_bad.cc
func_pointer_bad.cc:8:16: error: Literal is not a valid boolean PRBool zoo = 13; ^~ func_pointer_bad.cc:9:8: error: Non-boolean expression (*f)(foo, 44, 99); ^~~ func_pointer_bad.cc:9:17: error: Literal is not a valid boolean (*f)(foo, 44, 99); ^~ 3 errors generated.
The most complicated part of this is determining whether something is a valid boolean or not. As it turns out, PRBool isn’t the only boolean type in the mozilla code. Every once in a while, another (custom) boolean type gets assigned to a PRBool. In order to make sure that type is always used as a boolean and not a integer, that type is added to the list of types being checked for correctness.
In the end, we need to check about twelve different boolean types to ensure PRBool is only ever assigned booleans.
- PRBool
- PRPackedBool
- JSBool
- JSPackedBool
- cairo_bool_t
- pixman_bool_t
- gboolean
- PKIX_Boolean
- CK_BBOOL
- mdb_bool
- mork_bool
- realGLboolean
The actual number can vary by platform since types like gboolean would only be seen on Linux/gtk, but other platforms usually introduce their own boolean type in place of gboolean. The plugin takes a file containing a list of these types to check for boolean correctness so it can easily be used to help other projects switch to real booleans.
So afterwards, they’re just 10 boolean types left?
Getting rid of JSBool and JSPackedBool would be nice too, though that’s a bit more difficult than PRBool and PRPackedBool.
A few of the other boolean types are used in C where there is inconsistent support for bool – VC doesn’t support it.
Don’t touch realGLboolean!! OpenGL relies on its sizeof being 1, which bool doesn’t guarantee.
I’m not. I’m only changing Mozilla bool types where we have control over. However, it needs to be on the list of boolean types for this analysis to work out.
Coincidentally, I haven’t seen a platform where bool isn’t 8bit wide yet.
Very old GCC versions had sizeof(bool)==4. I concur that all current compilers seem to be using sizeof(bool)==1 but that would at least require a test (like a static assertion) before it’d be comfortable relying on.
Do the changes have to be manual? Couldn’t the plug-in rewrite the expressions too?
Good question.
The problem is that the only easy rewriting case is the case where it doesn’t matter. The bool type automatically converts non-zero values to 1, so surrounding every non-boolean expr with !!(expr) doesn’t buy you anything after switching PRBool to bool. Then, for the cases where PRBool isn’t the type the code should be using, there’s a judgement call about what type to use instead and if the code needs other fixes.
I’ll try to elaborate on this in a future post.
[...] Wu’s boolcheck tool is awesome. He wrote it to check that “typedef int” bools are really being used as [...]