The default bug view has changed. See this FAQ.

C++ new/delete aren't wrapped with jemalloc on Android

RESOLVED FIXED in Firefox 14

Status

()

Core
Build Config
--
blocker
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bjacob, Assigned: glandium)

Tracking

(Blocks: 1 bug)

unspecified
mozilla15
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox14 fixed, blocking-fennec1.0 betaN+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

That has caused countless crashes in C++ libraries that we import into libxul/libgkmedias: see bug 746794 for ANGLE and bug 740194 for Skia. We need a good definitive solution so we don't keep running into allocator mismatch crashes over and over again.

Updated

5 years ago
Blocks: 746794, 740194
(Assignee)

Comment 1

5 years ago
Other than ANGLE and Skia, there's also other-licenses/skia-npapi and ... js/src :-/
(Reporter)

Updated

5 years ago
Severity: normal → blocker
(Reporter)

Updated

5 years ago
blocking-fennec1.0: --- → ?
blocking-fennec1.0: ? → betaN+
(Assignee)

Updated

5 years ago
Depends on: 758773
(Assignee)

Updated

5 years ago
Blocks: 758858
(Assignee)

Comment 2

5 years ago
Created attachment 627451 [details] [diff] [review]
Wrap operator new/delete on Android

Doing this properly is going to be tough in the given timeframe, so for now, only address the immediate Android problem. Bug 758858 filed for the long-term solution.

Try builds with this patch applied will be there once built:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-b397cfa64854/
Attachment #627451 - Flags: review?(blassey.bugs)
(Assignee)

Updated

5 years ago
Attachment #627451 - Flags: review?(khuey)
Comment on attachment 627451 [details] [diff] [review]
Wrap operator new/delete on Android

Review of attachment 627451 [details] [diff] [review]:
-----------------------------------------------------------------

I'm just going to assume that your mangled names are correct.
Attachment #627451 - Flags: review?(khuey) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/ef0180b53eae
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
(Assignee)

Updated

5 years ago
Attachment #627451 - Flags: review?(blassey.bugs)
(Assignee)

Comment 5

5 years ago
Comment on attachment 627451 [details] [diff] [review]
Wrap operator new/delete on Android

[Approval Request Comment]
User impact if declined: random crashes due to memory allocator mismatch
Testing completed (on m-c, etc.): Landed on m-c today.
Risk to taking this patch (and alternatives if risky): It shouldn't break anything. In the worst case scenario it might unveil other allocator mismatches, but certainly much less severe than the ones being fixed (and it's a very hypothetical scenario)
String or UUID changes made by this patch: No string change.
Attachment #627451 - Flags: approval-mozilla-aurora?
Comment on attachment 627451 [details] [diff] [review]
Wrap operator new/delete on Android

Please land ASAP for the Fennec beta build.
Attachment #627451 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 7

5 years ago
http://hg.mozilla.org/releases/mozilla-aurora/rev/3a0cedb8ecec
status-firefox14: --- → fixed

Updated

5 years ago
Depends on: 695068
(Assignee)

Comment 8

5 years ago
Brown paper bag: while operator new/delete were correctly wrapped, they were wrapped to system malloc/free...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 9

5 years ago
Created attachment 628888 [details] [diff] [review]
Wrap operator new/delete to jemalloc on Android

Also make operator new distinguishible from operator new[] for valgrind. (likewise for delete/delete[]).
<none>
Attachment #628888 - Flags: review?(justin.lebar+bug)

Updated

5 years ago
status-firefox14: fixed → affected
(Assignee)

Comment 10

5 years ago
Created attachment 628904 [details] [diff] [review]
Wrap operator new/delete to jemalloc on Android.

Forgot to remove a (moved) prototype.
<none>
Attachment #628904 - Flags: review?(justin.lebar+bug)
(Assignee)

Updated

5 years ago
Attachment #628888 - Attachment is obsolete: true
Attachment #628888 - Flags: review?(justin.lebar+bug)
Attachment #628904 - Flags: review?(justin.lebar+bug) → review+
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/mozilla-central/rev/306365557d64
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 12

5 years ago
http://hg.mozilla.org/releases/mozilla-aurora/rev/540694f5df42
status-firefox14: affected → fixed
(In reply to Mike Hommey [:glandium] from comment #8)
> Brown paper bag: while operator new/delete were correctly wrapped, they were

Ah .. V's cross-DSO checking failed to pick this up because it intercepts
new/new[]/delete/delete[] as well as malloc/free, and so it would not see
that new etc were routing to the wrong place.  Ah well.
(In reply to Mike Hommey [:glandium] from comment #12)
> http://hg.mozilla.org/releases/mozilla-aurora/rev/540694f5df42

Can we also land on MOBILE140_2012053010_RELBRANCH on mozilla-aurora?
(In reply to Alex Keybl [:akeybl] from comment #14)
> (In reply to Mike Hommey [:glandium] from comment #12)
> > http://hg.mozilla.org/releases/mozilla-aurora/rev/540694f5df42
> 
> Can we also land on MOBILE140_2012053010_RELBRANCH on mozilla-aurora?

done https://hg.mozilla.org/releases/mozilla-aurora/rev/3dbfe52b2649
You need to log in before you can comment on or make changes to this bug.