394405 - Use-after-free guard in nsMacWindow::~nsMacWindow makes Jesse sad

Status: RESOLVED WONTFIX

(Core Graveyard :: Widget: Mac, defect)

Description

[Jesse Ruderman]

>  // Explicitly reset mMacEventHandler to null so that any use after free of
>  // this object crashes on a null-ptr deref rather than using freed memory.
>  // ~auto_ptr() only deletes the pointer, it does not set it to null.

Should be changed to make it clear that the line below simply makes it *more likely* that the crash will dereference null, rather than guaranteeing it.  The nsMacWindow's own memory is freed, after all.

>  mMacEventHandler.reset(nsnull);

In debug builds, it might be better to set this to 0xdddddddd, so a developer who sees the crash won't think it's a safe null-dereference crash.  (Or do debug builds automatically set all of the freed nsMacWindow's memory to 0xdddddddd anyway?)

Comment 1

[Mats Palmgren (inactive)]

> simply makes it *more likely*

Sure, that's what I meant.
This file is removed on trunk so I'm not sure either of these
nits are worth fixing.

> (Or do debug builds automatically set all of the freed nsMacWindow's memory
> to 0xdddddddd anyway?)

AFAIK, not for objects that are allocated with default malloc/new.
We only do that for arena-allocated objects.

Generic memory problems are better detected using external tools,
like valgrind, MallocDebug etc.  The MallocDebug stuff on MacOSX
are quite good IIRC.
http://developer.apple.com/documentation/Performance/Conceptual/ManagingMemory/Articles/MallocDebug.html
http://developer.apple.com/documentation/Performance/Conceptual/ManagingMemory/Articles/FindingLeaks.html

Comment 2

[Jesse Ruderman]

Oh, this is branch-only?  Feel free to ignore me, then :)

Comment 3

[Mats Palmgren (inactive)]

Yes, 1.8.1 branch only.

-> WONTFIX