Closed Bug 1396361 Opened 2 years ago Closed 2 years ago

Crash in <name omitted> | -[NSApplication _doUnhideWithoutActivationMaybeFakingIt:]

Categories

(Core :: Memory Allocator, defect, critical)

56 Branch
Unspecified
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: philipp, Assigned: glandium)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-9d037533-3112-436d-b5d9-fdfd60170903.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	libmozglue.dylib 	<name omitted> 	memory/build/zone.c:159
1 	AppKit 	-[NSApplication _doUnhideWithoutActivationMaybeFakingIt:] 	
2 	AppKit 	-[NSApplication sendEvent:] 	
3 	XUL 	-[GeckoNSApplication sendEvent:] 	widget/cocoa/nsAppShell.mm:113
4 	AppKit 	-[NSApplication run] 	
5 	XUL 	nsAppShell::Run() 	widget/cocoa/nsAppShell.mm:683
6 	XUL 	nsAppStartup::Run() 	toolkit/components/startup/nsAppStartup.cpp:287
7 	XUL 	XREMain::XRE_mainRun() 	toolkit/xre/nsAppRunner.cpp:4615
8 	XUL 	XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) 	toolkit/xre/nsAppRunner.cpp:4772
9 	XUL 	XRE_main(int, char**, mozilla::BootstrapConfig const&) 	toolkit/xre/nsAppRunner.cpp:4867
10 	firefox 	main 	browser/app/nsBrowserApp.cpp:236
11 	firefox 	start

these crash reports are popping up in the 56 cycle on macos installations with MOZ_RELEASE_ASSERT(zone) that got added in bug 1356701.
The crashing code looks like this:

static void
other_zone_free(malloc_zone_t* original_zone, void* ptr)
{
  // Sometimes, system libraries call malloc_zone_* functions with the wrong
  // zone (e.g. CoreFoundation does). In that case, we need to find the real
  // one. We can't call libSystem's free directly because we're exporting
  // free from libmozglue and we'd pick that one, so we manually find the
  // right zone and free with it.
  malloc_zone_t* zone = malloc_zone_from_ptr(ptr);
  // The system allocator crashes voluntarily by default when a pointer can't
  // be traced back to a zone. Do the same.
  MOZ_RELEASE_ASSERT(zone);
  MOZ_RELEASE_ASSERT(zone != original_zone);
  return malloc_zone_free(zone, ptr);
}

So somehow, this function is given a pointer that doesn't belong to any zone, such that malloc_zone_from_ptr returns NULL. And as mentioned in the comments, the system allocator crashes voluntarily in the same situation, per the definition of free() in https://opensource.apple.com/source/libmalloc/libmalloc-116.50.8/src/malloc.c.auto.html (malloc_debug_flags defaults to MALLOC_ABORT_ON_CORRUPTION per set_flags_from_environment).

Now, the notable difference between this code and the code from system free in libmalloc is that system free returns early when the pointer is NULL. It's plausible that malloc_zone_from_ptr would return NULL for a NULL pointer, and would lead to this crash.

Following the disassembly for the crashing code, it appears the rbx register contains the pointer passed into this function, and the crash reports do contain the value for rbx, which is 0, validating the hypothesis above.

What this means is that not only some system libraries do call malloc_zone_free with the wrong zone, but they even call it for NULL pointers...
Assignee: nobody → mh+mozilla
Comment on attachment 8904079 [details]
Bug 1396361 - Avoid crashing when some system library calls malloc_zone_free(zone, NULL).

https://reviewboard.mozilla.org/r/175826/#review180810
Attachment #8904079 - Flags: review?(n.nethercote) → review+
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/488a3f27fb35
Avoid crashing when some system library calls malloc_zone_free(zone, NULL). r=njn
Comment on attachment 8904079 [details]
Bug 1396361 - Avoid crashing when some system library calls malloc_zone_free(zone, NULL).

Approval Request Comment
[Feature/Bug causing the regression]: bug 1356701
[User impact if declined]: random crashes due to some system libraries doing unexpected things
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: n/a
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: it's a trivial change that essentially skips the (voluntarily) crashing code in a safe case (freeing a null pointer, so there's nothing to free in the first place)
[String changes made/needed]: n/a
Attachment #8904079 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/488a3f27fb35
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment on attachment 8904079 [details]
Bug 1396361 - Avoid crashing when some system library calls malloc_zone_free(zone, NULL).

Fix a crash. Beta56+.
Attachment #8904079 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.