PRBools abuse

Jonathan Protzenko jonathan.protzenko at gmail.com
Wed Jun 8 20:12:32 UTC 2011


Hi,

There was a lot of activity happening around malformed PRBools recently. 
Here's a rundown:
- in https://bugzilla.mozilla.org/show_bug.cgi?id=654126, we found that 
a severe segfault was due to a badly formed PRBool that was neither 0 or 
1, and that was blindly put into a JSBool, thus creating a JSBool that 
was neither 0 or 1;
- mrbkap said we should have some assertions for that, so I made it 
fatal to pass a bad boolean to the JS engine 
<https://bugzilla.mozilla.org/show_bug.cgi?id=658351>;
- quite a few bugs came out following this: 
https://bugzilla.mozilla.org/show_bug.cgi?id=662126, 
https://bugzilla.mozilla.org/show_bug.cgi?id=662125, 
https://bugzilla.mozilla.org/show_bug.cgi?id=659666, 
https://bugzilla.mozilla.org/show_bug.cgi?id=583699, 
https://bugzilla.mozilla.org/show_bug.cgi?id=661319 ;
- however, there was too much fallout, and this made Firefox really 
crashy, so we made xpcconvert.cpp check for badly formed PRBools, and 
fix them while issuing a warning if the value was not 0 or 1 — the 
warning reads "Passing a malformed PRBool through XPConnect".

Mark created a bug a few months ago asking that we review or at the very 
least audit our abusing of PRBools 
<https://bugzilla.mozilla.org/show_bug.cgi?id=582523>. Now that 
xpcconvert.cpp is doing the !! at the XPConvert boundary, we shouldn't 
crash JS anymore. However, I firmly believe that we should watch out for 
badly formed PRBools as a sign of possibly problematic / erroneous code. 
So, if any of you, while running debug builds, see the warning pop up, 
it might be a good time to audit the code that's causing it.

As to me, I'll probably patch my mozilla/ subdirectory to remove the !! 
in xpcconvert.cpp, so that I can catch any such mistakes. These are 
really trivial to debug, so I'll just have to remember to run 
thunderbird-bin with a debugger attached at all times.

Cheers,

jonathan



More information about the tb-planning mailing list