704304 - Check for miscompilations of the nsAutoPtr destructor

Status: RESOLVED WORKSFORME

(Core :: General, defect)

Description

[Joe Drew (not getting mail)]

In bug 685767, I was bitten by a miscompilation of nsAutoPtr's destructor inside gfxAlphaBoxBlur's destructor. As near as we (Bas Schouten and I) could tell, this was related to the fact that the contained class, mozilla::gfx::AlphaBoxBlur, is contained in namespaces.

This bug only shows up when compiling Mozilla on Windows with optimization on. Bas was working on a reduced testcase, but I don't know how much luck he had. We think it had to do with function deduplication, since the bug didn't occur in debug builds.

If you look for nsAutoPtrs with T=anything in a namespace, and then go to the disassembly of the containing class's destructor, you should not see anything containing a delete[] call; you should see something containing a regular delete call. The easiest way to verify this is to break in that class's destructor and step through the assembly for all of the calls to contained destructors.

Here's a list of things to do. You'll need a Windows optimized build. I was able to reproduce this bug with both MSVC 2005 (what we use on our build machines) and 2010.

1. Come up with a list of classes that contain nsAutoPtrs for namespaced classes.
2. For each of those classes, set a breakpoint in the destructor. Find a way to trigger that destructor; this might result require you to quit Firefox, depending on the class.
3. Go to the disassembly for the code you're debugging, and step instruction by instruction.
4. Match up the destructors being called with the members of the class. Destructors are called in the reverse order of their declaration in the class.
5. If you find any leaks, point them out!

Comment 1

[Jesse Ruderman]

Yikes.  Did you report the bug to Microsoft?

Comment 2

[Joe Drew (not getting mail)]

I don't remember if Bas ended up being able to create a reduced testcase or not, but if he did he sent it on to Microsoft.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

I think it is fixed