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

RESOLVED WONTFIX

Status

Core Graveyard
Widget: Mac
--
trivial
RESOLVED WONTFIX
10 years ago
8 years ago

People

(Reporter: Jesse Ruderman, Assigned: Josh Aas)

Tracking

({regression})

1.8 Branch
x86
Mac OS X
regression

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

10 years ago
>  // 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

10 years ago
> 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
Version: Trunk → 1.8 Branch
(Reporter)

Comment 2

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

Comment 3

10 years ago
Yes, 1.8.1 branch only.

-> WONTFIX
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WONTFIX

Updated

8 years ago
Component: Widget: Mac → Widget: Mac
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.